-
Notifications
You must be signed in to change notification settings - Fork 299
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
Fix backwards-incompatible calls to ASTHelpers.hasDirectAnnotationWithSimpleName #894
Conversation
@@ -21,7 +21,8 @@ plugins { | |||
} | |||
|
|||
configurations { | |||
nullawayJar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this configuration as it is no longer used
@yuxincs this fixes the issue that caused release 0.10.20 to be unusable on older Error Prone versions |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #894 +/- ##
=========================================
Coverage 86.94% 86.95%
- Complexity 1952 1953 +1
=========================================
Files 77 77
Lines 6314 6315 +1
Branches 1222 1222
=========================================
+ Hits 5490 5491 +1
Misses 420 420
Partials 404 404 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix! LGTM! I'll cut a new release once this and PR #895 land.
Ideally we would have our own EP check
Agreed, let's handle that in follow-up PRs instead.
// A bit of a hack: we add the dependencies of the oldest supported Error Prone version to the _beginning_ of the | ||
// classpath, so that they are used instead of the latest version. This exercises the scenario of building | ||
// NullAway against the latest supported Error Prone version but then running on the oldest supported version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Error Prone 2.24.0 introduces new overloads of
ASTHelpers.hasDirectionAnnotationWithSimpleName
; see google/error-prone@aa6e3e7. This causes backward compatibility issues when compiling against EP 2.24.0+ and then running on an older version of Error Prone. We introduce a wrapper method to avoid this issue. We also modify ourtestJdk8
tasks to properly test the scenario of compiling against the newest supported Error Prone version and then running on the oldest supported version, which would have caught this issue.Ideally we would have our own EP check preventing direct calls to
ASTHelpers.hasDirectionAnnotationWithSimpleName
, but that can be done in a follow-up.