diff --git a/AspNetCore.sln b/AspNetCore.sln index 2e2f7d19d4a2..2472df941390 100644 --- a/AspNetCore.sln +++ b/AspNetCore.sln @@ -1646,12 +1646,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Wasm.Prerendered.Client", " EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Wasm.Prerendered.Server", "src\Components\WebAssembly\testassets\Wasm.Prerendered.Server\Wasm.Prerendered.Server.csproj", "{6D365C86-3158-49F5-A21D-506C1E06E870}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.App.Analyzer", "src\Framework\Analyzer\src\Microsoft.AspNetCore.App.Analyzers.csproj", "{564CABB8-1B3F-4D9E-909D-260EF2B8614A}" -EndProject -Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Analyzer", "Analyzer", "{EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}" -EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.App.Analyzer.Test", "src\Framework\Analyzer\test\Microsoft.AspNetCore.App.Analyzers.Test.csproj", "{CF4CEC18-798D-46EC-B0A0-98D97496590F}" -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -7867,30 +7861,6 @@ Global {6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x64.Build.0 = Release|Any CPU {6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x86.ActiveCfg = Release|Any CPU {6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x86.Build.0 = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.Build.0 = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.ActiveCfg = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.Build.0 = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.ActiveCfg = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.Build.0 = Debug|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.ActiveCfg = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.Build.0 = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.ActiveCfg = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.Build.0 = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.ActiveCfg = Release|Any CPU - {564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.Build.0 = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.Build.0 = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.ActiveCfg = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.Build.0 = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.ActiveCfg = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.Build.0 = Debug|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.ActiveCfg = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.Build.0 = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.ActiveCfg = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.Build.0 = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.ActiveCfg = Release|Any CPU - {CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -8706,9 +8676,6 @@ Global {835A4E0F-A697-4B69-9736-3E99D163C4B9} = {48526D13-69E2-4409-A57B-C3FA3C64B4F7} {148A5B4F-C8A3-4468-92F6-51DB5641FB49} = {7D2B0799-A634-42AC-AE77-5D167BA51389} {6D365C86-3158-49F5-A21D-506C1E06E870} = {7D2B0799-A634-42AC-AE77-5D167BA51389} - {564CABB8-1B3F-4D9E-909D-260EF2B8614A} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD} - {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD} = {A4C26078-B6D8-4FD8-87A6-7C15A3482038} - {CF4CEC18-798D-46EC-B0A0-98D97496590F} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {3E8720B3-DBDD-498C-B383-2CC32A054E8F} diff --git a/eng/Dependencies.props b/eng/Dependencies.props index 4f64075c07cb..8a50fb9f218b 100644 --- a/eng/Dependencies.props +++ b/eng/Dependencies.props @@ -56,6 +56,7 @@ and are generated based on the last package release. + diff --git a/eng/Versions.props b/eng/Versions.props index 66da05084c94..ee5c29087ddf 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -188,6 +188,7 @@ 4.0.0-2.21354.7 4.0.0-2.21354.7 3.3.0 + 1.1.1-beta1.21413.1 1.0.0-20200708.1 6.10.0 6.10.0 diff --git a/src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj b/src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj index e7868125ebf3..3006e597737f 100644 --- a/src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj +++ b/src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj @@ -63,7 +63,11 @@ This package is an internal implementation of the .NET Core SDK and is not meant - + @@ -162,6 +166,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant + diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DelegateEndpointAnalyzer.cs similarity index 93% rename from src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DelegateEndpointAnalyzer.cs index aca8224bf9c7..927429ac5728 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DelegateEndpointAnalyzer.cs @@ -18,7 +18,8 @@ public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer { DiagnosticDescriptors.DoNotUseModelBindingAttributesOnDelegateEndpointParameters, DiagnosticDescriptors.DoNotReturnActionResultsFromMapActions, - DiagnosticDescriptors.DetectMisplacedLambdaAttribute + DiagnosticDescriptors.DetectMisplacedLambdaAttribute, + DiagnosticDescriptors.DetectMismatchedParameterOptionality }); public override void Initialize(AnalysisContext context) @@ -56,11 +57,13 @@ public override void Initialize(AnalysisContext context) DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol); DisallowReturningActionResultFromMapMethods(in operationAnalysisContext, wellKnownTypes, invocation, lambda); DetectMisplacedLambdaAttribute(operationAnalysisContext, invocation, lambda); + DetectMismatchedParameterOptionality(in operationAnalysisContext, invocation, lambda.Symbol); } else if (delegateCreation.Target.Kind == OperationKind.MethodReference) { var methodReference = (IMethodReferenceOperation)delegateCreation.Target; DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, methodReference.Method); + DetectMismatchedParameterOptionality(in operationAnalysisContext, invocation, methodReference.Method); var foundMethodReferenceBody = false; if (!methodReference.Method.DeclaringSyntaxReferences.IsEmpty) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMismatchedParameterOptionality.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMismatchedParameterOptionality.cs new file mode 100644 index 000000000000..8ccde050a525 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMismatchedParameterOptionality.cs @@ -0,0 +1,139 @@ +// 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.Linq; +using System.Collections.Generic; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer +{ + internal const string DetectMismatchedParameterOptionalityRuleId = "ASP0006"; + + private static void DetectMismatchedParameterOptionality( + in OperationAnalysisContext context, + IInvocationOperation invocation, + IMethodSymbol methodSymbol) + { + if (invocation.Arguments.Length < 2) + { + return; + } + + var value = invocation.Arguments[1].Value; + if (value.ConstantValue is not { HasValue: true } constant || + constant.Value is not string routeTemplate) + { + return; + } + + var allDeclarations = methodSymbol.GetAllMethodSymbolsOfPartialParts(); + foreach (var method in allDeclarations) + { + var parametersInArguments = method.Parameters; + var enumerator = new RouteTokenEnumerator(routeTemplate); + + while (enumerator.MoveNext()) + { + foreach (var parameter in parametersInArguments) + { + var paramName = parameter.Name; + // If this is not the methpd parameter associated with the route + // parameter then continue looking for it in the list + if (!enumerator.CurrentName.Equals(paramName.AsSpan(), StringComparison.OrdinalIgnoreCase)) + { + continue; + } + var argumentIsOptional = parameter.IsOptional || parameter.NullableAnnotation != NullableAnnotation.NotAnnotated; + var location = parameter.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax().GetLocation(); + var routeParamIsOptional = enumerator.CurrentQualifiers.IndexOf('?') > -1; + + if (!argumentIsOptional && routeParamIsOptional) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.DetectMismatchedParameterOptionality, + location, + paramName)); + } + } + } + } + } + + internal ref struct RouteTokenEnumerator + { + private ReadOnlySpan _routeTemplate; + + public RouteTokenEnumerator(string routeTemplateString) + { + _routeTemplate = routeTemplateString.AsSpan(); + CurrentName = default; + CurrentQualifiers = default; + } + + public ReadOnlySpan CurrentName { get; private set; } + public ReadOnlySpan CurrentQualifiers { get; private set; } + + public bool MoveNext() + { + if (_routeTemplate.IsEmpty) + { + return false; + } + + findStartBrace: + var startIndex = _routeTemplate.IndexOf('{'); + if (startIndex == -1) + { + return false; + } + + if (startIndex < _routeTemplate.Length - 1 && _routeTemplate[startIndex + 1] == '{') + { + // Escaped sequence + _routeTemplate = _routeTemplate.Slice(startIndex + 1); + goto findStartBrace; + } + + var tokenStart = startIndex + 1; + + findEndBrace: + var endIndex = IndexOf(_routeTemplate, tokenStart, '}'); + if (endIndex == -1) + { + return false; + } + if (endIndex < _routeTemplate.Length - 1 && _routeTemplate[endIndex + 1] == '}') + { + tokenStart = endIndex + 2; + goto findEndBrace; + } + + var token = _routeTemplate.Slice(startIndex + 1, endIndex - startIndex - 1); + var qualifier = token.IndexOfAny(new[] { ':', '=', '?' }); + CurrentName = qualifier == -1 ? token : token.Slice(0, qualifier); + CurrentQualifiers = qualifier == -1 ? null : token.Slice(qualifier); + + _routeTemplate = _routeTemplate.Slice(endIndex + 1); + return true; + } + } + + private static int IndexOf(ReadOnlySpan span, int startIndex, char c) + { + for (var i = startIndex; i < span.Length; i++) + { + if (span[i] == c) + { + return i; + } + } + + return -1; + } +} \ No newline at end of file diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMisplacedLambdaAttribute.cs similarity index 100% rename from src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMisplacedLambdaAttribute.cs diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs similarity index 79% rename from src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index 396ebef19efe..4cb94d5f26db 100644 --- a/src/Framework/Analyzer/src/DelegateEndpoints/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -34,5 +34,14 @@ internal static class DiagnosticDescriptors DiagnosticSeverity.Warning, isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + internal static readonly DiagnosticDescriptor DetectMismatchedParameterOptionality = new( + "ASP0006", + "Route parameter and argument optionality is mismatched", + "'{0}' argument should be annotated as optional or nullable to match route parameter", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); } } diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DisallowMvcBindArgumentsOnParameters.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowMvcBindArgumentsOnParameters.cs similarity index 100% rename from src/Framework/Analyzer/src/DelegateEndpoints/DisallowMvcBindArgumentsOnParameters.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowMvcBindArgumentsOnParameters.cs diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/DisallowReturningActionResultFromMapMethods.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowReturningActionResultFromMapMethods.cs similarity index 100% rename from src/Framework/Analyzer/src/DelegateEndpoints/DisallowReturningActionResultFromMapMethods.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowReturningActionResultFromMapMethods.cs diff --git a/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj similarity index 83% rename from src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj index e8be68688958..12662dd9bc7d 100644 --- a/src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj @@ -10,9 +10,10 @@ - + + diff --git a/src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WellKnownTypes.cs similarity index 100% rename from src/Framework/Analyzer/src/DelegateEndpoints/WellKnownTypes.cs rename to src/Framework/AspNetCoreAnalyzers/src/Analyzers/WellKnownTypes.cs diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/DetectMismatchedParameterOptionalityFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/DetectMismatchedParameterOptionalityFixer.cs new file mode 100644 index 000000000000..0b4f97ec0fe2 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/DetectMismatchedParameterOptionalityFixer.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Linq; +using System.Threading; +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzers.DelegateEndpoints; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints.Fixers; + +public class DetectMismatchedParameterOptionalityFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.DetectMismatchedParameterOptionality.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + { + foreach (var diagnostic in context.Diagnostics) + { + context.RegisterCodeFix( + CodeAction.Create("Fix mismatched route parameter and argument optionality", + cancellationToken => FixMismatchedParameterOptionality(diagnostic, context.Document, cancellationToken), + equivalenceKey: DiagnosticDescriptors.DetectMismatchedParameterOptionality.Id), + diagnostic); + } + + return Task.CompletedTask; + } + + private static async Task FixMismatchedParameterOptionality(Diagnostic diagnostic, Document document, CancellationToken cancellationToken) + { + DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken); + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + if (root == null) + { + return document; + } + + var param = root.FindNode(diagnostic.Location.SourceSpan); + if (param is ParameterSyntax { Type: { } parameterType } parameterSyntax) + { + var newParam = parameterSyntax.WithType(SyntaxFactory.NullableType(parameterType)); + editor.ReplaceNode(parameterSyntax, newParam); + } + + return editor.GetChangedDocument(); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj new file mode 100644 index 000000000000..d70178504e71 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj @@ -0,0 +1,17 @@ + + + CSharp CodeFixes for ASP.NET Core. + false + false + netstandard2.0 + false + Enable + Microsoft.AspNetCore.Analyzers + + + + + + + + diff --git a/src/Framework/Analyzer/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj similarity index 73% rename from src/Framework/Analyzer/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj rename to src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj index 8434a97bbfc2..6ec72c844de5 100644 --- a/src/Framework/Analyzer/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj +++ b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj @@ -11,11 +11,13 @@ - + + + diff --git a/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DetectMismatchedParameterOptionalityTest.cs b/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DetectMismatchedParameterOptionalityTest.cs new file mode 100644 index 000000000000..baa4841bb0cb --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DetectMismatchedParameterOptionalityTest.cs @@ -0,0 +1,396 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using VerifyCS = Microsoft.AspNetCore.Analyzers.DelegateEndpoints.CSharpDelegateEndpointsCodeFixVerifier< + Microsoft.AspNetCore.Analyzers.DelegateEndpoints.DelegateEndpointAnalyzer, + Microsoft.AspNetCore.Analyzers.DelegateEndpoints.Fixers.DetectMismatchedParameterOptionalityFixer>; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public partial class DetectMismatchedParameterOptionalityTest +{ + [Fact] + public async Task MatchingRequiredOptionality_CanBeFixed() + { + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}"", ({|#0:string name|}) => $""Hello {name}"");"; + + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}"", (string? name) => $""Hello {name}"");"; + + var expectedDiagnostics = new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0); + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource); + } + + [Fact] + public async Task MatchingMultipleRequiredOptionality_CanBeFixed() + { + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", ({|#0:string name|}, {|#1:string title|}) => $""Hello {name}, you are a {title}.""); +"; + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", (string? name, string? title) => $""Hello {name}, you are a {title}.""); +"; + var expectedDiagnostics = new[] { + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("title").WithLocation(1) + }; + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource); + + } + + [Fact] + public async Task MatchingSingleRequiredOptionality_CanBeFixed() + { + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", ({|#0:string name|}, string? title) => $""Hello {name}, you are a {title}.""); +"; + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", (string? name, string? title) => $""Hello {name}, you are a {title}.""); +"; + var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0); + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostic, fixedSource); + } + + [Fact] + public async Task MismatchedOptionalityInMethodGroup_CanBeFixed() + { + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +string SayHello({|#0:string name|}, {|#1:string title|}) => $""Hello {name}, you are a {title}.""; +app.MapGet(""/hello/{name?}/{title?}"", SayHello); +"; + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +string SayHello(string? name, string? title) => $""Hello {name}, you are a {title}.""; +app.MapGet(""/hello/{name?}/{title?}"", SayHello); +"; + + var expectedDiagnostics = new[] { + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("title").WithLocation(1) + }; + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource); + } + + [Fact] + public async Task MismatchedOptionalityInMethodGroupFromPartialMethod_CanBeFixed() + { + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", ExternalImplementation.SayHello); + +public partial class ExternalImplementation +{ + public static partial string SayHello({|#0:string name|}, {|#1:string title|}); +} + +public partial class ExternalImplementation +{ + public static partial string SayHello({|#2:string name|}, {|#3:string title|}) + { + return $""Hello {name}, you are a {title}.""; + } +} +"; + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", ExternalImplementation.SayHello); + +public partial class ExternalImplementation +{ + public static partial string SayHello(string? name, string? title); +} + +public partial class ExternalImplementation +{ + public static partial string SayHello(string? name, string? title) + { + return $""Hello {name}, you are a {title}.""; + } +} +"; + // Diagnostics are produced at both the declaration and definition syntax + // for partial method definitions to support the CodeFix correctly resolving both. + var expectedDiagnostics = new[] { + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("title").WithLocation(1), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(2), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("title").WithLocation(3) + }; + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource); + } + + [Fact] + public async Task MismatchedOptionalityInSeparateSource_CanBeFixed() + { + var usageSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}/{title?}"", Helpers.SayHello); +"; + var source = @" +#nullable enable +using System; + +public static class Helpers +{ + public static string SayHello({|#0:string name|}, {|#1:string title|}) + { + return $""Hello {name}, you are a {title}.""; + } +}"; + var fixedSource = @" +#nullable enable +using System; + +public static class Helpers +{ + public static string SayHello(string? name, string? title) + { + return $""Hello {name}, you are a {title}.""; + } +}"; + + var expectedDiagnostics = new[] { + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("name").WithLocation(0), + new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("title").WithLocation(1) + }; + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, usageSource); + } + + [Fact] + public async Task MatchingRequiredOptionality_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name}"", (string name) => $""Hello {name}""); +"; + + await VerifyCS.VerifyCodeFixAsync(source, source); + } + + [Fact] + public async Task ParameterFromRouteOrQuery_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name}"", (string name) => $""Hello {name}""); +"; + + await VerifyCS.VerifyCodeFixAsync(source, source); + } + + [Fact] + public async Task MatchingOptionality_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}"", (string? name) => $""Hello {name}""); +"; + + await VerifyCS.VerifyCodeFixAsync(source, source); + } + + [Fact] + public async Task RequiredRouteParamOptionalArgument_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name}"", (string? name) => $""Hello {name}""); +"; + + await VerifyCS.VerifyCodeFixAsync(source, source); + } + + [Fact] + public async Task OptionalRouteParamRequiredArgument_WithFromRoute_ProducesDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{Age?}"", ({|#0:[FromRoute] int age|}) => $""Age: {age}""); +"; + + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{Age?}"", ([FromRoute] int? age) => $""Age: {age}""); +"; + + var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("age").WithLocation(0); + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostic, fixedSource); + } + + [Fact] + public async Task OptionalRouteParamRequiredArgument_WithRegexConstraint_ProducesDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{age:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)?}"", ({|#0:int age|}) => $""Age: {age}""); +"; + + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{age:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)?}"", (int? age) => $""Age: {age}""); +"; + var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("age").WithLocation(0); + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostic, fixedSource); + } + + [Fact] + public async Task OptionalRouteParamRequiredArgument_WithTypeConstraint_ProducesDiagnostics() + { + // Arrange + var source = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{age:int?}"", ({|#0:int age|}) => $""Age: {age}""); +"; + + var fixedSource = @" +#nullable enable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{age:int?}"", (int? age) => $""Age: {age}""); +"; + + var expectedDiagnostic = new DiagnosticResult(DiagnosticDescriptors.DetectMismatchedParameterOptionality).WithArguments("age").WithLocation(0); + + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostic, fixedSource); + } + + [Fact] + public async Task MatchingRequiredOptionality_WithDisabledNullability() + { + var source = @" +#nullable disable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}"", (string name) => $""Hello {name}""); +"; + var fixedSource = @" +#nullable disable +using Microsoft.AspNetCore.Builder; + +var app = WebApplication.Create(); +app.MapGet(""/hello/{name?}"", (string name) => $""Hello {name}""); +"; + + await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + } + + [Theory] + [InlineData("{id}", new[] { "id" }, new[] { "" })] + [InlineData("{category}/product/{group}", new[] { "category", "group" }, new[] { "", "" })] + [InlineData("{category:int}/product/{group:range(10, 20)}?", new[] { "category", "group" }, new[] { ":int", ":range(10, 20)" })] + [InlineData("{person:int}/{ssn:regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)}", new[] { "person", "ssn" }, new[] { ":int", ":regex(^\\d{{3}}-\\d{{2}}-\\d{{4}}$)"})] + [InlineData("{area=Home}/{controller:required}/{id=0:int}", new[] { "area", "controller", "id" }, new[] { "=Home", ":required", "=0:int" })] + [InlineData("{category}/product/{group?}", new[] { "category", "group" }, new[] { "", "?"})] + [InlineData("{category}/{product}/{*sku}", new[] { "category", "product", "*sku" }, new[] { "", "", "" })] + [InlineData("{category}-product-{sku}", new[] { "category", "sku" }, new[] { "", "" } )] + [InlineData("category-{product}-sku", new[] { "product" }, new[] { "" } )] + [InlineData("{category}.{sku?}", new[] { "category", "sku" }, new[] { "", "?" })] + [InlineData("{category}.{product?}/{sku}", new[] { "category", "product", "sku" }, new[] { "", "?", "" })] + public void RouteTokenizer_Works_ForSimpleRouteTemplates(string template, string[] expectedNames, string[] expectedQualifiers) + { + // Arrange + var tokenizer = new DelegateEndpointAnalyzer.RouteTokenEnumerator(template); + var actualNames = new List(); + var actualQualifiers = new List(); + + // Act + while (tokenizer.MoveNext()) + { + actualNames.Add(tokenizer.CurrentName.ToString()); + actualQualifiers.Add(tokenizer.CurrentQualifiers.ToString()); + + } + + // Assert + Assert.Equal(expectedNames, actualNames); + Assert.Equal(expectedQualifiers, actualQualifiers); + } +} diff --git a/src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs b/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs similarity index 100% rename from src/Framework/Analyzer/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs rename to src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DetectMisplacedLambdaAttributeTest.cs diff --git a/src/Framework/Analyzer/test/MinimalActions/DisallowMvcBindArgumentsOnParametersTest.cs b/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DisallowMvcBindArgumentsOnParametersTest.cs similarity index 100% rename from src/Framework/Analyzer/test/MinimalActions/DisallowMvcBindArgumentsOnParametersTest.cs rename to src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DisallowMvcBindArgumentsOnParametersTest.cs diff --git a/src/Framework/Analyzer/test/MinimalActions/DisallowReturningActionResultsFromMapMethodsTest.cs b/src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DisallowReturningActionResultsFromMapMethodsTest.cs similarity index 100% rename from src/Framework/Analyzer/test/MinimalActions/DisallowReturningActionResultsFromMapMethodsTest.cs rename to src/Framework/AspNetCoreAnalyzers/test/MinimalActions/DisallowReturningActionResultsFromMapMethodsTest.cs diff --git a/src/Framework/Analyzer/test/TestDiagnosticAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs similarity index 100% rename from src/Framework/Analyzer/test/TestDiagnosticAnalyzer.cs rename to src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsAnalyzerVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsAnalyzerVerifier.cs new file mode 100644 index 000000000000..2fa14f67bccb --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsAnalyzerVerifier.cs @@ -0,0 +1,33 @@ +// 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.Immutable; +using System.Globalization; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Testing.Verifiers; +using Xunit; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public static class CSharpDelegateEndpointsAnalyzerVerifier + where TAnalyzer : DelegateEndpointAnalyzer, new() +{ + public static DiagnosticResult Diagnostic(string diagnosticId = null) + => CSharpDelegateEndpointsAnalyzerVerifier.Diagnostic(diagnosticId); + + public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor) + => new DiagnosticResult(descriptor); + + public static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new Test { TestCode = source }; + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + public class Test : CSharpCodeFixTest { } +} \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsCodeFixVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsCodeFixVerifier.cs new file mode 100644 index 000000000000..2883bb2fd389 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpDelegateEndpointsCodeFixVerifier.cs @@ -0,0 +1,85 @@ +// 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.Immutable; +using System.Globalization; +using Microsoft.AspNetCore.Analyzers.DelegateEndpoints.Fixers; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Testing.Verifiers; +using Xunit; + +namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints; + +public static class CSharpDelegateEndpointsCodeFixVerifier + where TAnalyzer : DelegateEndpointAnalyzer, new() + where TCodeFix : DetectMismatchedParameterOptionalityFixer, new() +{ + public static DiagnosticResult Diagnostic(string diagnosticId = null) + => CSharpCodeFixVerifier.Diagnostic(diagnosticId); + + public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor) + => new DiagnosticResult(descriptor); + + public static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new CSharpDelegateEndpointsAnalyzerVerifier.Test { TestCode = source }; + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + public static Task VerifyCodeFixAsync(string source, string fixedSource) + => VerifyCodeFixAsync(source, DiagnosticResult.EmptyDiagnosticResults, fixedSource); + + public static Task VerifyCodeFixAsync(string source, DiagnosticResult expected, string fixedSource) + => VerifyCodeFixAsync(source, new[] { expected }, fixedSource); + + public static Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource) + => VerifyCodeFixAsync(source, expected, fixedSource, string.Empty); + + public static Task VerifyCodeFixAsync(string sources, DiagnosticResult[] expected, string fixedSources, string usageSource = "") + { + var test = new DelegateEndpointAnalyzerTest + { + TestState = + { + Sources = { sources, usageSource }, + // We need to set the output type to an exe to properly + // support top-level programs in the tests. Otherwise, + // the test infra will assume we are trying to build a library. + OutputKind = OutputKind.ConsoleApplication + }, + FixedState = + { + Sources = { fixedSources, usageSource } + } + }; + + test.TestState.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + public class DelegateEndpointAnalyzerTest : CSharpCodeFixTest + { + public DelegateEndpointAnalyzerTest() + { + // We populate the ReferenceAssemblies used in the tests with the locally-built AspNetCore + // assemblies that are referenced in a minimal app to ensure that there are no reference + // errors during the build. + ReferenceAssemblies = ReferenceAssemblies.Net.Net60.AddAssemblies(ImmutableArray.Create( + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.WebApplication).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.DelegateEndpointRouteBuilderExtensions).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.IApplicationBuilder).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.IEndpointConventionBuilder).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.IHost).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.ModelBinding.IBinderTypeProviderMetadata).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.BindAttribute).Assembly.Location))); + + string TrimAssemblyExtension(string fullPath) => fullPath.Replace(".dll", string.Empty); + } + } +} \ No newline at end of file diff --git a/src/Framework/Analyzer/test/xunit.runner.json b/src/Framework/AspNetCoreAnalyzers/test/xunit.runner.json similarity index 100% rename from src/Framework/Analyzer/test/xunit.runner.json rename to src/Framework/AspNetCoreAnalyzers/test/xunit.runner.json diff --git a/src/Framework/Framework.slnf b/src/Framework/Framework.slnf index 94e9163086f8..bce83882a622 100644 --- a/src/Framework/Framework.slnf +++ b/src/Framework/Framework.slnf @@ -19,7 +19,8 @@ "src\\Extensions\\Features\\src\\Microsoft.Extensions.Features.csproj", "src\\FileProviders\\Embedded\\src\\Microsoft.Extensions.FileProviders.Embedded.csproj", "src\\FileProviders\\Manifest.MSBuildTask\\src\\Microsoft.Extensions.FileProviders.Embedded.Manifest.Task.csproj", - "src\\Framework\\Analyzer\\src\\Microsoft.AspNetCore.App.Analyzers.csproj", + "src\\Framework\\AspNetCoreAnalyzers\\src\\Analyzers\\Microsoft.AspNetCore.App.Analyzers.csproj", + "src\\Framework\\AspNetCoreAnalyzers\\src\\CodeFixes\\Microsoft.AspNetCore.App.CodeFixes.csproj", "src\\Framework\\Analyzer\\test\\Microsoft.AspNetCore.App.Analyzers.Test.csproj", "src\\Framework\\App.Ref\\src\\Microsoft.AspNetCore.App.Ref.csproj", "src\\Framework\\App.Runtime\\src\\Microsoft.AspNetCore.App.Runtime.csproj", diff --git a/src/Shared/Roslyn/CodeAnalysisExtensions.cs b/src/Shared/Roslyn/CodeAnalysisExtensions.cs index 4239bfa15732..5d22c7753abb 100644 --- a/src/Shared/Roslyn/CodeAnalysisExtensions.cs +++ b/src/Shared/Roslyn/CodeAnalysisExtensions.cs @@ -148,5 +148,26 @@ private static IEnumerable GetTypeHierarchy(this ITypeSymbol? typeS typeSymbol = typeSymbol.BaseType; } } + + // Adapted from https://github.com/dotnet/roslyn/blob/929272/src/Workspaces/Core/Portable/Shared/Extensions/IMethodSymbolExtensions.cs#L61 + public static IEnumerable GetAllMethodSymbolsOfPartialParts(this IMethodSymbol method) + { + if (method.PartialDefinitionPart != null) + { + Debug.Assert(method.PartialImplementationPart == null && !SymbolEqualityComparer.Default.Equals(method.PartialDefinitionPart, method)); + yield return method; + yield return method.PartialDefinitionPart; + } + else if (method.PartialImplementationPart != null) + { + Debug.Assert(!SymbolEqualityComparer.Default.Equals(method.PartialImplementationPart, method)); + yield return method.PartialImplementationPart; + yield return method; + } + else + { + yield return method; + } + } } }