Merge pull request #7014 from CollinAlpert/issue-6983

Don't emit CA1508 in Debug.Assert
dotnet · Nov 29, 2023 · d0e4558 · d0e4558
2 parents 96b6cc1 + de0cfa1
commit d0e4558
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 3 deletions.
diff --git a/...nalyzers/Core/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode.cs b/...nalyzers/Core/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode.cs
@@ -1,8 +1,10 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Linq;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
 using Microsoft.CodeAnalysis;
@@ -61,6 +63,9 @@ public override void Initialize(AnalysisContext context)
 
             context.RegisterCompilationStartAction(compilationContext =>
             {
+                var typeProvider = WellKnownTypeProvider.GetOrCreate(compilationContext.Compilation);
+                var debugAssert = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemDiagnosticsDebug);
+                var debugAssertMethods = debugAssert?.GetMembers(nameof(Debug.Assert)).OfType<IMethodSymbol>().ToArray() ?? Array.Empty<IMethodSymbol>();
                 compilationContext.RegisterOperationBlockAction(operationBlockContext =>
                 {
                     var owningSymbol = operationBlockContext.OwningSymbol;
@@ -73,13 +78,15 @@ public override void Initialize(AnalysisContext context)
 
                     foreach (var operationRoot in operationBlockContext.OperationBlocks)
                     {
-                        static bool ShouldAnalyze(IOperation op) =>
-                                (op as IBinaryOperation)?.IsComparisonOperator() == true ||
+                        bool ShouldAnalyze(IOperation op) =>
+                                ((op as IBinaryOperation)?.IsComparisonOperator() == true ||
                                 op is IInvocationOperation { TargetMethod.ReturnType.SpecialType: SpecialType.System_Boolean } ||
                                 op.Kind == OperationKind.Coalesce ||
                                 op.Kind == OperationKind.ConditionalAccess ||
                                 op.Kind == OperationKind.IsNull ||
-                                op.Kind == OperationKind.IsPattern;
+                                op.Kind == OperationKind.IsPattern)
+                                && (op.Parent is not IArgumentOperation { Parent: IInvocationOperation invocation } ||
+                                    !debugAssertMethods.Contains(invocation.TargetMethod, SymbolEqualityComparer.Default));
 
                         if (operationRoot.HasAnyOperationDescendant(ShouldAnalyze))
                         {

diff --git a/...ft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs b/...ft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Testing;
 using Test.Utilities;
 using Xunit;
 using CSharpLanguageVersion = Microsoft.CodeAnalysis.CSharp.LanguageVersion;
@@ -3378,5 +3379,71 @@ private void MayThrowException(int i)
                 LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8,
             }.RunAsync();
         }
+
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.ValueContentAnalysis)]
+        [Fact, WorkItem(6983, "https://github.com/dotnet/roslyn-analyzers/issues/6983")]
+        public Task DebugAssert_NoDiagnostic()
+        {
+            const string code = """
+                                using System.Diagnostics;
+                                
+                                public static class MyClass
+                                {
+                                    internal const int MyConstant = 16;
+                                
+                                    public static void MyMethod()
+                                    {
+                                        Debug.Assert(MyConstant == 16);
+                                    }
+                                }
+                                """;
+            return VerifyCS.VerifyAnalyzerAsync(code);
+        }
+
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.ValueContentAnalysis)]
+        [Fact, WorkItem(6983, "https://github.com/dotnet/roslyn-analyzers/issues/6983")]
+        public Task DebugAssertWithMessage_NoDiagnostic()
+        {
+            const string code = """
+                                using System.Diagnostics;
+                                
+                                public static class MyClass
+                                {
+                                    internal const int MyConstant = 16;
+                                
+                                    public static void MyMethod()
+                                    {
+                                        Debug.Assert(MyConstant == 16, "MyConstant is not 16");
+                                    }
+                                }
+                                """;
+            return VerifyCS.VerifyAnalyzerAsync(code);
+        }
+
+        [Trait(Traits.DataflowAnalysis, Traits.Dataflow.ValueContentAnalysis)]
+        [Fact, WorkItem(6983, "https://github.com/dotnet/roslyn-analyzers/issues/6983")]
+        public Task AsMethodArgument_Diagnostic()
+        {
+            const string code = """
+                                using System.Diagnostics;
+
+                                public static class MyClass
+                                {
+                                    internal const int MyConstant = 16;
+                                
+                                    public static void MyMethod()
+                                    {
+                                        Test({|#0:MyConstant == 16|});
+                                    }
+                                    
+                                    private static void Test(bool b) => throw null;
+                                }
+                                """;
+            return new VerifyCS.Test
+            {
+                TestCode = code,
+                ExpectedDiagnostics = { new DiagnosticResult(AvoidDeadConditionalCode.AlwaysTrueFalseOrNullRule).WithLocation(0).WithArguments("MyConstant == 16", "true") }
+            }.RunAsync();
+        }
     }
 }