Skip to content

Resolve parameter name with FromRouteAttribute in tooling #45720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal enum WellKnownType
Microsoft_AspNetCore_Http_Metadata_IFromHeaderMetadata,
Microsoft_AspNetCore_Http_Metadata_IFromQueryMetadata,
Microsoft_AspNetCore_Http_Metadata_IFromServiceMetadata,
Microsoft_AspNetCore_Http_Metadata_IFromRouteMetadata,
Microsoft_AspNetCore_Http_HeaderDictionaryExtensions,
Microsoft_AspNetCore_Routing_IEndpointRouteBuilder,
Microsoft_AspNetCore_Mvc_ControllerAttribute,
Expand Down Expand Up @@ -71,6 +72,7 @@ internal sealed class WellKnownTypes
"Microsoft.AspNetCore.Http.Metadata.IFromHeaderMetadata",
"Microsoft.AspNetCore.Http.Metadata.IFromQueryMetadata",
"Microsoft.AspNetCore.Http.Metadata.IFromServiceMetadata",
"Microsoft.AspNetCore.Http.Metadata.IFromRouteMetadata",
"Microsoft.AspNetCore.Http.HeaderDictionaryExtensions",
"Microsoft.AspNetCore.Routing.IEndpointRouteBuilder",
"Microsoft.AspNetCore.Mvc.ControllerAttribute",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,33 @@ static ImmutableArray<ParameterSymbol> ResolvedParametersCore(ISymbol symbol, IS
}
else
{
resolvedParameterSymbols.Add(new ParameterSymbol(child, topLevelSymbol));
var routeParameterName = ResolveRouteParameterName(child, wellKnownTypes);
resolvedParameterSymbols.Add(new ParameterSymbol(routeParameterName, child, topLevelSymbol));
}
}
return resolvedParameterSymbols.ToImmutable();
}

static string ResolveRouteParameterName(ISymbol parameterSymbol, WellKnownTypes wellKnownTypes)
{
var fromRouteMetadata = wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromRouteMetadata);
if (!parameterSymbol.HasAttributeImplementingInterface(fromRouteMetadata, out var attributeData))
{
return parameterSymbol.Name; // No route metadata attribute!
}

foreach (var namedArgument in attributeData.NamedArguments)
{
if (namedArgument.Key == "Name")
{
var routeParameterNameConstant = namedArgument.Value;
var routeParameterName = (string)routeParameterNameConstant.Value!;
return routeParameterName; // Have attribute & name is specified.
}
}

return parameterSymbol.Name; // We have the attribute, but name isn't specified!
}
}

public static ImmutableArray<ISymbol> GetParameterSymbols(ISymbol symbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ internal enum RouteUsageType
Component
}

internal record struct ParameterSymbol(ISymbol Symbol, ISymbol? TopLevelSymbol = null)
// RouteParameterName can be different from parameter name using FromRouteAttribute. e.g. [FromRoute(Name = "custom_name")]
internal record struct ParameterSymbol(string RouteParameterName, ISymbol Symbol, ISymbol? TopLevelSymbol = null)
{
public bool IsNested => TopLevelSymbol != null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.CodeAnalysis;

Expand All @@ -23,6 +24,26 @@ public static bool HasAttribute(this ISymbol symbol, INamedTypeSymbol attributeT
return false;
}

public static bool HasAttributeImplementingInterface(this ISymbol symbol, INamedTypeSymbol interfaceType)
{
return HasAttributeImplementingInterface(symbol, interfaceType, out var _);
}

public static bool HasAttributeImplementingInterface(this ISymbol symbol, INamedTypeSymbol interfaceType, [NotNullWhen(true)] out AttributeData? matchedAttribute)
{
foreach (var attributeData in symbol.GetAttributes())
{
if (attributeData.AttributeClass is not null && Implements(attributeData.AttributeClass, interfaceType))
{
matchedAttribute = attributeData;
return true;
}
}

matchedAttribute = null;
return false;
}

public static bool Implements(this ITypeSymbol type, ITypeSymbol interfaceType)
{
foreach (var t in type.AllInterfaces)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private void Analyze(

foreach (var parameter in routeUsage.UsageContext.ResolvedParameters)
{
routeParameterNames.Remove(parameter.Symbol.Name);
routeParameterNames.Remove(parameter.RouteParameterName);
}

foreach (var unusedParameterName in routeParameterNames)
Expand Down Expand Up @@ -143,7 +143,7 @@ private record struct InsertPoint(ISymbol ExistingParameter, bool Before);
continue;
}

var parameterSymbol = resolvedParameterSymbols.FirstOrDefault(s => string.Equals(s.Symbol.Name, routeParameter.Name, StringComparison.OrdinalIgnoreCase));
var parameterSymbol = resolvedParameterSymbols.FirstOrDefault(s => string.Equals(s.RouteParameterName, routeParameter.Name, StringComparison.OrdinalIgnoreCase));
if (parameterSymbol.Symbol != null)
{
var s = parameterSymbol.TopLevelSymbol ?? parameterSymbol.Symbol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ private static void ProvideParameterCompletions(EmbeddedCompletionContext contex
foreach (var parameterSymbol in context.RouteUsage.UsageContext.ResolvedParameters)
{
// Don't suggest parameter name if it already exists in the route.
if (!context.RouteUsage.RoutePattern.TryGetRouteParameter(parameterSymbol.Symbol.Name, out _))
if (!context.RouteUsage.RoutePattern.TryGetRouteParameter(parameterSymbol.RouteParameterName, out _))
{
context.AddIfMissing(parameterSymbol.Symbol.Name, suffix: null, description: null, WellKnownTags.Parameter, parentOpt: parentOpt);
context.AddIfMissing(parameterSymbol.RouteParameterName, suffix: null, description: null, WellKnownTags.Parameter, parentOpt: parentOpt);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Threading;
using Microsoft.AspNetCore.Analyzers.Infrastructure.RoutePattern;
using Microsoft.AspNetCore.Analyzers.Infrastructure.VirtualChars;
using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -22,27 +21,27 @@ internal class RoutePatternHighlighter : IAspNetCoreEmbeddedLanguageDocumentHigh
public ImmutableArray<AspNetCoreDocumentHighlights> GetDocumentHighlights(
SemanticModel semanticModel, SyntaxToken token, int position, CancellationToken cancellationToken)
{
var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation);
var routeUsageCache = RouteUsageCache.GetOrCreate(semanticModel.Compilation);
var routeUsage = routeUsageCache.Get(token, cancellationToken);
if (routeUsage is null)
{
return ImmutableArray<AspNetCoreDocumentHighlights>.Empty;
}

return GetHighlights(routeUsage.RoutePattern, semanticModel, wellKnownTypes, position, routeUsage.UsageContext.MethodSymbol, cancellationToken);
return GetHighlights(routeUsage, semanticModel, position, cancellationToken);
}

private static ImmutableArray<AspNetCoreDocumentHighlights> GetHighlights(
RoutePatternTree tree, SemanticModel semanticModel, WellKnownTypes wellKnownTypes, int position, IMethodSymbol? methodSymbol, CancellationToken cancellationToken)
RouteUsageModel routeUsage, SemanticModel semanticModel, int position, CancellationToken cancellationToken)
{
var virtualChar = tree.Text.Find(position);
var routePattern = routeUsage.RoutePattern;
var virtualChar = routePattern.Text.Find(position);
if (virtualChar == null)
{
return ImmutableArray<AspNetCoreDocumentHighlights>.Empty;
}

var node = FindParameterNode(tree.Root, virtualChar.Value);
var node = FindParameterNode(routePattern.Root, virtualChar.Value);
if (node == null)
{
return ImmutableArray<AspNetCoreDocumentHighlights>.Empty;
Expand All @@ -53,21 +52,17 @@ private static ImmutableArray<AspNetCoreDocumentHighlights> GetHighlights(
// Highlight the parameter in the route string, e.g. "{id}" highlights "id".
highlightSpans.Add(new AspNetCoreHighlightSpan(node.GetSpan(), AspNetCoreHighlightSpanKind.Reference));

if (methodSymbol != null)
if (routeUsage.UsageContext.MethodSymbol is { } methodSymbol)
{
// Resolve possible parameter symbols. Includes properties from AsParametersAttribute.
var routeParameters = RoutePatternParametersDetector.ResolvedParameters(methodSymbol, wellKnownTypes);
var parameterSymbols = routeParameters.Select(p => p.Symbol).ToArray();
var resolvedParameters = routeUsage.UsageContext.ResolvedParameters;

// Match route parameter to method parameter. Parameters in a route aren't case sensitive.
// First attempt an exact match, then a case insensitive match.
// It's possible to match multiple parameters, either based on parameter name, or [FromRoute(Name = "XXX")] attribute.
var parameterName = node.ParameterNameToken.Value!.ToString();
var matchingParameterSymbol = parameterSymbols.FirstOrDefault(s => s.Name == parameterName)
?? parameterSymbols.FirstOrDefault(s => string.Equals(s.Name, parameterName, StringComparison.OrdinalIgnoreCase));

if (matchingParameterSymbol != null)
foreach (var matchingParameter in resolvedParameters.Where(s => string.Equals(s.RouteParameterName, parameterName, StringComparison.OrdinalIgnoreCase)))
{
HighlightSymbol(semanticModel, methodSymbol, highlightSpans, matchingParameterSymbol, cancellationToken);
HighlightSymbol(semanticModel, methodSymbol, highlightSpans, matchingParameter.Symbol, cancellationToken);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,80 @@ public object TestAction()
});
}

[Fact]
public async Task ControllerAction_MatchRouteParameterWithFromRoute_NoDiagnostics()
{
// Arrange
var source = TestSource.Read(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;

class Program
{
static void Main()
{
}
}

public class TestController
{
[HttpGet(@""{id}"")]
public object TestAction([FromRoute(Name = ""id"")]string id1)
{
return null;
}
}
");

// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);

// Assert
Assert.Empty(diagnostics);
}

[Fact]
public async Task ControllerAction_MatchRouteParameterWithMultipleFromRoute_NoDiagnostics()
{
// Arrange
var source = TestSource.Read(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Mvc;

class Program
{
static void Main()
{
}
}

public class TestController
{
[HttpGet(@""{id}"")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should decide if we need an analyzer to pick up when route parameters are missing from the route pattern.

public object TestAction([CustomFromRoute(Name = ""id"")][FromRoute(Name = ""custom_id"")]string id1)
{
return null;
}
}

public class CustomFromRouteAttribute : Attribute, IFromRouteMetadata
{
public string? Name { get; set; }
}
");

// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);

// Assert
Assert.Empty(diagnostics);
}

[Fact]
public async Task MapGet_AsParameter_NoResults()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,31 @@ static void Main()
i => Assert.Equal("id", i.DisplayText));
}

[Fact]
public async Task Insertion_ParameterOpenBrace_EndpointMapGet_HasDelegate_FromRouteAttribute_ReturnDelegateParameterItem()
{
// Arrange & Act
var result = await GetCompletionsAndServiceAsync(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;

class Program
{
static void Main()
{
EndpointRouteBuilderExtensions.MapGet(null, @""{$$"", ([FromRoute(Name = ""id1"")]string id) => "");
}
}
");

// Assert
Assert.Collection(
result.Completions.ItemsList,
i => Assert.Equal("id1", i.DisplayText));
}

[Fact]
public async Task Insertion_ParameterOpenBrace_EndpointMapGet_HasMethod_ReturnDelegateParameterItem()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,72 @@ static void Main()
");
}

[Fact]
public async Task InParameterName_ExtensionMethod_MatchingDelegate_RouteMetadataWithoutName_HighlightParameter()
{
// Arrange & Act & Assert
await TestHighlightingAsync(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;

class Program
{
static void Main()
{
IEndpointRouteBuilder builder = null;
builder.MapGet(@""{$$[|id|]}"", ([FromRoute]string [|id|]) => $""{[|id|]}"");
}
}
");
}

[Fact]
public async Task InParameterName_ExtensionMethod_MatchingDelegate_RouteMetadataWithName_HighlightParameter()
{
// Arrange & Act & Assert
await TestHighlightingAsync(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;

class Program
{
static void Main()
{
IEndpointRouteBuilder builder = null;
builder.MapGet(@""{$$[|id|]}"", ([FromRoute(Name = ""id"")]string [|id1|]) => $""{[|id1|]}"");
}
}
");
}

[Fact]
public async Task InParameterName_ExtensionMethod_MatchingDelegate_RouteMetadataWithName_MultipleMatches_HighlightParameter()
{
// Arrange & Act & Assert
await TestHighlightingAsync(@"
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;

class Program
{
static void Main()
{
IEndpointRouteBuilder builder = null;
builder.MapGet(@""{$$[|id|]}"", ([FromRoute(Name = ""id"")]string [|id1|], string [|id|]) => $""{[|id1|]}"");
}
}
");
}

[Fact]
public async Task InParameterName_MatchingDelegate_HighlightParameter()
{
Expand Down