diff --git a/eng/Dependencies.props b/eng/Dependencies.props index b78ad89a9e3a..f01beada1fa9 100644 --- a/eng/Dependencies.props +++ b/eng/Dependencies.props @@ -191,6 +191,7 @@ and are generated based on the last package release. + diff --git a/eng/Versions.props b/eng/Versions.props index 92ed3147c45b..f3ab73436a65 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -254,6 +254,7 @@ 1.4.0 4.0.0 2.2.4 + 4.5.4 3.1.1 6.1.5 2.0.3 diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs b/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs index b3488d8c94e4..26c2d1b40ce3 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 override void Initialize(AnalysisContext context) 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 d9005518d1cd..3e2702cb88a4 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs @@ -25,5 +25,23 @@ internal static class DiagnosticDescriptors 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 000000000000..d98b2cc0b1ce --- /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 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 _routeTemplate; + + public RouteTokenEnumerator(string routeTemplateString) + { + _routeTemplate = routeTemplateString.AsSpan(); + Current = default; + } + + public ReadOnlySpan 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 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 040e511ad666..4536f3e866a3 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs +++ b/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs @@ -49,6 +49,12 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out We 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 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out We IResult = iResult, IActionResult = iActionResult, IConvertToActionResult = iConvertToActionResult, + IFromRouteMetadata = iFromRouteMetadata, }; return true; @@ -68,4 +75,5 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out We 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 e8be68688958..a9964836e4a1 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 @@ - + diff --git a/src/Framework/Analyzer/test/MinimalActions/RouteAttributeMismatchTest.cs b/src/Framework/Analyzer/test/MinimalActions/RouteAttributeMismatchTest.cs new file mode 100644 index 000000000000..007ee13604d9 --- /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(); + + // 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); + }); + } +}