Complain if a route parameter is not used or cannot be bound.prkrishn/route-analyzer

8 files changed, 336 insertions, 1 deletions

diff --git a/eng/Dependencies.props b/eng/Dependencies.props
index b78ad89a9e..f01beada1f 100644
--- a/eng/Dependencies.props
+++ b/eng/Dependencies.props
@@ -191,6 +191,7 @@ and are generated based on the last package release.
     <LatestPackageReference Include="Serilog.Extensions.Logging" />
     <LatestPackageReference Include="Serilog.Sinks.File" />
     <LatestPackageReference Include="StackExchange.Redis" />
+    <LatestPackageReference Include="System.Memory" />
     <LatestPackageReference Include="System.Reactive.Linq" />
     <LatestPackageReference Include="xunit.abstractions" />
     <LatestPackageReference Include="xunit.analyzers" />
diff --git a/eng/Versions.props b/eng/Versions.props
index 92ed3147c4..f3ab73436a 100644
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -254,6 +254,7 @@
     <SerilogExtensionsLoggingVersion>1.4.0</SerilogExtensionsLoggingVersion>
     <SerilogSinksFileVersion>4.0.0</SerilogSinksFileVersion>
     <StackExchangeRedisVersion>2.2.4</StackExchangeRedisVersion>
+    <SystemMemoryVersion>4.5.4</SystemMemoryVersion>
     <SystemReactiveLinqVersion>3.1.1</SystemReactiveLinqVersion>
     <SwashbuckleAspNetCoreVersion>6.1.5</SwashbuckleAspNetCoreVersion>
     <XunitAbstractionsVersion>2.0.3</XunitAbstractionsVersion>
diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs
index b3488d8c94..26c2d1b40c 100644
--- a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs
+++ b/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs
@@ -18,6 +18,8 @@ public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
     {
         DiagnosticDescriptors.DoNotUseModelBindingAttributesOnDelegateEndpointParameters,
         DiagnosticDescriptors.DoNotReturnActionResultsFromMapActions,
+        DiagnosticDescriptors.RouteValueIsUnused,
+        DiagnosticDescriptors.RouteParameterCannotBeBound,
     });
 
     public override void Initialize(AnalysisContext context)
@@ -54,6 +56,7 @@ public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
                     var lambda = ((IAnonymousFunctionOperation)delegateCreation.Target);
                     DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol);
                     DisallowReturningActionResultFromMapMethods(in operationAnalysisContext, wellKnownTypes, invocation, lambda);
+                    RouteAttributeMismatch(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol);
                 }
                 else if (delegateCreation.Target.Kind == OperationKind.MethodReference)
                 {
diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs
index d9005518d1..3e2702cb88 100644
--- a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs
+++ b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs
@@ -25,5 +25,23 @@ namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints
             DiagnosticSeverity.Warning,
             isEnabledByDefault: true,
             helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+        internal static readonly DiagnosticDescriptor RouteValueIsUnused = new(
+            "ASP0005",
+            "Route value is unused",
+            "The route value '{0}' does not get bound and can be removed",
+            "Usage",
+            DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            helpLinkUri: "https://aka.ms/minimal-action/analyzer");
+
+        internal static readonly DiagnosticDescriptor RouteParameterCannotBeBound = new(
+            "ASP0006",
+            "Route parameter is not bound",
+            "Route parameter does not have a corresponding route token and cannot be bound",
+            "Usage",
+            DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            helpLinkUri: "https://aka.ms/minimal-action/analyzer");
     }
 }
diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/RouteAttributeMismatch.cs b/src/Framework/Analyzer/src/DelegateEndpoints/RouteAttributeMismatch.cs
new file mode 100644
index 0000000000..d98b2cc0b1
--- /dev/null
+++ b/src/Framework/Analyzer/src/DelegateEndpoints/RouteAttributeMismatch.cs
@@ -0,0 +1,167 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints;
+
+public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
+{
+    private static void RouteAttributeMismatch(
+        in OperationAnalysisContext context,
+        WellKnownTypes wellKnownTypes,
+        IInvocationOperation mapInvocation,
+        IMethodSymbol mapDelegate)
+    {
+        var value = mapInvocation.Arguments[1].Value;
+        if (value.ConstantValue is not { HasValue: true } constant ||
+            constant.Value is not string routeTemplate)
+        {
+            return;
+        }
+
+        var fromRouteParameters = GetFromRouteParameters(wellKnownTypes, mapDelegate.Parameters);
+
+        var enumerator = new RouteTokenEnumerator(routeTemplate);
+
+        while (enumerator.MoveNext())
+        {
+            var bound = false;
+            for (var i = fromRouteParameters.Length - 1; i >= 0; i--)
+            {
+                if (enumerator.Current.Equals(fromRouteParameters[i].RouteName.AsSpan(), StringComparison.Ordinal))
+                {
+                    fromRouteParameters = fromRouteParameters.RemoveAt(i);
+                    bound = true;
+                }
+            }
+
+            if (!bound)
+            {
+                // If we didn't bind to an explicit FromRoute parameter, look for
+                // an implicit one.
+                foreach (var parameter in mapDelegate.Parameters)
+                {
+                    if (enumerator.Current.Equals(parameter.Name.AsSpan(), StringComparison.Ordinal))
+                    {
+                        bound = true;
+                    }
+                }
+            }
+
+            if (!bound)
+            {
+                context.ReportDiagnostic(Diagnostic.Create(
+                    DiagnosticDescriptors.RouteValueIsUnused,
+                    mapInvocation.Arguments[1].Syntax.GetLocation(),
+                    enumerator.Current.ToString()));
+            }
+        }
+
+        // Report diagnostics for any FromRoute parameter that does is not represented in the route template.
+        foreach (var parameter in fromRouteParameters)
+        {
+            context.ReportDiagnostic(Diagnostic.Create(
+                DiagnosticDescriptors.RouteParameterCannotBeBound,
+                parameter.Parameter.Locations.FirstOrDefault(),
+                enumerator.Current.ToString()));
+        }
+    }
+
+    private static ImmutableArray<(IParameterSymbol Parameter, string RouteName)> GetFromRouteParameters(
+        WellKnownTypes wellKnownTypes, ImmutableArray<IParameterSymbol> parameters)
+    {
+        var result = ImmutableArray<(IParameterSymbol, string)>.Empty;
+        foreach (var parameter in parameters)
+        {
+            var attribute = parameter.GetAttributes(wellKnownTypes.IFromRouteMetadata).FirstOrDefault();
+            if (attribute is not null)
+            {
+                var fromRouteName = parameter.Name;
+                var nameProperty = attribute.NamedArguments.FirstOrDefault(static f => f.Key == "Name");
+                if (nameProperty.Value is { IsNull: false, Type: { SpecialType: SpecialType.System_String } })
+                {
+                    fromRouteName = (string)nameProperty.Value.Value;
+                }
+
+                result = result.Add((parameter, fromRouteName));
+            }
+        }
+
+        return result;
+    }
+
+    internal ref struct RouteTokenEnumerator
+    {
+        private ReadOnlySpan<char> _routeTemplate;
+
+        public RouteTokenEnumerator(string routeTemplateString)
+        {
+            _routeTemplate = routeTemplateString.AsSpan();
+            Current = default;
+        }
+
+        public ReadOnlySpan<char> Current { get; private set; }
+
+        public bool MoveNext()
+        {
+            if (_routeTemplate.IsEmpty)
+            {
+                return false;
+            }
+
+            findStartBrace:
+            var startIndex = _routeTemplate.IndexOf('{');
+            if (startIndex == -1)
+            {
+                return false;
+            }
+
+            if (startIndex < _routeTemplate.Length - 1 && _routeTemplate[startIndex + 1] == '{')
+            {
+                // Escaped sequence
+                _routeTemplate = _routeTemplate.Slice(startIndex + 1);
+                goto findStartBrace;
+            }
+
+            var tokenStart = startIndex + 1;
+
+            findEndBrace:
+            var endIndex = IndexOf(_routeTemplate, tokenStart, '}');
+            if (endIndex == -1)
+            {
+                return false;
+            }
+            if (endIndex < _routeTemplate.Length - 1 && _routeTemplate[endIndex + 1] == '}')
+            {
+                tokenStart = endIndex + 2;
+                goto findEndBrace;
+            }
+
+            var token = _routeTemplate.Slice(startIndex + 1, endIndex - startIndex - 1);
+            var qualifier = token.IndexOfAny(new[] { ':', '=' });
+            Current = qualifier == -1 ? token : token.Slice(0, qualifier);
+
+            _routeTemplate = _routeTemplate.Slice(endIndex + 1);
+            return true;
+        }
+
+        private static int IndexOf(ReadOnlySpan<char> span, int startIndex, char c)
+        {
+            for (var i = startIndex; i < span.Length; i++)
+            {
+                if (span[i] == c)
+                {
+                    return i;
+                }
+            }
+
+            return -1;
+        }
+    }
+}
diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs b/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs
index 040e511ad6..4536f3e866 100644
--- a/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs
+++ b/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs
@@ -49,6 +49,12 @@ internal sealed class WellKnownTypes
             return false;
         }
 
+        const string IFromRouteMetadata = "Microsoft.AspNetCore.Http.Metadata.IFromRouteMetadata";
+        if (compilation.GetTypeByMetadataName(IFromRouteMetadata) is not { } iFromRouteMetadata)
+        {
+            return false;
+        }
+
         wellKnownTypes = new WellKnownTypes
         {
             DelegateEndpointRouteBuilderExtensions = delegateEndpointRouteBuilderExtensions,
@@ -57,6 +63,7 @@ internal sealed class WellKnownTypes
             IResult = iResult,
             IActionResult = iActionResult,
             IConvertToActionResult = iConvertToActionResult,
+            IFromRouteMetadata = iFromRouteMetadata,
         };
 
         return true;
@@ -68,4 +75,5 @@ internal sealed class WellKnownTypes
     public INamedTypeSymbol IResult { get; private init; }
     public INamedTypeSymbol IActionResult { get; private init; }
     public INamedTypeSymbol IConvertToActionResult { get; private init; }
