uber / NullAway

A tool to help eliminate NullPointerExceptions (NPEs) in your Java code with low build-time overhead
MIT License
3.63k stars 293 forks source link

Fix assertion check for structure of enhanced-for loop over a Map keySet #868

Closed msridhar closed 10 months ago

msridhar commented 10 months ago

Fixes #866

Before, we would check that an enhanced-for loop includes a call to Set.iterator() on the result of calling Map.keySet(). However, it is possible and legal that statically, the target of this call is instead Collection.iterator(). So, we change our check to test the receiver type passed into the call (which must still be a Set). Also, opportunistically switch a couple of places we were throwing RuntimeException around this check to throw the more meaningful VerifyException. Unfortunately, we have not found a way to add a test in open-source to reproduce the failure from #866 but we have confirmed this change fixes the problem.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (8f4f8a6) 87.02% compared to head (ff60b56) 87.04%.

Files Patch % Lines
...llaway/dataflow/AccessPathNullnessPropagation.java 50.00% 3 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #868 +/- ## ============================================ + Coverage 87.02% 87.04% +0.02% Complexity 1922 1922 ============================================ Files 77 77 Lines 6219 6221 +2 Branches 1210 1208 -2 ============================================ + Hits 5412 5415 +3 - Misses 403 404 +1 + Partials 404 402 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

msridhar commented 10 months ago

The test coverage failure is expected here as we do not expect the exceptional cases to actually occur

msridhar commented 10 months ago

@yuxincs @lazaroclapp could one of you review?