From bb4069e8d8d36d581fda6e61c19be1a5b426b2c3 Mon Sep 17 00:00:00 2001 From: Evangelink Date: Fri, 7 Aug 2020 15:31:57 +0200 Subject: [PATCH] CA1024: Do not report on tasks return types --- .../UsePropertiesWhereAppropriate.cs | 82 +++++++++++-------- .../UsePropertiesWhereAppropriateTests.cs | 39 +++++++++ 2 files changed, 88 insertions(+), 33 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriate.cs b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriate.cs index 8aab7ef31e..972602ce61 100644 --- a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriate.cs +++ b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriate.cs @@ -42,47 +42,63 @@ public override void Initialize(AnalysisContext analysisContext) analysisContext.EnableConcurrentExecution(); analysisContext.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - analysisContext.RegisterOperationBlockStartAction(context => + analysisContext.RegisterCompilationStartAction(context => { + var taskTypesBuilder = ImmutableHashSet.CreateBuilder(); - if (!(context.OwningSymbol is IMethodSymbol methodSymbol) || - methodSymbol.ReturnsVoid || - methodSymbol.ReturnType.Kind == SymbolKind.ArrayType || - !methodSymbol.Parameters.IsEmpty || - !methodSymbol.MatchesConfiguredVisibility(context.Options, Rule, context.Compilation, context.CancellationToken) || - methodSymbol.IsAccessorMethod() || - !IsPropertyLikeName(methodSymbol.Name)) - { - return; - } + taskTypesBuilder.AddIfNotNull( + context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask)); + taskTypesBuilder.AddIfNotNull( + context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask1)); + taskTypesBuilder.AddIfNotNull( + context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksValueTask)); + taskTypesBuilder.AddIfNotNull( + context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksValueTask1)); - // A few additional checks to reduce the noise for this diagnostic: - // Ensure that the method is non-generic, non-virtual/override, has no overloads and doesn't have special names: 'GetHashCode' or 'GetEnumerator'. - // Also avoid generating this diagnostic if the method body has any invocation expressions. - // Also avoid implicit interface implementation (explicit are handled through the member accessibility) - if (methodSymbol.IsGenericMethod || - methodSymbol.IsVirtual || - methodSymbol.IsOverride || - methodSymbol.ContainingType.GetMembers(methodSymbol.Name).Length > 1 || - methodSymbol.Name == GetHashCodeName || - methodSymbol.Name == GetEnumeratorName || - methodSymbol.IsImplementationOfAnyImplicitInterfaceMember()) - { - return; - } + var taskTypes = taskTypesBuilder.ToImmutable(); - bool hasInvocations = false; - context.RegisterOperationAction(operationContext => + context.RegisterOperationBlockStartAction(context => { - hasInvocations = true; - }, OperationKind.Invocation); + if (!(context.OwningSymbol is IMethodSymbol methodSymbol) || + methodSymbol.ReturnsVoid || + methodSymbol.ReturnType.Kind == SymbolKind.ArrayType || + !methodSymbol.Parameters.IsEmpty || + !methodSymbol.MatchesConfiguredVisibility(context.Options, Rule, context.Compilation, context.CancellationToken) || + methodSymbol.IsAccessorMethod() || + !IsPropertyLikeName(methodSymbol.Name)) + { + return; + } - context.RegisterOperationBlockEndAction(endContext => - { - if (!hasInvocations) + // A few additional checks to reduce the noise for this diagnostic: + // Ensure that the method is non-generic, non-virtual/override, has no overloads and doesn't have special names: 'GetHashCode' or 'GetEnumerator'. + // Also avoid generating this diagnostic if the method body has any invocation expressions. + // Also avoid implicit interface implementation (explicit are handled through the member accessibility) + if (methodSymbol.IsGenericMethod || + methodSymbol.IsVirtual || + methodSymbol.IsOverride || + methodSymbol.Name == GetHashCodeName || + methodSymbol.Name == GetEnumeratorName || + methodSymbol.ContainingType.GetMembers(methodSymbol.Name).Length > 1 || + taskTypes.Contains(methodSymbol.ReturnType.OriginalDefinition) || + methodSymbol.IsImplementationOfAnyImplicitInterfaceMember()) { - endContext.ReportDiagnostic(endContext.OwningSymbol.CreateDiagnostic(Rule)); + return; } + + bool hasInvocations = false; + context.RegisterOperationAction(operationContext => + { + hasInvocations = true; + }, OperationKind.Invocation); + + context.RegisterOperationBlockEndAction(endContext => + { + if (!hasInvocations) + { + endContext.ReportDiagnostic(endContext.OwningSymbol.CreateDiagnostic(Rule)); + } + }); }); }); } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriateTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriateTests.cs index ac31cd7e45..a123583892 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriateTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/UsePropertiesWhereAppropriateTests.cs @@ -439,6 +439,45 @@ public object GetContent() "); } + [Fact, WorkItem(3877, "https://github.com/dotnet/roslyn-analyzers/issues/3877")] + public async Task CA1024_ReturnsTask_NoDiagnostic() + { + await VerifyCS.VerifyAnalyzerAsync(@" +using System.Threading.Tasks; + +public class Something +{ + public Task GetTask() => default(Task); + public Task GetGenericTask() => default(Task); + + public ValueTask GetValueTask() => default(ValueTask); + public ValueTask GetGenericValueTask() => default(ValueTask); +} +"); + + await VerifyVB.VerifyAnalyzerAsync(@" +Imports System.Threading.Tasks + +Public Class Something + Public Function GetTask() As Task + Return Nothing + End Function + + Public Function GetGenericTask() As Task(Of Integer) + Return Nothing + End Function + + Public Function GetValueTask() As ValueTask + Return Nothing + End Function + + Public Function GetGenericValueTask() As ValueTask(Of Integer) + Return Nothing + End Function +End Class +"); + } + private static DiagnosticResult GetCA1024CSharpResultAt(int line, int column, string methodName) => VerifyCS.Diagnostic() .WithLocation(line, column)