S6931: Support for ASP.NET Core #8939
Conversation
mary-georgiou-sonarsource
force-pushed
the
mary/S6931-aspnetcore-2.x
branch
14 times, most recently
from
5899532
to
6cb3a8e
Compare
mary-georgiou-sonarsource
force-pushed
the
mary/S6931-aspnetcore-2.x
branch
3 times, most recently
from
9e9b345
to
7e09abf
Compare
github-actions
bot
assigned martin-strecker-sonarsource and unassigned mary-georgiou-sonarsource
| compilationStartContext.RegisterSymbolStartAction(symbolStartContext => | ||
| { | ||
| var symbol = (INamedTypeSymbol)symbolStartContext.Symbol; | ||
| if (symbol.IsControllerType() && symbol.ContainingSymbol is not INamedTypeSymbol) |
There was a problem hiding this comment.
The && symbol.ContainingSymbol is not INamedTypeSymbol part should go into symbol.IsControllerType()
| if (symbol.IsControllerType() && symbol.ContainingSymbol is not INamedTypeSymbol) | |
| if (symbol.IsControllerType()) |
| // 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 '/' |
There was a problem hiding this comment.
| // 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 '/' |
martin-strecker-sonarsource
assigned mary-georgiou-sonarsource and unassigned martin-strecker-sonarsource
mary-georgiou-sonarsource
force-pushed
the
mary/S6931-aspnetcore-2.x
branch
from
7e09abf
to
fe80c19
Compare
github-actions
bot
assigned martin-strecker-sonarsource and unassigned mary-georgiou-sonarsource
mary-georgiou-sonarsource
force-pushed
the
mary/S6931-aspnetcore-2.x
branch
from
c1942ad
to
a2a1d08
Compare
mary-georgiou-sonarsource
force-pushed
the
mary/S6931-aspnetcore-2.x
branch
from
a2a1d08
to
1314122
Compare
|
|
| {{attribute}} // Secondary | ||
| public ActionResult SomeAction() => View(); |
There was a problem hiding this comment.
These test cases contradict with S6934 "How to fix" option 3).
There was a problem hiding this comment.
@martin-strecker-sonarsource @antonioaversa maybe we can exclude in S6931 routes that start with /[controller] as they seem pretty intentional.
WDYT?
| public IActionResult Index4() => View(); | ||
| } | ||
|
|
||
| public class NoncompliantNoControllerRouteController : Controller // Noncompliant {{Change the paths of the actions of this controller to be relative and add a controller route with the common prefix.}} |
There was a problem hiding this comment.
This contradicts suggestion 3) for rule S6934. This is not necessarily a bad thing as option 3) is still not ideal. But if you want to do route value binding for a single action only, it is a viable alternative. You just have to accept the S6931 violation afterward. I think we should cross-link between the two RSpecs.
Please look at the next test case as well. We allow absolute path in this rule as well if there is mixed routing.
/cc @costin-zaharia-sonarsource and @antonioaversa
|
|
||
| public class CompliantNoControllerRouteNoActionRouteController : Controller // Compliant | ||
| { | ||
| public IActionResult Index1() => View(); // Default route -> relative |
There was a problem hiding this comment.
| public IActionResult Index1() => View(); // Default route -> relative | |
| public IActionResult Index1() => View(); // Conventional route -> relative |
There was a problem hiding this comment.
fixed here 1b3c553
| public class CompliantNoControllerRouteEmptyActionRouteController : Controller // Compliant | ||
| { | ||
| [HttpGet] | ||
| public IActionResult Index1() => View(); // Empty route -> relative |
There was a problem hiding this comment.
| public IActionResult Index1() => View(); // Empty route -> relative | |
| public IActionResult Index1() => View(); // Empty route template -> relative conventional routing |
There was a problem hiding this comment.
fixed here 1b3c553
github-actions
bot
assigned mary-georgiou-sonarsource and unassigned martin-strecker-sonarsource
Related to #8870
@martin-strecker-sonarsource Please ignore the squash-rebase commit.I rebased.