From 41364ed3c73bc6d1b25c341bce559fe9b603c1ec Mon Sep 17 00:00:00 2001 From: Christoph Amrein Date: Wed, 17 Nov 2021 11:24:51 +0100 Subject: [PATCH 1/2] Now excluding sub-types too --- ...owsInPotentiallyAsyncMethodAnalyzerTest.cs | 20 ++++++++++++++++ .../ThrowsInPotentiallyAsyncMethodAnalyzer.cs | 23 +++++++++++-------- .../Extensions/TypeSymbolExtensions.cs | 15 ++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/ParallelHelper.Test/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzerTest.cs b/src/ParallelHelper.Test/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzerTest.cs index b09c9f2..c42ded0 100644 --- a/src/ParallelHelper.Test/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzerTest.cs +++ b/src/ParallelHelper.Test/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzerTest.cs @@ -110,5 +110,25 @@ public Task DoWorkAsync(int input) { .AddAnalyzerOption("dotnet_diagnostic.PH_S032.exclusions", "System.InvalidOperationException") .VerifyDiagnostic(); } + + [TestMethod] + public void DoesNotReportNonAsyncMethodWithAsyncSuffixAndReturningTaskThatUsesThrowsExpressionWithSubTypeOfExcludedType() { + const string source = @" +using System; +using System.Threading.Tasks; + +class Test { + private string value; + + public Task DoWorkAsync(string value) { + this.value = value ?? throw new ArgumentNullException(nameof(value)); + return Task.FromResult(value.Length); + } +}"; + CreateAnalyzerCompilationBuilder() + .AddSourceTexts(source) + .AddAnalyzerOption("dotnet_diagnostic.PH_S032.exclusions", "System.ArgumentException") + .VerifyDiagnostic(); + } } } diff --git a/src/ParallelHelper/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzer.cs b/src/ParallelHelper/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzer.cs index 4776d73..c425ac0 100644 --- a/src/ParallelHelper/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzer.cs +++ b/src/ParallelHelper/Analyzer/Smells/ThrowsInPotentiallyAsyncMethodAnalyzer.cs @@ -42,7 +42,7 @@ public class ThrowsInPotentiallyAsyncMethodAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); private const string AsyncSuffix = "Async"; - private const string DefaultExcludedTypes = "System.ArgumentException System.NotImplementedException"; + private const string DefaultExcludedBaseTypes = "System.ArgumentException System.NotImplementedException"; public override void Initialize(AnalysisContext context) { context.EnableConcurrentExecution(); @@ -73,8 +73,10 @@ public override void Analyze() { } } - private IEnumerable GetExcludedExceptionTypes() { - return Context.Options.GetConfig(Rule, "exclusions", DefaultExcludedTypes).Split(); + private IEnumerable GetExcludedExceptionBaseTypes() { + return Context.Options.GetConfig(Rule, "exclusions", DefaultExcludedBaseTypes) + .Split() + .SelectMany(SemanticModel.GetTypesByName); } private bool IsPotentiallyAsyncMethod() { @@ -89,24 +91,25 @@ private bool ReturnsTaskObject() { } private IEnumerable GetAllThrowsStatementsAndExpressionsInSameActivationFrame() { - var excludedExceptionTypes = GetExcludedExceptionTypes().ToArray(); + var excludedBaseTypes = GetExcludedExceptionBaseTypes().ToArray(); return Root.DescendantNodesInSameActivationFrame() .WithCancellation(CancellationToken) - .Where(node => IsThrowsWithoutExcludedType(node, excludedExceptionTypes)); + .Where(node => IsThrowsWithoutExcludedType(node, excludedBaseTypes)); } - private bool IsThrowsWithoutExcludedType(SyntaxNode node, IEnumerable excludedTypes) { + private bool IsThrowsWithoutExcludedType(SyntaxNode node, IEnumerable excludedBaseTypes) { + return node switch { - ThrowStatementSyntax statement => !IsAnyOfType(statement.Expression, excludedTypes), - ThrowExpressionSyntax expression => !IsAnyOfType(expression.Expression, excludedTypes), + ThrowStatementSyntax statement => !IsAnySubTypeOf(statement.Expression, excludedBaseTypes), + ThrowExpressionSyntax expression => !IsAnySubTypeOf(expression.Expression, excludedBaseTypes), _ => false }; } - private bool IsAnyOfType(SyntaxNode node, IEnumerable types) { + private bool IsAnySubTypeOf(SyntaxNode node, IEnumerable types) { var nodeType = SemanticModel.GetTypeInfo(node, CancellationToken).Type; return nodeType != null - && types.Any(type => SemanticModel.IsEqualType(nodeType, type)); + && types.Any(baseType => baseType.IsBaseTypeOf(nodeType, CancellationToken)); } } } diff --git a/src/ParallelHelper/Extensions/TypeSymbolExtensions.cs b/src/ParallelHelper/Extensions/TypeSymbolExtensions.cs index b8e4c23..caa2e1a 100644 --- a/src/ParallelHelper/Extensions/TypeSymbolExtensions.cs +++ b/src/ParallelHelper/Extensions/TypeSymbolExtensions.cs @@ -1,6 +1,7 @@ using Microsoft.CodeAnalysis; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace ParallelHelper.Extensions { /// @@ -77,5 +78,19 @@ public static IEnumerable GetAllPublicMembers(this ITypeSymbol type) { public static IEnumerable GetAllMembers(this ITypeSymbol type) { return GetAllBaseTypesAndSelf(type).SelectMany(baseType => baseType.GetMembers()); } + + /// + /// Checks if the given type is a base type of the given type. + /// + /// The base type to check. + /// The type to check if it's a sub type of the given base type. + /// A token to stop the check before completion. + /// True if the given type is a sub-type of the given base type. + /// This check does not include interfaces. + public static bool IsBaseTypeOf(this ITypeSymbol baseType, ITypeSymbol subType, CancellationToken cancellationToken) { + return subType.GetAllBaseTypesAndSelf() + .WithCancellation(cancellationToken) + .Any(type => baseType.Equals(type, SymbolEqualityComparer.Default)); + } } } From 25164dfaf613ef19936a7b91696d16aada426ecd Mon Sep 17 00:00:00 2001 From: Christoph Amrein Date: Wed, 17 Nov 2021 11:26:02 +0100 Subject: [PATCH 2/2] Added sub-type exclusion note --- doc/analyzers/PH_S032.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/analyzers/PH_S032.md b/doc/analyzers/PH_S032.md index 47ae604..8d0d289 100644 --- a/doc/analyzers/PH_S032.md +++ b/doc/analyzers/PH_S032.md @@ -12,7 +12,7 @@ Encapsulate the exceptions in a task using `Task.FromException(...)`. ## Options ```ini -# A white-space separated list of exception types to not report +# A white-space separated list of exception types to not report. The sub-types of these types will be excluded too. # Format: ... dotnet_diagnostic.PH_S032.exclusions = System.ArgumentException System.NotImplementedException ```