Skip to content

Commit 6475919

Browse files
pranavkmpranavkm
authored
Add an analyzer that complains when using using model binding attributes with a minimal action delegate (#35359)
* [x] Complain when using MVC binding attributes with minimal action Fixes #35088
1 parent ff9ed0f commit 6475919

22 files changed

+458
-28
lines changed

‎AspNetCore.sln‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
Microsoft Visual Studio Solution File, Format Version 12.00
33
# Visual Studio Version 17
4-
VisualStudioVersion = 17.0.31613.370
4+
VisualStudioVersion = 17.0.31606.5
55
MinimumVisualStudioVersion = 15.0.26124.0
66
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "eng", "eng", "{C28A32F6-8314-412E-9F3B-CBD31C23E878}"
77
EndProject
@@ -1642,6 +1642,12 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "testassets", "testassets",
16421642
EndProject
16431643
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Identity.DefaultUI.WebSite", "src\Identity\testassets\Identity.DefaultUI.WebSite\Identity.DefaultUI.WebSite.csproj", "{835A4E0F-A697-4B69-9736-3E99D163C4B9}"
16441644
EndProject
1645+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.App.Analyzer", "src\Framework\Analyzer\src\Microsoft.AspNetCore.App.Analyzers.csproj", "{564CABB8-1B3F-4D9E-909D-260EF2B8614A}"
1646+
EndProject
1647+
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Analyzer", "Analyzer", "{EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}"
1648+
EndProject
1649+
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}"
1650+
EndProject
16451651
Global
16461652
GlobalSection(SolutionConfigurationPlatforms) = preSolution
16471653
Debug|Any CPU = Debug|Any CPU
@@ -7833,6 +7839,30 @@ Global
78337839
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.Release|x64.Build.0 = Release|Any CPU
78347840
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.Release|x86.ActiveCfg = Release|Any CPU
78357841
{835A4E0F-A697-4B69-9736-3E99D163C4B9}.Release|x86.Build.0 = Release|Any CPU
7842+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
7843+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|Any CPU.Build.0 = Debug|Any CPU
7844+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.ActiveCfg = Debug|Any CPU
7845+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x64.Build.0 = Debug|Any CPU
7846+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.ActiveCfg = Debug|Any CPU
7847+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Debug|x86.Build.0 = Debug|Any CPU
7848+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.ActiveCfg = Release|Any CPU
7849+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|Any CPU.Build.0 = Release|Any CPU
7850+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.ActiveCfg = Release|Any CPU
7851+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x64.Build.0 = Release|Any CPU
7852+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.ActiveCfg = Release|Any CPU
7853+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A}.Release|x86.Build.0 = Release|Any CPU
7854+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
7855+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|Any CPU.Build.0 = Debug|Any CPU
7856+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.ActiveCfg = Debug|Any CPU
7857+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x64.Build.0 = Debug|Any CPU
7858+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.ActiveCfg = Debug|Any CPU
7859+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Debug|x86.Build.0 = Debug|Any CPU
7860+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.ActiveCfg = Release|Any CPU
7861+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|Any CPU.Build.0 = Release|Any CPU
7862+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.ActiveCfg = Release|Any CPU
7863+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x64.Build.0 = Release|Any CPU
7864+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.ActiveCfg = Release|Any CPU
7865+
{CF4CEC18-798D-46EC-B0A0-98D97496590F}.Release|x86.Build.0 = Release|Any CPU
78367866
EndGlobalSection
78377867
GlobalSection(SolutionProperties) = preSolution
78387868
HideSolutionNode = FALSE
@@ -8646,6 +8676,9 @@ Global
86468676
{514726D2-3D2E-44C1-B056-163E37DE3E8B} = {7B976D8F-EA31-4C0B-97BD-DFD9B3CC86FB}
86478677
{48526D13-69E2-4409-A57B-C3FA3C64B4F7} = {9F21A235-436E-4020-A076-1DF4F89D0CA0}
86488678
{835A4E0F-A697-4B69-9736-3E99D163C4B9} = {48526D13-69E2-4409-A57B-C3FA3C64B4F7}
8679+
{564CABB8-1B3F-4D9E-909D-260EF2B8614A} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}
8680+
{EE39397E-E4AF-4D3F-9B9C-D637F9222CDD} = {A4C26078-B6D8-4FD8-87A6-7C15A3482038}
8681+
{CF4CEC18-798D-46EC-B0A0-98D97496590F} = {EE39397E-E4AF-4D3F-9B9C-D637F9222CDD}
86498682
EndGlobalSection
86508683
GlobalSection(ExtensibilityGlobals) = postSolution
86518684
SolutionGuid = {3E8720B3-DBDD-498C-B383-2CC32A054E8F}

