Skip to content

Add an analyzer that complains when using using model binding attributes with a minimal action delegate #35359

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 7 commits into from
Aug 19, 2021
Merged
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
35 changes: 34 additions & 1 deletion AspNetCore.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31613.370
VisualStudioVersion = 17.0.31606.5
MinimumVisualStudioVersion = 15.0.26124.0
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "eng", "eng", "{C28A32F6-8314-412E-9F3B-CBD31C23E878}"
EndProject
@@ -1642,6 +1642,12 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "testassets", "testassets",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Identity.DefaultUI.WebSite", "src\Identity\testassets\Identity.DefaultUI.WebSite\Identity.DefaultUI.WebSite.csproj", "{835A4E0F-A697-4B69-9736-3E99D163C4B9}"
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
@@ -7833,6 +7839,30 @@ Global
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.Release|x64.Build.0 = Release|Any CPU
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.Release|x86.ActiveCfg = Release|Any CPU
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.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
@@ -8646,6 +8676,9 @@ Global
{514726D2-3D2E-44C1-B056-163E37DE3E8B} = {7B976D8F-EA31-4C0B-97BD-DFD9B3CC86FB}
{48526D13-69E2-4409-A57B-C3FA3C64B4F7} = {9F21A235-436E-4020-A076-1DF4F89D0CA0}
{835A4E0F-A697-4B69-9736-3E99D163C4B9} = {48526D13-69E2-4409-A57B-C3FA3C64B4F7}
{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}
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
@@ -190,7 +190,7 @@
<MicrosoftBuildUtilitiesCoreVersion>16.9.0</MicrosoftBuildUtilitiesCoreVersion>
<MicrosoftCodeAnalysisCommonVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCommonVersion>
<MicrosoftCodeAnalysisCSharpVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpVersion>
<MicrosoftCodeAnalysisCSharpWorkspacesVersion>3.8.0</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
<MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
<MicrosoftCodeAnalysisPublicApiAnalyzersVersion>3.3.0</MicrosoftCodeAnalysisPublicApiAnalyzersVersion>
<MicrosoftCssParserVersion>1.0.0-20200708.1</MicrosoftCssParserVersion>
<MicrosoftIdentityModelLoggingVersion>6.10.0</MicrosoftIdentityModelLoggingVersion>
6 changes: 4 additions & 2 deletions src/Analyzers/Analyzers/src/StartupAnalysisBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
@@ -18,7 +18,9 @@ public StartupAnalysisBuilder(StartupAnalyzer analyzer, StartupSymbols startupSy
_analyzer = analyzer;
StartupSymbols = startupSymbols;

_analysesByType = new Dictionary<INamedTypeSymbol, List<object>>();
#pragma warning disable RS1024 // Compare symbols correctly
_analysesByType = new Dictionary<INamedTypeSymbol, List<object>>(SymbolEqualityComparer.Default);
#pragma warning restore RS1024 // Compare symbols correctly
_lock = new object();
}

6 changes: 3 additions & 3 deletions src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@ static Diagnostics()

internal static readonly DiagnosticDescriptor UnsupportedUseMvcWithEndpointRouting = new DiagnosticDescriptor(
"MVC1005",
"Cannot use UseMvc with Endpoint Routing.",
"Cannot use UseMvc with Endpoint Routing",
"Using '{0}' to configure MVC is not supported while using Endpoint Routing. To continue using '{0}', please set 'MvcOptions.EnableEndpointRouting = false' inside '{1}'.",
"Usage",
DiagnosticSeverity.Warning,
@@ -47,8 +47,8 @@ static Diagnostics()

internal static readonly DiagnosticDescriptor IncorrectlyConfiguredAuthorizationMiddleware = new DiagnosticDescriptor(
"ASP0001",
"Authorization middleware is incorrectly configured.",
"The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated.",
"Authorization middleware is incorrectly configured",
"The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated",
"Usage",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
Original file line number Diff line number Diff line change
@@ -26,9 +26,15 @@ public static TestSource Read(string rawSource)
for (var i = 0; i < lines.Length; i++)
{
var line = lines[i];
var markerStartIndex = line.IndexOf(MarkerStart, StringComparison.Ordinal);
if (markerStartIndex != -1)

while (true)
{
var markerStartIndex = line.IndexOf(MarkerStart, StringComparison.Ordinal);
if (markerStartIndex == -1)
{
break;
}

var markerEndIndex = line.IndexOf(MarkerEnd, markerStartIndex, StringComparison.Ordinal);
var markerName = line.Substring(markerStartIndex + 2, markerEndIndex - markerStartIndex - 2);
var markerLocation = new DiagnosticLocation(i + 1, markerStartIndex + 1);
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Description>CSharp Analyzers 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" />

<InternalsVisibleTo Include="Microsoft.AspNetCore.App.Analyzers.Test" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)IsExternalInit.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)Roslyn\CodeAnalysisExtensions.cs" LinkBase="Shared" />
</ItemGroup>

</Project>
20 changes: 20 additions & 0 deletions src/Framework/Analyzer/src/MinimalActions/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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;

namespace Microsoft.AspNetCore.Analyzers.MinimalActions
{
[System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking")]
internal static class DiagnosticDescriptors
{
internal static readonly DiagnosticDescriptor DoNotUseModelBindingAttributesOnMinimalActionParameters = new(
"ASP0003",
"Do not use model binding attributes with Map actions",
"{0} should not be specified for a {1} delegate parameter",
"Usage",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
helpLinkUri: "https://aka.ms/aspnet/analyzers");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// 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 Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.AspNetCore.Analyzers.MinimalActions;

public partial class MinimalActionAnalyzer : DiagnosticAnalyzer
{
private static void DisallowMvcBindArgumentsOnParameters(
in OperationAnalysisContext context,
WellKnownTypes wellKnownTypes,
IInvocationOperation invocation,
IMethodSymbol methodSymbol)
{
foreach (var parameter in methodSymbol.Parameters)
{
var modelBindingAttribute = parameter.GetAttributes(wellKnownTypes.IBinderTypeProviderMetadata).FirstOrDefault() ??
parameter.GetAttributes(wellKnownTypes.BindAttribute).FirstOrDefault();

if (modelBindingAttribute is not null)
{
var location = Location.None;
if (!parameter.DeclaringSyntaxReferences.IsEmpty)
{
var syntax = parameter.DeclaringSyntaxReferences[0].GetSyntax(context.CancellationToken);
location = syntax.GetLocation();
}

var methodName = invocation.TargetMethod.Name;

context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.DoNotUseModelBindingAttributesOnMinimalActionParameters,
location,
modelBindingAttribute.AttributeClass.Name,
methodName));
}
}
}
}
72 changes: 72 additions & 0 deletions src/Framework/Analyzer/src/MinimalActions/MinimalActionAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.AspNetCore.Analyzers.MinimalActions;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public partial class MinimalActionAnalyzer : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(new[]
{
DiagnosticDescriptors.DoNotUseModelBindingAttributesOnMinimalActionParameters,
});

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(static compilationStartAnalysisContext =>
{
var compilation = compilationStartAnalysisContext.Compilation;
if (!WellKnownTypes.TryCreate(compilation, out var wellKnownTypes))
{
return;
}

compilationStartAnalysisContext.RegisterOperationAction(operationAnalysisContext =>
{
var invocation = (IInvocationOperation)operationAnalysisContext.Operation;
var targetMethod = invocation.TargetMethod;
if (IsMapActionInvocation(wellKnownTypes, invocation, targetMethod))
{
return;
}

var delegateCreation = invocation.Arguments[2].Descendants().OfType<IDelegateCreationOperation>().FirstOrDefault();
if (delegateCreation is null)
{
return;
}

if (delegateCreation.Target.Kind == OperationKind.AnonymousFunction)
{
var lambda = ((IAnonymousFunctionOperation)delegateCreation.Target);
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol);
}
else if (delegateCreation.Target.Kind == OperationKind.MethodReference)
{
var methodReference = (IMethodReferenceOperation)delegateCreation.Target;
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, methodReference.Method);
}
}, OperationKind.Invocation);
});
}

private static bool IsMapActionInvocation(
WellKnownTypes wellKnownTypes,
IInvocationOperation invocation,
IMethodSymbol targetMethod)
{
return !targetMethod.Name.StartsWith("Map", StringComparison.Ordinal) ||
!SymbolEqualityComparer.Default.Equals(wellKnownTypes.MinimalActionEndpointRouteBuilderExtensions, targetMethod.ContainingType) ||
invocation.Arguments.Length != 3;
}
}
47 changes: 47 additions & 0 deletions src/Framework/Analyzer/src/MinimalActions/WellKnownTypes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;

namespace Microsoft.AspNetCore.Analyzers.MinimalActions;

internal sealed class WellKnownTypes
{
public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out WellKnownTypes? wellKnownTypes)
{
wellKnownTypes = default;
const string MinimalActionEndpointRouteBuilderExtensions = "Microsoft.AspNetCore.Builder.MinimalActionEndpointRouteBuilderExtensions";
if (compilation.GetTypeByMetadataName(MinimalActionEndpointRouteBuilderExtensions) is not { } minimalActionEndpointRouteBuilderExtensions)
{
return false;
}

const string IBinderTypeProviderMetadata = "Microsoft.AspNetCore.Mvc.ModelBinding.IBinderTypeProviderMetadata";
if (compilation.GetTypeByMetadataName(IBinderTypeProviderMetadata) is not { } ibinderTypeProviderMetadata)
{
return false;
}

// There isn't a good way to distinguish BindAttribute from other allowed
// MVC's attributes like From*.
const string BindAttribute = "Microsoft.AspNetCore.Mvc.BindAttribute";
if (compilation.GetTypeByMetadataName(BindAttribute) is not { } bindAttribute)
{
return false;
}

wellKnownTypes = new WellKnownTypes
{
MinimalActionEndpointRouteBuilderExtensions = minimalActionEndpointRouteBuilderExtensions,
IBinderTypeProviderMetadata = ibinderTypeProviderMetadata,
BindAttribute = bindAttribute,
};

return true;
}

public ITypeSymbol MinimalActionEndpointRouteBuilderExtensions { get; private init; }
public INamedTypeSymbol IBinderTypeProviderMetadata { get; private init; }
public INamedTypeSymbol BindAttribute { get; private init; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<PreserveCompilationContext>true</PreserveCompilationContext>
<RootNamespace>Microsoft.AspNetCore.Analyzers</RootNamespace>
</PropertyGroup>

<ItemGroup>
<None Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\src\Microsoft.AspNetCore.App.Analyzers.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" />
</ItemGroup>

</Project>
Loading