S3900: Basic rule implementation

analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.cs

@@ -18,16 +18,51 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+using SonarAnalyzer.SymbolicExecution.Constraints;
+
 namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks.CSharp;
 
 public class PublicMethodArgumentsShouldBeCheckedForNull : SymbolicRuleCheck
 {
     private const string DiagnosticId = "S3900";
-    private const string MessageFormat = "Refactor this method to add validation of parameter '{0}' before using it.";
+    private const string MessageFormat = "{0}";
 
     internal static readonly DiagnosticDescriptor S3900 = DescriptorFactory.Create(DiagnosticId, MessageFormat);
 
     protected override DiagnosticDescriptor Rule => S3900;
 
-    public override bool ShouldExecute() => false;
+    public override bool ShouldExecute() =>
+        Node is BaseMethodDeclarationSyntax or AccessorDeclarationSyntax;
+
+    protected override ProgramState PreProcessSimple(SymbolicContext context)
+    {
+        var operation = context.Operation.Instance;
+        if (operation.Kind == OperationKindEx.ParameterReference
+            && operation.ToParameterReference().Parameter is var parameter
+            && !parameter.Type.IsValueType
+            && IsParameterDereferenced(context.Operation)
+            && NullableStateIsNotKnownForParameter(parameter)
+            && !parameter.HasAttribute(KnownType.Microsoft_AspNetCore_Mvc_FromServicesAttribute))
+        {
+            var message = SemanticModel.GetDeclaredSymbol(Node).IsConstructor()
+                ? "Refactor this constructor to avoid using members of parameter '{0}' because it could be null."
+                : "Refactor this method to add validation of parameter '{0}' before using it.";
+            ReportIssue(operation, string.Format(message, operation.Syntax), context);
+        }
+
+        return context.State;
+
+        bool NullableStateIsNotKnownForParameter(IParameterSymbol symbol) =>
+            context.State[symbol] is null || !context.State[symbol].HasConstraint<ObjectConstraint>();
+    }
+
+    private static bool IsParameterDereferenced(IOperationWrapperSonar operation) =>
+        operation.Parent != null
+        && operation.Parent.IsAnyKind(
+            OperationKindEx.Invocation,
+            OperationKindEx.FieldReference,
+            OperationKindEx.PropertyReference,
+            OperationKindEx.EventReference,
+            OperationKindEx.Await,
+            OperationKindEx.ArrayElementReference);
 }

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs

@@ -69,6 +69,9 @@ internal static IFieldReferenceOperationWrapper ToFieldReference(this IOperation
         internal static IPropertyReferenceOperationWrapper ToPropertyReference(this IOperation operation) =>
             IPropertyReferenceOperationWrapper.FromOperation(operation);
 
+        internal static IParameterReferenceOperationWrapper ToParameterReference(this IOperation operation) =>
+            IParameterReferenceOperationWrapper.FromOperation(operation);
+
         internal static IEventReferenceOperationWrapper ToEventReference(this IOperation operation) =>
             IEventReferenceOperationWrapper.FromOperation(operation);
 

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.CSharp11.cs

@@ -5,15 +5,22 @@
 
     public void NotCompliantCases(object[] o)
     {
-        o[1].ToString(); // FIXME Non-compliant {{Refactor this method to add validation of parameter 'o' before using it.}}
+        o[1].ToString(); // Noncompliant {{Refactor this method to add validation of parameter 'o' before using it.}}
     }
 
-    public void Compliant(object[] o)
+    public void ListPattern1(object[] o)
     {
         if (o is [not null, not null])
         {
-            o.ToString();       // Compliant
-            o[1].ToString();    // Compliant
+            o.ToString();       // Noncompliant - FP
+        }
+    }
+
+    public void ListPattern2(object[] o)
+    {
+        if (o is [not null, not null])
+        {
+            o[1].ToString();    // Noncompliant - FP
         }
     }
 }
@@ -22,6 +29,6 @@ public interface ISomeInterface
 {
     public static virtual void NotCompliantCases(object o)
     {
-        o.ToString(); // FIXME Non-compliant {{Refactor this method to add validation of parameter 'o' before using it.}}
+        o.ToString(); // Noncompliant {{Refactor this method to add validation of parameter 'o' before using it.}}
     }
 }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs

@@ -9,7 +9,7 @@ public void NullCoalescenceAssignment_NotNull(string s1)
     public void NullCoalescenceAssignment_Null(string s1)
     {
         s1 ??= null;
-        s1.ToString(); // FN
+        s1.ToString(); // Covered by S2259
     }
 }
 
@@ -20,7 +20,7 @@ public interface IWithDefaultMembers
 
     void Reset(string s)
     {
-        s.ToString(); // FIXME Non-compliant
+        s.ToString(); // Noncompliant
     }
 }
 
@@ -30,12 +30,12 @@ public void Method(object arg)
     {
         string LocalFunction(object o)
         {
-            return o.ToString(); // FN: local functions are not supported by the CFG
+            return o.ToString(); // Compliant - local methods are not accessible from other assemblies
         }
 
         static string LocalStaticFunction(object o)
         {
-            return o.ToString(); // FN: local functions are not supported by the CFG
+            return o.ToString();
         }
     }
 }
@@ -66,7 +66,7 @@ public void OnlyDiscardBranch_Noncompliant(string s, bool b)
     {
         var result = b switch
         {
-            _ => s.ToString() // FIXME Non-compliant
+            _ => s.ToString() // Noncompliant
         };
     }
 
@@ -75,7 +75,7 @@ public void MultipleBranches_Noncompliant(string s, int val)
         var result = val switch
         {
             1 => "a",
-            2 => s.ToString(), // FIXME Non-compliant
+            2 => s.ToString(), // Noncompliant
             _ => "b"
         };
     }
@@ -87,7 +87,7 @@ public void Nested_Noncompliant(string s, int val, bool condition)
             1 => "a",
             2 => condition switch
             {
-                _ => s.ToString() // FIXME Non-compliant
+                _ => s.ToString() // Noncompliant
             },
             _ => "b"
         };
@@ -97,8 +97,8 @@ public void MultipleBranches_HandleNull(string s, int val)
     {
         var result = s switch
         {
-            null => s.ToString(),   // FIXME Non-compliant
-            _ => s.ToString()       // Compliant as null was already handled
+            null => s.ToString(), // Covered by S2259
+            _ => s.ToString()
         };
     }
 
@@ -116,7 +116,7 @@ public string MultipleBranches_PropertyPattern(Address address, string s)
     {
         return address switch
         {
-            { State: "WA" } addr => s.ToString(), // FIXME Non-compliant
+            { State: "WA" } addr => s.ToString(), // Noncompliant
             _ => string.Empty
         };
     }
@@ -125,7 +125,7 @@ public string MultipleBranches_PropertyPattern_FP(string s)
     {
         return s switch
         {
-            { Length: 5 } => s.ToString(), // FIXME Non-compliant - FP we know that the length is 5 so the string cannot be null
+            { Length: 5 } => s.ToString(), // Compliant - we know that the length is 5 so the string cannot be null
             _ => string.Empty
         };
     }
@@ -134,7 +134,7 @@ public string MultipleBranches_RecursivePattern(Person person, string s)
     {
         return person switch
         {
-            { Address: { State: "WA" } } pers => s.ToString(), // FIXME Non-compliant
+            { Address: { State: "WA" } } pers => s.ToString(), // Noncompliant
             _ => string.Empty
         };
     }
@@ -143,7 +143,7 @@ public string MultipleBranches_TuplePattern(Address address, string s)
     {
         return address switch
         {
-            var (name, state) => s.ToString(), // FN
+            var (name, state) => s.ToString(), // Noncompliant
             _ => string.Empty
         };
     }
@@ -152,8 +152,8 @@ public string MultipleBranches_WhenClause(Address address, string s)
     {
         return address switch
         {
-            Address addr when addr.Name.Length > 0 => s.ToString(),         // FIXME Non-compliant
-            Address addr when addr.Name.Length == s.Length => string.Empty, // FIXME Non-compliant
+            Address addr when addr.Name.Length > 0 => s.ToString(),         // Noncompliant
+            Address addr when addr.Name.Length == s.Length => string.Empty, // Noncompliant
             _ => string.Empty
         };
     }
@@ -162,7 +162,7 @@ public string MultipleBranches_VarDeclaration(Address address, string s)
     {
         return address switch
         {
-            Address addr => s.ToString(), // FIXME Non-compliant
+            Address addr => s.ToString(), // Noncompliant
             _ => string.Empty
         };
     }
@@ -171,8 +171,8 @@ public string TwoBranches_NoDefault(bool condition, string s)
     {
         return condition switch
         {
-            true => s.ToString(), // FIXME Non-compliant
-            false => s.ToString() // FIXME Non-compliant
+            true => s.ToString(), // Noncompliant
+            false => s.ToString() // Noncompliant
         };
     }
 }
@@ -187,7 +187,7 @@ public void Test(string s)
                 break;
 
             default:
-                s.ToString(); // Compliant - the null is handled by the case null branch.
+                s.ToString(); // Noncompliant - FP
                 break;
         }
     }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.CSharp9.cs

@@ -9,7 +9,7 @@ public class Sample
 
     public void TargetTypedNew(object arg)
     {
-        arg.ToString();         // FN, can't build CFG for this method
+        arg.ToString();         // Noncompliant
 
         StringBuilder sb = new();
     }
@@ -23,29 +23,29 @@ public void PatternMatching(object arg)
 
         if (arg is int and > 0 and > 1)
         {
-            arg.ToString();     // Compliant
+            arg.ToString();     // Noncompliant - FP
         }
 
         if (arg is int or bool or long)
         {
-            arg.ToString();     // Compliant
+            arg.ToString();     // Noncompliant - FP
         }
 
         if (arg is null)
         {
-            arg.ToString();     // FN
+            arg.ToString();     // Covered by S2259
         }
         if (arg is int or bool or null)
         {
-            arg.ToString();     // FN
+            arg.ToString();
         }
         else if (arg is not not null)
         {
-            arg.ToString();     // FN
+            arg.ToString();
         }
         else if (!(arg is not null))
         {
-            arg.ToString();     // FN
+            arg.ToString();
         }
     }
 
@@ -63,7 +63,7 @@ public object PropertySet
         get => null;
         set
         {
-            field = value.ToString();   // FIXME Non-compliant
+            field = value.ToString();   // Noncompliant
         }
     }
 
@@ -72,7 +72,7 @@ public object PropertyInit
         get => null;
         init
         {
-            field = value.ToString();   // FIXME Non-compliant
+            field = value.ToString();   // Noncompliant
         }
     }
 }
@@ -81,7 +81,7 @@ public record Record
 {
     public void Method(object arg)
     {
-        arg.ToString();                 // FIXME Non-compliant
+        arg.ToString();                 // Noncompliant
     }
 }
 
@@ -94,7 +94,7 @@ public partial class Partial
 {
     public partial void Method(object arg)
     {
-        arg.ToString();                 // FIXME Non-compliant
+        arg.ToString();                 // Noncompliant
     }
 }
 
@@ -107,7 +107,7 @@ public int GetProductMultipleAttr([FromServices][FromRoute] IService service) =>
         service.GetValue();             // Compliant, it's attributed with FromServices attribute
 
     public int GetPrice(IService service) =>
-        service.GetValue();             // FIXME Non-compliant
+        service.GetValue();             // Noncompliant
 
     public interface IService
     {