+    public INamedTypeSymbol IFromRouteMetadata { get; private init; }
 }
diff --git a/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj b/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj
index e8be686889..a9964836e4 100644
--- a/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj
+++ b/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj
@@ -11,7 +11,7 @@
 
   <ItemGroup>
     <Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="All" />
-
+    <Reference Include="System.Memory" />
     <InternalsVisibleTo Include="Microsoft.AspNetCore.App.Analyzers.Test" />
   </ItemGroup>
 
diff --git a/src/Framework/Analyzer/test/MinimalActions/RouteAttributeMismatchTest.cs b/src/Framework/Analyzer/test/MinimalActions/RouteAttributeMismatchTest.cs
new file mode 100644
index 0000000000..007ee13604
--- /dev/null
+++ b/src/Framework/Analyzer/test/MinimalActions/RouteAttributeMismatchTest.cs
@@ -0,0 +1,137 @@
+// 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 class RouteAttributeMismatchTest
+{
+    private TestDiagnosticAnalyzerRunner Runner { get; } = new(new DelegateEndpointAnalyzer());
+
+    [Theory]
+    [InlineData("{id}", new[] { "id" })]
+    [InlineData("{category}/product/{group}", new[] { "category", "group" })]
+    [InlineData("{category:int}/product/{group:range(10, 20)}?", new[] { "category", "group" })]
+    [InlineData("{person:int}/{ssn:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)}", new[] { "person", "ssn" })]
+    [InlineData("{area=Home}/{controller:required}/{id=0:int}", new[] { "area", "controller", "id" })]
+    public void RouteTokenizer_Works_ForSimpleRouteTemplates(string template, string[] expected)
+    {
+        // Arrange
+        var tokenizer = new DelegateEndpointAnalyzer.RouteTokenEnumerator(template);
+        var actual = new List<string>();
+
+        // Act
+        while (tokenizer.MoveNext())
+        {
+            actual.Add(tokenizer.Current.ToString());
+        }
+
+        // Assert
+        Assert.Equal(expected, actual);
+    }
+
+    [Fact]
+    public async Task MinimalAction_UnusedRouteValueProducesDiagnostics()
+    {
+        // Arrange
+        var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+var webApp = WebApplication.Create();
+webApp.MapPost(/*MM*/""/{id}"", () => {});
+");
+        // Act
+        var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+
+        // Assert
+        var diagnostic = Assert.Single(diagnostics);
+        Assert.Same(DiagnosticDescriptors.RouteValueIsUnused, diagnostic.Descriptor);
+        Assert.Equal($"The route value 'id' does not get bound and can be removed", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+        AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+    }
+
+    [Fact]
+    public async Task MinimalAction_SomeUnusedTokens_ProducesDiagnostics()
+    {
+        // Arrange
+        var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+var webApp = WebApplication.Create();
+webApp.MapPost(/*MM*/""/{id:int}/{location:alpha}"", (int id, string loc) => {});
+");
+        // Act
+        var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+
+        // Assert
+        var diagnostic = Assert.Single(diagnostics);
+        Assert.Same(DiagnosticDescriptors.RouteValueIsUnused, diagnostic.Descriptor);
+        Assert.Equal($"The route value 'location' does not get bound and can be removed", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+        AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+    }
+
+    [Fact]
+    public async Task MinimalAction_FromRouteParameterWithMatchingToken_Works()
+    {
+        // Arrange
+        var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+var webApp = WebApplication.Create();
+webApp.MapPost(/*MM*/""/{id:int}"", ([FromRoute] int id, string loc) => {});
+");
+        // Act
+        var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+
+        // Assert
+        Assert.Empty(diagnostics);
+    }
+
+    [Fact]
+    public async Task MinimalAction_FromRouteParameterUsingName_Works()
+    {
+        // Arrange
+        var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+var webApp = WebApplication.Create();
+webApp.MapPost(/*MM*/""/{userId}"", ([FromRoute(Name = ""userId"")] int id) => {});
+");
+        // Act
+        var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+
+        // Assert
+        Assert.Empty(diagnostics);
+    }
+
+    [Fact]
+    public async Task MinimalAction_FromRouteParameterWithNoMatchingToken_ProducesDiagnostics()
+    {
+        // Arrange
+        var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+var webApp = WebApplication.Create();
+webApp.MapPost(/*MM1*/""/{userId:int}"", ([FromRoute] int /*MM2*/id, string loc) => {});
+");
+        // Act
+        var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+
+        // Assert
+        Assert.Collection(
+            diagnostics.OrderBy(d => d.Descriptor.Id),
+            diagnostic =>
+            {
+                Assert.Same(DiagnosticDescriptors.RouteValueIsUnused, diagnostic.Descriptor);
+                AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic.Location);
+            },
+            diagnostic =>
+            {
+                Assert.Same(DiagnosticDescriptors.RouteParameterCannotBeBound, diagnostic.Descriptor);
+                AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM2"], diagnostic.Location);
+            });
+    }
+}