Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S6931: Support for ASP.NET Core #8939

Merged
merged 3 commits into from
Mar 21, 2024
Merged
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
11 changes: 6 additions & 5 deletions analyzers/src/SonarAnalyzer.Common/Helpers/AspNetMvcHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ public static bool IsControllerMethod(this IMethodSymbol methodSymbol) =>
/// Returns a value indicating whether the provided type symbol is a ASP.NET MVC
/// controller.
/// </summary>
public static bool IsControllerType(this INamedTypeSymbol containingType) =>
containingType != null
&& (containingType.DerivesFromAny(ControllerTypes)
|| containingType.GetAttributes(ControllerAttributeTypes).Any())
&& !containingType.GetAttributes(NonControllerAttributeTypes).Any();
public static bool IsControllerType(this INamedTypeSymbol namedType) =>
namedType != null
&& namedType.ContainingSymbol is not INamedTypeSymbol
&& (namedType.DerivesFromAny(ControllerTypes)
|| namedType.GetAttributes(ControllerAttributeTypes).Any())
&& !namedType.GetAttributes(NonControllerAttributeTypes).Any();

public static bool ReferencesControllers(this Compilation compilation) =>
compilation.GetTypeByMetadataName(KnownType.System_Web_Mvc_Controller) is not null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private void ReportIssues(SonarSymbolReportingContext context, INamedTypeSymbol
// If one of the following conditions is true, the rule won't raise an issue
// 1. The controller does not have any actions defined
// 2. At least one action is not annotated with a route attribute or is annotated with a parameterless attribute
// 3. At least one route template defined in a route attribute over any action does not start with '/'
// 3. There is at least one action with a route template that does not start with '/'
if (!actions.Any() || actions.Any(x => !x.RouteParameters.Any() || x.RouteParameters.Values.Any(x => !x.StartsWith("/"))))
{
return;
Expand All @@ -91,7 +91,7 @@ SyntaxNode ControllerDeclarationSyntax(INamedTypeSymbol symbol) =>
private static Dictionary<Location, string> RouteAttributeTemplateArguments(ImmutableArray<AttributeData> attributes)
{
var templates = new Dictionary<Location, string>();
var routeAttributes = attributes.Where(x => x.AttributeClass.IsAny(KnownType.RouteAttributes));
var routeAttributes = attributes.Where(x => x.AttributeClass.IsAny(KnownType.RouteAttributes) || x.AttributeClass.DerivesFrom(KnownType.Microsoft_AspNetCore_Mvc_Routing_HttpMethodAttribute));
foreach (var attribute in routeAttributes)
{
if (attribute.TryGetAttributeValue<string>("template", out var templateParameter))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2024 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

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

namespace SonarAnalyzer.Test.Rules;

[TestClass]
public class RouteTemplateShouldNotStartWithSlashTest
{
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.RouteTemplateShouldNotStartWithSlash>();

#if NET
private static IEnumerable<MetadataReference> AspNetCoreReferences =>
[
AspNetCoreMetadataReference.MicrosoftAspNetCore, // For WebApplication
AspNetCoreMetadataReference.MicrosoftExtensionsHostingAbstractions, // For IHost
AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpAbstractions, // For HttpContext, RouteValueDictionary
AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpFeatures,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures,
AspNetCoreMetadataReference.MicrosoftAspNetCoreRouting, // For IEndpointRouteBuilder
AspNetCoreMetadataReference.MicrosoftAspNetCoreRazorPages, // For RazorPagesEndpointRouteBuilderExtensions.MapFallbackToPage
];

[TestMethod]
public void RouteTemplateShouldNotStartWithSlash_CS() =>
builderCS
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNetCore.cs")
.WithOptions(ParseOptionsHelper.FromCSharp12)
.AddReferences(AspNetCoreReferences)
.Verify();
#endif

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

public class WithUserDefinedAttributeDerivedFromHttpMethodAttributeController : Controller // Noncompliant: MyHttpMethodAttribute derives from HttpMethodAttribute
{
[MyHttpMethod("/Index")] // Secondary
public IActionResult WithUserDefinedAttribute() => View();

private sealed class MyHttpMethodAttribute(string template) : HttpMethodAttribute([template]) { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,156 @@ public class RouteTemplateShouldNotStartWithSlashTest
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.RouteTemplateShouldNotStartWithSlash>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.RouteTemplateShouldNotStartWithSlash>();

#if NETFRAMEWORK
#if NET
private static IEnumerable<MetadataReference> AspNetCoreReferences =>
[
AspNetCoreMetadataReference.MicrosoftAspNetCore, // For WebApplication
AspNetCoreMetadataReference.MicrosoftExtensionsHostingAbstractions, // For IHost
AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpAbstractions, // For HttpContext, RouteValueDictionary
AspNetCoreMetadataReference.MicrosoftAspNetCoreHttpFeatures,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures,
AspNetCoreMetadataReference.MicrosoftAspNetCoreRouting, // For IEndpointRouteBuilder
AspNetCoreMetadataReference.MicrosoftAspNetCoreRazorPages, // For RazorPagesEndpointRouteBuilderExtensions.MapFallbackToPage
];

[TestMethod]
public void RouteTemplateShouldNotStartWithSlash_CS() =>
builderCS
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNetCore.cs")
.AddReferences(AspNetCoreReferences)
.Verify();

[DataRow("""[HttpGet("/Index1")]""", "", false)]
[DataRow("""[Route("/Index2")]""", "", false)]
[DataRow("""[HttpGet("\\Index1")]""", "", true)]
[DataRow("""[Route("\\Index2")]""", "", true)]
[DataRow("""[HttpGet("Index1/SubPath")]""", "", true)]
[DataRow("""[Route("Index2/SubPath")]""", "", true)]
[DataRow("""[Route("/IndexA")]""", """[Route("/IndexB")]""", false)]
[DataRow("""[Route("/IndexC")]""", """[HttpGet("/IndexD")]""", false)]
[DataTestMethod]
public void RouteTemplateShouldNotStartWithSlash_RouteAttributes(string firstAttribute, string secondAttribute, bool compliant)
{
var builder = builderCS.AddReferences(AspNetCoreReferences)
.AddSnippet($$"""
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

[Route("[controller]")]
public class BasicsController : Controller {{(compliant ? string.Empty : " // Noncompliant")}}
{
{{firstAttribute}}{{(compliant ? string.Empty : " // Secondary")}}
{{secondAttribute}}{{(compliant || string.IsNullOrEmpty(secondAttribute) ? string.Empty : " // Secondary")}}
public ActionResult SomeAction() => View();
}
""");

if (compliant)
{
builder.VerifyNoIssueReported();
}
else
{
builder.Verify();
}
}

[DataRow("""[HttpGet("/IndexGet")]""")]
[DataRow("""[HttpPost("/IndexPost")]""")]
[DataRow("""[HttpPut("/IndexPut")]""")]
[DataRow("""[HttpDelete("/IndexDelete")]""")]
[DataRow("""[HttpPatch("/IndexPatch")]""")]
[DataRow("""[HttpHead("/IndexHead")]""")]
[DataRow("""[HttpOptions("/IndexOptions")]""")]
public void RouteTemplateShouldNotStartWithSlash_HttpAttributes(string attribute)
{
var builder = builderCS.AddReferences(AspNetCoreReferences)
.AddSnippet($$"""
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

public class BasicsController : Controller // Noncompliant
{
{{attribute}} // Secondary
public ActionResult SomeAction() => View();
Comment on lines +104 to +105

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test cases contradict with S6934 "How to fix" option 3).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martin-strecker-sonarsource @antonioaversa maybe we can exclude in S6931 routes that start with /[controller] as they seem pretty intentional.
WDYT?

}
""");
builder.Verify();
}

[DataRow("""[Route(template: @"/[action]", Name = "a", Order = 42)]""")]
[DataRow("""[RouteAttribute(@"/[action]")]""")]
[DataRow("""[Microsoft.AspNetCore.Mvc.RouteAttribute(@"/[action]")]""")]
[DataRow("""[method:Route(@"/[action]")]""")]
[DataTestMethod]
public void RouteTemplateShouldNotStartWithSlash_AttributeSyntaxVariations(string attribute)
{
var builder = builderCS.AddReferences(AspNetCoreReferences)
.AddSnippet($$"""
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

public class BasicsController : Controller // Noncompliant
{
{{attribute}} // Secondary
public ActionResult SomeAction() => View();
}
""");
builder.Verify();
}

[DataRow("""(@"/[action]")""", false)]
[DataRow("""("/[action]")""", false)]
[DataRow("""("\u002f[action]")""", false)]
[DataRow("""($"/{ConstA}/[action]")""", false)]
[DataRow(""""("""/[action]""")"""", false)]
[DataRow(""""""(""""/[action]"""")"""""", false)]
[DataRow(""""($$"""/{{ConstA}}/[action]""")"""", false)]
[DataRow("""(@"/[action]", Name = "a", Order = 42)""", false)]
[DataRow("""($"{ConstA}/[action]")""", true)]
[DataRow("""($"{ConstSlash}[action]")""", false)]
[DataTestMethod]
public void RouteTemplateShouldNotStartWithSlash_WithAllTypesOfStrings(string attributeParameter, bool compliant)
{
var builder = builderCS
.AddReferences(AspNetCoreReferences)
.WithOptions(ParseOptionsHelper.FromCSharp11)
.AddSnippet($$"""
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;

public class BasicsController : Controller {{(compliant ? string.Empty : " // Noncompliant")}}
{
private const string ConstA = "A";
private const string ConstSlash = "/";

[Route{{attributeParameter}}] {{(compliant ? string.Empty : " // Secondary")}}
public ActionResult SomeAction() => View();
}
""");

if (compliant)
{
builder.VerifyNoIssueReported();
}
else
{
builder.Verify();
}
}

[TestMethod]
public void RouteTemplateShouldNotStartWithSlash_VB() =>
builderVB
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNetCore.vb")
.AddReferences(AspNetCoreReferences)
.Verify();

#endif

#if NETFRAMEWORK
// ASP.NET 4x MVC 3 and 4 don't support attribute routing, nor MapControllerRoute and similar
public static IEnumerable<object[]> AspNet4xMvcVersionsUnderTest => [["5.2.7"] /* Most used */, [Constants.NuGetLatestVersion]];

Expand All @@ -42,17 +190,17 @@ private static IEnumerable<MetadataReference> AspNet4xReferences(string aspNetMv
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
public void RouteTemplateShouldNotStartWithSlash_CS(string aspNetMvcVersion) =>
builderCS
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNet4x.cs")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNet4x.cs")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

[TestMethod]
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
public void RouteTemplateShouldNotStartWithSlash_VB(string aspNetMvcVersion) =>
builderVB
.AddPaths("RouteTemplateShouldNotStartWithSlash.vb")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();
.AddPaths("RouteTemplateShouldNotStartWithSlash.AspNet4x.vb")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

[TestMethod]
[DynamicData(nameof(AspNet4xMvcVersionsUnderTest))]
Expand Down Expand Up @@ -182,7 +330,6 @@ End Class
""")
.AddReferences(AspNet4xReferences(aspNetMvcVersion))
.Verify();

#endif

}
Loading
Loading