-
Notifications
You must be signed in to change notification settings - Fork 863
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
Adopt "errorprone" tool instead of Spotbugs #1525
Conversation
Compared to spotbugs, errorprone seems to run faster, finds a number of useful things, is easier to configure, and is updated often. * Add errorprone to the build for all non-test code * Suppress or disable warnings that are too much for us to fix now * Fix other errors and warnings * Remove spotbugs (it and errorprone don't always agree)
This change improves build times a little bit, makes the code more consistent and modern, and found three bugs:
Given that, I'm going to go ahead and merge it, although it may require be to do a bit more work on a few of the in-flight PRs. |
@@ -667,7 +666,7 @@ private Field[] getAccessibleFields(boolean includeProtected, boolean includePri | |||
for (Field field : declared) { | |||
int mod = field.getModifiers(); | |||
if (includePrivate || isPublic(mod) || isProtected(mod)) { | |||
if (!field.isAccessible()) field.setAccessible(true); | |||
field.trySetAccessible(); |
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.
@gbrail is there any reason why we not forward this to the vm bridge?
I guess performance might be no real reason because hopefully the vm inlines the call
@@ -328,8 +328,7 @@ private void discoverAccessibleMethods( | |||
if (isPublic(mods) || isProtected(mods) || includePrivate) { | |||
MethodSignature sig = new MethodSignature(method); | |||
if (!map.containsKey(sig)) { | |||
if (includePrivate && !method.isAccessible()) | |||
method.setAccessible(true); | |||
if (includePrivate) method.trySetAccessible(); |
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.
Same question as below
Compared to spotbugs, errorprone seems to run faster, finds a number of
useful things, is easier to configure, and is updated often.
Other notes: