Fix S1066 FP: Combination of dynamic and out should not raise

[mgirolet-gl]

Description

Whenever SonarQube detects an if statement alone in another if statements, it flags csharpsquid:S1066 to indicate to the developers that the two if statements can (and should) be merged into one.

However, C# has a very interesting type resolving feature that allows to declare variables within ifs:

dynamic anything = 1; // Assume that this variable could be anything of any type

if(anything is int n)
	Console.WriteLine(n); // Here, n is declared and is an int containing value 1
	
Console.WriteLine(n); // Error CS0165: Here, n is still declared but is marked as unassigned, so an error is thrown.

This is also possible with 'out' variables, although it behaves differently:

string something = "1"; // Assume that this variable could also contain "hello"

if(int.TryParse(something, out int n))
	Console.WriteLine(n); // Here, n is declared and is an int containing value 1
	
Console.WriteLine(n); // Here, n is still declared and will be at value 1 if int.TryParse returned true, or default(int) if it returned false

Now here's the glitch: for some reason, if you combine both possibilities into one statement, the compiler rejects it:

dynamic anything = "2024-04-29"; // Assume that this variable could be anything of any type
if (anything is string && DateTime.TryParseExact(anything, "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.None, out DateTime dt))
    return dt; // Error: CS0165 - Use of unassigned local variable 'dt'

The workaround is to use two nested statements, which gets csharpsquid:S1066 flagged even though we CANNOT merge both statements here:

dynamic anything = "2024-04-29"; // Assume that this variable could be anything of any type
if (anything is string)
    if (DateTime.TryParseExact(anything, "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.None, out DateTime dt))
        return dt; // Works, but csharpsquid:S1066 is flagged

Repro steps

DateTime? ThisWorks() {
	dynamic anything = "2024-04-29"; // Obviously, this value is calculated by other means but for this example I'm making it fixed

	if (anything is string)
		if(DateTime.TryParseExact(anything, "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.None, out DateTime dt))
			return dt; // Works, but csharpsquid:S1066 is flagged

	return null;
}

DateTime? ThisWontWork() {
	dynamic anything = "2024-04-29";

	if (anything is string && DateTime.TryParseExact(anything, "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.None, out DateTime dt))
		return dt; // Error: CS0165 - Use of unassigned local variable 'dt'

	return null;
}

Expected behavior

SonarQube detects that the two if statements cannot be merged into a single one and doesn't flag it as an issue.

Actual behavior

SonarQube assumes the two statements can be merged and flags it as a S1066 issue.

Known workarounds

Flagging the issue as a false positive, or separating the conditions in different functions (which would make the code more complex and harder to understand in my specific case)

Related information

• C#/VB.NET Plugins version: 9.19.0.84025
• Visual Studio version: 2022
• MSBuild version: 17.7.4
• Dotnet version: .NET 6.0
• SonarScanner for .NET version: 6.2.0
• Operating System: Windows/Linux

I have also reported the issue on the Sonar Community forum.

Thanks for taking the time to read!

[pavel-mikula-sonarsource]

Hi @mgirolet-gl,

Thank you for reporting this case, it's a really interesting one. I confirm it as a False Positive.

The reason is, that using dynamic anywhere, like DoSomething(anything), doesn't just use dynamic argument. The whole statement becomes dynamic by nature, because the compiler doesn't really know what will really be invoked and what are the usual consequences.

So anything is string && DateTime.DoSomething-well-known is in reality interpreted
as anything is string && There's-some-other-uknown-dynamic-statement-here,-have-fun

Using dynamic type anywhere in the statement should prevent the rule from raising, so also IsTrue(anything) && ... should not raise

[mgirolet-gl]

Hey there, thanks for your reply!

Thanks for the explanation, I hadn't thought of it this way.
I had (wrongly) assumed it was due to a misinterpretation of the is and out keywords being combined in the if, but it indeed seems to be a quirk of how the compiler interprets dynamic variables in conditions as you explained since there's no compiling error when the input type isn't dynamic.

Looking forward to see it fixed.

Thanks for all your work, you guys are the best!