Add analyzer for detecting mismatched endpoint parameter optionality (#36154)

* Add analyzer for detecting mismatched endpoint parameter optionality

* Address feedback from code review

* Factor out CodeFixes and Analyzers to separate assemblies

* Address more feedback from review

* Address code checks

Author: captainsafia

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}

eng/Dependencies.props

@@ -56,6 +56,7 @@ and are generated based on the last package release.
     <LatestPackageReference Include="Microsoft.Extensions.Options" />
     <LatestPackageReference Include="Microsoft.Extensions.Primitives" />
     <LatestPackageReference Include="Microsoft.Win32.Registry" />
+    <LatestPackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" />
     <LatestPackageReference Include="System.Buffers" />
     <LatestPackageReference Include="System.CodeDom" />
     <LatestPackageReference Include="System.CommandLine.Experimental" />

eng/Versions.props

@@ -192,6 +192,7 @@
     <MicrosoftCodeAnalysisCSharpVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpVersion>
     <MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
     <MicrosoftCodeAnalysisPublicApiAnalyzersVersion>3.3.0</MicrosoftCodeAnalysisPublicApiAnalyzersVersion>
+    <MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>1.1.1-beta1.21413.1</MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>
     <MicrosoftCssParserVersion>1.0.0-20200708.1</MicrosoftCssParserVersion>
     <MicrosoftIdentityModelLoggingVersion>6.10.0</MicrosoftIdentityModelLoggingVersion>
     <MicrosoftIdentityModelProtocolsOpenIdConnectVersion>6.10.0</MicrosoftIdentityModelProtocolsOpenIdConnectVersion>

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
   <ItemGroup>
     <!-- Note: do not add _TransitiveExternalAspNetCoreAppReference to this list. This is intentionally not listed as a direct package reference. -->
     <Reference Include="@(AspNetCoreAppReference);@(AspNetCoreAppReferenceAndPackage);@(ExternalAspNetCoreAppReference)" />
-    <ProjectReference Include="..\..\Analyzer\src\Microsoft.AspNetCore.App.Analyzers.csproj"
+    <ProjectReference Include="..\..\AspNetCoreAnalyzers\src\Analyzers\Microsoft.AspNetCore.App.Analyzers.csproj"
+      ReferenceOutputAssembly="false"
+      SkipGetTargetFrameworkProperties="true"
+      UndefineProperties="TargetFramework;TargetFrameworks;RuntimeIdentifier;PublishDir" />
+    <ProjectReference Include="..\..\AspNetCoreAnalyzers\src\CodeFixes\Microsoft.AspNetCore.App.CodeFixes.csproj"
       ReferenceOutputAssembly="false"
       SkipGetTargetFrameworkProperties="true"
       UndefineProperties="TargetFramework;TargetFrameworks;RuntimeIdentifier;PublishDir" />
@@ -170,6 +174,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant
 
       <RefPackContent Include="$(PkgMicrosoft_Internal_Runtime_AspNetCore_Transport)\$(AnalyzersPackagePath)**\*.*" PackagePath="$(AnalyzersPackagePath)" />
       <RefPackContent Include="$(ArtifactsDir)bin\Microsoft.AspNetCore.App.Analyzers\$(Configuration)\netstandard2.0\Microsoft.AspNetCore.App.Analyzers.dll" PackagePath="$(AnalyzersPackagePath)dotnet/cs/" />
+      <RefPackContent Include="$(ArtifactsDir)bin\Microsoft.AspNetCore.App.CodeFixes\$(Configuration)\netstandard2.0\Microsoft.AspNetCore.App.CodeFixes.dll" PackagePath="$(AnalyzersPackagePath)dotnet/cs/" />
 
       <RefPackContent Include="@(AspNetCoreReferenceAssemblyPath)" PackagePath="$(RefAssemblyPackagePath)" />
       <RefPackContent Include="@(AspNetCoreReferenceDocXml)" PackagePath="$(RefAssemblyPackagePath)" />

src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs renamed to 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)

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<char> _routeTemplate;
+
+        public RouteTokenEnumerator(string routeTemplateString)
+        {
+            _routeTemplate = routeTemplateString.AsSpan();
+            CurrentName = default;
+            CurrentQualifiers = default;
+        }
+
+        public ReadOnlySpan<char> CurrentName { get; private set; }
+        public ReadOnlySpan<char> 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<char> span, int startIndex, char c)
+    {
+        for (var i = startIndex; i < span.Length; i++)
+        {
+            if (span[i] == c)
+            {
+                return i;
+            }
+        }
+
+        return -1;
+    }
+}

src/Framework/Analyzer/src/DelegateEndpoints/DetectMisplacedLambdaAttribute.cs renamed to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMisplacedLambdaAttribute.cs

@@ -43,4 +43,13 @@ internal static class DiagnosticDescriptors
         DiagnosticSeverity.Warning,
         isEnabledByDefault: true,
         helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+    internal static readonly DiagnosticDescriptor DetectMismatchedParameterOptionality = new(
+        "ASP0007",
+        "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");
 }

src/Framework/Analyzer/src/DiagnosticDescriptors.cs renamed to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs

@@ -10,9 +10,10 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="All" />
+    <Reference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="All" />
 
     <InternalsVisibleTo Include="Microsoft.AspNetCore.App.Analyzers.Test" />
+    <InternalsVisibleTo Include="Microsoft.AspNetCore.App.CodeFixes" />
   </ItemGroup>
 
   <ItemGroup>

src/Framework/Analyzer/src/DelegateEndpoints/DisallowMvcBindArgumentsOnParameters.cs renamed to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowMvcBindArgumentsOnParameters.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<string> 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<Document> 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();
+    }
+}

src/Framework/Analyzer/src/DelegateEndpoints/DisallowReturningActionResultFromMapMethods.cs renamed to src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowReturningActionResultFromMapMethods.cs

@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Description>CSharp CodeFixes for ASP.NET Core.</Description>
+    <IsShippingPackage>false</IsShippingPackage>
+    <AddPublicApiAnalyzers>false</AddPublicApiAnalyzers>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <IncludeBuildOutput>false</IncludeBuildOutput>
+    <Nullable>Enable</Nullable>
+    <RootNamespace>Microsoft.AspNetCore.Analyzers</RootNamespace>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="All" />
+    <ProjectReference Include="..\Analyzers\Microsoft.AspNetCore.App.Analyzers.csproj" />
+  </ItemGroup>
+
+</Project>

src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj renamed to src/Framework/AspNetCoreAnalyzers/src/Analyzers/Microsoft.AspNetCore.App.Analyzers.csproj

@@ -11,11 +11,13 @@
   </ItemGroup>
 
   <ItemGroup>
-    <ProjectReference Include="..\src\Microsoft.AspNetCore.App.Analyzers.csproj" />
+    <ProjectReference Include="..\src\Analyzers\Microsoft.AspNetCore.App.Analyzers.csproj" />
+    <ProjectReference Include="..\src\CodeFixes\Microsoft.AspNetCore.App.CodeFixes.csproj" />
     <ProjectReference Include="$(RepoRoot)src\Analyzers\Microsoft.AspNetCore.Analyzer.Testing\src\Microsoft.AspNetCore.Analyzer.Testing.csproj" />
     <Reference Include="Microsoft.AspNetCore" />
     <Reference Include="Microsoft.AspNetCore.Mvc" />
     <Reference Include="Microsoft.AspNetCore.Http.Results" />
+    <Reference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" />
   </ItemGroup>
 
 </Project>