Skip to content

Commit

Permalink
Fix S6934 FP + FN: Inherited Route and HttpMethod attributes are not …
Browse files Browse the repository at this point in the history
…considered at the controller or action level (#8979)
  • Loading branch information
costin-zaharia-sonarsource authored Mar 25, 2024
1 parent 9478382 commit 4d28640
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
}
compilationStart.RegisterSymbolStartAction(symbolStart =>
{
if (symbolStart.Symbol.GetAttributes().Any(x => x.AttributeClass.Is(KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute)))
if (symbolStart.Symbol.GetAttributesWithInherited().Any(x => x.AttributeClass.Is(KnownType.Microsoft_AspNetCore_Mvc_RouteAttribute)))
{
return;
}
Expand All @@ -53,7 +53,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
var methodDeclaration = (MethodDeclarationSyntax)nodeContext.Node;
if (nodeContext.SemanticModel.GetDeclaredSymbol(methodDeclaration, nodeContext.Cancel) is { } method
&& method.IsControllerMethod()
&& method.GetAttributes().Any(x => !CanBeIgnored(x.GetAttributeRouteTemplate(RouteTemplateAttributes))))
&& method.GetAttributesWithInherited().Any(x => !CanBeIgnored(x.GetAttributeRouteTemplate(RouteTemplateAttributes))))
{
secondaryLocations.Push(methodDeclaration.Identifier.GetLocation());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,30 @@ public class DerivedController : Controller { }

[Controller]
public class Endpoint { }

[Route("api/[controller]")]
public class ControllerWithRouteAttribute : Controller { }

public class ControllerWithInheritedRoute : ControllerWithRouteAttribute // Compliant, attribute is inherited
{
[HttpGet("Test")] // Route: api/ControllerWithInheritedRoute/Test
public string Index() => "Hi!";
}

public class BaseControllerWithActionWithRoute : Controller // Noncompliant
{
[HttpGet("Test")] //Route: /Test (AmbiguousMatchException raised because of the override in ControllerOverridesActionWithRoute)
public virtual string Index() => "Hi!"; // Secondary

// Route: BaseControllerWithActionWithRoute/Index/1
public virtual string Index(int id) => "Hi!"; // Compliant
}

public class ControllerOverridesActionWithRoute : BaseControllerWithActionWithRoute // Noncompliant
{
// Route: /Test (AmbiguousMatchException raised because the base method is also in scope)
public override string Index() => "Hi!"; // Secondary

// Route: ControllerOverridesActionWithRoute/Index/1
public override string Index(int id) => "Hi!"; // Compliant
}

0 comments on commit 4d28640

Please sign in to comment.