From 83303491636edf14ce966df124bf76f813fdc9fa Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Sat, 21 Aug 2021 21:09:33 +0000 Subject: [PATCH 1/6] Add analyzer for detecting misplaced attributes on delegates --- .../DelegateEndpointAnalyzer.cs | 2 + .../DetectMisplacedLambdaAttribute.cs | 62 +++++++ .../DiagnosticDescriptors.cs | 9 + .../DetectMisplacedLambdaAttributeTest.cs | 154 ++++++++++++++++++ 4 files changed, 227 insertions(+) create mode 100644 src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs create mode 100644 src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs index b3488d8c94e4..aca8224bf9c7 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs @@ -18,6 +18,7 @@ public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer { DiagnosticDescriptors.DoNotUseModelBindingAttributesOnDelegateEndpointParameters, DiagnosticDescriptors.DoNotReturnActionResultsFromMapActions, + DiagnosticDescriptors.DetectMisplacedLambdaAttribute }); public override void Initialize(AnalysisContext context) @@ -54,6 +55,7 @@ public override void Initialize(AnalysisContext context) var lambda = ((IAnonymousFunctionOperation)delegateCreation.Target); DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol); DisallowReturningActionResultFromMapMethods(in operationAnalysisContext, wellKnownTypes, invocation, lambda); + DetectMisplacedLambdaAttribute(operationAnalysisContext, invocation, lambda); } else if (delegateCreation.Target.Kind == OperationKind.MethodReference) { diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs new file mode 100644 index 000000000000..1babc392a2e3 --- /dev/null +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer +{ + private static void DetectMisplacedLambdaAttribute( + in OperationAnalysisContext context, + IInvocationOperation invocation, + IAnonymousFunctionOperation lambda) + { + // This analyzer will only process invocations that are immediate children of the + // AnonymousFunctionOperation provided as the delegate endpoint. We'll support checking + // for misplaced attributes in `() => Hello()` and `() => { return Hello(); }` but not in: + // () => { + // Hello(); + // return "foo"; + // } + InvocationExpressionSyntax? targetInvocation = null; + + // () => Hello() has a single child which is a BlockOperation so we check to see if + // expression associated with that operation is an invocation expression + if (lambda.Children.First().Syntax is InvocationExpressionSyntax innerInvocation) + { + targetInvocation = innerInvocation; + } + + if (lambda.Children.First().Children.First() is IReturnOperation returnOperation + && returnOperation.ReturnedValue is IInvocationOperation returnedInvocation) + + { + targetInvocation = (InvocationExpressionSyntax)returnedInvocation.Syntax; + } + + if (targetInvocation is null) + { + return; + } + + + var methodOperation = invocation.SemanticModel.GetSymbolInfo(targetInvocation); + + var attributes = methodOperation.Symbol.GetAttributes(); + var location = lambda.Syntax.GetLocation(); + + foreach (var attribute in attributes) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.DetectMisplacedLambdaAttribute, + location, + attribute.AttributeClass?.Name, + methodOperation.Symbol.Name)); + } + } +} \ No newline at end of file diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs index d9005518d1cd..44e25c76aea7 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs @@ -25,5 +25,14 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + internal static readonly DiagnosticDescriptor DetectMisplacedLambdaAttribute = new( + "ASP0005", + "Do not place attribute on invoked method", + "'{0}' should not be placed on '{1}'. Place '{0}' on the endpoint delegate instead.", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); } } diff --git a/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs b/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs new file mode 100644 index 000000000000..d6e2e57794de --- /dev/null +++ b/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs @@ -0,0 +1,154 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using Microsoft.AspNetCore.Analyzer.Testing; +using Xunit; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public partial class DetectMisplacedLambdaAttributeTest +{ + private TestDiagnosticAnalyzerRunner Runner { get; } = new(new DelegateEndpointAnalyzer()); + + [Fact] + public async Task MinimalAction_WithCorrectlyPlacedAttribute_Works() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", [Authorize] () => Hello()); +void Hello() { } +"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert + Assert.Empty(diagnostics); + } + + [Fact] + public async Task MinimalAction_WithMisplacedAttribute_ProducesDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/() => Hello()); +[Authorize] +void Hello() { } +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var diagnostic = Assert.Single(diagnostics); + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task MinimalAction_WithMisplacedAttributeAndBlockSyntax_ProducesDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/() => { return Hello(); }); +[Authorize] +string Hello() { return ""foo""; } +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var diagnostic = Assert.Single(diagnostics); + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task MinimalAction_WithMultipleMisplacedAttributes_ProducesDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/() => Hello()); +[Authorize] +[Produces(""application/xml"")] +void Hello() { } +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Collection(diagnostics, + diagnostic => { + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + }, + diagnostic => { + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'ProducesAttribute' should not be placed on 'Hello'. Place 'ProducesAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + ); + } + + [Fact] + public async Task MinimalAction_WithSingleMisplacedAttribute_ProducesDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/[Authorize]() => Hello()); +[Produces(""application/xml"")] +void Hello() { } +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Collection(diagnostics, + diagnostic => { + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'ProducesAttribute' should not be placed on 'Hello'. Place 'ProducesAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + ); + } + + [Fact] + public async Task MinimalAction_DoesNotWarnOnNonReturningMethods() + { + // Arrange + var source = @" +using System; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/() => { + Console.WriteLine(""foo""); + return ""foo""; +});"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert + Assert.Empty(diagnostics); + } +} + From 15a1dc7b3c920e4a774ae48a8ff807bdb1ac1bc5 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Fri, 27 Aug 2021 19:17:00 +0000 Subject: [PATCH 2/6] Address feedback from peer review --- .../DetectMisplacedLambdaAttribute.cs | 43 +++++++-- .../DiagnosticDescriptors.cs | 4 +- .../DetectMisplacedLambdaAttributeTest.cs | 96 ++++++++++++++++++- 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs index 1babc392a2e3..ce117c421166 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -27,14 +27,13 @@ private static void DetectMisplacedLambdaAttribute( // () => Hello() has a single child which is a BlockOperation so we check to see if // expression associated with that operation is an invocation expression - if (lambda.Children.First().Syntax is InvocationExpressionSyntax innerInvocation) + if (lambda.Children.FirstOrDefault().Syntax is InvocationExpressionSyntax innerInvocation) { targetInvocation = innerInvocation; } - if (lambda.Children.First().Children.First() is IReturnOperation returnOperation + if (lambda.Children.FirstOrDefault().Children.FirstOrDefault() is IReturnOperation returnOperation && returnOperation.ReturnedValue is IInvocationOperation returnedInvocation) - { targetInvocation = (InvocationExpressionSyntax)returnedInvocation.Syntax; } @@ -44,19 +43,43 @@ private static void DetectMisplacedLambdaAttribute( return; } - var methodOperation = invocation.SemanticModel.GetSymbolInfo(targetInvocation); + var methodSymbol = methodOperation.Symbol ?? methodOperation.CandidateSymbols.FirstOrDefault(); + + // If no method definition was found for the lambda, then abort. + if (methodSymbol is null) + { + return; + } - var attributes = methodOperation.Symbol.GetAttributes(); + var attributes = methodSymbol.GetAttributes(); var location = lambda.Syntax.GetLocation(); foreach (var attribute in attributes) { - context.ReportDiagnostic(Diagnostic.Create( - DiagnosticDescriptors.DetectMisplacedLambdaAttribute, - location, - attribute.AttributeClass?.Name, - methodOperation.Symbol.Name)); + if (IsInValidNamespace(attribute)) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.DetectMisplacedLambdaAttribute, + location, + attribute.AttributeClass?.Name, + methodSymbol.Name)); + } + } + + bool IsInValidNamespace(AttributeData attribute) + { + var supportedNamespace = "Microsoft.AspNetCore"; + var symbolDisplayFormat = new SymbolDisplayFormat( + typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces); + + if (attribute.AttributeClass is null) + { + return false; + } + + var fullyQualifiedName = attribute.AttributeClass.ContainingNamespace.ToDisplayString(symbolDisplayFormat); + return fullyQualifiedName.StartsWith(supportedNamespace, System.StringComparison.OrdinalIgnoreCase); } } } \ No newline at end of file diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs index 44e25c76aea7..396ebef19efe 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs @@ -28,8 +28,8 @@ internal static class DiagnosticDescriptors internal static readonly DiagnosticDescriptor DetectMisplacedLambdaAttribute = new( "ASP0005", - "Do not place attribute on invoked method", - "'{0}' should not be placed on '{1}'. Place '{0}' on the endpoint delegate instead.", + "Do not place attribute on route handlers", + "'{0}' should be placed on the endpoint delegate to be effective", "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, diff --git a/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs b/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs index d6e2e57794de..c25bfbcbadb4 100644 --- a/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs +++ b/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs @@ -48,7 +48,7 @@ void Hello() { } var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal("'AuthorizeAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -70,7 +70,7 @@ public async Task MinimalAction_WithMisplacedAttributeAndBlockSyntax_ProducesDia var diagnostic = Assert.Single(diagnostics); Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal("'AuthorizeAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); } [Fact] @@ -95,12 +95,12 @@ void Hello() { } diagnostic => { Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal("'AuthorizeAttribute' should not be placed on 'Hello'. Place 'AuthorizeAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal("'AuthorizeAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); }, diagnostic => { Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal("'ProducesAttribute' should not be placed on 'Hello'. Place 'ProducesAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal("'ProducesAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); } ); } @@ -126,7 +126,7 @@ void Hello() { } diagnostic => { Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); - Assert.Equal("'ProducesAttribute' should not be placed on 'Hello'. Place 'ProducesAttribute' on the endpoint delegate instead.", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + Assert.Equal("'ProducesAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); } ); } @@ -150,5 +150,91 @@ public async Task MinimalAction_DoesNotWarnOnNonReturningMethods() // Assert Assert.Empty(diagnostics); } + + [Fact] + public async Task MinimalAction_DoesNotWarnOrErrorOnNonExistantLambdas() + { + // Arrange + var source = @" +using System; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", () => ThereIsNoMethod());"; + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source); + + // Assert that there is a singal error but it is not ours (CS0103) + var diagnostic = Assert.Single(diagnostics); + Assert.Equal("CS0103", diagnostic.Descriptor.Id); + } + + [Fact] + public async Task MinimalAction_WithMisplacedAttributeOnMethodGroup_DoesNotProduceDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", Hello); +[Authorize] +void Hello() { } +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Empty(diagnostics); + } + + [Fact] + public async Task MinimalAction_WithMisplacedAttributeOnExternalReference_DoesNotProduceDiagnostics() + { + // Arrange + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +class Program { + public static void Main(string[] args) + { + var app = WebApplication.Create(); + app.MapGet(""/"", /*MM*/() => Helpers.Hello()); + } +} + +public static class Helpers +{ + [Authorize] + public static void Hello() { } +} +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + var diagnostic = Assert.Single(diagnostics); + Assert.Same(DiagnosticDescriptors.DetectMisplacedLambdaAttribute, diagnostic.Descriptor); + AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location); + Assert.Equal("'AuthorizeAttribute' should be placed on the endpoint delegate to be effective", diagnostic.GetMessage(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task MinimalAction_OutOfScope_ProducesDiagnostics() + { + // Arrange + // WriteLine has nullability annotation attributes that should + // not warn + var source = TestSource.Read(@" +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +var app = WebApplication.Create(); +app.MapGet(""/"", /*MM*/() => System.Console.WriteLine(""foo"")); +"); + // Act + var diagnostics = await Runner.GetDiagnosticsAsync(source.Source); + + // Assert + Assert.Empty(diagnostics); + } } From f86beebbaaccad38fd45473dc1695afe5a9d2a2f Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 30 Aug 2021 23:15:35 +0000 Subject: [PATCH 3/6] Update IsInValidNamespace check --- .../DetectMisplacedLambdaAttribute.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs index ce117c421166..2f4c8088c390 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -57,7 +57,7 @@ private static void DetectMisplacedLambdaAttribute( foreach (var attribute in attributes) { - if (IsInValidNamespace(attribute)) + if (IsInValidNamespace(attribute.AttributeClass?.ContainingNamespace)) { context.ReportDiagnostic(Diagnostic.Create( DiagnosticDescriptors.DetectMisplacedLambdaAttribute, @@ -67,19 +67,21 @@ private static void DetectMisplacedLambdaAttribute( } } - bool IsInValidNamespace(AttributeData attribute) + static bool IsInValidNamespace(INamespaceSymbol? containingNamespace) { var supportedNamespace = "Microsoft.AspNetCore"; - var symbolDisplayFormat = new SymbolDisplayFormat( - typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces); - if (attribute.AttributeClass is null) + if (containingNamespace != null) { - return false; + if (containingNamespace.ToString().Equals(supportedNamespace, System.StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + return IsInValidNamespace(containingNamespace.ContainingNamespace); } - var fullyQualifiedName = attribute.AttributeClass.ContainingNamespace.ToDisplayString(symbolDisplayFormat); - return fullyQualifiedName.StartsWith(supportedNamespace, System.StringComparison.OrdinalIgnoreCase); + return false; } } } \ No newline at end of file From b65f1a8c87691b70b8f8a3c1e4646ccbfb040017 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 31 Aug 2021 20:12:48 +0000 Subject: [PATCH 4/6] Avoid using ToString in namespace comparison --- .../DetectMisplacedLambdaAttribute.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs index 2f4c8088c390..569031a65b1c 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -67,18 +67,17 @@ private static void DetectMisplacedLambdaAttribute( } } - static bool IsInValidNamespace(INamespaceSymbol? containingNamespace) + bool IsInValidNamespace(INamespaceSymbol? @namespace) { - var supportedNamespace = "Microsoft.AspNetCore"; - - if (containingNamespace != null) + if (@namespace != null && !@namespace.IsGlobalNamespace) { - if (containingNamespace.ToString().Equals(supportedNamespace, System.StringComparison.OrdinalIgnoreCase)) + // Check for Microsoft.AspNetCore in the ContainingNamespaces for this type + if (@namespace.Name.Equals("AspNetCore") && @namespace.ContainingNamespace.Name.Equals("Microsoft")) { return true; } - return IsInValidNamespace(containingNamespace.ContainingNamespace); + return IsInValidNamespace(@namespace.ContainingNamespace); } return false; From a0dab23ced13ef59530436c4628a60ab5f06529d Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 31 Aug 2021 14:07:37 -0700 Subject: [PATCH 5/6] Update src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs Co-authored-by: Pranav K --- .../src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs index 569031a65b1c..52d3f4b7a3b4 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -72,7 +72,7 @@ bool IsInValidNamespace(INamespaceSymbol? @namespace) if (@namespace != null && !@namespace.IsGlobalNamespace) { // Check for Microsoft.AspNetCore in the ContainingNamespaces for this type - if (@namespace.Name.Equals("AspNetCore") && @namespace.ContainingNamespace.Name.Equals("Microsoft")) + if (@namespace.Name.Equals("AspNetCore", StringComparison.Ordinal) && @namespace.ContainingNamespace.Name.Equals("Microsoft", StringComparison.Ordinal)) { return true; } From 8fc6816fdbbbd814887e03627c710898d0eae196 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 31 Aug 2021 14:20:38 -0700 Subject: [PATCH 6/6] Update src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs --- .../src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs index 52d3f4b7a3b4..cb5d66041056 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs @@ -72,7 +72,7 @@ bool IsInValidNamespace(INamespaceSymbol? @namespace) if (@namespace != null && !@namespace.IsGlobalNamespace) { // Check for Microsoft.AspNetCore in the ContainingNamespaces for this type - if (@namespace.Name.Equals("AspNetCore", StringComparison.Ordinal) && @namespace.ContainingNamespace.Name.Equals("Microsoft", StringComparison.Ordinal)) + if (@namespace.Name.Equals("AspNetCore", System.StringComparison.Ordinal) && @namespace.ContainingNamespace.Name.Equals("Microsoft", System.StringComparison.Ordinal)) { return true; }