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

Improve AspNetMvcHelper.ReferencesControllers method: add parameter for .NET Core #9251

Closed
zsolt-kolbay-sonarsource opened this issue May 3, 2024 · 1 comment · Fixed by #9309
Assignees
Labels
Area: C# C# rules related issues. Sprint: Hardening Fix FPs/FNs/improvements Type: Improvement Making existing code better.
Projects
Milestone

Comments

@zsolt-kolbay-sonarsource
Copy link
Contributor

zsolt-kolbay-sonarsource commented May 3, 2024

Currently the AspNetMvcHelper.ReferencesController() method returns true if the analyzed project references ASP.NET MVC Core or ASP.NET MVC (.NET Framework).
Several rules misuse this method to check whether the analyzer should be registered for the given project.
e.g. AnnotateApiActionsWithHttpVerb is only implemented for ASP.NET Core, but it uses this method to check if the analyzer should be used on the project or not. If the project uses ASP.NET MVC controllers under the old .NET Framework then it will still (needlessly) analyze it, but it won't report anything, because the rule only works with types from ASP.NET Core.

TODO

Add a bool parameter to the method: ReferencesController(bool onlyAspNetCore). When set to true it will only return true if the project references ASP.NET Core MVC Controllers. When the parameter is set to false, it will return true if the project references either ASP.NET Core MVC Controller or the classical .NET Framework MVC Controllers.
Then update all ASP.NET rules to use it correctly: if the rule is only implemented for ASP.NET Core then call ReferencesController(onlyAspNetCore: true)

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource added Type: Improvement Making existing code better. Area: C# C# rules related issues. labels May 3, 2024
@antonioaversa
Copy link
Contributor

Alternative proposal: having two distinct methods, one for core (e.g. referencesCoreControllers) and one for framework (e.g. referencesFrameworkControllers).
We would have very few rules targeting ASP.NET 4.x in addition to ASP.NET Core, so where something like if (referencesCoreControllers() && referencesFrameworkController()) would be required.
I think it would be more explicit and simpler to read than a flag passed as input parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Sprint: Hardening Fix FPs/FNs/improvements Type: Improvement Making existing code better.
Projects
Best Kanban
  
Validate Peach
Development

Successfully merging a pull request may close this issue.

3 participants