diff --git a/src/Framework/App.Ref/src/CompatibilitySuppressions.xml b/src/Framework/App.Ref/src/CompatibilitySuppressions.xml index e2ec12ef3b32..6aae8c381577 100644 --- a/src/Framework/App.Ref/src/CompatibilitySuppressions.xml +++ b/src/Framework/App.Ref/src/CompatibilitySuppressions.xml @@ -4,4 +4,8 @@ PKV004 net7.0 + + PKV004 + net8.0 + \ No newline at end of file diff --git a/src/Framework/App.Runtime/src/CompatibilitySuppressions.xml b/src/Framework/App.Runtime/src/CompatibilitySuppressions.xml index 056469f78b07..61fb1abb7e35 100644 --- a/src/Framework/App.Runtime/src/CompatibilitySuppressions.xml +++ b/src/Framework/App.Runtime/src/CompatibilitySuppressions.xml @@ -4,4 +4,8 @@ PKV0001 net7.0 + + PKV0001 + net8.0 + \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternTree.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternTree.cs index 88844a842688..bd40589e5fab 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternTree.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Infrastructure/RoutePattern/RoutePatternTree.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using Microsoft.AspNetCore.Analyzers.Infrastructure.EmbeddedSyntax; using Microsoft.AspNetCore.Analyzers.Infrastructure.VirtualChars; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Text; namespace Microsoft.AspNetCore.Analyzers.Infrastructure.RoutePattern; diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/SymbolExtensions.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/SymbolExtensions.cs index 8a23fe3f33fd..e80bb5054eff 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/SymbolExtensions.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/SymbolExtensions.cs @@ -26,14 +26,14 @@ public static bool HasAttribute(this ISymbol symbol, INamedTypeSymbol attributeT public static bool HasAttributeImplementingInterface(this ISymbol symbol, INamedTypeSymbol interfaceType) { - return HasAttributeImplementingInterface(symbol, interfaceType, out var _); + return symbol.HasAttributeImplementingInterface(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)) + if (attributeData.AttributeClass is not null && attributeData.AttributeClass.Implements(interfaceType)) { matchedAttribute = attributeData; return true; diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs index 036c4bbf37f8..0e90cbc31b45 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs @@ -64,8 +64,7 @@ private static void DisallowNonParsableComplexTypesOnParameters( location )) { continue; } - // Match handler parameter against route parameters. If it is a route parameter it needs to be parsable/bindable in some fashion. - if (routeUsage.RoutePattern.TryGetRouteParameter(handlerDelegateParameter.Name, out var routeParameter)) + if (IsRouteParameter(routeUsage, handlerDelegateParameter)) { var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes); var bindability = ParsabilityHelper.GetBindability(parameterTypeSymbol, wellKnownTypes); @@ -77,7 +76,7 @@ private static void DisallowNonParsableComplexTypesOnParameters( context.ReportDiagnostic(Diagnostic.Create( descriptor, location, - routeParameter.Name, + handlerDelegateParameter.Name, parameterTypeSymbol.Name )); } @@ -86,16 +85,22 @@ private static void DisallowNonParsableComplexTypesOnParameters( } } - static bool HasAttributeImplementingFromMetadataInterfaceType(IParameterSymbol parameter, WellKnownType fromMetadataInterfaceType, WellKnownTypes wellKnownTypes) + static bool IsRouteParameter(RouteUsageModel routeUsage, IParameterSymbol handlerDelegateParameter) { - var fromMetadataInterfaceTypeSymbol = wellKnownTypes.Get(fromMetadataInterfaceType); - return parameter.GetAttributes().Any(ad => ad.AttributeClass.Implements(fromMetadataInterfaceTypeSymbol)); + // This gets the ParameterSymbol (concept from RouteEmbeddedLanguage) regardless of whether it is in + // the route pattern or not. If it is and it has a custom [FromRoute(Name = "blah")] attribute then + // RouteParameterName and we'll be able to find it by looking it up in the route pattern (thus confirming + // that it is a route parameter). + var resolvedParameter = routeUsage.UsageContext.ResolvedParameters.FirstOrDefault(rp => rp.Symbol.Name == handlerDelegateParameter.Name); + var isRouteParameter = routeUsage.RoutePattern.TryGetRouteParameter(resolvedParameter.RouteParameterName, out var _); + return isRouteParameter; } static bool ReportFromAttributeDiagnostic(OperationAnalysisContext context, WellKnownType fromMetadataInterfaceType, WellKnownTypes wellKnownTypes, IParameterSymbol parameter, INamedTypeSymbol parameterTypeSymbol, Location location) { + var fromMetadataInterfaceTypeSymbol = wellKnownTypes.Get(fromMetadataInterfaceType); var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes); - if (HasAttributeImplementingFromMetadataInterfaceType(parameter, fromMetadataInterfaceType, wellKnownTypes) && parsability != Parsability.Parsable) + if (parameter.HasAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable) { var descriptor = SelectDescriptor(parsability, Bindability.NotBindable); diff --git a/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/DisallowNonParsableComplexTypesOnParametersTest.cs b/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/DisallowNonParsableComplexTypesOnParametersTest.cs index e9bcd6bd8791..a4bffcbc3a0f 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/DisallowNonParsableComplexTypesOnParametersTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/DisallowNonParsableComplexTypesOnParametersTest.cs @@ -203,6 +203,91 @@ public class Customer await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostic); } + [Fact] + public async Task Route_Parameter_withNameChanged_viaFromRoute_whenNotParsable_Fails() + { + // Arrange + var source = $$""" +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +var webApp = WebApplication.Create(); +webApp.MapGet("/customers/{cust}", ({|#0:[FromRoute(Name = "cust")]Customer customer|}) => {}); + +public class Customer +{ +} +"""; + + var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable) + .WithArguments("customer", "Customer") + .WithLocation(0); + + // Act + await VerifyCS.VerifyAnalyzerAsync(source, expectedDiagnostic); + } + + [Fact] + public async Task Route_Parameter_withTwoReceivingHandlerParameters_Works() + { + // Arrange + var source = $$""" +using System; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +var webApp = WebApplication.Create(); +webApp.MapGet("/customers/{cust}", ([FromRoute(Name = "cust")]Customer customer, [FromRoute(Name = "cust")]string customerKey) => {}); + +public class Customer +{ + public static bool TryParse(string s, IFormatProvider provider, out Customer result) + { + result = new Customer(); + return true; + } +} +"""; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Route_Parameter_withNameChanged_viaFromRoute_whenParsable_Works() + { + // Arrange + var source = $$""" +using System; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +var webApp = WebApplication.Create(); +webApp.MapGet("/customers/{cust}", ({|#0:[FromRoute(Name = "cust")]Customer customer|}) => {}); + +public class Customer : IParsable +{ + public static Customer Parse(string s, IFormatProvider provider) + { + if (TryParse(s, provider, out Customer customer)) + { + return customer; + } + else + { + throw new ArgumentException(s); + } + } + + public static bool TryParse(string s, IFormatProvider provider, out Customer result) + { + result = new Customer(); + return true; + } +} +"""; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + [Fact] public async Task Route_Parameter_withBindAsyncMethodThatReturnsTask_of_T_Fails() { @@ -489,7 +574,7 @@ public class Customer } [Fact] - public async Task Handler_Parameter_withWellknownTypes_Works() + public async Task Handler_Parameter_withWellKnownTypes_Works() { // Arrange var source = $$""" @@ -538,6 +623,108 @@ public async Task Handler_Parameter_with_FromService_Attribute_Works() public class MyService { +} +"""; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Handler_Parameter_withServiceInterface_Works() + { + // Arrange + var source = $$""" +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Builder; + +var webApp = WebApplication.Create(); +webApp.MapGet("/weatherforecast", (HttpContext context, IDownstreamWebApi downstreamWebApi) => {}); + +// This type doesn't need to be parsable because it should be assumed to be a service type. +public interface IDownstreamWebApi +{ +} +"""; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Route_Parameter_withAbstractBaseType_Works() + { + // Arrange + var source = $$""" +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Builder; + +var builder = WebApplication.CreateBuilder(args); +var app = builder.Build(); + +app.MapGet("/", () => "Hello World!"); +app.MapGet("/{customer}", (BaseCustomer customer) => { }); +app.Run(); + +public abstract class BaseCustomer : IParsable +{ + public static BaseCustomer Parse(string s, IFormatProvider? provider) + { + return new CommercialCustomer(); + } + + public static bool TryParse(string? s, IFormatProvider? provider, out BaseCustomer result) + { + result = new CommercialCustomer(); + return true; + } +} + +public class CommercialCustomer : BaseCustomer +{ + +} +"""; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Route_Parameter_withInterfaceType_Works() + { + // Arrange + var source = $$""" +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Builder; + +var builder = WebApplication.CreateBuilder(args); +var app = builder.Build(); + +app.MapGet("/", () => "Hello World!"); +app.MapGet("/{customer}", (ICustomer customer) => { }); +app.Run(); + +public interface ICustomer +{ + public static ICustomer Parse(string s, IFormatProvider? provider) + { + return new CommercialCustomer(); + } + + public static bool TryParse(string? s, IFormatProvider? provider, out ICustomer result) + { + result = new CommercialCustomer(); + return true; + } +} + +public class CommercialCustomer : ICustomer +{ + } """;