Fix S2259 FP: PropertyReference does not learn from underlying symb…

…ol (#9030)
SonarSource · Apr 9, 2024 · 1a054c2 · 1a054c2
1 parent 8f54a08
commit 1a054c2
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 20 deletions.
diff --git a/...yzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/References.cs b/...yzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/References.cs
@@ -73,20 +73,23 @@ protected override IPropertyReferenceOperationWrapper Convert(IOperation operati
 
     protected override ProgramState PreProcess(ProgramState state, IPropertyReferenceOperationWrapper operation, bool isInLoop)
     {
-        var symbol = operation.Instance.TrackedSymbol(state);
-        if (symbol is not null)
+        if (operation.WrappedOperation.TrackedSymbol(state) is { } propertySymbol && state[propertySymbol] is { } propertyValue)
+        {
+            state = state.SetOperationValue(operation, propertyValue);
+        }
+        var instanceSymbol = operation.Instance.TrackedSymbol(state);
+        if (instanceSymbol is not null)
         {
             if (!IsNullableProperty(operation, "HasValue"))
             {
-                state = state.SetSymbolConstraint(symbol, ObjectConstraint.NotNull);
+                state = state.SetSymbolConstraint(instanceSymbol, ObjectConstraint.NotNull);
             }
-            if (IsNullableProperty(operation, "Value") && state[symbol] is { } value)
+            if (IsNullableProperty(operation, "Value") && state[instanceSymbol] is { } instanceValue)
             {
-                state = state.SetOperationValue(operation, value);
+                state = state.SetOperationValue(operation, instanceValue);
             }
         }
-        state = CollectionTracker.LearnFrom(state, operation, symbol);
-        return state;
+        return CollectionTracker.LearnFrom(state, operation, instanceSymbol);
     }
 
     protected override SymbolicConstraint BoolConstraintFromOperation(ProgramState state, IPropertyReferenceOperationWrapper operation) =>

diff --git a/...sts/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs b/...sts/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs
@@ -18,7 +18,6 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-using System.Data;
 using Microsoft.CodeAnalysis.Operations;
 using SonarAnalyzer.SymbolicExecution;
 using SonarAnalyzer.SymbolicExecution.Constraints;
@@ -1066,16 +1065,22 @@ public void PropertyReference_Write_SetsNotNull()
     [TestMethod]
     public void PropertyReference_AutoProperty_IsTracked()
     {
-        const string code = """
-                AutoProperty = null;
-                Tag("AfterSetNull", AutoProperty);
-                AutoProperty.ToString();
-                Tag("AfterReadReference", AutoProperty);
-                """;
+        var code = """
+            AutoProperty = null;
+            var x = AutoProperty;
+            Tag("AfterSetNull", AutoProperty);
+            Tag("AfterSetNull_Operation", x);
+            AutoProperty.ToString();
+            x = AutoProperty;
+            Tag("AfterReadReference", AutoProperty);
+            Tag("AfterReadReference_Operation", x);
+            """;
         var validator = SETestContext.CreateCS(code).Validator;
         validator.ValidateContainsOperation(OperationKind.PropertyReference);
         validator.TagValue("AfterSetNull").Should().HaveOnlyConstraints(ObjectConstraint.Null);
+        validator.TagValue("AfterSetNull_Operation").Should().HaveOnlyConstraints(ObjectConstraint.Null);
         validator.TagValue("AfterReadReference").Should().HaveOnlyConstraints(ObjectConstraint.NotNull);
+        validator.TagValue("AfterReadReference_Operation").Should().HaveOnlyConstraints(ObjectConstraint.NotNull);
     }
 
     [TestMethod]

diff --git a/...ers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/NullPointerDereference.cs b/...ers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/NullPointerDereference.cs
@@ -140,7 +140,7 @@ public void M()
                 Property = null;
                 _ = Property.HasValue;  // Compliant
                 Property = null;
-                Property.GetType();     // FN https://github.com/SonarSource/sonar-dotnet/issues/6930
+                Property.GetType();     // Noncompliant
                 Property = default(T);
                 Property.GetType();     // Compliant
             }
@@ -366,7 +366,7 @@ class MyClass
             public void M()
             {
                 Property = null;
-                Property.ToString();  // FN https://github.com/SonarSource/sonar-dotnet/issues/6930
+                Property.ToString();  // Noncompliant
             }
         }
 
@@ -2123,7 +2123,7 @@ private void Foo()
 
         if (Value != null)                      // Both branches enter here because PropertyReference is not learning from TrackedSymbol
         {
-            Console.WriteLine(value.Length);    // Noncompliant FP
+            Console.WriteLine(value.Length);    // Compliant
         }
     }
 }

diff --git a/...ers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/NullPointerDereference.vb b/...ers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/NullPointerDereference.vb
@@ -133,9 +133,7 @@ Public Class Program
             Prop = Nothing
             Dim X = Prop.HasValue   ' Compliant
             Prop = Nothing
-            Prop.GetType()          ' FN https://github.com/SonarSource/sonar-dotnet/issues/6930
-            Prop = Nothing
-            Prop.GetType()          ' Compliant
+            Prop.GetType()          ' Noncompliant
         End Sub
     End Class