False positive S3241 (Change return type to 'void') on lambda function

Clean Code Report False-positive / False-negative...
1
  • What language is this for? => C#
  • Which rule? => S3241: Change return type to ‘void’; not a single caller uses the returned value.
  • Why do you believe it’s a false-positive/false-negative? => Applying Sonar’s suggested change would cause a compilation error.
  • Are you using
    • SonarCloud => Yes
    • SonarLint => Yes
    • Which version => Reproduceable released versions downloaded on 30-May-2023.
    • in connected mode => not relevant
    • How can we reproduce the problem => See snippet (simplified extract from actual code)

The false positive warning applies to the lambda function Stuff. The false positive will be not raised when omitting the second line: Stuff(0);

public static IList<string> GetListOfStuff(IStuffer stuffer) {
	var aListOfStuff = new List<string>();
	Stuff(0);
	stuffer?.DoSomeStuffing(Stuff);
	return aListOfStuff;

	bool Stuff(int anInteger) {
		if( anInteger % 2 == 0) {
			aListOfStuff.Add($"Hello {anInteger}");
			return true;
		}
		return false;
	}
}

public interface IStuffer {
	void DoSomeStuffing(Func<int, bool> stuff);
}
4

Hello @Ginkgo, and thanks for reporting this.

Based on how the rule works, this is not a false positive.

In the reproducer, Stuff is used twice, once as an argument to another method and once it’s invoked.
The rule checks the invocations of methods with private accessibility, and in this case, there’s no invocation where the return value of Stuff is used.

This is why when you remove the invocation of the method Stuff(0), the rule no longer raises an issue.
Another way to not raise an issue is to use a discard like so:
_ = Stuff(0);.

I hope this helped!

5

Hello @Mary_Georgiou , thanks for looking into this.

Sonar claims: “ not a single caller uses the returned value ”. That is not true: there is obviously an implementation of the interface that uses the return value, sorry for not making that clear enough.

Then it ordains a change that would break the code in a spectacular way.

How is this not a bug?

6

Hello @Ginkgo,

Thanks for your reply.

That is not true: there is obviously an implementation of the interface that uses the return value

In which part of this snippet is the return value used by the implementation of IStuffer.DoSomething?

7

@Mary_Georgiou

public class Stuffer: IStuffer {
	public DoSomething(Func<int, bool> stuff) {
		if( stuff(2) )
			Console.WriteLine(“Here it is used”);
}

The thing about interfaces is that Sonar never knows nor needs to know nor should assume to know what its implementations might do.

8

Hello @Ginkgo,

Apologies for the late reply.

I see what you mean, you are right.
It should not be related only to interfaces but in any case that the delegate is passed as an argument.

I opened an issue and added it to our backlog to fix in a future sprint.

thanks!
Best Regards
Mary