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

Fix S6964 FP: Do not raise in properties with required modifier #9285

Closed
cremor opened this issue May 16, 2024 · 3 comments · Fixed by #9307
Closed

Fix S6964 FP: Do not raise in properties with required modifier #9285

cremor opened this issue May 16, 2024 · 3 comments · Fixed by #9307
Assignees
Labels
Area: C# C# rules related issues. Sprint: Hardening Fix FPs/FNs/improvements Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@cremor
Copy link

cremor commented May 16, 2024

Description

The rule S6964 should not be active for model properties that use the required keyword. Those properties can never be under-posted because the JSON deserialization for them fails.

Repro steps

  1. Create a new ASP.NET Core Web API project.
  2. Add a model with required properties (e.g. required int) and use it in a POST controller method.
  3. Call the POST method without providing the property. See that ASP.NET Core returns an error, even though you didn't apply the [Required] attribute manually.

Expected behavior

Rule should not be reported.

Actual behavior

Rule is reported.

Known workarounds

I had to create a new quality profile in SonarCloud to disable the rule completely.

Related information

  • C#/VB.NET Plugins version: 9.25.0.90414 (used by the SonarCloudPrepare@1 task)
  • Visual Studio version: not relevant
  • MSBuild / dotnet version: 8.0.300
  • SonarScanner for .NET version: SonarScanner for MSBuild 5.15 (used by the SonarCloudAnalyze@1 task)
  • Operating System: windows-latest build agent on Azure DevOps
@mary-georgiou-sonarsource
Copy link
Contributor

Hello @cremor.

The rule does take into consideration the RequiredAttribute.
Could you please provide me with a reproducer so I can investigate further?

Thanks!

@cremor
Copy link
Author

cremor commented May 17, 2024

My issue is about the required C# keyword. Not about the RequiredAttribute class.

@mary-georgiou-sonarsource
Copy link
Contributor

mary-georgiou-sonarsource commented May 17, 2024

@cremor I misread - my bad. Thanks a lot!

I confirm this is a false positive. We'll tackle it soon, as we plan to harden the ASP.NET rules in the following weeks.

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: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants