Add ambiguous route analyzer (dotnet#45582)

Co-authored-by: Youssef Victor <[email protected]>

src/Framework/AspNetCoreAnalyzers/samples/WebAppSample/Program.cs

@@ -17,8 +17,8 @@
 });
 
 app.MapGet("/posts/{**rest}", (string rest) => $"Routing to {rest}");
-app.MapGet("/todos/{id}", (int id) => db.Todos.Find(id));
-app.MapGet("/todos/{text}", (string text) => db.Todos.Where(t => t.Text.Contains(text)));
+app.MapGet("/todos/{id:int}", (int id) => db.Todos.Find(id));
+app.MapGet("/todos/{text:alpha}", (string text) => db.Todos.Where(t => t.Text.Contains(text)));
 app.MapGet("/posts/{slug:regex(^[a-z0-9_-]+$)}", (string slug) =>
 {
     return $"Match slug: {Regex.IsMatch(slug, "[a-z0-9_-]+")}";

src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs

@@ -178,4 +178,13 @@ internal static class DiagnosticDescriptors
         DiagnosticSeverity.Error,
         isEnabledByDefault: true,
         helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+    internal static readonly DiagnosticDescriptor AmbiguousRouteHandlerRoute = new(
+        "ASP0022",
+        "Route conflict detected between route handlers",
+        "Route '{0}' conflicts with another handler route. An HTTP request that matches multiple routes results in an ambiguous match error. Fix the conflict by changing the route's pattern, HTTP method, or route constraints.",
+        "Usage",
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        helpLinkUri: "https://aka.ms/aspnet/analyzers");
 }

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/AmbiguousRoutePatternComparer.cs

@@ -0,0 +1,158 @@
+// 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.Generic;
+using System.Linq;
+using Microsoft.AspNetCore.Analyzers.Infrastructure.RoutePattern;
+
+namespace Microsoft.AspNetCore.Analyzers.Infrastructure;
+
+/// <summary>
+/// This route pattern comparer checks to see if two route patterns match the same URL and create ambiguous match exceptions at runtime.
+/// It doesn't check two routes exactly equal each other. For example, "/product/{id}" and "/product/{name}" aren't exactly equal but will match the same URL.
+/// </summary>
+internal sealed class AmbiguousRoutePatternComparer : IEqualityComparer<RoutePatternTree>
+{
+    public static AmbiguousRoutePatternComparer Instance { get; } = new();
+
+    public bool Equals(RoutePatternTree x, RoutePatternTree y)
+    {
+        if (x.Root.Parts.Length != y.Root.Parts.Length)
+        {
+            return false;
+        }
+
+        for (var i = 0; i < x.Root.Parts.Length; i++)
+        {
+            var xPart = x.Root.Parts[i];
+            var yPart = y.Root.Parts[i];
+
+            var equal = xPart switch
+            {
+                RoutePatternSegmentSeparatorNode _ => yPart is RoutePatternSegmentSeparatorNode,
+                RoutePatternSegmentNode xSegment => Equals(xSegment, yPart as RoutePatternSegmentNode),
+                _ => throw new InvalidOperationException($"Unexpected part type '{xPart.Kind}'."),
+            };
+
+            if (!equal)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private static bool Equals(RoutePatternSegmentNode x, RoutePatternSegmentNode? y)
+    {
+        if (y is null)
+        {
+            return false;
+        }
+
+        if (x.Children.Length != y.Children.Length)
+        {
+            return false;
+        }
+
+        for (var i = 0; i < x.Children.Length; i++)
+        {
+            var xChild = x.Children[i];
+            var yChild = y.Children[i];
+
+            var equal = xChild switch
+            {
+                RoutePatternOptionalSeparatorNode _ => yChild is RoutePatternOptionalSeparatorNode,
+                RoutePatternReplacementNode xReplacement => yChild is RoutePatternReplacementNode yReplacement && IgnoreCaseEquals(xReplacement.TextToken.Value, yReplacement.TextToken.Value),
+                RoutePatternLiteralNode xLiteral => yChild is RoutePatternLiteralNode yLiteral && IgnoreCaseEquals(xLiteral.LiteralToken.Value, yLiteral.LiteralToken.Value),
+                RoutePatternParameterNode xParameter => Equals(xParameter, yChild as RoutePatternParameterNode),
+                _ => throw new InvalidOperationException($"Unexpected segment node type '{xChild.Kind}'."),
+            };
+
+            if (!equal)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private static bool IgnoreCaseEquals(object? value1, object? value2)
+    {
+        var s1 = value1 as string;
+        var s2 = value2 as string;
+
+        if (s1 is null || s2 is null)
+        {
+            return false;
+        }
+
+        return string.Equals(s1, s2, StringComparison.OrdinalIgnoreCase);
+    }
+
+    private static bool Equals(RoutePatternParameterNode x, RoutePatternParameterNode? y)
+    {
+        if (y is null)
+        {
+            return false;
+        }
+
+        // Only parameter policies differentiate between parameters.
+        var xParameterPolicies = x.ParameterParts.Where(p => p.Kind == RoutePatternKind.ParameterPolicy).OfType<RoutePatternPolicyParameterPartNode>().ToList();
+        var yParameterPolicies = y.ParameterParts.Where(p => p.Kind == RoutePatternKind.ParameterPolicy).OfType<RoutePatternPolicyParameterPartNode>().ToList();
+
+        if (xParameterPolicies.Count != yParameterPolicies.Count)
+        {
+            return false;
+        }
+
+        for (var i = 0; i < xParameterPolicies.Count; i++)
+        {
+            var xPolicy = xParameterPolicies[i];
+            var yPolicy = yParameterPolicies[i];
+
+            if (!Equals(xPolicy, yPolicy))
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private static bool Equals(RoutePatternPolicyParameterPartNode x, RoutePatternPolicyParameterPartNode y)
+    {
+        if (x.PolicyFragments.Length != y.PolicyFragments.Length)
+        {
+            return false;
+        }
+
+        for (var i = 0; i < x.PolicyFragments.Length; i++)
+        {
+            var xPart = x.PolicyFragments[i];
+            var yPart = y.PolicyFragments[i];
+
+            var equal = xPart switch
+            {
+                RoutePatternPolicyFragment xFragment => yPart is RoutePatternPolicyFragment yFragment && Equals(xFragment.ArgumentToken.Value, yFragment.ArgumentToken.Value),
+                RoutePatternPolicyFragmentEscapedNode xFragmentEscaped => yPart is RoutePatternPolicyFragmentEscapedNode yFragmentEscaped && Equals(xFragmentEscaped.ArgumentToken.Value, yFragmentEscaped.ArgumentToken.Value),
+                _ => throw new InvalidOperationException($"Unexpected policy node type '{xPart.Kind}'."),
+            };
+
+            if (!equal)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    public int GetHashCode(RoutePatternTree obj)
+    {
+        // TODO: Improve hash code calculation. This is rudimentary and will generate a lot of collisions.
+        return obj.Root.ChildCount;
+    }
+}

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/IRoutePatternNodeVisitor.cs

@@ -10,8 +10,8 @@ internal interface IRoutePatternNodeVisitor
     void Visit(RoutePatternReplacementNode node);
     void Visit(RoutePatternParameterNode node);
     void Visit(RoutePatternLiteralNode node);
-    void Visit(RoutePatternSegmentSeperatorNode node);
-    void Visit(RoutePatternOptionalSeperatorNode node);
+    void Visit(RoutePatternSegmentSeparatorNode node);
+    void Visit(RoutePatternOptionalSeparatorNode node);
     void Visit(RoutePatternCatchAllParameterPartNode node);
     void Visit(RoutePatternNameParameterPartNode node);
     void Visit(RoutePatternPolicyParameterPartNode node);

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternKind.cs

@@ -9,7 +9,7 @@ internal enum RoutePatternKind
     EndOfFile,
     Segment,
     CompilationUnit,
-    Seperator,
+    Separator,
     Literal,
     Replacement,
     Parameter,

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternLexer.cs

@@ -102,7 +102,7 @@ private bool TextAt(int position, string val)
 
             if (ch.Value == '/')
             {
-                // Literal ends at a seperator or start of a parameter.
+                // Literal ends at a separator or start of a parameter.
                 break;
             }
             else if (ch.Value == '{' && IsUnescapedChar(ref Position, '{'))

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternNodes.cs

@@ -43,11 +43,11 @@ public override void Accept(IRoutePatternNodeVisitor visitor)
 
 internal sealed class RoutePatternSegmentNode : RoutePatternRootPartNode
 {
-    public ImmutableArray<RoutePatternNode> Children { get; }
+    public ImmutableArray<RoutePatternSegmentPartNode> Children { get; }
 
     internal override int ChildCount => Children.Length;
 
-    public RoutePatternSegmentNode(ImmutableArray<RoutePatternNode> children)
+    public RoutePatternSegmentNode(ImmutableArray<RoutePatternSegmentPartNode> children)
         : base(RoutePatternKind.Segment)
     {
         Children = children;
@@ -162,47 +162,47 @@ public override void Accept(IRoutePatternNodeVisitor visitor)
         => visitor.Visit(this);
 }
 
-internal sealed class RoutePatternOptionalSeperatorNode : RoutePatternSegmentPartNode
+internal sealed class RoutePatternOptionalSeparatorNode : RoutePatternSegmentPartNode
 {
-    public RoutePatternOptionalSeperatorNode(RoutePatternToken seperatorToken)
-        : base(RoutePatternKind.Seperator)
+    public RoutePatternOptionalSeparatorNode(RoutePatternToken separatorToken)
+        : base(RoutePatternKind.Separator)
     {
-        Debug.Assert(seperatorToken.Kind == RoutePatternKind.DotToken);
-        SeperatorToken = seperatorToken;
+        Debug.Assert(separatorToken.Kind == RoutePatternKind.DotToken);
+        SeparatorToken = separatorToken;
     }
 
-    public RoutePatternToken SeperatorToken { get; }
+    public RoutePatternToken SeparatorToken { get; }
 
     internal override int ChildCount => 1;
 
     internal override RoutePatternNodeOrToken ChildAt(int index)
         => index switch
         {
-            0 => SeperatorToken,
+            0 => SeparatorToken,
             _ => throw new InvalidOperationException(),
         };
 
     public override void Accept(IRoutePatternNodeVisitor visitor)
         => visitor.Visit(this);
 }
 
-internal sealed class RoutePatternSegmentSeperatorNode : RoutePatternRootPartNode
+internal sealed class RoutePatternSegmentSeparatorNode : RoutePatternRootPartNode
 {
-    public RoutePatternSegmentSeperatorNode(RoutePatternToken seperatorToken)
-        : base(RoutePatternKind.Seperator)
+    public RoutePatternSegmentSeparatorNode(RoutePatternToken separatorToken)
+        : base(RoutePatternKind.Separator)
     {
-        Debug.Assert(seperatorToken.Kind == RoutePatternKind.SlashToken);
-        SeperatorToken = seperatorToken;
+        Debug.Assert(separatorToken.Kind == RoutePatternKind.SlashToken);
+        SeparatorToken = separatorToken;
     }
 
-    public RoutePatternToken SeperatorToken { get; }
+    public RoutePatternToken SeparatorToken { get; }
 
     internal override int ChildCount => 1;
 
     internal override RoutePatternNodeOrToken ChildAt(int index)
         => index switch
         {
-            0 => SeperatorToken,
+            0 => SeparatorToken,
             _ => throw new InvalidOperationException(),
         };
 

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternParser.cs

@@ -171,7 +171,7 @@ private static void ValidateStart(RoutePatternCompilationUnit root, IList<Embedd
                     // No problem if tilde is followed by slash.
                     if (root.ChildCount > 2 &&
                         root.ChildAt(1).Node is var secondNode &&
-                        secondNode?.Kind == RoutePatternKind.Seperator)
+                        secondNode?.Kind == RoutePatternKind.Separator)
                     {
                         return;
                     }
@@ -351,18 +351,18 @@ private static void ValidateParameterParts(RoutePatternCompilationUnit root, ILi
 
     private static void ValidateNoConsecutiveSeparators(RoutePatternCompilationUnit root, IList<EmbeddedDiagnostic> diagnostics)
     {
-        RoutePatternSegmentSeperatorNode? previousNode = null;
+        RoutePatternSegmentSeparatorNode? previousNode = null;
         foreach (var part in root)
         {
-            if (part.TryGetNode(RoutePatternKind.Seperator, out var seperatorNode))
+            if (part.TryGetNode(RoutePatternKind.Separator, out var separatorNode))
             {
-                var currentNode = (RoutePatternSegmentSeperatorNode)seperatorNode;
+                var currentNode = (RoutePatternSegmentSeparatorNode)separatorNode;
                 if (previousNode != null)
                 {
                     diagnostics.Add(
                         new EmbeddedDiagnostic(
                             Resources.TemplateRoute_CannotHaveConsecutiveSeparators,
-                            EmbeddedSyntaxHelpers.GetSpan(previousNode.SeperatorToken, currentNode.SeperatorToken)));
+                            EmbeddedSyntaxHelpers.GetSpan(previousNode.SeparatorToken, currentNode.SeparatorToken)));
                 }
                 previousNode = currentNode;
             }
@@ -421,13 +421,13 @@ private ImmutableArray<RoutePatternRootPartNode> ParseRootParts()
     private RoutePatternRootPartNode ParseRootPart()
         => _currentToken.Kind switch
         {
-            RoutePatternKind.SlashToken => ParseSegmentSeperator(),
+            RoutePatternKind.SlashToken => ParseSegmentSeparator(),
             _ => ParseSegment(),
         };
 
     private RoutePatternSegmentNode ParseSegment()
     {
-        var result = ImmutableArray.CreateBuilder<RoutePatternNode>();
+        var result = ImmutableArray.CreateBuilder<RoutePatternSegmentPartNode>();
 
         while (_currentToken.Kind != RoutePatternKind.EndOfFile &&
             _currentToken.Kind != RoutePatternKind.SlashToken)
@@ -686,7 +686,7 @@ private RoutePatternPolicyParameterPartNode ParsePolicy()
         return new(colonToken, fragments.ToImmutable());
     }
 
-    private RoutePatternSegmentSeperatorNode ParseSegmentSeperator()
+    private RoutePatternSegmentSeparatorNode ParseSegmentSeparator()
         => new(ConsumeCurrentToken());
 
     private TextSpan GetTokenStartPositionSpan(RoutePatternToken token)

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/WellKnownTypes.cs

@@ -57,7 +57,13 @@ internal enum WellKnownType
     System_Threading_Tasks_ValueTask_T,
     System_Reflection_ParameterInfo,
     Microsoft_AspNetCore_Http_IBindableFromHttpContext_T,
-    System_IParsable_T
+    System_IParsable_T,
+    Microsoft_AspNetCore_Builder_AuthorizationEndpointConventionBuilderExtensions,
+    Microsoft_AspNetCore_Http_OpenApiRouteHandlerBuilderExtensions,
+    Microsoft_AspNetCore_Builder_CorsEndpointConventionBuilderExtensions,
+    Microsoft_Extensions_DependencyInjection_OutputCacheConventionBuilderExtensions,
+    Microsoft_AspNetCore_Builder_RateLimiterEndpointConventionBuilderExtensions,
+    Microsoft_AspNetCore_Builder_RoutingEndpointConventionBuilderExtensions,
 }
 
 internal sealed class WellKnownTypes
@@ -110,7 +116,13 @@ internal sealed class WellKnownTypes
         "System.Threading.Tasks.ValueTask`1",
         "System.Reflection.ParameterInfo",
         "Microsoft.AspNetCore.Http.IBindableFromHttpContext`1",
-        "System.IParsable`1"
+        "System.IParsable`1",
+        "Microsoft.AspNetCore.Builder.AuthorizationEndpointConventionBuilderExtensions",
+        "Microsoft.AspNetCore.Http.OpenApiRouteHandlerBuilderExtensions",
+        "Microsoft.AspNetCore.Builder.CorsEndpointConventionBuilderExtensions",
+        "Microsoft.Extensions.DependencyInjection.OutputCacheConventionBuilderExtensions",
+        "Microsoft.AspNetCore.Builder.RateLimiterEndpointConventionBuilderExtensions",
+        "Microsoft.AspNetCore.Builder.RoutingEndpointConventionBuilderExtensions",
     };
 
     public static WellKnownTypes GetOrCreate(Compilation compilation) =>