C# S3626 false positive when returning before local functions

SonarQube for IDE Visual Studio
1
  • Operating system: Windows 10
  • Visual Studio version: Visual Studio 2022
  • SonarLint plugin version: 7.4.0.80741
  • Programming language you’re coding in: C# 12
  • Is connected mode used: No

I’ve created a few local functions inside a method in a custom Windows Forms control I’m creating. According to JetBrains ReSharper, local functions should be at the end of the method body after a return statement. However, when I add the empty return statement, SonarLint flags it because the method it’s in is a void method.

Here is a skeleton of the method in question:

protected override void OnPaint(PaintEventArgs e)
{
	// code excluded

	return;

	void DrawDigit(char digit, Point digitOrigin)
	{
		// code excluded
	}

	IEnumerable<char> GetDigits()
	{
		// code excluded
	}

	IEnumerable<Segment> GetSegmentsForDigit(char digit)
	{
		// code excluded
	}
}

Here is a screenshot of the return statement being flagged:
image

Sonar Community Roundup, February 22 - February 28
4

Hello @fishnet37222,

Welcome to the community.

I don’t think this is a false positive.
In the case of your OnPaint method, it returns void, and using a return at the end of it doesn’t have any impact on the flow of the method execution. Thus, the statement is redundant.

As for the conflicting issues with ReSharper, I would say this is a matter of opinion, as I personally see this suggestion as purely cosmetic.

Have a good day,

6

@sebastien.marichal You are incorrect. The return is for the benefit of the local methods. See the excellent explanation on the ReSharper blog here:

7

I agree that S3626 should have an exception when it sees only local functions below the return statement. There is certainly precedent for in SonarQube’s rules suppressing themselves for certain exceptions to the general case. Jetbrains’ description on this is logical and the return adds clarity.

When you have one or more local functions defined at the end of a method, JetBrains Rider suggests adding an explicit return between the last executable statement and the local function definition. The method will return at that point anyway, but making this explicit will enhance the clarity of the method’s control flow for readers. This is particularly beneficial if the local function definition is lengthy, or if there are multiple local functions. In these instances, the reader will not have to scroll to the end of the method to check for more executable statements after the local function definitions.

8

Hello @voidpointer, @Balfa,

Considering the attraction this issue has, I had a discussion with the rest of the team.
We will add an exception for this scenario in the rule.
Whenever a return statement is used right before local functions the rule will not raise anymore.

I have created a ticket in our backlog to implement it in the future.

Thank you for your feedback!

1 Like