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

Identical Expression Comparison analyzer #7066

Merged
merged 11 commits into from
Aug 21, 2020

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Aug 20, 2020

What type of PR is this?

Tool

What does this PR do? Why is it needed?

This PR introduces a static code analyzer that reports a diagnostic when a boolean comparison expression (==, !=, >=, <=, >, <) is comparing two identical expressions. Occurrences of such code in Prysm have been found here: https://deepsource.io/gh/rkapka/prysm/issue/SCC-SA4000/occurrences. This is especially bad in tests because an assertion such as x == x will always pass, even if the tested code becomes broken.

Which issues(s) does this PR fix?

Part of #6747

Other notes for review

This PR will likely fail to build because of the mentioned occurrences. Please add commits to this branch with fixes because I don't know what the correct expressions should be.

@rkapka rkapka requested a review from a team as a code owner August 20, 2020 19:17
@rkapka rkapka marked this pull request as draft August 20, 2020 20:25
],
)

# gazelle:exclude analyzer_test.go
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to ignore tests because of #7074

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #7066 into master will increase coverage by 2.13%.
The diff coverage is 71.82%.

@@            Coverage Diff             @@
##           master    #7066      +/-   ##
==========================================
+ Coverage   60.07%   62.20%   +2.13%     
==========================================
  Files         323      404      +81     
  Lines       27422    31502    +4080     
==========================================
+ Hits        16473    19595    +3122     
- Misses       8733     9125     +392     
- Partials     2216     2782     +566     

@rkapka rkapka marked this pull request as ready for review August 21, 2020 08:57
@prylabs-bulldozer prylabs-bulldozer bot merged commit 6228b3c into master Aug 21, 2020
@farazdagi farazdagi deleted the identical-expression-comparison-analyzer branch August 25, 2020 15:48
@rkapka rkapka changed the title Identical expression comparison analyzer Identical Expression Comparison analyzer Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants