Skip to content

Commit

Permalink
S6931: Support partial classes (#8965)
Browse files Browse the repository at this point in the history
  • Loading branch information
mary-georgiou-sonarsource committed Mar 22, 2024
1 parent 2d909a7 commit b993838
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ public abstract class RouteTemplateShouldNotStartWithSlashBase<TSyntaxKind>() :
var symbol = (INamedTypeSymbol)symbolStartContext.Symbol;
if (symbol.IsControllerType())
{
var controllerActionInfo = new ConcurrentStack<ControllerActionInfo>();
var controllerActionInfo = new ConcurrentStack<ActionParametersInfo>();
symbolStartContext.RegisterSyntaxNodeAction(nodeContext =>
{
if (nodeContext.SemanticModel.GetDeclaredSymbol(nodeContext.Node) is IMethodSymbol methodSymbol && methodSymbol.IsControllerMethod())
{
controllerActionInfo.Push(new ControllerActionInfo(methodSymbol, RouteAttributeTemplateArguments(methodSymbol.GetAttributes())));
controllerActionInfo.Push(new ActionParametersInfo(RouteAttributeTemplateArguments(methodSymbol.GetAttributes())));
}
}, Language.SyntaxKind.MethodDeclarations);
Expand All @@ -64,7 +64,7 @@ public abstract class RouteTemplateShouldNotStartWithSlashBase<TSyntaxKind>() :
}, SymbolKind.NamedType);
});

private void ReportIssues(SonarSymbolReportingContext context, INamedTypeSymbol controllerSymbol, ConcurrentStack<ControllerActionInfo> actions)
private void ReportIssues(SonarSymbolReportingContext context, INamedTypeSymbol controllerSymbol, ConcurrentStack<ActionParametersInfo> actions)
{
// If one of the following conditions is true, the rule won't raise an issue
// 1. The controller does not have any actions defined
Expand All @@ -78,18 +78,14 @@ private void ReportIssues(SonarSymbolReportingContext context, INamedTypeSymbol
var issueMessage = controllerSymbol.GetAttributes().Any(x => x.AttributeClass.IsAny(KnownType.RouteAttributes) || x.AttributeClass.Is(KnownType.System_Web_Mvc_RoutePrefixAttribute))
? MessageOnlyActions
: MessageActionsAndController;
var attributeLocations = actions.SelectMany(x => x.RouteParameters).Select(x => x.Key);

if (ControllerDeclarationSyntax(controllerSymbol) is { } controllerSyntax
&& Language.Syntax.NodeIdentifier(controllerSyntax) is { } identifier)
var secondaryLocations = actions.SelectMany(x => x.RouteParameters.Keys);
foreach (var classDeclaration in controllerSymbol.DeclaringSyntaxReferences.Select(x => x.GetSyntax()))
{
context.ReportIssue(Language.GeneratedCodeRecognizer, Diagnostic.Create(Rule, identifier.GetLocation(), attributeLocations, issueMessage));
context.ReportIssue(
Language.GeneratedCodeRecognizer,
Diagnostic.Create(Rule, Language.Syntax.NodeIdentifier(classDeclaration)?.GetLocation(), secondaryLocations, issueMessage));
}

SyntaxNode ControllerDeclarationSyntax(INamedTypeSymbol symbol) =>
symbol.DeclaringSyntaxReferences
.FirstOrDefault(x => !x.GetSyntax().SyntaxTree.IsGenerated(Language.GeneratedCodeRecognizer))
?.GetSyntax();
}

private static Dictionary<Location, string> RouteAttributeTemplateArguments(ImmutableArray<AttributeData> attributes)
Expand All @@ -105,5 +101,5 @@ private void ReportIssues(SonarSymbolReportingContext context, INamedTypeSymbol
return templates;
}

private readonly record struct ControllerActionInfo(IMethodSymbol Action, Dictionary<Location, string> RouteParameters);
private readonly record struct ActionParametersInfo(Dictionary<Location, string> RouteParameters);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
*/

using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;

namespace SonarAnalyzer.Test.Rules;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public class RouteTemplateShouldNotStartWithSlashTest
[TestMethod]
public void RouteTemplateShouldNotStartWithSlash_CS() =>
builderCS
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNetCore.cs")
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNetCore.cs", "RouteTemplateShouldNotStartWithSlash.AspNetCore.PartialAutogenerated.cs")
.WithConcurrentAnalysis(false)
.AddReferences(AspNetCoreReferences)
.Verify();

Expand Down Expand Up @@ -190,7 +191,8 @@ public class BasicsController : Controller {{(compliant ? string.Empty : " // No
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
public void RouteTemplateShouldNotStartWithSlash_CS(string aspNetMvcVersion) =>
builderCS
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNet4x.cs")
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNet4x.cs", "RouteTemplateShouldNotStartWithSlash.AspNet4x.PartialAutogenerated.cs")
.WithConcurrentAnalysis(false)
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

Expand All @@ -202,23 +204,6 @@ public class BasicsController : Controller {{(compliant ? string.Empty : " // No
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

[TestMethod]
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
public void RouteTemplateShouldNotStartWithSlash_DoesNotReportInAutoGeneratedCode_CS(string aspNetMvcVersion) =>
builderCS
.AddSnippet("""
// <auto-generated/>
using System.Web.Mvc;
[Route("[controller]")]
public class NoncompliantController : Controller // Compliant, we don't report in autogenerated controllers
{
[Route("/Index1")]
public ActionResult Index1() => View();
}
""")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

[DataRow("/Index2", false)]
[DataRow("\\Index2", true)]
[DataRow("Index1/SubPath", true)]
Expand Down Expand Up @@ -308,28 +293,6 @@ public class BasicsController : Controller {{(compliant ? string.Empty : " // No
builder.Verify();
}
}

[TestMethod]
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
public void RouteTemplateShouldNotStartWithSlash_DoesNotReportInAutoGeneratedCode_VB(string aspNetMvcVersion) =>
builderVB
.AddSnippet("""
' <auto-generated/>
Imports System.Web.Mvc

<Route("[controller]")>
Public Class NoncompliantController ' Compliant it's in an autogenerated file
Inherits Controller

<Route("/Index1")>
Public Function Index1() As ActionResult
Return View()
End Function

End Class
""")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();
#endif

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// <auto-generated/>
using System.Web.Mvc;

[Route("[controller]")]
public partial class NonCompliantPartialController : Controller // This is non-compliant but primary issues are not reported on auto-generated classes
{
[Route("/[action]")] // Secondary [first, second]
public ActionResult Index5() => View();

[Route("/SubPath/Index6_1")] // Secondary [first, second]
[Route("/[controller]/Index6_2")] // Secondary [first, second]
public ActionResult Index6() => View();
}

[Route("[controller]")]
public partial class PartialCompliantController : Controller // This class makes its partial partial part in RouteTemplateShouldNotStartWithSlash.AspNet4x.cs compliant
{
[Route("[action]")]
public ActionResult Index4() => View();
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,50 @@ public class WithFullQualifiedPartiallyAliasedNameController : Controller // Non
public ActionResult Index() => View();
}
}

[Route("[controller]")]
public partial class NonCompliantPartialController : Controller // Noncompliant [first]
{
[Route("/Index1")] // Secondary [first, second]
public ActionResult Index1() => View();

[Route("/SubPath/Index2_1")] // Secondary [first, second]
[Route("/[controller]/Index2_2")] // Secondary [first, second]
public ActionResult Index2() => View();
}

[Route("[controller]")]
public partial class NonCompliantPartialController : Controller // Noncompliant [second]
{
[Route("/[action]")] // Secondary [first, second]
public ActionResult Index3() => View();

[Route("/SubPath/Index4_1")] // Secondary [first, second]

[Route("/[controller]/Index4_2")] // Secondary [first, second]
public ActionResult Index4() => View();
}

[Route("[controller]")]
public partial class CompliantPartialController : Controller // Compliant, due to the other partial part of this class
{
[Route("/Index1")]
public ActionResult Index1() => View();

[Route("/SubPath/Index2")]
public ActionResult Index2() => View();
}

[Route("[controller]")]
public partial class CompliantPartialController : Controller
{
[Route("[action]")]
public ActionResult Index3() => View();
}

[Route("[controller]")]
public partial class PartialCompliantController : Controller // Compliant - its autogenerated part has at least one compliant action
{
[Route("/[action]")]
public ActionResult Index3() => View();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// <auto-generated/>
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

[Route("[controller]")]
public partial class NoncompliantPartialAutogeneratedController : Controller // This is non-compliant but primary issues are not reported on auto-generated classes
{
[HttpGet("/[action]")] // Secondary
public IActionResult Index3() => View();

[HttpGet("/SubPath/Index4_1")] // Secondary
public IActionResult Index4() => View();
}

[Route("[controller]")]
public partial class CompliantPartialAutogeneratedController : Controller // This class makes its partial partial part in RouteTemplateShouldNotStartWithSlash.AspNet4x.cs compliant
{
[HttpGet("[action]")]
public IActionResult Index3() => View();
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class NoncompliantNoControllerRouteController : Controller // Noncomplian

public class CompliantNoControllerRouteNoActionRouteController : Controller // Compliant
{
public IActionResult Index1() => View(); // Default route -> relative
public IActionResult Index1() => View(); // Conventional route -> relative

[Route("/SubPath/Index2")]
public IActionResult Index2() => View();
Expand All @@ -99,7 +99,7 @@ public class CompliantNoControllerRouteNoActionRouteController : Controller // C
public class CompliantNoControllerRouteEmptyActionRouteController : Controller // Compliant
{
[HttpGet]
public IActionResult Index1() => View(); // Empty route -> relative
public IActionResult Index1() => View(); // Empty route template -> relative conventional routing

[Route("/SubPath/Index2")]
public IActionResult Index2() => View();
Expand Down Expand Up @@ -257,3 +257,60 @@ public class PublicNestedController : Controller // Compliant, actions
public IActionResult Index() => View();
}
}

[Route("[controller]")]
public partial class NoncompliantPartialController : Controller // Noncompliant [first]
{
[Route("/Index1")] // Secondary [first, second]
public IActionResult Index1() => View();

[Route("/SubPath/Index2")] // Secondary [first, second]
public IActionResult Index2() => View();
}

[Route("[controller]")]
public partial class NoncompliantPartialController : Controller // Noncompliant [second]
{
[HttpGet("/[action]")] // Secondary [first, second]
public IActionResult Index3() => View();

[HttpGet("/SubPath/Index4_1")] // Secondary [first, second]
public IActionResult Index4() => View();
}

[Route("[controller]")]
public partial class CompliantPartialController : Controller // Compliant, due to the other partial part of this class
{
[Route("/Index1")]
public IActionResult Index1() => View();

[Route("/SubPath/Index2")]
public IActionResult Index2() => View();
}

[Route("[controller]")]
public partial class CompliantPartialController : Controller
{
[HttpGet("[action]")]
public IActionResult Index3() => View();
}

[Route("[controller]")]
public partial class NoncompliantPartialAutogeneratedController : Controller // Noncompliant {{Change the paths of the actions of this controller to be relative and adapt the controller route accordingly.}}
{
[Route("/Index1")] // Secondary
public IActionResult Index1() => View();

[Route("/SubPath/Index2")] // Secondary
public IActionResult Index2() => View();
}

[Route("[controller]")]
public partial class CompliantPartialAutogeneratedController : Controller // Compliant, as its autogenerated partial class is compliant
{
[Route("/Index1")]
public IActionResult Index1() => View();

[Route("/SubPath/Index2")]
public IActionResult Index2() => View();
}

0 comments on commit b993838

Please sign in to comment.