False positive S1172 on certain captured method parameters

[dalestan (Dale)]

The following code has a false positive S1172 on the value parameter. The passed value for value is used when key is null. The analyzer treats localFunction as live code, since it doesn’t issue S1172s on the other two parameters, but it apparently does not recognize that the TryGetValue call is conditional. (See embedded comments for workarounds and non-workarounds.)

#pragma warning disable S1144, IDE0051 // Unused private types or members should be removed
internal class Test {
    private void Method(object? value, Dictionary<string, object?> dict, string? key) {
        // Changing this to a lambda expression (i.e. `Action localFunction = () => {`
        // fixes SonarQube's complaints, but VS will tell me to change it back with IDE0039.
        void localFunction() {
            // Splitting this into two nested if statements does not help the analyzer:
            if (key != null && dict.TryGetValue(key, out value)) {
                //...
            }

            if (value != null) {
                //...
            }
        }

        DoSomethingWith(localFunction);
    }

    private static void DoSomethingWith(Action _) { /* ... */ }
}

Possibly related to C# S1172 and S1854 - False positives with local functions and nullability operator"!", but that is five years old.
Similar to Fix S1172 FP: Parameter captured in a referenced local function · Issue #3134 · SonarSource/sonar-dotnet · GitHub and C# S1172 - Parameters not recognized in value tuples, but both are fixed.

I’m using SonarQube 8.26.0.14164 on Visual Studio 17.14.14 and Windows 11 24H2.

[Mary (Mary Georgiou)]

Hello @dalestan and thanks for reporting this.

From my POV in this particular example the issue is a true positive.
The out value will overwrite whatever is in the value argument to null or to the value that is assigned to the key.
Result is that the parameter value of value will never be read.

You’ll see that if you remove the dict.TryGetValue(key, out value)) the issue disappears.

EDIT: No it’s an FP - because if the key is null then the second if condition will actually read the value of value.

Sorry of the confusion.

I’ll add it to our backlog to deal with it in a future sprint

Thanks
Mary