diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SpecifyRouteAttribute.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SpecifyRouteAttribute.cs index 4fb98417299..c8006d76e85 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SpecifyRouteAttribute.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SpecifyRouteAttribute.cs @@ -27,42 +27,40 @@ public sealed class SpecifyRouteAttribute() : SonarDiagnosticAnalyzer RouteTemplateAttributes = - ImmutableArray.Create(KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute, KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute); + private static readonly ImmutableArray RouteTemplateAttributes = ImmutableArray.Create( + KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute, + KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute); - protected override string MessageFormat => "Specify the RouteAttribute when an HttpMethodAttribute is specified at an action level."; + protected override string MessageFormat => "Specify the RouteAttribute when an HttpMethodAttribute or RouteAttribute is specified at an action level."; protected override ILanguageFacade Language => CSharpFacade.Instance; protected override void Initialize(SonarAnalysisContext context) => context.RegisterCompilationStartAction(compilationStart => + { + if (!UsesAttributeRouting(compilationStart.Compilation)) + { + return; + } + compilationStart.RegisterSymbolStartAction(symbolStart => { - if (compilationStart.Compilation.GetTypeByMetadataName(KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute) is null) + if (symbolStart.Symbol.GetAttributes().Any(x => x.AttributeClass.Is(KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute))) { return; } - compilationStart.RegisterSymbolStartAction(symbolStart => + var secondaryLocations = new ConcurrentStack(); + symbolStart.RegisterSyntaxNodeAction(nodeContext => { - if (symbolStart.Symbol.GetAttributes().Any(x => x.AttributeClass.Is(KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute))) + var methodDeclaration = (MethodDeclarationSyntax)nodeContext.Node; + if (nodeContext.SemanticModel.GetDeclaredSymbol(methodDeclaration, nodeContext.Cancel) is { } method + && method.IsControllerMethod() + && method.GetAttributes().Any(x => !CanBeIgnored(x.GetAttributeRouteTemplate(RouteTemplateAttributes)))) { - return; + secondaryLocations.Push(methodDeclaration.Identifier.GetLocation()); } - var secondaryLocations = new ConcurrentStack(); - symbolStart.RegisterSyntaxNodeAction(nodeContext => - { - var node = (MethodDeclarationSyntax)nodeContext.Node; - if (nodeContext.SemanticModel.GetDeclaredSymbol(node, nodeContext.Cancel) is { } method - && method.IsControllerMethod() - && method.GetAttributes().Any(x => - x.AttributeClass.DerivesFromAny(RouteTemplateAttributes) - && x.TryGetAttributeValue("template", out var template) - && !CanBeIgnored(template))) - { - secondaryLocations.Push(node.Identifier.GetLocation()); - } - }, SyntaxKind.MethodDeclaration); - symbolStart.RegisterSymbolEndAction(symbolEnd => ReportIssues(symbolEnd, symbolStart.Symbol, secondaryLocations)); - }, SymbolKind.NamedType); - }); + }, SyntaxKind.MethodDeclaration); + symbolStart.RegisterSymbolEndAction(symbolEnd => ReportIssues(symbolEnd, symbolStart.Symbol, secondaryLocations)); + }, SymbolKind.NamedType); + }); private void ReportIssues(SonarSymbolReportingContext context, ISymbol symbol, ConcurrentStack secondaryLocations) { @@ -80,6 +78,9 @@ private void ReportIssues(SonarSymbolReportingContext context, ISymbol symbol, C } } + private static bool UsesAttributeRouting(Compilation compilation) => + compilation.GetTypeByMetadataName(KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute) is not null; + private static bool CanBeIgnored(string template) => string.IsNullOrWhiteSpace(template) // See: https://learn.microsoft.com/en-us/aspnet/core/mvc/controllers/routing#combining-attribute-routes diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs index 3ed347b455d..eaea75c7f1f 100644 --- a/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs @@ -28,6 +28,12 @@ public static bool HasName(this AttributeData attribute, string name) => public static bool HasAnyName(this AttributeData attribute, params string[] names) => names.Any(x => attribute.HasName(x)); + public static string GetAttributeRouteTemplate(this AttributeData attribute, ImmutableArray attributeTypes) => + attribute.AttributeClass.DerivesFromAny(attributeTypes) + && attribute.TryGetAttributeValue("template", out var template) + ? template + : null; + public static bool TryGetAttributeValue(this AttributeData attribute, string valueName, out T value) { // named arguments take precedence over constructor arguments of the same name. For [Attr(valueName: false, valueName = true)] "true" is returned. diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/RouteTemplateShouldNotStartWithSlashBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/RouteTemplateShouldNotStartWithSlashBase.cs index 1a46a6f66f3..431bbfef745 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/RouteTemplateShouldNotStartWithSlashBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/RouteTemplateShouldNotStartWithSlashBase.cs @@ -22,15 +22,19 @@ namespace SonarAnalyzer.Rules; -public abstract class RouteTemplateShouldNotStartWithSlashBase : SonarDiagnosticAnalyzer +public abstract class RouteTemplateShouldNotStartWithSlashBase() : SonarDiagnosticAnalyzer(DiagnosticId) where TSyntaxKind : struct { private const string DiagnosticId = "S6931"; private const string MessageOnlyActions = "Change the paths of the actions of this controller to be relative and adapt the controller route accordingly."; private const string MessageActionsAndController = "Change the paths of the actions of this controller to be relative and add a controller route with the common prefix."; + private static readonly ImmutableArray RouteTemplateAttributes = ImmutableArray.Create( + KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute, + KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute, + KnownType.System_Web_Mvc_RouteAttribute); + protected override string MessageFormat => "{0}"; - protected RouteTemplateShouldNotStartWithSlashBase() : base(DiagnosticId) { } protected override void Initialize(SonarAnalysisContext context) => context.RegisterCompilationStartAction(compilationStartContext => @@ -91,10 +95,9 @@ SyntaxNode ControllerDeclarationSyntax(INamedTypeSymbol symbol) => private static Dictionary RouteAttributeTemplateArguments(ImmutableArray attributes) { var templates = new Dictionary(); - var routeAttributes = attributes.Where(x => x.AttributeClass.IsAny(KnownType.RouteAttributes) || x.AttributeClass.DerivesFrom(KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute)); - foreach (var attribute in routeAttributes) + foreach (var attribute in attributes) { - if (attribute.TryGetAttributeValue("template", out var templateParameter)) + if (attribute.GetAttributeRouteTemplate(RouteTemplateAttributes) is { } templateParameter) { templates.Add(attribute.ApplicationSyntaxReference.GetSyntax().GetLocation(), templateParameter); } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/SpecifyRouteAttributeTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/SpecifyRouteAttributeTest.cs index 0074fb3fa66..65dd63ae318 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/SpecifyRouteAttributeTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/SpecifyRouteAttributeTest.cs @@ -31,63 +31,55 @@ public class SpecifyRouteAttributeTest .WithOptions(ParseOptionsHelper.FromCSharp12) .AddReferences([ AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore, - AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpAbstractions, AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures, - AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions, - AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpFeatures, + AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions ]); [TestMethod] public void SpecifyRouteAttribute_CS() => - builder.AddPaths("SpecifyRouteAttribute.cs").Verify(); + builder.AddPaths("SpecifyRouteAttribute.CSharp12.cs").Verify(); [TestMethod] public void SpecifyRouteAttribute_PartialClasses_CS() => builder .AddSnippet(""" - using Microsoft.AspNetCore.Mvc; - using Microsoft.AspNetCore.Mvc.Routing; + using Microsoft.AspNetCore.Mvc; - public partial class HomeController : Controller // Noncompliant [first] - { - [HttpGet("Test")] - public IActionResult Index() => View(); // Secondary [first, second] - } - """) + public partial class HomeController : Controller // Noncompliant [first] + { + [HttpGet("Test")] + public IActionResult Index() => View(); // Secondary [first, second] + } + """) .AddSnippet(""" - using Microsoft.AspNetCore.Mvc; - using Microsoft.AspNetCore.Mvc.Routing; + using Microsoft.AspNetCore.Mvc; - public partial class HomeController : Controller // Noncompliant [second] - { - } - """) + public partial class HomeController : Controller { } // Noncompliant [second] + """) .Verify(); [TestMethod] public void SpecifyRouteAttribute_PartialClasses_OneGenerated_CS() => builder .AddSnippet(""" - // - using Microsoft.AspNetCore.Mvc; - using Microsoft.AspNetCore.Mvc.Routing; + // + using Microsoft.AspNetCore.Mvc; - public partial class HomeController : Controller - { - [HttpGet("Test")] - public IActionResult ActionInGeneratedCode() => View(); // Secondary - } - """) + public partial class HomeController : Controller + { + [HttpGet("Test")] + public IActionResult ActionInGeneratedCode() => View(); // Secondary + } + """) .AddSnippet(""" - using Microsoft.AspNetCore.Mvc; - using Microsoft.AspNetCore.Mvc.Routing; + using Microsoft.AspNetCore.Mvc; - public partial class HomeController : Controller // Noncompliant - { - [HttpGet("Test")] - public IActionResult Index() => View(); // Secondary - } - """) + public partial class HomeController : Controller // Noncompliant + { + [HttpGet("Test")] + public IActionResult Index() => View(); // Secondary + } + """) .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.CSharp12.cs similarity index 98% rename from analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.cs rename to analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.CSharp12.cs index d71afc44283..4a63cab0d20 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SpecifyRouteAttribute.CSharp12.cs @@ -35,7 +35,7 @@ public class RouteTemplateIsNotSpecifiedController : Controller public IActionResult Error() => View(); // Compliant } -public class RouteTemplatesAreSpecifiedController : Controller // Noncompliant [controller] {{Specify the RouteAttribute when an HttpMethodAttribute is specified at an action level.}} +public class RouteTemplatesAreSpecifiedController : Controller // Noncompliant [controller] {{Specify the RouteAttribute when an HttpMethodAttribute or RouteAttribute is specified at an action level.}} // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ { private const string ConstantRoute = "ConstantRoute"; @@ -175,4 +175,4 @@ public class NonController public class DerivedController : Controller { } [Controller] -public class Endpoint {} +public class Endpoint { }