Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,13 @@ internal static class DiagnosticDescriptors
DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: AnalyzersLink);

internal static readonly DiagnosticDescriptor AttributeRoutingUsedForNonActionMethod = new(
"ASP0029",
CreateLocalizableResourceString(nameof(Resources.Analyzer_AttributeRoutingUsedForNonActionMethod_Title)),
CreateLocalizableResourceString(nameof(Resources.Analyzer_AttributeRoutingUsedForNonActionMethod_Message)),
Usage,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: AnalyzersLink);
}
28 changes: 24 additions & 4 deletions src/Framework/AspNetCoreAnalyzers/src/Analyzers/Mvc/MvcAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ namespace Microsoft.AspNetCore.Analyzers.Mvc;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public partial class MvcAnalyzer : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [
DiagnosticDescriptors.AmbiguousActionRoute,
DiagnosticDescriptors.OverriddenAuthorizeAttribute
);
DiagnosticDescriptors.OverriddenAuthorizeAttribute,
DiagnosticDescriptors.AttributeRoutingUsedForNonActionMethod];

public override void Initialize(AnalysisContext context)
{
Expand Down Expand Up @@ -59,12 +59,21 @@ public override void Initialize(AnalysisContext context)
// Visit Actions
foreach (var member in namedTypeSymbol.GetMembers())
{
if (member is IMethodSymbol methodSymbol && MvcDetector.IsAction(methodSymbol, wellKnownTypes))
if (member is not IMethodSymbol methodSymbol)
{
continue;
}

if (MvcDetector.IsAction(methodSymbol, wellKnownTypes))
{
PopulateActionRoutes(context, wellKnownTypes, routeUsageCache, pooledItems.ActionRoutes, methodSymbol);
DetectOverriddenAuthorizeAttributeOnAction(context, wellKnownTypes, methodSymbol, pooledItems.AuthorizeAttributes, allowAnonClass);
pooledItems.AuthorizeAttributes.Clear();
}
else
{
AnalyzeNonActionMethod(methodSymbol, wellKnownTypes, context);
}
}

RoutePatternTree? controllerRoutePattern = null;
Expand All @@ -88,6 +97,17 @@ public override void Initialize(AnalysisContext context)
});
}

private static void AnalyzeNonActionMethod(IMethodSymbol method, WellKnownTypes wellKnownTypes, SymbolAnalysisContext context)
{
var routeTemplateProviderType = wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Mvc_Routing_IRouteTemplateProvider);

if (method.TryGetAttributeImplementingInterface(routeTemplateProviderType, out var attribute))
{
var syntax = attribute.ApplicationSyntaxReference!.GetSyntax(context.CancellationToken);
context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.AttributeRoutingUsedForNonActionMethod, syntax.GetLocation(), method.Name));
}
}

private static void PopulateActionRoutes(SymbolAnalysisContext context, WellKnownTypes wellKnownTypes, RouteUsageCache routeUsageCache, List<ActionRoute> actionRoutes, IMethodSymbol methodSymbol)
{
// [Route("xxx")] attributes don't have a HTTP method and instead use the HTTP methods of other attributes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,4 +333,10 @@
<data name="Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Message" xml:space="preserve">
<value>If the server does not specifically reject IPv6, IPAddress.IPv6Any is preferred over IPAddress.Any usage for safety and performance reasons. See https://aka.ms/aspnetcore-warnings/ASP0028 for more details.</value>
</data>
<data name="Analyzer_AttributeRoutingUsedForNonActionMethod_Title" xml:space="preserve">
<value>Attribute routing is used for a method that is not an action</value>
</data>
<data name="Analyzer_AttributeRoutingUsedForNonActionMethod_Message" xml:space="preserve">
<value>Attribute routing is used for '{0}' method that is not an action. Remove the attribute or make it a public method that can be an action.</value>
</data>
</root>
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.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Analyzers.Verifiers;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Testing;

namespace Microsoft.AspNetCore.Analyzers;

public static class CSharpAnalyzerTestExtensions
{
extension<TAnalyzer, TVerifier>(CSharpAnalyzerTest<TAnalyzer, TVerifier>)
where TAnalyzer : DiagnosticAnalyzer, new()
where TVerifier : IVerifier, new()
{
public static CSharpAnalyzerTest<TAnalyzer, TVerifier> Create([StringSyntax("C#-test")] string source, params ReadOnlySpan<DiagnosticResult> expectedDiagnostics)
{
var test = new CSharpAnalyzerTest<TAnalyzer, TVerifier>
{
TestCode = source.ReplaceLineEndings(),
// We need to set the output type to an exe to properly
// support top-level programs in the tests. Otherwise,
// the test infra will assume we are trying to build a library.
TestState = { OutputKind = OutputKind.ConsoleApplication },
ReferenceAssemblies = CSharpAnalyzerVerifier<TAnalyzer>.GetReferenceAssemblies(),
};

test.ExpectedDiagnostics.AddRange(expectedDiagnostics);
return test;
}
}

public static CSharpAnalyzerTest<TAnalyzer, TVerifier> WithSource<TAnalyzer, TVerifier>(this CSharpAnalyzerTest<TAnalyzer, TVerifier> test, [StringSyntax("C#-test")] string source)
where TAnalyzer : DiagnosticAnalyzer, new()
where TVerifier : IVerifier, new()
{
test.TestState.Sources.Add(source);
return test;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// 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.CSharp.Testing;
using Microsoft.CodeAnalysis.Testing;

using CSTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerTest<
Microsoft.AspNetCore.Analyzers.Mvc.MvcAnalyzer,
Microsoft.CodeAnalysis.Testing.DefaultVerifier>;

namespace Microsoft.AspNetCore.Analyzers.Mvc;

public class AttributeRoutingUsedForNonActionMethodTest
{
private const string Program = """
public class Program
{
public static void Main() { }
}
""";

[Theory]
[InlineData("AcceptVerbs(\"get\")")]
[InlineData("HttpDelete")]
[InlineData("HttpGet")]
[InlineData("HttpHead")]
[InlineData("HttpOptions")]
[InlineData("HttpPatch")]
[InlineData("HttpPost")]
[InlineData("HttpPut")]
[InlineData("Route(\"test\")")]
public async Task NonPublicMethod_HasDiagnostics(string attribute)
{
// Arrange
var source = $$"""
using Microsoft.AspNetCore.Mvc;

public class TestController : ControllerBase
{
[{|#0:{{attribute}}|}]
protected object Test1() => new();

[{|#1:{{attribute}}|}]
private object Test2() => new();

[{|#2:{{attribute}}|}]
internal object Test3() => new();

[{|#3:{{attribute}}|}]
protected internal object Test4() => new();
}
""";

var test = CSTest.Create(
source,
CreateDiagnostic("Test1", location: 0),
CreateDiagnostic("Test2", location: 1),
CreateDiagnostic("Test3", location: 2),
CreateDiagnostic("Test4", location: 3))
.WithSource(Program);

// Act & Assert
await test.RunAsync();
}

[Theory]
[InlineData("AcceptVerbs(\"get\")")]
[InlineData("HttpDelete")]
[InlineData("HttpGet")]
[InlineData("HttpHead")]
[InlineData("HttpOptions")]
[InlineData("HttpPatch")]
[InlineData("HttpPost")]
[InlineData("HttpPut")]
[InlineData("Route(\"test\")")]
public async Task MethodMarkedAsNonAction_HasDiagnostics(string attribute)
{
// Arrange
var source = $$"""
using Microsoft.AspNetCore.Mvc;

public class TestController : ControllerBase
{
[NonAction]
[{|#0:{{attribute}}|}]
public object Test() => new();
}
""";

var test = CSTest.Create(source, CreateDiagnostic("Test")).WithSource(Program);

// Act & Assert
await test.RunAsync();
}

private static DiagnosticResult CreateDiagnostic(string methodName, int location = 0)
{
return new DiagnosticResult(DiagnosticDescriptors.AttributeRoutingUsedForNonActionMethod).WithArguments(methodName).WithLocation(location);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.InternalTesting;
Expand Down Expand Up @@ -30,19 +31,9 @@ public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor)
=> CSharpAnalyzerVerifier<TAnalyzer, DefaultVerifier>.Diagnostic(descriptor);

/// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.VerifyAnalyzerAsync(string, DiagnosticResult[])"/>
public static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected)
public static async Task VerifyAnalyzerAsync([StringSyntax("C#-test")] string source, params DiagnosticResult[] expected)
{
var test = new CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>
{
TestCode = source.ReplaceLineEndings(),
// We need to set the output type to an exe to properly
// support top-level programs in the tests. Otherwise,
// the test infra will assume we are trying to build a library.
TestState = { OutputKind = OutputKind.ConsoleApplication },
ReferenceAssemblies = GetReferenceAssemblies(),
};

test.ExpectedDiagnostics.AddRange(expected);
var test = CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>.Create(source, expected);
await test.RunAsync(CancellationToken.None);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Shared/RoslynUtils/WellKnownTypeData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public enum WellKnownType
Microsoft_AspNetCore_Mvc_SkipStatusCodePagesAttribute,
Microsoft_AspNetCore_Mvc_ValidateAntiForgeryTokenAttribute,
Microsoft_AspNetCore_Mvc_ModelBinding_EmptyBodyBehavior,
Microsoft_AspNetCore_Mvc_Routing_IRouteTemplateProvider,
Microsoft_AspNetCore_Authorization_AllowAnonymousAttribute,
Microsoft_AspNetCore_Authorization_AuthorizeAttribute,
Microsoft_Extensions_DependencyInjection_PolicyServiceCollectionExtensions,
Expand Down Expand Up @@ -233,6 +234,7 @@ public enum WellKnownType
"Microsoft.AspNetCore.Mvc.SkipStatusCodePagesAttribute",
"Microsoft.AspNetCore.Mvc.ValidateAntiForgeryTokenAttribute",
"Microsoft.AspNetCore.Mvc.ModelBinding.EmptyBodyBehavior",
"Microsoft.AspNetCore.Mvc.Routing.IRouteTemplateProvider",
"Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute",
"Microsoft.AspNetCore.Authorization.AuthorizeAttribute",
"Microsoft.Extensions.DependencyInjection.PolicyServiceCollectionExtensions",
Expand Down
Loading