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 another bug with Lombok equals() parameter annotations #880

Open msridhar opened 10 months ago

msridhar commented 10 months ago

We were getting override errors in relation to the parameter of some Lombok-generated equals() method due to the fix in #874. This error is not reported with the latest version of Error Prone since it properly handles @SuppressWarnings("all"), which is generated by Lombok (see https://github.com/google/error-prone/pull/4076). For compatibility with older Error Prone versions we add our own check for @SuppressWarnings("all").

Getting a test case here required some work since this issue can only be reproduced on older Error Prone versions. We re-jigger some test projects to build against whatever Error Prone version is specified with epApiVersion, so that we catch issues like these in the future.

codecov[bot] commented 10 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 87.07%. Comparing base (28cc318) to head (7092ef6).

Files Patch % Lines
.../src/main/java/com/uber/nullaway/ErrorBuilder.java 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #880 +/- ## ============================================ - Coverage 87.09% 87.07% -0.02% Complexity 1990 1990 ============================================ Files 77 77 Lines 6430 6431 +1 Branches 1245 1245 ============================================ Hits 5600 5600 Misses 422 422 - Partials 408 409 +1 ```

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

msridhar commented 10 months ago

FYI I am going to hold off on landing this, as I realized it doesn't completely fix the problems we were seeing. Once we figure out a plan for moving forward, I will land this PR (assuming it's part of that plan).