rspec
rspec copied to clipboard
Create rule S6932: Use model binding instead of reading raw request data
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for:
- [ ] logical errors and incorrect information
- [ ] information gaps and missing content
- [ ] text style and tone
- [ ] PR summary and labels follow the guidelines
This can be extended to Razor Pages (despite not being in the scope of the sprint, the concept is the same).
The PageModel
class also has a Request property, the same type as the Request property in MVC Controllers.
So the description will be pretty much the same. The later implementation as well. The only difference will be that instead of Controllers
the rule will look through PageModel
classes.
public class HomePageModel: PageModel
{
public IActionResult OnPost()
{
var name = Request.Form["name"]; // Noncompliant
...
}
}
@martin-strecker-sonarsource Please check the CI:
Rule csharp:S6932 has a "How to fix it" section for an unsupported framework: "ASP.NET Core"
The framework mentioned in the "How to fix it" section should be included in the list of frameworks here.
We will not handle the VariableDeclarationSyntax. We will call semanticModel.GetConstantValue(argument.Expression) instead. Do you want me to handle the whole constant expressions section of the spec?
No, knowing a bit how the implementation will look like, I think cover all constant expressions would be an overkill that doesn't bring much value. It's good as it is IMO.
@martin-strecker-sonarsource
I missed sending some pending comments. They might be outdated by now.
I went through all resolved conversations and checked whether there was something new. It should be all good now.
Quality Gate passed for 'rspec-frontend'
Issues
0 New issues
0 Fixed issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
Quality Gate passed for 'rspec-tools'
Issues
0 New issues
0 Fixed issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
@zsolt-kolbay-sonarsource Thank you for bringing this up. I did not have the time to look into this in more detail, but you are right that this looks like a low-hanging fruit. I added comments next to the parameterized tests, that we can extend these to PageModel 201582063b49d5e04bf2d7659ffbe9ee097a066f. Such an addition needs more tests and should result in an update of the RSpec.