Skip to content

Add analyzer for detecting mismatched endpoint parameter optionality #36154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions AspNetCore.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down
1 change: 1 addition & 0 deletions eng/Dependencies.props
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
1 change: 1 addition & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -162,6 +166,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)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Loading