Go: Fix FPs in go/incorrect-integer-conversion query
#16234
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
smowton
previously approved these changes
Apr 17, 2024
|
@smowton I only did MRVA. I will do a QA run. |
It was introduced in github/codeql-go#718 in response to github/codeql-go#717, to check that we don't have type assertions as sinks. We now have other tests covering type assertions.
owen-mc
force-pushed
the
go/incorrect-integer-conversion-type-switch-fp
branch
from
99c7b97 to
212a0f2
Compare
|
I did a DCA run. No alert changes. The two biggest repos had a slight increase in analysis time, of 2-3%, but almost all the rest showed no difference at all. I suspect this may be due to the field flow branch limit. I will do a QA run just to be sure. |
|
QA results look fine. No increases in alerts - not sure why that's different from MRVA. Big decreases on a couple of repos, just for |
smowton
approved these changes
Apr 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found the type switch FPs while looking at the results for #16120, which fixed some broken flow into type switch clauses. While thinking about how best to fix them I also realised that any type assertion should transform the flow state, so I implemented that as well.
A before and after MRVA run suggests that on the top 1000 repos this PR removes 218 FPs, but the more detailed picture is more nuanced. On a very small number of repos it increased the number of alerts. This seems counter-intuitive, as this PR only adds barriers and hence should only remove results. I investigated and found that this was because removing some flow allowed us to find more results within the default limit for
fieldFlowBranchLimit.