Fix ambiguous route analyzer false positive with action replacement (#51880) (#52034)

Author: JamesNK

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Mvc/DetectAmbiguousActionRoutes.cs

@@ -18,8 +18,10 @@ namespace Microsoft.AspNetCore.Analyzers.Mvc;
 
 public partial class MvcAnalyzer
 {
-    private static void DetectAmbiguousActionRoutes(SymbolAnalysisContext context, WellKnownTypes wellKnownTypes, List<ActionRoute> actionRoutes)
+    private static void DetectAmbiguousActionRoutes(SymbolAnalysisContext context, WellKnownTypes wellKnownTypes, RoutePatternTree? controllerRoutePattern, List<ActionRoute> actionRoutes)
     {
+        var controllerHasActionReplacement = controllerRoutePattern != null ? HasActionReplacementToken(controllerRoutePattern) : false;
+
         // Ambiguous action route detection is conservative in what it detects to avoid false positives.
         //
         // Successfully matched action routes must:
@@ -31,36 +33,76 @@ private static void DetectAmbiguousActionRoutes(SymbolAnalysisContext context, W
         {
             // Group action routes together. When multiple match in a group, then report action routes to diagnostics.
             var groupedByParent = actionRoutes
-                .GroupBy(ar => new ActionRouteGroupKey(ar.ActionSymbol, ar.RouteUsageModel.RoutePattern, ar.HttpMethods, wellKnownTypes));
+                .GroupBy(ar => new ActionRouteGroupKey(ar.ActionSymbol, ar.RouteUsageModel.RoutePattern, ar.HttpMethods, controllerHasActionReplacement, wellKnownTypes));
 
-            foreach (var ambigiousGroup in groupedByParent.Where(g => g.Count() >= 2))
+            foreach (var ambiguousGroup in groupedByParent.Where(g => g.Count() >= 2))
             {
-                foreach (var ambigiousActionRoute in ambigiousGroup)
+                foreach (var ambiguousActionRoute in ambiguousGroup)
                 {
                     context.ReportDiagnostic(Diagnostic.Create(
                         DiagnosticDescriptors.AmbiguousActionRoute,
-                        ambigiousActionRoute.RouteUsageModel.UsageContext.RouteToken.GetLocation(),
-                        ambigiousActionRoute.RouteUsageModel.RoutePattern.Root.ToString()));
+                        ambiguousActionRoute.RouteUsageModel.UsageContext.RouteToken.GetLocation(),
+                        ambiguousActionRoute.RouteUsageModel.RoutePattern.Root.ToString()));
+                }
+            }
+        }
+    }
+
+    private static bool HasActionReplacementToken(RoutePatternTree routePattern)
+    {
+        for (var i = 0; i < routePattern.Root.Parts.Length; i++)
+        {
+            if (routePattern.Root.Parts[i] is RoutePatternSegmentNode segment)
+            {
+                for (var j = 0; j < segment.Children.Length; j++)
+                {
+                    if (segment.Children[j] is RoutePatternReplacementNode replacementNode)
+                    {
+                        if (!replacementNode.TextToken.IsMissing)
+                        {
+                            var name = replacementNode.TextToken.Value!.ToString();
+                            if (string.Equals(name, "action", StringComparison.OrdinalIgnoreCase))
+                            {
+                                return true;
+                            }
+                        }
+                    }
                 }
             }
         }
+
+        return false;
     }
 
     private readonly struct ActionRouteGroupKey : IEquatable<ActionRouteGroupKey>
     {
         public IMethodSymbol ActionSymbol { get; }
         public RoutePatternTree RoutePattern { get; }
         public ImmutableArray<string> HttpMethods { get; }
+        public string ActionName { get; }
+        public bool HasActionReplacement { get; }
         private readonly WellKnownTypes _wellKnownTypes;
 
-        public ActionRouteGroupKey(IMethodSymbol actionSymbol, RoutePatternTree routePattern, ImmutableArray<string> httpMethods, WellKnownTypes wellKnownTypes)
+        public ActionRouteGroupKey(IMethodSymbol actionSymbol, RoutePatternTree routePattern, ImmutableArray<string> httpMethods, bool controllerHasActionReplacement, WellKnownTypes wellKnownTypes)
         {
             Debug.Assert(!httpMethods.IsDefault);
 
             ActionSymbol = actionSymbol;
             RoutePattern = routePattern;
             HttpMethods = httpMethods;
             _wellKnownTypes = wellKnownTypes;
+            ActionName = GetActionName(ActionSymbol, _wellKnownTypes);
+            HasActionReplacement = controllerHasActionReplacement || HasActionReplacementToken(RoutePattern);
+        }
+
+        private static string GetActionName(IMethodSymbol actionSymbol, WellKnownTypes wellKnownTypes)
+        {
+            var actionNameAttribute = actionSymbol.GetAttributes(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Mvc_ActionNameAttribute), inherit: true).FirstOrDefault();
+            if (actionNameAttribute != null && actionNameAttribute.ConstructorArguments.Length > 0 && actionNameAttribute.ConstructorArguments[0].Value is string name)
+            {
+                return name;
+            }
+            return actionSymbol.Name;
         }
 
         public override bool Equals(object obj)
@@ -76,6 +118,7 @@ public bool Equals(ActionRouteGroupKey other)
         {
             return
                 AmbiguousRoutePatternComparer.Instance.Equals(RoutePattern, other.RoutePattern) &&
+                (!HasActionReplacement || string.Equals(ActionName, other.ActionName, StringComparison.OrdinalIgnoreCase)) &&
                 HasMatchingHttpMethods(HttpMethods, other.HttpMethods) &&
                 CanMatchActions(_wellKnownTypes, ActionSymbol, other.ActionSymbol);
         }

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Mvc/MvcAnalyzer.cs

@@ -5,7 +5,9 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Linq;
 using System.Threading;
+using Microsoft.AspNetCore.Analyzers.Infrastructure.RoutePattern;
 using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
 using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
 using Microsoft.CodeAnalysis;
@@ -47,9 +49,20 @@ public override void Initialize(AnalysisContext context)
                         actionRoutes = new List<ActionRoute>();
                     }
 
+                    RoutePatternTree? controllerRoutePattern = null;
+                    var controllerRouteAttribute = namedTypeSymbol.GetAttributes(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Mvc_RouteAttribute), inherit: true).FirstOrDefault();
+                    if (controllerRouteAttribute != null)
+                    {
+                        var routeUsage = GetRouteUsageModel(controllerRouteAttribute, routeUsageCache, context.CancellationToken);
+                        if (routeUsage != null)
+                        {
+                            controllerRoutePattern = routeUsage.RoutePattern;
+                        }
+                    }
+
                     PopulateActionRoutes(context, wellKnownTypes, routeUsageCache, namedTypeSymbol, actionRoutes);
 
-                    DetectAmbiguousActionRoutes(context, wellKnownTypes, actionRoutes);
+                    DetectAmbiguousActionRoutes(context, wellKnownTypes, controllerRoutePattern, actionRoutes);
 
                     // Return to the pool.
                     actionRoutes.Clear();