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 S3655: Support C# 9 and C# 10 syntax #6794

Closed
6 tasks done
pavel-mikula-sonarsource opened this issue Feb 27, 2023 · 5 comments · Fixed by #7000
Closed
6 tasks done

Improve S3655: Support C# 9 and C# 10 syntax #6794

pavel-mikula-sonarsource opened this issue Feb 27, 2023 · 5 comments · Fixed by #7000
Assignees
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll Type: Improvement Making existing code better.
Projects
Milestone

Comments

@pavel-mikula-sonarsource
Copy link
Contributor

pavel-mikula-sonarsource commented Feb 27, 2023

Improve precision and detection of S3655 by migrating it to the new SE engine. While keeping the old implementation for Roslyn v1.x and v2.x series.

PRs:

  • Clone TestCases from Sonar directory to Roslyn. Mark all Noncompliant as FIXME Non-compliant
  • Create a dummy without implementation, register it in SymbolicExecutionRunner, add test methods
  • Implement ShouldExecute
  • Implement the rule - this depends on the engine capabilities
  • ...investigate and fix whatever is missing
  • Remove remaining FIXME annotations, close FP repro issues as fixed in this sprint
@pavel-mikula-sonarsource pavel-mikula-sonarsource added Type: Improvement Making existing code better. Area: CFG/SE CFG and SE related issues. Area: C# C# rules related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll labels Feb 27, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource added this to the 8.56 milestone Feb 27, 2023
@github-actions github-actions bot added this to To do in Best Kanban Feb 27, 2023
@antonioaversa antonioaversa moved this from To do to In progress in Best Kanban Mar 1, 2023
@pavel-mikula-sonarsource
Copy link
Contributor Author

The idea is, that nullable symbol will share constraints with it's value. So the nullable itself will have Null or NotNull constraint. You can implement the rule based on this.

The rule will need a special handling for members like .HasValue and .GetValueOrDefault() that should not raise an issue when accessed.

@antonioaversa
Copy link
Contributor

antonioaversa commented Mar 3, 2023

The rule will need a special handling for members like .HasValue and .GetValueOrDefault() that should not raise an issue when accessed.

I would rather say that Value should be the only one raising an issue when accessed. Every other method (e.g.ToString, GetHashCode, GetType, ...) on a Nullable<T> doesn't seem to raise an exception, no matter what's the value of the nullable.

@pavel-mikula-sonarsource
Copy link
Contributor Author

The rule will need a special handling for members like .HasValue and .GetValueOrDefault() that should not raise an issue when accessed.

I would rather say that Value should be the only one raising an issue when accessed. Every other method (e.g.ToString, GetHashCode, GetType, ...) on a Nullable<T> doesn't seem to raise an exception, no matter what's the value of the nullable.

Yes, it's actually S2259 that needs special handling. And that one is extracted to #6840

@antonioaversa
Copy link
Contributor

New issues, documenting behaviors detected during peach validation:

@antonioaversa antonioaversa moved this from Validate Peach to Done in Best Kanban Apr 4, 2023
@martin-strecker-sonarsource
Copy link
Contributor

Closed as completed.

Best Kanban automation moved this from Done to Validate Peach Apr 18, 2023
@martin-strecker-sonarsource martin-strecker-sonarsource moved this from Validate Peach to Done in Best Kanban Apr 18, 2023
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. Area: CFG/SE CFG and SE related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll Type: Improvement Making existing code better.
Projects
Best Kanban
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants