Skip to content

Commit c279156

Browse files
captainsafiacaptainsafia
committed
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
1 parent e23fd04 commit c279156

28 files changed

+776
-38
lines changed

‎AspNetCore.sln‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,12 +1646,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Wasm.Prerendered.Client", "
16461646
EndProject
16471647
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}"
16481648
EndProject
1649-
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.App.Analyzer", "src\Framework\Analyzer\src\Microsoft.AspNetCore.App.Analyzers.csproj", "{564CABB8-1B3F-4D9E-909D-260EF2B8614A}"
1650-
EndProject
1651-
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Analyzer", "Analyzer", "{EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}"
1652-
EndProject
1653-
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}"
1654-
EndProject
16551649
Global
16561650
GlobalSection(SolutionConfigurationPlatforms) = preSolution
16571651
Debug|Any CPU = Debug|Any CPU
@@ -7867,30 +7861,6 @@ Global
78677861
{6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x64.Build.0 = Release|Any CPU
78687862
{6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x86.ActiveCfg = Release|Any CPU
78697863
{6D365C86-3158-49F5-A21D-506C1E06E870}.Release|x86.Build.0 = Release|Any CPU
7870-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
7871-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.Build.0 = Debug|Any CPU
7872-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.ActiveCfg = Debug|Any CPU
7873-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.Build.0 = Debug|Any CPU
7874-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.ActiveCfg = Debug|Any CPU
7875-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.Build.0 = Debug|Any CPU
7876-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.ActiveCfg = Release|Any CPU
7877-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.Build.0 = Release|Any CPU
7878-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.ActiveCfg = Release|Any CPU
7879-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.Build.0 = Release|Any CPU
7880-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.ActiveCfg = Release|Any CPU
7881-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.Build.0 = Release|Any CPU
7882-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
7883-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.Build.0 = Debug|Any CPU
7884-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.ActiveCfg = Debug|Any CPU
7885-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.Build.0 = Debug|Any CPU
7886-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.ActiveCfg = Debug|Any CPU
7887-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.Build.0 = Debug|Any CPU
7888-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.ActiveCfg = Release|Any CPU
7889-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.Build.0 = Release|Any CPU
7890-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.ActiveCfg = Release|Any CPU
7891-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.Build.0 = Release|Any CPU
7892-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.ActiveCfg = Release|Any CPU
7893-
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.Build.0 = Release|Any CPU
78947864
EndGlobalSection
78957865
GlobalSection(SolutionProperties) = preSolution
78967866
HideSolutionNode = FALSE
@@ -8706,9 +8676,6 @@ Global
87068676
{835A4E0F-A697-4B69-9736-3E99D163C4B9} = {48526D13-69E2-4409-A57B-C3FA3C64B4F7}
87078677
{148A5B4F-C8A3-4468-92F6-51DB5641FB49} = {7D2B0799-A634-42AC-AE77-5D167BA51389}
87088678
{6D365C86-3158-49F5-A21D-506C1E06E870} = {7D2B0799-A634-42AC-AE77-5D167BA51389}
8709-
{564CABB8-1B3F-4D9E-909D-260EF2B8614A} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}
8710-
{EE39397E-E4AF-4D3F-9B9C-D637F9222CDD} = {A4C26078-B6D8-4FD8-87A6-7C15A3482038}
8711-
{CF4CEC18-798D-46EC-B0A0-98D97496590F} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}
87128679
EndGlobalSection
87138680
GlobalSection(ExtensibilityGlobals) = postSolution
87148681
SolutionGuid = {3E8720B3-DBDD-498C-B383-2CC32A054E8F}

‎eng/Dependencies.props‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ and are generated based on the last package release.
5656
<LatestPackageReference Include="Microsoft.Extensions.Options" />
5757
<LatestPackageReference Include="Microsoft.Extensions.Primitives" />
5858
<LatestPackageReference Include="Microsoft.Win32.Registry" />
59+
<LatestPackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" />
5960
<LatestPackageReference Include="System.Buffers" />
6061
<LatestPackageReference Include="System.CodeDom" />
6162
<LatestPackageReference Include="System.CommandLine.Experimental" />

‎eng/Versions.props‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@
192192
<MicrosoftCodeAnalysisCSharpVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpVersion>
193193
<MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
194194
<MicrosoftCodeAnalysisPublicApiAnalyzersVersion>3.3.0</MicrosoftCodeAnalysisPublicApiAnalyzersVersion>
195+
<MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>1.1.1-beta1.21413.1</MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>
195196
<MicrosoftCssParserVersion>1.0.0-20200708.1</MicrosoftCssParserVersion>
196197
<MicrosoftIdentityModelLoggingVersion>6.10.0</MicrosoftIdentityModelLoggingVersion>
197198
<MicrosoftIdentityModelProtocolsOpenIdConnectVersion>6.10.0</MicrosoftIdentityModelProtocolsOpenIdConnectVersion>

‎src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ This package is an internal implementation of the .NET Core SDK and is not meant
6363
<ItemGroup>
6464
<!-- Note: do not add _TransitiveExternalAspNetCoreAppReference to this list. This is intentionally not listed as a direct package reference. -->
6565
<Reference Include="@(AspNetCoreAppReference);@(AspNetCoreAppReferenceAndPackage);@(ExternalAspNetCoreAppReference)" />
66-
<ProjectReference Include="..\..\Analyzer\src\Microsoft.AspNetCore.App.Analyzers.csproj"
66+
<ProjectReference Include="..\..\AspNetCoreAnalyzers\src\Analyzers\Microsoft.AspNetCore.App.Analyzers.csproj"
67+
ReferenceOutputAssembly="false"
68+
SkipGetTargetFrameworkProperties="true"
69+
UndefineProperties="TargetFramework;TargetFrameworks;RuntimeIdentifier;PublishDir" />
70+
<ProjectReference Include="..\..\AspNetCoreAnalyzers\src\CodeFixes\Microsoft.AspNetCore.App.CodeFixes.csproj"
6771
ReferenceOutputAssembly="false"
6872
SkipGetTargetFrameworkProperties="true"
6973
UndefineProperties="TargetFramework;TargetFrameworks;RuntimeIdentifier;PublishDir" />
@@ -170,6 +174,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant
170174

171175
<RefPackContent Include="$(PkgMicrosoft_Internal_Runtime_AspNetCore_Transport)\$(AnalyzersPackagePath)**\*.*" PackagePath="$(AnalyzersPackagePath)" />
172176
<RefPackContent Include="$(ArtifactsDir)bin\Microsoft.AspNetCore.App.Analyzers\$(Configuration)\netstandard2.0\Microsoft.AspNetCore.App.Analyzers.dll" PackagePath="$(AnalyzersPackagePath)dotnet/cs/" />
177+
<RefPackContent Include="$(ArtifactsDir)bin\Microsoft.AspNetCore.App.CodeFixes\$(Configuration)\netstandard2.0\Microsoft.AspNetCore.App.CodeFixes.dll" PackagePath="$(AnalyzersPackagePath)dotnet/cs/" />
173178

174179
<RefPackContent Include="@(AspNetCoreReferenceAssemblyPath)" PackagePath="$(RefAssemblyPackagePath)" />
175180
<RefPackContent Include="@(AspNetCoreReferenceDocXml)" PackagePath="$(RefAssemblyPackagePath)" />

‎src/Framework/Analyzer/src/DelegateEndpoints/DelegateEndpointAnalyzer.cs‎ renamed to ‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DelegateEndpointAnalyzer.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
1818
{
1919
DiagnosticDescriptors.DoNotUseModelBindingAttributesOnDelegateEndpointParameters,
2020
DiagnosticDescriptors.DoNotReturnActionResultsFromMapActions,
21-
DiagnosticDescriptors.DetectMisplacedLambdaAttribute
21+
DiagnosticDescriptors.DetectMisplacedLambdaAttribute,
22+
DiagnosticDescriptors.DetectMismatchedParameterOptionality
2223
});
2324

2425
public override void Initialize(AnalysisContext context)
@@ -56,11 +57,13 @@ public override void Initialize(AnalysisContext context)
5657
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol);
5758
DisallowReturningActionResultFromMapMethods(in operationAnalysisContext, wellKnownTypes, invocation, lambda);
5859
DetectMisplacedLambdaAttribute(operationAnalysisContext, invocation, lambda);
60+
DetectMismatchedParameterOptionality(in operationAnalysisContext, invocation, lambda.Symbol);
5961
}
6062
else if (delegateCreation.Target.Kind == OperationKind.MethodReference)
6163
{
6264
var methodReference = (IMethodReferenceOperation)delegateCreation.Target;
6365
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, methodReference.Method);
66+
DetectMismatchedParameterOptionality(in operationAnalysisContext, invocation, methodReference.Method);
6467

6568
var foundMethodReferenceBody = false;
6669
if (!methodReference.Method.DeclaringSyntaxReferences.IsEmpty)

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DetectMismatchedParameterOptionality.cs‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Linq;
6+
using System.Collections.Generic;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
using Microsoft.CodeAnalysis.Diagnostics;
10+
using Microsoft.CodeAnalysis.Operations;
11+
12+
namespace Microsoft.AspNetCore.Analyzers.DelegateEndpoints;
13+
14+
public partial class DelegateEndpointAnalyzer : DiagnosticAnalyzer
15+
{
16+
internal const string DetectMismatchedParameterOptionalityRuleId = "ASP0006";
17+
18+
private static void DetectMismatchedParameterOptionality(
19+
in OperationAnalysisContext context,
20+
IInvocationOperation invocation,
21+
IMethodSymbol methodSymbol)
22+
{
23+
if (invocation.Arguments.Length < 2)
24+
{
25+
return;
26+
}
27+
28+
var value = invocation.Arguments[1].Value;
29+
if (value.ConstantValue is not { HasValue: true } constant ||
30+
constant.Value is not string routeTemplate)
31+
{
32+
return;
33+
}
34+
35+
var allDeclarations = methodSymbol.GetAllMethodSymbolsOfPartialParts();
36+
foreach (var method in allDeclarations)
37+
{
38+
var parametersInArguments = method.Parameters;
39+
var enumerator = new RouteTokenEnumerator(routeTemplate);
40+
41+
while (enumerator.MoveNext())
42+
{
43+
foreach (var parameter in parametersInArguments)
44+
{
45+
var paramName = parameter.Name;
46+
// If this is not the methpd parameter associated with the route
47+
// parameter then continue looking for it in the list
48+
if (!enumerator.CurrentName.Equals(paramName.AsSpan(), StringComparison.OrdinalIgnoreCase))
49+
{
50+
continue;
51+
}
52+
var argumentIsOptional = parameter.IsOptional || parameter.NullableAnnotation != NullableAnnotation.NotAnnotated;
53+
var location = parameter.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax().GetLocation();
54+
var routeParamIsOptional = enumerator.CurrentQualifiers.IndexOf('?') > -1;
55+
56+
if (!argumentIsOptional && routeParamIsOptional)
57+
{
58+
context.ReportDiagnostic(Diagnostic.Create(
59+
DiagnosticDescriptors.DetectMismatchedParameterOptionality,
60+
location,
61+
paramName));
62+
}
63+
}
64+
}
65+
}
66+
}
67+
68+
internal ref struct RouteTokenEnumerator
69+
{
70+
private ReadOnlySpan<char> _routeTemplate;
71+
72+
public RouteTokenEnumerator(string routeTemplateString)
73+
{
74+
_routeTemplate = routeTemplateString.AsSpan();
75+
CurrentName = default;
76+
CurrentQualifiers = default;
77+
}
78+
79+
public ReadOnlySpan<char> CurrentName { get; private set; }
80+
public ReadOnlySpan<char> CurrentQualifiers { get; private set; }
81+
82+
public bool MoveNext()
83+
{
84+
if (_routeTemplate.IsEmpty)
85+
{
86+
return false;
87+
}
88+
89+
findStartBrace:
90+
var startIndex = _routeTemplate.IndexOf('{');
91+
if (startIndex == -1)
92+
{
93+
return false;
94+
}
95+
96+
if (startIndex < _routeTemplate.Length - 1 && _routeTemplate[startIndex + 1] == '{')
97+
{
98+
// Escaped sequence
99+
_routeTemplate = _routeTemplate.Slice(startIndex + 1);
100+
goto findStartBrace;
101+
}
102+
103+
var tokenStart = startIndex + 1;
104+
105+
findEndBrace:
106+
var endIndex = IndexOf(_routeTemplate, tokenStart, '}');
107+
if (endIndex == -1)
108+
{
109+
return false;
110+
}
111+
if (endIndex < _routeTemplate.Length - 1 && _routeTemplate[endIndex + 1] == '}')
112+
{
113+
tokenStart = endIndex + 2;
114+
goto findEndBrace;
115+
}
116+
117+
var token = _routeTemplate.Slice(startIndex + 1, endIndex - startIndex - 1);
118+
var qualifier = token.IndexOfAny(new[] { ':', '=', '?' });
119+
CurrentName = qualifier == -1 ? token : token.Slice(0, qualifier);
120+
CurrentQualifiers = qualifier == -1 ? null : token.Slice(qualifier);
121+
122+
_routeTemplate = _routeTemplate.Slice(endIndex + 1);
123+
return true;
124+
}
125+
}
126+
127+
private static int IndexOf(ReadOnlySpan<char> span, int startIndex, char c)
128+
{
129+
for (var i = startIndex; i < span.Length; i++)
130+
{
131+
if (span[i] == c)
132+
{
133+
return i;
134+
}
135+
}
136+
137+
return -1;
138+
}
139+
}

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

File renamed without changes.

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,13 @@ internal static class DiagnosticDescriptors
4343
DiagnosticSeverity.Warning,
4444
isEnabledByDefault: true,
4545
helpLinkUri: "https://aka.ms/aspnet/analyzers");
46+
47+
internal static readonly DiagnosticDescriptor DetectMismatchedParameterOptionality = new(
48+
"ASP0007",
49+
"Route parameter and argument optionality is mismatched",
50+
"'{0}' argument should be annotated as optional or nullable to match route parameter",
51+
"Usage",
52+
DiagnosticSeverity.Warning,
53+
isEnabledByDefault: true,
54+
helpLinkUri: "https://aka.ms/aspnet/analyzers");
4655
}

‎src/Framework/Analyzer/src/DelegateEndpoints/DisallowMvcBindArgumentsOnParameters.cs‎ renamed to ‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DisallowMvcBindArgumentsOnParameters.cs‎

File renamed without changes.

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

File renamed without changes.

0 commit comments

Comments
 (0)