‎eng/Versions.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@
190190
<MicrosoftBuildUtilitiesCoreVersion>16.9.0</MicrosoftBuildUtilitiesCoreVersion>
191191
<MicrosoftCodeAnalysisCommonVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCommonVersion>
192192
<MicrosoftCodeAnalysisCSharpVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpVersion>
193-
<MicrosoftCodeAnalysisCSharpWorkspacesVersion>3.8.0</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
193+
<MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.0.0-2.21354.7</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
194194
<MicrosoftCodeAnalysisPublicApiAnalyzersVersion>3.3.0</MicrosoftCodeAnalysisPublicApiAnalyzersVersion>
195195
<MicrosoftCssParserVersion>1.0.0-20200708.1</MicrosoftCssParserVersion>
196196
<MicrosoftIdentityModelLoggingVersion>6.10.0</MicrosoftIdentityModelLoggingVersion>

‎src/Analyzers/Analyzers/src/StartupAnalysisBuilder.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

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

21-
_analysesByType = new Dictionary<INamedTypeSymbol, List<object>>();
21+
#pragma warning disable RS1024 // Compare symbols correctly
22+
_analysesByType = new Dictionary<INamedTypeSymbol, List<object>>(SymbolEqualityComparer.Default);
23+
#pragma warning restore RS1024 // Compare symbols correctly
2224
_lock = new object();
2325
}
2426

‎src/Analyzers/Analyzers/src/StartupAnalyzer.Diagnostics.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ static Diagnostics()
3838

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

4848
internal static readonly DiagnosticDescriptor IncorrectlyConfiguredAuthorizationMiddleware = new DiagnosticDescriptor(
4949
"ASP0001",
50-
"Authorization middleware is incorrectly configured.",
51-
"The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated.",
50+
"Authorization middleware is incorrectly configured",
51+
"The call to UseAuthorization should appear between app.UseRouting() and app.UseEndpoints(..) for authorization to be correctly evaluated",
5252
"Usage",
5353
DiagnosticSeverity.Warning,
5454
isEnabledByDefault: true,

‎src/Analyzers/Microsoft.AspNetCore.Analyzer.Testing/src/TestSource.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,15 @@ public static TestSource Read(string rawSource)
2626
for (var i = 0; i < lines.Length; i++)
2727
{
2828
var line = lines[i];
29-
var markerStartIndex = line.IndexOf(MarkerStart, StringComparison.Ordinal);
30-
if (markerStartIndex != -1)
29+
30+
while (true)
3131
{
32+
var markerStartIndex = line.IndexOf(MarkerStart, StringComparison.Ordinal);
33+
if (markerStartIndex == -1)
34+
{
35+
break;
36+
}
37+
3238
var markerEndIndex = line.IndexOf(MarkerEnd, markerStartIndex, StringComparison.Ordinal);
3339
var markerName = line.Substring(markerStartIndex + 2, markerEndIndex - markerStartIndex - 2);
3440
var markerLocation = new DiagnosticLocation(i + 1, markerStartIndex + 1);

‎src/Components/Web.JS/dist/Release/blazor.server.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Framework/Analyzer/src/Microsoft.AspNetCore.App.Analyzers.csproj‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<Description>CSharp Analyzers for ASP.NET Core.</Description>
4+
<IsShippingPackage>false</IsShippingPackage>
5+
<AddPublicApiAnalyzers>false</AddPublicApiAnalyzers>
6+
<TargetFramework>netstandard2.0</TargetFramework>
7+
<IncludeBuildOutput>false</IncludeBuildOutput>
8+
<Nullable>Enable</Nullable>
9+
<RootNamespace>Microsoft.AspNetCore.Analyzers</RootNamespace>
10+
</PropertyGroup>
11+
12+
<ItemGroup>
13+
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" PrivateAssets="All" />
14+
15+
<InternalsVisibleTo Include="Microsoft.AspNetCore.App.Analyzers.Test" />
16+
</ItemGroup>
17+
18+
<ItemGroup>
19+
<Compile Include="$(SharedSourceRoot)IsExternalInit.cs" LinkBase="Shared" />
20+
<Compile Include="$(SharedSourceRoot)Roslyn\CodeAnalysisExtensions.cs" LinkBase="Shared" />
21+
</ItemGroup>
22+
23+
</Project>

‎src/Framework/Analyzer/src/MinimalActions/DiagnosticDescriptors.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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 Microsoft.CodeAnalysis;
5+
6+
namespace Microsoft.AspNetCore.Analyzers.MinimalActions
7+
{
8+
[System.Diagnostics.CodeAnalysis.SuppressMessage("MicrosoftCodeAnalysisReleaseTracking", "RS2008:Enable analyzer release tracking")]
9+
internal static class DiagnosticDescriptors
10+
{
11+
internal static readonly DiagnosticDescriptor DoNotUseModelBindingAttributesOnMinimalActionParameters = new(
12+
"ASP0003",
13+
"Do not use model binding attributes with Map actions",
14+
"{0} should not be specified for a {1} delegate parameter",
15+
"Usage",
16+
DiagnosticSeverity.Warning,
17+
isEnabledByDefault: true,
18+
helpLinkUri: "https://aka.ms/aspnet/analyzers");
19+
}
20+
}

‎src/Framework/Analyzer/src/MinimalActions/DisallowMvcBindArgumentsOnParameters.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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.Linq;
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.Diagnostics;
7+
using Microsoft.CodeAnalysis.Operations;
8+
9+
namespace Microsoft.AspNetCore.Analyzers.MinimalActions;
10+
11+
public partial class MinimalActionAnalyzer : DiagnosticAnalyzer
12+
{
13+
private static void DisallowMvcBindArgumentsOnParameters(
14+
in OperationAnalysisContext context,
15+
WellKnownTypes wellKnownTypes,
16+
IInvocationOperation invocation,
17+
IMethodSymbol methodSymbol)
18+
{
19+
foreach (var parameter in methodSymbol.Parameters)
20+
{
21+
var modelBindingAttribute = parameter.GetAttributes(wellKnownTypes.IBinderTypeProviderMetadata).FirstOrDefault() ??
22+
parameter.GetAttributes(wellKnownTypes.BindAttribute).FirstOrDefault();
23+
24+
if (modelBindingAttribute is not null)
25+
{
26+
var location = Location.None;
27+
if (!parameter.DeclaringSyntaxReferences.IsEmpty)
28+
{
29+
var syntax = parameter.DeclaringSyntaxReferences[0].GetSyntax(context.CancellationToken);
30+
location = syntax.GetLocation();
31+
}
32+
33+
var methodName = invocation.TargetMethod.Name;
34+
35+
context.ReportDiagnostic(Diagnostic.Create(
36+
DiagnosticDescriptors.DoNotUseModelBindingAttributesOnMinimalActionParameters,
37+
location,
38+
modelBindingAttribute.AttributeClass.Name,
39+
methodName));
40+
}
41+
}
42+
}
43+
}

‎src/Framework/Analyzer/src/MinimalActions/MinimalActionAnalyzer.cs‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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.Collections.Immutable;
6+
using System.Linq;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.Diagnostics;
9+
using Microsoft.CodeAnalysis.Operations;
10+
11+
namespace Microsoft.AspNetCore.Analyzers.MinimalActions;
12+
13+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
14+
public partial class MinimalActionAnalyzer : DiagnosticAnalyzer
15+
{
16+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(new[]
17+
{
18+
DiagnosticDescriptors.DoNotUseModelBindingAttributesOnMinimalActionParameters,
19+
});
20+
21+
public override void Initialize(AnalysisContext context)
22+
{
23+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
24+
context.EnableConcurrentExecution();
25+
26+
context.RegisterCompilationStartAction(static compilationStartAnalysisContext =>
27+
{
28+
var compilation = compilationStartAnalysisContext.Compilation;
29+
if (!WellKnownTypes.TryCreate(compilation, out var wellKnownTypes))
30+
{
31+
return;
32+
}
33+
34+
compilationStartAnalysisContext.RegisterOperationAction(operationAnalysisContext =>
35+
{
36+
var invocation = (IInvocationOperation)operationAnalysisContext.Operation;
37+
var targetMethod = invocation.TargetMethod;
38+
if (IsMapActionInvocation(wellKnownTypes, invocation, targetMethod))
39+
{
40+
return;
41+
}
42+
43+
var delegateCreation = invocation.Arguments[2].Descendants().OfType<IDelegateCreationOperation>().FirstOrDefault();
44+
if (delegateCreation is null)
45+
{
46+
return;
47+
}
48+
49+
if (delegateCreation.Target.Kind == OperationKind.AnonymousFunction)
50+
{
51+
var lambda = ((IAnonymousFunctionOperation)delegateCreation.Target);
52+
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, lambda.Symbol);
53+
}
54+
else if (delegateCreation.Target.Kind == OperationKind.MethodReference)
55+
{
56+
var methodReference = (IMethodReferenceOperation)delegateCreation.Target;
57+
DisallowMvcBindArgumentsOnParameters(in operationAnalysisContext, wellKnownTypes, invocation, methodReference.Method);
58+
}
59+
}, OperationKind.Invocation);
60+
});
61+
}
62+
63+
private static bool IsMapActionInvocation(
64+
WellKnownTypes wellKnownTypes,
65+
IInvocationOperation invocation,
66+
IMethodSymbol targetMethod)
67+
{
68+
return !targetMethod.Name.StartsWith("Map", StringComparison.Ordinal) ||
69+
!SymbolEqualityComparer.Default.Equals(wellKnownTypes.MinimalActionEndpointRouteBuilderExtensions, targetMethod.ContainingType) ||
70+
invocation.Arguments.Length != 3;
71+
}
72+
}

0 commit comments

Comments
 (0)