From 9ff9267096425b5caf39df8dcf134e33faaaba26 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 5 Aug 2021 09:15:58 -0500 Subject: [PATCH 1/8] Set EndpointName automatically from method name --- ...malActionEndpointRouteBuilderExtensions.cs | 9 ++++ .../src/Microsoft.AspNetCore.Routing.csproj | 1 + ...ctionEndpointRouteBuilderExtensionsTest.cs | 49 +++++++++++++++++++ .../EndpointMetadataApiDescriptionProvider.cs | 8 +-- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 1 + src/Shared/TypeHelper.cs | 28 +++++++++++ 6 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 src/Shared/TypeHelper.cs diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index 02addb8e468b..422378eeddff 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; @@ -184,6 +185,14 @@ public static MinimalActionEndpointConventionBuilder Map( // Add MethodInfo as metadata to assist with OpenAPI generation for the endpoint. builder.Metadata.Add(action.Method); + // Methods defined in a top-level program are generated as statics so the delegate + // target will be null. Inline lambdas are compiler generated properties so they can + // be filtered that way. + if (action.Target == null || !TypeHelper.IsCompilerGenerated(action.Method.Name)) + { + builder.Metadata.Add(new EndpointNameMetadata(action.Method.Name)); + } + // Add delegate attributes as metadata var attributes = action.Method.GetCustomAttributes(); diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index 4c0c0a1a5443..b992da41da2c 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -24,6 +24,7 @@ Microsoft.AspNetCore.Routing.RouteCollection + diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index 4dd5bbaa4569..ecbe22a7b813 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -359,6 +359,55 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder() Assert.Equal(int.MaxValue, routeEndpointBuilder.Order); } + [Fact] + // This test scenario simulates methods defined in a top-level program + // which are compiler generated. + public void MapMethod_SetsEndpointNameForInnerMethod() + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + string InnerGetString() => "TestString"; + _ = builder.MapDelete("/", InnerGetString); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointName = endpoint.Metadata.GetMetadata(); + Assert.NotNull(endpointName); + Assert.Equal("InnerGetString", endpointName?.EndpointName); + } + + [Fact] + public void MapMethod_SetsEndpointNameForMethodGroup() + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + _ = builder.MapDelete("/", GetString); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointName = endpoint.Metadata.GetMetadata(); + Assert.NotNull(endpointName); + Assert.Equal("GetString", endpointName?.EndpointName); + } + + private string GetString() => "TestString"; + + [Fact] + public void MapMethod_DoesNotSetEndpointNameForLambda() + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + _ = builder.MapDelete("/", () => { }); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointName = endpoint.Metadata.GetMetadata(); + Assert.Null(endpointName); + } + class FromRoute : Attribute, IFromRouteMetadata { public string? Name { get; set; } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index fc3f69701a1e..d6a93763e6e4 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -78,7 +78,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string // For now, put all methods defined the same declaring type together. string controllerName; - if (methodInfo.DeclaringType is not null && !IsCompilerGenerated(methodInfo.DeclaringType)) + if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGenerated(methodInfo.DeclaringType.Name, methodInfo.DeclaringType)) { controllerName = methodInfo.DeclaringType.Name; } @@ -363,11 +363,5 @@ private static void AddActionDescriptorEndpointMetadata( actionDescriptor.EndpointMetadata = new List(endpointMetadata); } } - - // The CompilerGeneratedAttribute doesn't always get added so we also check if the type name starts with "<" - // For example, "<>c" is a "declaring" type the C# compiler will generate without the attribute for a top-level lambda - // REVIEW: Is there a better way to do this? - private static bool IsCompilerGenerated(Type type) => - Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || type.Name.StartsWith('<'); } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 4b86355d1087..a9748b7bc86b 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Shared/TypeHelper.cs b/src/Shared/TypeHelper.cs new file mode 100644 index 000000000000..1f81ee50526f --- /dev/null +++ b/src/Shared/TypeHelper.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime.CompilerServices +{ + internal static class TypeHelper + { + /// + /// Checks to see if a given type is compiler generated. + /// + /// The compiler doesn't always annotate every time it generates with the + /// CompilerGeneratedAttribute so sometimes we have to check if the type's + /// identifier represents a generated type. Follows the same heuristics seen + /// in https://github.com/dotnet/roslyn/blob/b57c1f89c1483da8704cde7b535a20fd029748db/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/GeneratedMetadataNames.cs#L19 + /// + /// + /// The type to evaluate. Can be null if evaluating only on name. + /// The identifier associated wit the type. + /// if is compiler generated + /// or represents a compiler generated identifier. + internal static bool IsCompilerGenerated(string name, Type? type = null) + { + return (type is Type && Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute))) + || name.StartsWith("<", StringComparison.Ordinal) + || (name.IndexOf('$') >= 0); + } + } +} \ No newline at end of file From 704673f0fe5613bb0ccde399511e6ccb0bae696c Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 17 Aug 2021 15:28:20 +0000 Subject: [PATCH 2/8] Only support adding names from local functions --- .../MinimalActionEndpointRouteBuilderExtensions.cs | 12 ++++++++---- ...inimalActionEndpointRouteBuilderExtensionsTest.cs | 8 ++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index 422378eeddff..aa278dbdf63e 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -185,10 +185,14 @@ public static MinimalActionEndpointConventionBuilder Map( // Add MethodInfo as metadata to assist with OpenAPI generation for the endpoint. builder.Metadata.Add(action.Method); - // Methods defined in a top-level program are generated as statics so the delegate - // target will be null. Inline lambdas are compiler generated properties so they can - // be filtered that way. - if (action.Target == null || !TypeHelper.IsCompilerGenerated(action.Method.Name)) + // We only add endpoint names for types that are not compiler generated since + // compiler generated types are mangled by default. This logic can be changed once + // https://github.com/dotnet/roslyn/issues/55651 is addressed. For now, this will + // not set the endpoint name metadata for: + // - Local functions + // - Inline lambdas + // - Static functions + if (!TypeHelper.IsCompilerGenerated(action.Method.Name)) { builder.Metadata.Add(new EndpointNameMetadata(action.Method.Name)); } diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index ecbe22a7b813..ec8480c072cf 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -361,8 +361,9 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder() [Fact] // This test scenario simulates methods defined in a top-level program - // which are compiler generated. - public void MapMethod_SetsEndpointNameForInnerMethod() + // which are compiler generated. This can be re-examined once + // https://github.com/dotnet/roslyn/issues/55651 is addressed. + public void MapMethod_DoesNotEndpointNameForInnerMethod() { var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); string InnerGetString() => "TestString"; @@ -373,8 +374,7 @@ public void MapMethod_SetsEndpointNameForInnerMethod() var endpoint = Assert.Single(dataSource.Endpoints); var endpointName = endpoint.Metadata.GetMetadata(); - Assert.NotNull(endpointName); - Assert.Equal("InnerGetString", endpointName?.EndpointName); + Assert.Null(endpointName); } [Fact] From c5fa42fd44200738e7117ea8254af0a21205f303 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 17 Aug 2021 18:53:05 +0000 Subject: [PATCH 3/8] Supoort local functions via Roslyn parsing logic --- ...malActionEndpointRouteBuilderExtensions.cs | 24 ++- ...tingEndpointConventionBuilderExtensions.cs | 1 + .../src/Microsoft.AspNetCore.Routing.csproj | 5 +- ...ctionEndpointRouteBuilderExtensionsTest.cs | 22 ++- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 2 +- .../RoslynUtils/GeneratedNameConstants.cs | 17 ++ src/Shared/RoslynUtils/GeneratedNameKind.cs | 70 ++++++++ src/Shared/RoslynUtils/GeneratedNameParser.cs | 170 ++++++++++++++++++ src/Shared/{ => RoslynUtils}/TypeHelper.cs | 0 9 files changed, 297 insertions(+), 14 deletions(-) create mode 100644 src/Shared/RoslynUtils/GeneratedNameConstants.cs create mode 100644 src/Shared/RoslynUtils/GeneratedNameKind.cs create mode 100644 src/Shared/RoslynUtils/GeneratedNameParser.cs rename src/Shared/{ => RoslynUtils}/TypeHelper.cs (100%) diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index aa278dbdf63e..f34254be0816 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.CodeAnalysis.CSharp.Symbols; namespace Microsoft.AspNetCore.Builder { @@ -185,16 +186,21 @@ public static MinimalActionEndpointConventionBuilder Map( // Add MethodInfo as metadata to assist with OpenAPI generation for the endpoint. builder.Metadata.Add(action.Method); - // We only add endpoint names for types that are not compiler generated since - // compiler generated types are mangled by default. This logic can be changed once - // https://github.com/dotnet/roslyn/issues/55651 is addressed. For now, this will - // not set the endpoint name metadata for: - // - Local functions - // - Inline lambdas - // - Static functions - if (!TypeHelper.IsCompilerGenerated(action.Method.Name)) + // Methods defined in a top-level program are generated as statics so the delegate + // target will be null. Inline lambdas are compiler generated properties so they can + // be filtered that way. + if (action.Target == null || !TypeHelper.IsCompilerGenerated(action.Method.Name)) { - builder.Metadata.Add(new EndpointNameMetadata(action.Method.Name)); + if (GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)) + { + builder.Metadata.Add(new EndpointNameMetadata(endpointName)); + builder.Metadata.Add(new RouteNameMetadata(endpointName)); + } + else + { + builder.Metadata.Add(new EndpointNameMetadata(action.Method.Name)); + builder.Metadata.Add(new RouteNameMetadata(action.Method.Name)); + } } // Add delegate attributes as metadata diff --git a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs index 24b9b3df6c88..4c29a1dce365 100644 --- a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Linq; using Microsoft.AspNetCore.Routing; namespace Microsoft.AspNetCore.Builder diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index b992da41da2c..dabaeffa8273 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -24,7 +24,10 @@ Microsoft.AspNetCore.Routing.RouteCollection - + + + + diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index ec8480c072cf..6bb049c116aa 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -361,8 +361,8 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder() [Fact] // This test scenario simulates methods defined in a top-level program - // which are compiler generated. This can be re-examined once - // https://github.com/dotnet/roslyn/issues/55651 is addressed. + // which are compiler generated. We currently do some manually parsing leveraging + // code in Roslyn to support this scenario. More info at https://github.com/dotnet/roslyn/issues/55651. public void MapMethod_DoesNotEndpointNameForInnerMethod() { var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); @@ -374,7 +374,8 @@ public void MapMethod_DoesNotEndpointNameForInnerMethod() var endpoint = Assert.Single(dataSource.Endpoints); var endpointName = endpoint.Metadata.GetMetadata(); - Assert.Null(endpointName); + Assert.NotNull(endpointName); + Assert.Equal("InnerGetString", endpointName?.EndpointName); } [Fact] @@ -392,6 +393,21 @@ public void MapMethod_SetsEndpointNameForMethodGroup() Assert.Equal("GetString", endpointName?.EndpointName); } + [Fact] + public void WithNameOverridesDefaultEndpointName() + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + _ = builder.MapDelete("/", GetString).WithName("SomeCustomName"); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointName = endpoint.Metadata.GetMetadata(); + Assert.NotNull(endpointName); + Assert.Equal("SomeCustomName", endpointName?.EndpointName); + } + private string GetString() => "TestString"; [Fact] diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index a9748b7bc86b..a1170d9393e4 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/Shared/RoslynUtils/GeneratedNameConstants.cs b/src/Shared/RoslynUtils/GeneratedNameConstants.cs new file mode 100644 index 000000000000..b771fb784464 --- /dev/null +++ b/src/Shared/RoslynUtils/GeneratedNameConstants.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameConstants.cs +// and exist to address the issues with extracting original method names for +// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 +// for more info. +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + internal static class GeneratedNameConstants + { + internal const char DotReplacementInTypeNames = '-'; + internal const string SynthesizedLocalNamePrefix = "CS$"; + internal const string SuffixSeparator = "__"; + internal const char LocalFunctionNameTerminator = '|'; + } +} \ No newline at end of file diff --git a/src/Shared/RoslynUtils/GeneratedNameKind.cs b/src/Shared/RoslynUtils/GeneratedNameKind.cs new file mode 100644 index 000000000000..262ebf5264d8 --- /dev/null +++ b/src/Shared/RoslynUtils/GeneratedNameKind.cs @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameKind.cs +// and exist to address the issues with extracting original method names for +// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 +// for more info. +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + internal enum GeneratedNameKind + { + None = 0, + + // Used by EE: + ThisProxyField = '4', + HoistedLocalField = '5', + DisplayClassLocalOrField = '8', + LambdaMethod = 'b', + LambdaDisplayClass = 'c', + StateMachineType = 'd', + LocalFunction = 'g', // note collision with Deprecated_InitializerLocal, however this one is only used for method names + + // Used by EnC: + AwaiterField = 'u', + HoistedSynthesizedLocalField = 's', + + // Currently not parsed: + StateMachineStateField = '1', + IteratorCurrentBackingField = '2', + StateMachineParameterProxyField = '3', + ReusableHoistedLocalField = '7', + LambdaCacheField = '9', + FixedBufferField = 'e', + AnonymousType = 'f', + TransparentIdentifier = 'h', + AnonymousTypeField = 'i', + AnonymousTypeTypeParameter = 'j', + AutoPropertyBackingField = 'k', + IteratorCurrentThreadIdField = 'l', + IteratorFinallyMethod = 'm', + BaseMethodWrapper = 'n', + AsyncBuilderField = 't', + DynamicCallSiteContainerType = 'o', + DynamicCallSiteField = 'p', + AsyncIteratorPromiseOfValueOrEndBackingField = 'v', + DisposeModeField = 'w', + CombinedTokensField = 'x', // last + + // Deprecated - emitted by Dev12, but not by Roslyn. + // Don't reuse the values because the debugger might encounter them when consuming old binaries. + [Obsolete] + Deprecated_OuterscopeLocals = '6', + [Obsolete] + Deprecated_IteratorInstance = 'a', + [Obsolete] + Deprecated_InitializerLocal = 'g', + [Obsolete] + Deprecated_DynamicDelegate = 'q', + [Obsolete] + Deprecated_ComrefCallLocal = 'r', + } + + internal static class GeneratedNameKindExtensions + { + internal static bool IsTypeName(this GeneratedNameKind kind) + => kind is GeneratedNameKind.LambdaDisplayClass or GeneratedNameKind.StateMachineType or GeneratedNameKind.DynamicCallSiteContainerType; + } +} \ No newline at end of file diff --git a/src/Shared/RoslynUtils/GeneratedNameParser.cs b/src/Shared/RoslynUtils/GeneratedNameParser.cs new file mode 100644 index 000000000000..ba6086484861 --- /dev/null +++ b/src/Shared/RoslynUtils/GeneratedNameParser.cs @@ -0,0 +1,170 @@ +// 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.Diagnostics.CodeAnalysis; +using System.Globalization; + +// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameParser.cs +// and exist to address the issues with extracting original method names for +// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 +// for more info. +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + internal static class GeneratedNameParser + { + // Parse the generated name. Returns true for names of the form + // [CS$]<[middle]>c[__[suffix]] where [CS$] is included for certain + // generated names, where [middle] and [__[suffix]] are optional, + // and where c is a single character in [1-9a-z] + // (csharp\LanguageAnalysis\LIB\SpecialName.cpp). + internal static bool TryParseGeneratedName( + string name, + out GeneratedNameKind kind, + out int openBracketOffset, + out int closeBracketOffset) + { + openBracketOffset = -1; + if (name.StartsWith("CS$<", StringComparison.Ordinal)) + { + openBracketOffset = 3; + } + else if (name.StartsWith("<", StringComparison.Ordinal)) + { + openBracketOffset = 0; + } + + if (openBracketOffset >= 0) + { + closeBracketOffset = IndexOfBalancedParenthesis(name, openBracketOffset, '>'); + if (closeBracketOffset >= 0 && closeBracketOffset + 1 < name.Length) + { + int c = name[closeBracketOffset + 1]; + if (c is >= '1' and <= '9' or >= 'a' and <= 'z') // Note '0' is not special. + { + kind = (GeneratedNameKind)c; + return true; + } + } + } + + kind = GeneratedNameKind.None; + openBracketOffset = -1; + closeBracketOffset = -1; + return false; + } + + private static int IndexOfBalancedParenthesis(string str, int openingOffset, char closing) + { + char opening = str[openingOffset]; + + int depth = 1; + for (int i = openingOffset + 1; i < str.Length; i++) + { + var c = str[i]; + if (c == opening) + { + depth++; + } + else if (c == closing) + { + depth--; + if (depth == 0) + { + return i; + } + } + } + + return -1; + } + + internal static bool TryParseSourceMethodNameFromGeneratedName(string generatedName, GeneratedNameKind requiredKind, [NotNullWhen(true)] out string? methodName) + { + if (!TryParseGeneratedName(generatedName, out var kind, out int openBracketOffset, out int closeBracketOffset)) + { + methodName = null; + return false; + } + + if (requiredKind != 0 && kind != requiredKind) + { + methodName = null; + return false; + } + + methodName = generatedName.Substring(openBracketOffset + 1, closeBracketOffset - openBracketOffset - 1); + + if (kind.IsTypeName()) + { + methodName = methodName.Replace(GeneratedNameConstants.DotReplacementInTypeNames, '.'); + } + + return true; + } + + /// + /// Parses generated local function name out of a generated method name. + /// + internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhen(true)] out string? localFunctionName) + { + localFunctionName = null; + + // '<' containing-method-name '>' 'g' '__' local-function-name '|' method-ordinal '_' lambda-ordinal + if (!TryParseGeneratedName(generatedName, out var kind, out _, out int closeBracketOffset) || kind != GeneratedNameKind.LocalFunction) + { + return false; + } + + int localFunctionNameStart = closeBracketOffset + 2 + GeneratedNameConstants.SuffixSeparator.Length; + if (localFunctionNameStart >= generatedName.Length) + { + return false; + } + + int localFunctionNameEnd = generatedName.IndexOf(GeneratedNameConstants.LocalFunctionNameTerminator, localFunctionNameStart); + if (localFunctionNameEnd < 0) + { + return false; + } + + localFunctionName = generatedName.Substring(localFunctionNameStart, localFunctionNameEnd - localFunctionNameStart); + return true; + } + + // Extracts the slot index from a name of a field that stores hoisted variables or awaiters. + // Such a name ends with "__{slot index + 1}". + // Returned slot index is >= 0. + internal static bool TryParseSlotIndex(string fieldName, out int slotIndex) + { + int lastUnder = fieldName.LastIndexOf('_'); + if (lastUnder - 1 < 0 || lastUnder == fieldName.Length || fieldName[lastUnder - 1] != '_') + { + slotIndex = -1; + return false; + } + + if (int.TryParse(fieldName.AsSpan(lastUnder + 1), NumberStyles.None, CultureInfo.InvariantCulture, out slotIndex) && slotIndex >= 1) + { + slotIndex--; + return true; + } + + slotIndex = -1; + return false; + } + + internal static bool TryParseAnonymousTypeParameterName(string typeParameterName, [NotNullWhen(true)] out string? propertyName) + { + if (typeParameterName.StartsWith("<", StringComparison.Ordinal) && + typeParameterName.EndsWith(">j__TPar", StringComparison.Ordinal)) + { + propertyName = typeParameterName.Substring(1, typeParameterName.Length - 9); + return true; + } + + propertyName = null; + return false; + } + } +} \ No newline at end of file diff --git a/src/Shared/TypeHelper.cs b/src/Shared/RoslynUtils/TypeHelper.cs similarity index 100% rename from src/Shared/TypeHelper.cs rename to src/Shared/RoslynUtils/TypeHelper.cs From 8fb48c34fe62eb660fbdc090be1641c12417bdf6 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 18 Aug 2021 01:06:29 +0000 Subject: [PATCH 4/8] Set DisplayName using endpoint name --- ...nimalActionEndpointRouteBuilderExtensions.cs | 17 ++++++++--------- ...outingEndpointConventionBuilderExtensions.cs | 1 - ...lActionEndpointRouteBuilderExtensionsTest.cs | 16 ++++++++-------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index f34254be0816..20a3814bba3c 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -109,7 +109,8 @@ public static MinimalActionEndpointConventionBuilder MapMethods( } var builder = endpoints.Map(RoutePatternFactory.Parse(pattern), action); - builder.WithDisplayName($"{pattern} HTTP: {string.Join(", ", httpMethods)}"); + // Prepends the HTTP method to the DisplayName produced with pattern + method name + builder.Add(b => b.DisplayName = $"HTTP: {string.Join(", ", httpMethods)} {b.DisplayName}"); builder.WithMetadata(new HttpMethodMetadata(httpMethods)); return builder; } @@ -191,16 +192,14 @@ public static MinimalActionEndpointConventionBuilder Map( // be filtered that way. if (action.Target == null || !TypeHelper.IsCompilerGenerated(action.Method.Name)) { - if (GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)) + if (!GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)) { - builder.Metadata.Add(new EndpointNameMetadata(endpointName)); - builder.Metadata.Add(new RouteNameMetadata(endpointName)); - } - else - { - builder.Metadata.Add(new EndpointNameMetadata(action.Method.Name)); - builder.Metadata.Add(new RouteNameMetadata(action.Method.Name)); + endpointName = action.Method.Name; } + + builder.Metadata.Add(new EndpointNameMetadata(endpointName)); + builder.Metadata.Add(new RouteNameMetadata(endpointName)); + builder.DisplayName = $"{builder.DisplayName} => {endpointName}"; } // Add delegate attributes as metadata diff --git a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs index 4c29a1dce365..24b9b3df6c88 100644 --- a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Linq; using Microsoft.AspNetCore.Routing; namespace Microsoft.AspNetCore.Builder diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index 6bb049c116aa..0337f28fbf3e 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -99,7 +99,7 @@ public void MapGet_BuildsEndpointWithCorrectMethod() Assert.Equal("GET", method); var routeEndpointBuilder = GetRouteEndpointBuilder(builder); - Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName); + Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName); Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText); } @@ -125,7 +125,7 @@ public async Task MapGetWithRouteParameter_BuildsEndpointWithRouteSpecificBindin Assert.Equal("GET", method); var routeEndpointBuilder = GetRouteEndpointBuilder(builder); - Assert.Equal("/{id} HTTP: GET", routeEndpointBuilder.DisplayName); + Assert.Equal("HTTP: GET /{id}", routeEndpointBuilder.DisplayName); Assert.Equal("/{id}", routeEndpointBuilder.RoutePattern.RawText); // Assert that we don't fallback to the query string @@ -163,7 +163,7 @@ public async Task MapGetWithoutRouteParameter_BuildsEndpointWithQuerySpecificBin Assert.Equal("GET", method); var routeEndpointBuilder = GetRouteEndpointBuilder(builder); - Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName); + Assert.Equal("HTTP: GET /", routeEndpointBuilder.DisplayName); Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText); // Assert that we don't fallback to the route values @@ -205,7 +205,7 @@ public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Action Date: Wed, 18 Aug 2021 19:07:29 +0000 Subject: [PATCH 5/8] Address code review and clean up parser --- .../src/Microsoft.AspNetCore.Routing.csproj | 2 - ...ctionEndpointRouteBuilderExtensionsTest.cs | 27 ++- .../RoslynUtils/GeneratedNameConstants.cs | 17 -- src/Shared/RoslynUtils/GeneratedNameKind.cs | 70 -------- src/Shared/RoslynUtils/GeneratedNameParser.cs | 155 +----------------- src/Shared/RoslynUtils/TypeHelper.cs | 4 +- 6 files changed, 30 insertions(+), 245 deletions(-) delete mode 100644 src/Shared/RoslynUtils/GeneratedNameConstants.cs delete mode 100644 src/Shared/RoslynUtils/GeneratedNameKind.cs diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index dabaeffa8273..e92de9551933 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -26,8 +26,6 @@ Microsoft.AspNetCore.Routing.RouteCollection - - diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index 0337f28fbf3e..f28298a2d9e2 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -365,6 +365,7 @@ public void MapFallbackWithoutPath_BuildsEndpointWithLowestRouteOrder() // code in Roslyn to support this scenario. More info at https://github.com/dotnet/roslyn/issues/55651. public void MapMethod_DoesNotEndpointNameForInnerMethod() { + var name = "InnerGetString"; var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); string InnerGetString() => "TestString"; _ = builder.MapDelete("/", InnerGetString); @@ -374,13 +375,17 @@ public void MapMethod_DoesNotEndpointNameForInnerMethod() var endpoint = Assert.Single(dataSource.Endpoints); var endpointName = endpoint.Metadata.GetMetadata(); - Assert.NotNull(endpointName); - Assert.Equal("InnerGetString", endpointName?.EndpointName); + var routeName = endpoint.Metadata.GetMetadata(); + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal(name, endpointName?.EndpointName); + Assert.Equal(name, routeName?.RouteName); + Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName); } [Fact] public void MapMethod_SetsEndpointNameForMethodGroup() { + var name = "GetString"; var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); _ = builder.MapDelete("/", GetString); @@ -389,23 +394,31 @@ public void MapMethod_SetsEndpointNameForMethodGroup() var endpoint = Assert.Single(dataSource.Endpoints); var endpointName = endpoint.Metadata.GetMetadata(); - Assert.NotNull(endpointName); - Assert.Equal("GetString", endpointName?.EndpointName); + var routeName = endpoint.Metadata.GetMetadata(); + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal(name, endpointName?.EndpointName); + Assert.Equal(name, routeName?.RouteName); + Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName); } [Fact] public void WithNameOverridesDefaultEndpointName() { + var name = "SomeCustomName"; var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); - _ = builder.MapDelete("/", GetString).WithName("SomeCustomName"); + _ = builder.MapDelete("/", GetString).WithName(name); var dataSource = GetBuilderEndpointDataSource(builder); // Trigger Endpoint build by calling getter. var endpoint = Assert.Single(dataSource.Endpoints); var endpointName = endpoint.Metadata.GetMetadata(); - Assert.NotNull(endpointName); - Assert.Equal("SomeCustomName", endpointName?.EndpointName); + var routeName = endpoint.Metadata.GetMetadata(); + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal(name, endpointName?.EndpointName); + Assert.Equal(name, routeName?.RouteName); + // Will still use the original method name, not the custom endpoint name + Assert.Equal("HTTP: DELETE / => GetString", routeEndpointBuilder.DisplayName); } private string GetString() => "TestString"; diff --git a/src/Shared/RoslynUtils/GeneratedNameConstants.cs b/src/Shared/RoslynUtils/GeneratedNameConstants.cs deleted file mode 100644 index b771fb784464..000000000000 --- a/src/Shared/RoslynUtils/GeneratedNameConstants.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameConstants.cs -// and exist to address the issues with extracting original method names for -// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 -// for more info. -namespace Microsoft.CodeAnalysis.CSharp.Symbols -{ - internal static class GeneratedNameConstants - { - internal const char DotReplacementInTypeNames = '-'; - internal const string SynthesizedLocalNamePrefix = "CS$"; - internal const string SuffixSeparator = "__"; - internal const char LocalFunctionNameTerminator = '|'; - } -} \ No newline at end of file diff --git a/src/Shared/RoslynUtils/GeneratedNameKind.cs b/src/Shared/RoslynUtils/GeneratedNameKind.cs deleted file mode 100644 index 262ebf5264d8..000000000000 --- a/src/Shared/RoslynUtils/GeneratedNameKind.cs +++ /dev/null @@ -1,70 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameKind.cs -// and exist to address the issues with extracting original method names for -// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 -// for more info. -namespace Microsoft.CodeAnalysis.CSharp.Symbols -{ - internal enum GeneratedNameKind - { - None = 0, - - // Used by EE: - ThisProxyField = '4', - HoistedLocalField = '5', - DisplayClassLocalOrField = '8', - LambdaMethod = 'b', - LambdaDisplayClass = 'c', - StateMachineType = 'd', - LocalFunction = 'g', // note collision with Deprecated_InitializerLocal, however this one is only used for method names - - // Used by EnC: - AwaiterField = 'u', - HoistedSynthesizedLocalField = 's', - - // Currently not parsed: - StateMachineStateField = '1', - IteratorCurrentBackingField = '2', - StateMachineParameterProxyField = '3', - ReusableHoistedLocalField = '7', - LambdaCacheField = '9', - FixedBufferField = 'e', - AnonymousType = 'f', - TransparentIdentifier = 'h', - AnonymousTypeField = 'i', - AnonymousTypeTypeParameter = 'j', - AutoPropertyBackingField = 'k', - IteratorCurrentThreadIdField = 'l', - IteratorFinallyMethod = 'm', - BaseMethodWrapper = 'n', - AsyncBuilderField = 't', - DynamicCallSiteContainerType = 'o', - DynamicCallSiteField = 'p', - AsyncIteratorPromiseOfValueOrEndBackingField = 'v', - DisposeModeField = 'w', - CombinedTokensField = 'x', // last - - // Deprecated - emitted by Dev12, but not by Roslyn. - // Don't reuse the values because the debugger might encounter them when consuming old binaries. - [Obsolete] - Deprecated_OuterscopeLocals = '6', - [Obsolete] - Deprecated_IteratorInstance = 'a', - [Obsolete] - Deprecated_InitializerLocal = 'g', - [Obsolete] - Deprecated_DynamicDelegate = 'q', - [Obsolete] - Deprecated_ComrefCallLocal = 'r', - } - - internal static class GeneratedNameKindExtensions - { - internal static bool IsTypeName(this GeneratedNameKind kind) - => kind is GeneratedNameKind.LambdaDisplayClass or GeneratedNameKind.StateMachineType or GeneratedNameKind.DynamicCallSiteContainerType; - } -} \ No newline at end of file diff --git a/src/Shared/RoslynUtils/GeneratedNameParser.cs b/src/Shared/RoslynUtils/GeneratedNameParser.cs index ba6086484861..39315c86a4bc 100644 --- a/src/Shared/RoslynUtils/GeneratedNameParser.cs +++ b/src/Shared/RoslynUtils/GeneratedNameParser.cs @@ -1,169 +1,30 @@ // 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.Diagnostics.CodeAnalysis; -using System.Globalization; -// These sources are copied from https://github.com/dotnet/roslyn/blob/7d7bf0cc73e335390d73c9de6d7afd1e49605c9d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameParser.cs -// and exist to address the issues with extracting original method names for -// generated local functions. See https://github.com/dotnet/roslyn/issues/55651 +// This code is a stop-gapp and exists to address the issues with extracting +// original method names from generated local functions. See https://github.com/dotnet/roslyn/issues/55651 // for more info. namespace Microsoft.CodeAnalysis.CSharp.Symbols { internal static class GeneratedNameParser { - // Parse the generated name. Returns true for names of the form - // [CS$]<[middle]>c[__[suffix]] where [CS$] is included for certain - // generated names, where [middle] and [__[suffix]] are optional, - // and where c is a single character in [1-9a-z] - // (csharp\LanguageAnalysis\LIB\SpecialName.cpp). - internal static bool TryParseGeneratedName( - string name, - out GeneratedNameKind kind, - out int openBracketOffset, - out int closeBracketOffset) - { - openBracketOffset = -1; - if (name.StartsWith("CS$<", StringComparison.Ordinal)) - { - openBracketOffset = 3; - } - else if (name.StartsWith("<", StringComparison.Ordinal)) - { - openBracketOffset = 0; - } - - if (openBracketOffset >= 0) - { - closeBracketOffset = IndexOfBalancedParenthesis(name, openBracketOffset, '>'); - if (closeBracketOffset >= 0 && closeBracketOffset + 1 < name.Length) - { - int c = name[closeBracketOffset + 1]; - if (c is >= '1' and <= '9' or >= 'a' and <= 'z') // Note '0' is not special. - { - kind = (GeneratedNameKind)c; - return true; - } - } - } - - kind = GeneratedNameKind.None; - openBracketOffset = -1; - closeBracketOffset = -1; - return false; - } - - private static int IndexOfBalancedParenthesis(string str, int openingOffset, char closing) - { - char opening = str[openingOffset]; - - int depth = 1; - for (int i = openingOffset + 1; i < str.Length; i++) - { - var c = str[i]; - if (c == opening) - { - depth++; - } - else if (c == closing) - { - depth--; - if (depth == 0) - { - return i; - } - } - } - - return -1; - } - - internal static bool TryParseSourceMethodNameFromGeneratedName(string generatedName, GeneratedNameKind requiredKind, [NotNullWhen(true)] out string? methodName) - { - if (!TryParseGeneratedName(generatedName, out var kind, out int openBracketOffset, out int closeBracketOffset)) - { - methodName = null; - return false; - } - - if (requiredKind != 0 && kind != requiredKind) - { - methodName = null; - return false; - } - - methodName = generatedName.Substring(openBracketOffset + 1, closeBracketOffset - openBracketOffset - 1); - - if (kind.IsTypeName()) - { - methodName = methodName.Replace(GeneratedNameConstants.DotReplacementInTypeNames, '.'); - } - - return true; - } - /// /// Parses generated local function name out of a generated method name. /// - internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhen(true)] out string? localFunctionName) + internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhen(true)] out string? originalName) { - localFunctionName = null; - - // '<' containing-method-name '>' 'g' '__' local-function-name '|' method-ordinal '_' lambda-ordinal - if (!TryParseGeneratedName(generatedName, out var kind, out _, out int closeBracketOffset) || kind != GeneratedNameKind.LocalFunction) - { - return false; - } + originalName = null; - int localFunctionNameStart = closeBracketOffset + 2 + GeneratedNameConstants.SuffixSeparator.Length; - if (localFunctionNameStart >= generatedName.Length) - { - return false; - } - - int localFunctionNameEnd = generatedName.IndexOf(GeneratedNameConstants.LocalFunctionNameTerminator, localFunctionNameStart); - if (localFunctionNameEnd < 0) - { - return false; - } - - localFunctionName = generatedName.Substring(localFunctionNameStart, localFunctionNameEnd - localFunctionNameStart); - return true; - } - - // Extracts the slot index from a name of a field that stores hoisted variables or awaiters. - // Such a name ends with "__{slot index + 1}". - // Returned slot index is >= 0. - internal static bool TryParseSlotIndex(string fieldName, out int slotIndex) - { - int lastUnder = fieldName.LastIndexOf('_'); - if (lastUnder - 1 < 0 || lastUnder == fieldName.Length || fieldName[lastUnder - 1] != '_') - { - slotIndex = -1; - return false; - } - - if (int.TryParse(fieldName.AsSpan(lastUnder + 1), NumberStyles.None, CultureInfo.InvariantCulture, out slotIndex) && slotIndex >= 1) - { - slotIndex--; - return true; - } - - slotIndex = -1; - return false; - } - - internal static bool TryParseAnonymousTypeParameterName(string typeParameterName, [NotNullWhen(true)] out string? propertyName) - { - if (typeParameterName.StartsWith("<", StringComparison.Ordinal) && - typeParameterName.EndsWith(">j__TPar", StringComparison.Ordinal)) + var startIndex = generatedName.LastIndexOf(">g__", StringComparison.OrdinalIgnoreCase); + var endIndex = generatedName.LastIndexOf("|", StringComparison.OrdinalIgnoreCase); + if (startIndex >= 0 && endIndex >= 0 && endIndex - startIndex > 4) { - propertyName = typeParameterName.Substring(1, typeParameterName.Length - 9); + originalName = generatedName.Substring(startIndex + 4, endIndex - startIndex - 4); return true; } - propertyName = null; return false; } } diff --git a/src/Shared/RoslynUtils/TypeHelper.cs b/src/Shared/RoslynUtils/TypeHelper.cs index 1f81ee50526f..f87e62d55f97 100644 --- a/src/Shared/RoslynUtils/TypeHelper.cs +++ b/src/Shared/RoslynUtils/TypeHelper.cs @@ -15,12 +15,12 @@ internal static class TypeHelper /// /// /// The type to evaluate. Can be null if evaluating only on name. - /// The identifier associated wit the type. + /// The identifier associated with the type. /// if is compiler generated /// or represents a compiler generated identifier. internal static bool IsCompilerGenerated(string name, Type? type = null) { - return (type is Type && Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute))) + return (type is Type && Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute))) || name.StartsWith("<", StringComparison.Ordinal) || (name.IndexOf('$') >= 0); } From 1e7403145fea569f5f541a1e94163ed6c5627754 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 18 Aug 2021 21:27:54 +0000 Subject: [PATCH 6/8] Address feedback from Roslyn review --- ...malActionEndpointRouteBuilderExtensions.cs | 2 +- .../EndpointMetadataApiDescriptionProvider.cs | 2 +- src/Shared/RoslynUtils/GeneratedNameParser.cs | 6 ++-- src/Shared/RoslynUtils/TypeHelper.cs | 36 ++++++++++++------- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index 20a3814bba3c..6c25cf70114b 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -190,7 +190,7 @@ public static MinimalActionEndpointConventionBuilder Map( // Methods defined in a top-level program are generated as statics so the delegate // target will be null. Inline lambdas are compiler generated properties so they can // be filtered that way. - if (action.Target == null || !TypeHelper.IsCompilerGenerated(action.Method.Name)) + if (action.Target == null || !TypeHelper.IsCompilerGeneratedMethod(action.Method)) { if (!GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)) { diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index d6a93763e6e4..b27c729a747c 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -78,7 +78,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string // For now, put all methods defined the same declaring type together. string controllerName; - if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGenerated(methodInfo.DeclaringType.Name, methodInfo.DeclaringType)) + if (methodInfo.DeclaringType is not null && !TypeHelper.IsCompilerGeneratedType(methodInfo.DeclaringType)) { controllerName = methodInfo.DeclaringType.Name; } diff --git a/src/Shared/RoslynUtils/GeneratedNameParser.cs b/src/Shared/RoslynUtils/GeneratedNameParser.cs index 39315c86a4bc..5976fa27b7b3 100644 --- a/src/Shared/RoslynUtils/GeneratedNameParser.cs +++ b/src/Shared/RoslynUtils/GeneratedNameParser.cs @@ -3,7 +3,7 @@ using System.Diagnostics.CodeAnalysis; -// This code is a stop-gapp and exists to address the issues with extracting +// This code is a stop-gap and exists to address the issues with extracting // original method names from generated local functions. See https://github.com/dotnet/roslyn/issues/55651 // for more info. namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -17,8 +17,8 @@ internal static bool TryParseLocalFunctionName(string generatedName, [NotNullWhe { originalName = null; - var startIndex = generatedName.LastIndexOf(">g__", StringComparison.OrdinalIgnoreCase); - var endIndex = generatedName.LastIndexOf("|", StringComparison.OrdinalIgnoreCase); + var startIndex = generatedName.LastIndexOf(">g__", StringComparison.Ordinal); + var endIndex = generatedName.LastIndexOf("|", StringComparison.Ordinal); if (startIndex >= 0 && endIndex >= 0 && endIndex - startIndex > 4) { originalName = generatedName.Substring(startIndex + 4, endIndex - startIndex - 4); diff --git a/src/Shared/RoslynUtils/TypeHelper.cs b/src/Shared/RoslynUtils/TypeHelper.cs index f87e62d55f97..2eeba0f343c2 100644 --- a/src/Shared/RoslynUtils/TypeHelper.cs +++ b/src/Shared/RoslynUtils/TypeHelper.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Reflection; + namespace System.Runtime.CompilerServices { internal static class TypeHelper @@ -8,21 +10,31 @@ internal static class TypeHelper /// /// Checks to see if a given type is compiler generated. /// - /// The compiler doesn't always annotate every time it generates with the - /// CompilerGeneratedAttribute so sometimes we have to check if the type's - /// identifier represents a generated type. Follows the same heuristics seen - /// in https://github.com/dotnet/roslyn/blob/b57c1f89c1483da8704cde7b535a20fd029748db/src/ExpressionEvaluator/Core/Source/ResultProvider/Helpers/GeneratedMetadataNames.cs#L19 + /// The compiler will annotate either the target type or the declaring type + /// with the CompilerGenerated attribute. We walk up the declaring types until + /// we find a CompilerGenerated attribute or declare the type as not compiler + /// generated otherwise. /// /// - /// The type to evaluate. Can be null if evaluating only on name. - /// The identifier associated with the type. - /// if is compiler generated - /// or represents a compiler generated identifier. - internal static bool IsCompilerGenerated(string name, Type? type = null) + /// The type to evaluate. + /// if is compiler generated. + internal static bool IsCompilerGeneratedType(Type? type = null) + { + if (type is Type) + { + return Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(type.DeclaringType); + } + return false; + } + + /// + /// Checks to see if a given method is compiler generated. + /// + /// The method to evaluate. + /// if is compiler generated. + internal static bool IsCompilerGeneratedMethod(MethodInfo method) { - return (type is Type && Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute))) - || name.StartsWith("<", StringComparison.Ordinal) - || (name.IndexOf('$') >= 0); + return Attribute.IsDefined(method, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(method.DeclaringType); } } } \ No newline at end of file From 54ee5cdf958d77dfd7019c41798a137c7887840b Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 18 Aug 2021 23:08:59 +0000 Subject: [PATCH 7/8] Clean up type null validation --- src/Shared/RoslynUtils/TypeHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/RoslynUtils/TypeHelper.cs b/src/Shared/RoslynUtils/TypeHelper.cs index 2eeba0f343c2..b7e1f068d62e 100644 --- a/src/Shared/RoslynUtils/TypeHelper.cs +++ b/src/Shared/RoslynUtils/TypeHelper.cs @@ -20,7 +20,7 @@ internal static class TypeHelper /// if is compiler generated. internal static bool IsCompilerGeneratedType(Type? type = null) { - if (type is Type) + if (type is not null) { return Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || IsCompilerGeneratedType(type.DeclaringType); } From 56c98a372710235ffc0af074b49886f6f08cb361 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 19 Aug 2021 01:12:37 +0000 Subject: [PATCH 8/8] Fix scenario for local function --- ...malActionEndpointRouteBuilderExtensions.cs | 10 ++++----- ...ctionEndpointRouteBuilderExtensionsTest.cs | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index 6c25cf70114b..15281a28ccb3 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -188,14 +188,12 @@ public static MinimalActionEndpointConventionBuilder Map( builder.Metadata.Add(action.Method); // Methods defined in a top-level program are generated as statics so the delegate - // target will be null. Inline lambdas are compiler generated properties so they can + // target will be null. Inline lambdas are compiler generated method so they can // be filtered that way. - if (action.Target == null || !TypeHelper.IsCompilerGeneratedMethod(action.Method)) + if (GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName) + || !TypeHelper.IsCompilerGeneratedMethod(action.Method)) { - if (!GeneratedNameParser.TryParseLocalFunctionName(action.Method.Name, out var endpointName)) - { - endpointName = action.Method.Name; - } + endpointName ??= action.Method.Name; builder.Metadata.Add(new EndpointNameMetadata(endpointName)); builder.Metadata.Add(new RouteNameMetadata(endpointName)); diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index f28298a2d9e2..2d1edcbd7094 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -382,6 +382,28 @@ public void MapMethod_DoesNotEndpointNameForInnerMethod() Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName); } + [Fact] + public void MapMethod_DoesNotEndpointNameForInnerMethodWithTarget() + { + var name = "InnerGetString"; + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + var testString = "TestString"; + string InnerGetString() => testString; + _ = builder.MapDelete("/", InnerGetString); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointName = endpoint.Metadata.GetMetadata(); + var routeName = endpoint.Metadata.GetMetadata(); + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal(name, endpointName?.EndpointName); + Assert.Equal(name, routeName?.RouteName); + Assert.Equal("HTTP: DELETE / => InnerGetString", routeEndpointBuilder.DisplayName); + } + + [Fact] public void MapMethod_SetsEndpointNameForMethodGroup() {