Ignore conditions in locks statement

SonarSource · Aug 14, 2023 · 5f7493a · 5f7493a
1 parent 2ef77e5
commit 5f7493a
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 4 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs
@@ -73,4 +73,6 @@ protected override bool IsForLoopIncrementor(SyntaxNode syntax) =>
     protected override bool IsUsing(SyntaxNode syntax) =>
         (syntax.IsKind(SyntaxKind.VariableDeclaration) && syntax.Parent.IsKind(SyntaxKind.UsingStatement))
         || (syntax is LocalDeclarationStatementSyntax local && local.UsingKeyword().IsKind(SyntaxKind.UsingKeyword));
+    protected override bool IsLockStatement(SyntaxNode syntax) =>
+        syntax.IsKind(SyntaxKind.LockStatement);
 }
diff --git a/...arAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs b/...arAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/ConditionEvaluatesToConstantBase.cs
@@ -18,6 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+using Microsoft.CodeAnalysis.CSharp;
 using SonarAnalyzer.CFG.Roslyn;
 using SonarAnalyzer.SymbolicExecution.Constraints;
 
@@ -44,6 +45,7 @@ public abstract class ConditionEvaluatesToConstantBase : SymbolicRuleCheck
     protected abstract bool IsConditionalAccessExpression(SyntaxNode syntax);
     protected abstract bool IsForLoopIncrementor(SyntaxNode syntax);
     protected abstract bool IsUsing(SyntaxNode syntax);
+    protected abstract bool IsLockStatement(SyntaxNode syntax);
 
     public override ProgramState[] PreProcess(SymbolicContext context)
     {
@@ -55,7 +57,7 @@ public override ProgramState ConditionEvaluated(SymbolicContext context)
     {
         var operation = context.Operation.Instance;
         if (operation.Kind is not OperationKindEx.Literal
-            && !operation.Syntax.Ancestors().Any(IsUsing)
+            && !operation.Syntax.Ancestors().Any(x => IsUsing(x) || IsLockStatement(x))
             && operation.TrackedSymbol(context.State) is not IFieldSymbol { IsConst: true }
             && !IsDiscardPattern(operation))
         {

diff --git a/...rs/src/SonarAnalyzer.VisualBasic/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/...rs/src/SonarAnalyzer.VisualBasic/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs
@@ -73,4 +73,6 @@ syntax.Parent is BinaryConditionalExpressionSyntax { } binary
 
     protected override bool IsUsing(SyntaxNode syntax) =>
         syntax.IsKind(SyntaxKind.VariableDeclarator) && syntax.Parent.IsKind(SyntaxKind.UsingStatement);
+    protected override bool IsLockStatement(SyntaxNode syntax) =>
+        syntax.IsKind(SyntaxKind.SyncLockBlock);
 }
diff --git a/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs
@@ -1221,7 +1221,7 @@ public static Singleton Instance
                     {
                         lock (syncRoot)
                         {
-                            if (instance == null) // Noncompliant FP
+                            if (instance == null) // We don't check conditions that are in lock statements.
                             {
                                 instance = new Singleton();
                             }
@@ -3019,7 +3019,8 @@ public void InitWithLock(ref object field)
             {
                 lock (gate)
                 {
-                    if (field == null) // Noncompliant FP: in multithreading context it makes sense to check for null twice
+                    if (field == null) // We don't check conditions in lock statements.
+                                       // In multithreading context it makes sense to check for null twice
                     {
                         field = new object();
                     }

diff --git a/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.vb b/...SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.vb
@@ -1003,7 +1003,7 @@ End Sub
             Get
                 If instanceField Is Nothing Then
                     SyncLock syncRoot
-                        If instanceField Is Nothing Then ' Noncompliant FP
+                        If instanceField Is Nothing Then ' We don't raise in conditions in synclock blocks as it's raising many FPs.
                             instanceField = New Singleton()
                         End If
                     End SyncLock