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.

Sign up for GitHub

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

Jump to bottom

Fix S1905 FP: Nullability context and array of anonymous types #6753

Merged
merged 23 commits into from Mar 14, 2023
Merged

Fix S1905 FP: Nullability context and array of anonymous types #6753

merged 23 commits into from Mar 14, 2023

Conversation

Tim-Pohlmann
Copy link
Contributor

Fixes #6438

@Tim-Pohlmann Tim-Pohlmann marked this pull request as ready for review February 15, 2023 13:06
martin-strecker-sonarsource
martin-strecker-sonarsource requested changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nice refactoring and the solution seems also right to me.

analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantCast.CSharp8.cs Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantCast.CSharp8.cs Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
martin-strecker-sonarsource
martin-strecker-sonarsource requested changes Feb 16, 2023
View reviewed changes
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/RedundantCastTest.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantCast.CSharp8.cs Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantCast.CSharp8.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
martin-strecker-sonarsource
martin-strecker-sonarsource requested changes Feb 27, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you bump the coverage in the touched code paths?

analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/TestCases/RedundantCast.CSharp8.cs Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs Outdated Show resolved Hide resolved
@Tim-Pohlmann Tim-Pohlmann mentioned this pull request Feb 28, 2023
Closed
Tim-Pohlmann and others added 20 commits March 1, 2023 10:26
@Tim-Pohlmann
Add FP test cases
56a5ca2
@Tim-Pohlmann
Clean up some code
fa0d931
@Tim-Pohlmann
Add check for anaonymous objects in implicit arrays and switch expres…
2ce6132 
…sions
@Tim-Pohlmann
Improve pattern matching
655120e
@Tim-Pohlmann
Fix a newly introduced FN
12e5164
@Tim-Pohlmann
Change approach to check for Nullability
f87cadd
@Tim-Pohlmann
Change code to use FlowState when available
eaa9553
@Tim-Pohlmann
Refactor and improve tests
c85f588
@Tim-Pohlmann
Clean up CheckCastExpression
cbb4a49
@Tim-Pohlmann @martin-strecker-sonarsource
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
92fe174 
Co-authored-by: Martin Strecker <103252490+martin-strecker-sonarsource@users.noreply.github.com>
@Tim-Pohlmann @martin-strecker-sonarsource
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
cdf4780 
Co-authored-by: Martin Strecker <103252490+martin-strecker-sonarsource@users.noreply.github.com>
@Tim-Pohlmann @martin-strecker-sonarsource
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
8a5b500 
Co-authored-by: Martin Strecker <103252490+martin-strecker-sonarsource@users.noreply.github.com>
@Tim-Pohlmann
Fixe wrong parameter order
ab8ecd8
@Tim-Pohlmann
Refactor some code and add tests for as expressions
b3db542
@Tim-Pohlmann
Improve code
05578e3
@Tim-Pohlmann
Refactor GetElementType
2d51c5f
@Tim-Pohlmann
Add more tests
11d61ce
@Tim-Pohlmann
Use Is(KnownType...) instead of name check
aae24ab
@Tim-Pohlmann
Simplify GetReportLocation
28ddab0
@Tim-Pohlmann
Add more tests, including FNs
943592d
@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.5% 90.5% Coverage
0.0% 0.0% Duplication

martin-strecker-sonarsource
martin-strecker-sonarsource approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Tim-Pohlmann
Copy link
Contributor Author

@pavel-mikula-sonarsource can you merge this? lack of coverage is due to 3 lines of defensive programming.

@pavel-mikula-sonarsource pavel-mikula-sonarsource merged commit 8e33179 into master Mar 14, 2023
24 of 25 checks passed
@pavel-mikula-sonarsource pavel-mikula-sonarsource deleted the Tim/Fix-1905-FP branch March 14, 2023 09:20
@Tim-Pohlmann Tim-Pohlmann mentioned this pull request Mar 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martin-strecker-sonarsource martin-strecker-sonarsource martin-strecker-sonarsource approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix S1905 FP: Nullability context and array of anonymous types
3 participants
@Tim-Pohlmann @martin-strecker-sonarsource @pavel-mikula-sonarsource