From 37cb4d6a9867be2a6eb4d1cd7f94a9b58d980216 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Wed, 5 Jun 2019 08:02:08 -0700 Subject: [PATCH] Handle catch clauses without exception variable in CA1031 Fixes #2518 Also changed the diagnostic span to be just the catch keyword instead of the entire catch block. --- .../DoNotCatchGeneralExceptionTypes.cs | 4 ++-- .../DoNotCatchGeneralExceptionTypesTests.cs | 21 +++++++++++++++++++ .../DoNotCatchCorruptedStateExceptions.cs | 6 ++---- .../DoNotCatchGeneralUnlessRethrown.cs | 11 +++------- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypes.cs b/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypes.cs index 9e7a4ca3cc..85e3a028f0 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypes.cs +++ b/src/Microsoft.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/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypesTests.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypesTests.cs index a722b0bc57..91ca35069e 100644 --- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/DoNotCatchGeneralExceptionTypesTests.cs +++ b/src/Microsoft.CodeQuality.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 index 8f1cb281df..7672b53094 100644 --- 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()); } } } \ No newline at end of file diff --git a/src/Utilities/Compiler/DoNotCatchGeneralUnlessRethrown.cs b/src/Utilities/Compiler/DoNotCatchGeneralUnlessRethrown.cs index 3d63f5916c..8b931ddea9 100644 --- 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)