Add analyzer for detecting misplaced attributes on delegates (#35779)

* Add analyzer for detecting misplaced attributes on delegates

* Address feedback from peer review

* Update IsInValidNamespace check

* Avoid using ToString in namespace comparison

* Update src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs

Co-authored-by: Pranav K <[email protected]>

* Update src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs

Co-authored-by: Pranav K <[email protected]>

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 partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
                     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)
                 {

src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs

@@ -0,0 +1,86 @@
+// 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.FirstOrDefault().Syntax is InvocationExpressionSyntax innerInvocation)
+        {
+            targetInvocation = innerInvocation;
+        }
+
+        if (lambda.Children.FirstOrDefault().Children.FirstOrDefault() is IReturnOperation returnOperation
+            && returnOperation.ReturnedValue is IInvocationOperation returnedInvocation)
+        {
+            targetInvocation = (InvocationExpressionSyntax)returnedInvocation.Syntax;
+        }
+
+        if (targetInvocation is null)
+        {
+            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 = methodSymbol.GetAttributes();
+        var location = lambda.Syntax.GetLocation();
+
+        foreach (var attribute in attributes)
+        {
+            if (IsInValidNamespace(attribute.AttributeClass?.ContainingNamespace))
+            {
+                context.ReportDiagnostic(Diagnostic.Create(
+                    DiagnosticDescriptors.DetectMisplacedLambdaAttribute,
+                    location,
+                    attribute.AttributeClass?.Name,
+                    methodSymbol.Name));
+            }
+        }
+
+        bool IsInValidNamespace(INamespaceSymbol? @namespace)
+        {
+            if (@namespace != null && [email protected])
+            {
+                // Check for Microsoft.AspNetCore in the ContainingNamespaces for this type
+                if (@namespace.Name.Equals("AspNetCore", System.StringComparison.Ordinal) && @namespace.ContainingNamespace.Name.Equals("Microsoft", System.StringComparison.Ordinal))
+                {
+                    return true;
+                }
+
+                return IsInValidNamespace(@namespace.ContainingNamespace);
+            }
+
+            return false;
+        }
+    }
+}

src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs

@@ -25,5 +25,14 @@ namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints
             DiagnosticSeverity.Warning,
             isEnabledByDefault: true,
             helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+        internal static readonly DiagnosticDescriptor DetectMisplacedLambdaAttribute = new(
+            "ASP0005",
+            "Do not place attribute on route handlers",
+            "'{0}' should be placed on the endpoint delegate to be effective",
+            "Usage",
+            DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            helpLinkUri: "https://aka.ms/aspnet/analyzers");
     }
 }

src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs

@@ -0,0 +1,240 @@
+// 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 be placed on the endpoint delegate to be effective", 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 be placed on the endpoint delegate to be effective", 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 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 be placed on the endpoint delegate to be effective", 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 be placed on the endpoint delegate to be effective", 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);
+    }
+
+    [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);
+    }
+}
+