diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index 2c8c60168300..ef2bef8f2240 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -196,4 +196,13 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + internal static readonly DiagnosticDescriptor AtMostOneFromBodyAttribute = new( + "ASP0024", + new LocalizableResourceString(nameof(Resources.Analyzer_MultipleFromBody_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_MultipleFromBody_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 7b858c8e70df..d69b04c8126c 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -207,4 +207,10 @@ Suggest using IHeaderDictionary.Append or the indexer - \ No newline at end of file + + Route handler has multiple parameters with the [FromBody] attribute or a parameter with an [AsParameters] attribute where the parameter type contains multiple members with [FromBody] attributes. Only one parameter can have a [FromBody] attribute. + + + Route handler has multiple parameters with the [FromBody] attribute. + + diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/AtMostOneFromBodyAttribute.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/AtMostOneFromBodyAttribute.cs new file mode 100644 index 000000000000..9081a15d3dd0 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/AtMostOneFromBodyAttribute.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure; +using Microsoft.AspNetCore.App.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.RouteHandlers; + +using WellKnownType = WellKnownTypeData.WellKnownType; + +public partial class RouteHandlerAnalyzer : DiagnosticAnalyzer +{ + private static void AtMostOneFromBodyAttribute( + in OperationAnalysisContext context, + WellKnownTypes wellKnownTypes, + IMethodSymbol methodSymbol) + { + var fromBodyMetadataInterfaceType = wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromBodyMetadata); + var asParametersAttributeType = wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_AsParametersAttribute); + + var asParametersDecoratedParameters = methodSymbol.Parameters.Where(p => p.HasAttribute(asParametersAttributeType)); + + foreach (var asParameterDecoratedParameter in asParametersDecoratedParameters) + { + var fromBodyMetadataInterfaceMembers = asParameterDecoratedParameter.Type.GetMembers().Where( + m => m.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType) + ); + + if (fromBodyMetadataInterfaceMembers.Count() >= 2) + { + ReportDiagnostics(context, fromBodyMetadataInterfaceMembers); + } + } + + var fromBodyMetadataInterfaceParameters = methodSymbol.Parameters.Where(p => p.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType)); + + if (fromBodyMetadataInterfaceParameters.Count() >= 2) + { + ReportDiagnostics(context, fromBodyMetadataInterfaceParameters); + } + + static void ReportDiagnostics(OperationAnalysisContext context, IEnumerable symbols) + { + foreach (var symbol in symbols) + { + if (symbol.DeclaringSyntaxReferences.Length > 0) + { + var syntax = symbol.DeclaringSyntaxReferences[0].GetSyntax(context.CancellationToken); + var location = syntax.GetLocation(); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.AtMostOneFromBodyAttribute, + location + )); + } + } + } + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs index 6f6d8c8cb71f..ce65f2d1e42f 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs @@ -17,11 +17,10 @@ public partial class RouteHandlerAnalyzer : DiagnosticAnalyzer { private static void DisallowNonParsableComplexTypesOnParameters( in OperationAnalysisContext context, + WellKnownTypes wellKnownTypes, RouteUsageModel routeUsage, IMethodSymbol methodSymbol) { - var wellKnownTypes = WellKnownTypes.GetOrCreate(context.Compilation); - foreach (var handlerDelegateParameter in methodSymbol.Parameters) { // If the parameter is decorated with a FromServices attribute then we can skip it. diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/RouteHandlerAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/RouteHandlerAnalyzer.cs index 18eba86d37dc..c5dd2f0a9d8c 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/RouteHandlerAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/RouteHandlerAnalyzer.cs @@ -27,7 +27,8 @@ public partial class RouteHandlerAnalyzer : DiagnosticAnalyzer DiagnosticDescriptors.DetectMismatchedParameterOptionality, DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsableOrBindable, DiagnosticDescriptors.BindAsyncSignatureMustReturnValueTaskOfT, - DiagnosticDescriptors.AmbiguousRouteHandlerRoute + DiagnosticDescriptors.AmbiguousRouteHandlerRoute, + DiagnosticDescriptors.AtMostOneFromBodyAttribute ); public override void Initialize(AnalysisContext context) @@ -105,17 +106,19 @@ void DoOperationAnalysis(OperationAnalysisContext context, ConcurrentDictionary< { var lambda = (IAnonymousFunctionOperation)delegateCreation.Target; DisallowMvcBindArgumentsOnParameters(in context, wellKnownTypes, invocation, lambda.Symbol); - DisallowNonParsableComplexTypesOnParameters(in context, routeUsage, lambda.Symbol); + DisallowNonParsableComplexTypesOnParameters(in context, wellKnownTypes, routeUsage, lambda.Symbol); DisallowReturningActionResultFromMapMethods(in context, wellKnownTypes, invocation, lambda, delegateCreation.Syntax); DetectMisplacedLambdaAttribute(context, lambda); DetectMismatchedParameterOptionality(in context, routeUsage, lambda.Symbol); + AtMostOneFromBodyAttribute(in context, wellKnownTypes, lambda.Symbol); } else if (delegateCreation.Target.Kind == OperationKind.MethodReference) { var methodReference = (IMethodReferenceOperation)delegateCreation.Target; DisallowMvcBindArgumentsOnParameters(in context, wellKnownTypes, invocation, methodReference.Method); - DisallowNonParsableComplexTypesOnParameters(in context, routeUsage, methodReference.Method); + DisallowNonParsableComplexTypesOnParameters(in context, wellKnownTypes, routeUsage, methodReference.Method); DetectMismatchedParameterOptionality(in context, routeUsage, methodReference.Method); + AtMostOneFromBodyAttribute(in context, wellKnownTypes, methodReference.Method); var foundMethodReferenceBody = false; if (!methodReference.Method.DeclaringSyntaxReferences.IsEmpty) diff --git a/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/AtMostOneFromBodyAttributeTest.cs b/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/AtMostOneFromBodyAttributeTest.cs new file mode 100644 index 000000000000..472ad4a90f1f --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/RouteHandlers/AtMostOneFromBodyAttributeTest.cs @@ -0,0 +1,173 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.Policy; +using Microsoft.CodeAnalysis.Testing; +using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpAnalyzerVerifier; + +namespace Microsoft.AspNetCore.Analyzers.RouteHandlers; + +public partial class AtMostOneFromBodyAttributeTest +{ + private TestDiagnosticAnalyzerRunner Runner { get; } = new(new RouteHandlerAnalyzer()); + + [Fact] + public async Task Handler_With_No_FromBody_Attributes_Works() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", (string productId, Product product) => {}); + +public class Product +{ +} +"; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Handler_With_One_FromBody_Attributes_Works() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", (string productId, [FromBody]Product product) => {}); + +public class Product +{ +} +"; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task Handler_With_Two_FromBody_Attributes_Fails() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", (string productId, {|#0:[FromBody]Product product1|}, {|#1:[FromBody]Product product2|}) => {}); + +public class Product +{ +} +"; + + var expectedDiagnostic1 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(0); + var expectedDiagnostic2 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(1); + + // Act + await VerifyCS.VerifyAnalyzerAsync( + source, + expectedDiagnostic1, + expectedDiagnostic2 + ); + } + + [Fact] + public async Task MethodGroup_Handler_With_Two_FromBody_Attributes_Fails() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", MyHandlers.ProcessRequest); + +public static class MyHandlers +{ + public static void ProcessRequest(string productId, {|#0:[FromBody]Product product1|}, {|#1:[FromBody]Product product2|}) + { + } +} + +public class Product +{ +} +"; + + var expectedDiagnostic1 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(0); + var expectedDiagnostic2 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(1); + + // Act + await VerifyCS.VerifyAnalyzerAsync( + source, + expectedDiagnostic1, + expectedDiagnostic2 + ); + } + + [Fact] + public async Task Handler_Handler_With_AsParameters_Argument_With_TwoFromBody_Attributes_Fails() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", ([AsParameters]GetProductRequest request) => {}); + +public class GetProductRequest +{ + {|#0:[FromBody] + public Product Product1 { get; set; }|} + + {|#1:[FromBody] + public Product Product2 { get; set; }|} +} + +public class Product +{ +} +"; + + var expectedDiagnostic1 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(0); + var expectedDiagnostic2 = new DiagnosticResult(DiagnosticDescriptors.AtMostOneFromBodyAttribute).WithLocation(1); + + // Act + await VerifyCS.VerifyAnalyzerAsync( + source, + expectedDiagnostic1, + expectedDiagnostic2 + ); + } + + [Fact] + public async Task Handler_Handler_With_AsParameters_Argument_With_OneFromBody_Attributes_Works() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Builder; +var webApp = WebApplication.Create(); +webApp.MapPost(""/products/{productId}"", ([AsParameters]GetProductRequest request) => {}); + +public class GetProductRequest +{ + {|#0:[FromBody] + public Product Product1 { get; set; }|} + + public Product Product2 { get; set; } +} + +public class Product +{ +} +"; + + // Act + await VerifyCS.VerifyAnalyzerAsync(source); + } +}