Merge pull request #2519 from mavasani/FixCA1031

Handle catch clauses without exception variable in CA1031
dotnet · Jun 5, 2019 · 7f096af · 7f096af
2 parents c20436c + 37cb4d6
commit 7f096af
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 14 deletions.
diff --git a/...crosoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypes.cs b/...crosoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypes.cs
@@ -35,9 +35,9 @@ public DoNotCatchGeneralExceptionTypesAnalyzer() : base(shouldCheckLambdas: true
         {
         }
 
-        protected override Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxNode catchNode)
+        protected override Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxToken catchKeyword)
         {
-            return catchNode.CreateDiagnostic(Rule, containingMethod.Name);
+            return catchKeyword.CreateDiagnostic(Rule, containingMethod.Name);
         }
     }
 }
diff --git a/...deQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypesTests.cs b/...deQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypesTests.cs
@@ -650,6 +650,27 @@ End Namespace
             GetCA1031BasicResultAt(11, 29, "TestMethod"));
         }
 
+        [Fact, WorkItem(2518, "https://github.com/dotnet/roslyn-analyzers/issues/2518")]
+        public void CSharp_NoDiagnostic_SpecificExceptionWithoutVariable()
+        {
+            VerifyCSharp(@"
+            using System;
+
+            public class Class1
+            {
+                void M()
+                {
+                    try
+                    {
+                    }
+                    catch (OperationCanceledException)
+                    {
+                        // Comment
+                    }
+                }
+            }");
+        }
+
         private static DiagnosticResult GetCA1031CSharpResultAt(int line, int column, string signature)
         {
             return GetCSharpResultAt(line, column, DoNotCatchGeneralExceptionTypesAnalyzer.Rule, signature);

diff --git a/src/Microsoft.NetFramework.Analyzers/Core/DoNotCatchCorruptedStateExceptions.cs b/src/Microsoft.NetFramework.Analyzers/Core/DoNotCatchCorruptedStateExceptions.cs
@@ -1,10 +1,8 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System.Collections.Immutable;
-using System.Linq;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
-using Microsoft.NetFramework.Analyzers.Helpers;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 
@@ -37,9 +35,9 @@ public DoNotCatchCorruptedStateExceptionsAnalyzer() : base(shouldCheckLambdas: f
         {
         }
 
-        protected override Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxNode catchNode)
+        protected override Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxToken catchKeyword)
         {
-            return catchNode.CreateDiagnostic(Rule, containingMethod.ToDisplayString());
+            return catchKeyword.CreateDiagnostic(Rule, containingMethod.ToDisplayString());
         }
     }
 }
diff --git a/src/Utilities/Compiler/DoNotCatchGeneralUnlessRethrown.cs b/src/Utilities/Compiler/DoNotCatchGeneralUnlessRethrown.cs
@@ -24,7 +24,7 @@ protected DoNotCatchGeneralUnlessRethrownAnalyzer(bool shouldCheckLambdas, strin
             _enablingMethodAttributeFullyQualifiedName = enablingMethodAttributeFullyQualifiedName;
         }
 
-        protected abstract Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxNode catchNode);
+        protected abstract Diagnostic CreateDiagnostic(IMethodSymbol containingMethod, SyntaxToken catchKeyword);
 
         public override void Initialize(AnalysisContext analysisContext)
         {
@@ -62,7 +62,7 @@ public override void Initialize(AnalysisContext analysisContext)
 
                         foreach (var catchClause in walker.CatchClausesForDisallowedTypesWithoutRethrow)
                         {
-                            operationBlockAnalysisContext.ReportDiagnostic(CreateDiagnostic(method, catchClause.Syntax));
+                            operationBlockAnalysisContext.ReportDiagnostic(CreateDiagnostic(method, catchClause.Syntax.GetFirstToken()));
                         }
                     }
                 });
@@ -142,12 +142,7 @@ public override void VisitThrow(IThrowOperation operation)
 
             private bool IsCatchTooGeneral(ICatchClauseOperation operation)
             {
-                return IsGenericCatch(operation) || _disallowedCatchTypes.Any(type => operation.ExceptionType.Equals(type));
-            }
-
-            private static bool IsGenericCatch(ICatchClauseOperation operation)
-            {
-                return operation.ExceptionDeclarationOrExpression == null;
+                return _disallowedCatchTypes.Any(type => operation.ExceptionType.Equals(type));
             }
 
             private static bool MightBeFilteringBasedOnTheCaughtException(ICatchClauseOperation operation)