Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improvement: Bump errorprone to 2.11.0 #2069

Merged
merged 8 commits into from
Mar 28, 2022
Merged

Improvement: Bump errorprone to 2.11.0 #2069

merged 8 commits into from
Mar 28, 2022

Conversation

robert3005
Copy link
Contributor

Before this PR

After this PR

==COMMIT_MSG==
Upgrade errorprone to 2.11.0
==COMMIT_MSG==

Since we bundle mockito errorprone rules and there was a slight api break in errorprone we need mockito to release their updated rules first mockito/mockito#2553

Possible downsides?

@policy-bot policy-bot bot requested a review from ferozco February 3, 2022 18:57
robert3005
robert3005 commented Feb 3, 2022
View reviewed changes
baseline-error-prone/build.gradle Outdated
check("StrictUnusedVariable", CheckSeverity.OFF)
}
}

// Incorrectly identifies tests as junit4 usage
tasks.checkJUnitDependencies.enabled = false
tasks.javadoc.enabled = false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how to pass --add-exports flags to javadoc in gradle (options are definitely not being respected) and at this point I have given up and disabled javadoc for errorprone.

robert3005
robert3005 commented Feb 3, 2022
View reviewed changes
build.gradle
@@ -42,6 +42,7 @@ allprojects {
tasks.withType(JavaCompile) {
options.compilerArgs += ['-Werror', '-Xlint:deprecation']
options.errorprone {
enabled = false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be reenabled once we release this pr and pick it up - otherwise we fail because of the break

ferozco
ferozco reviewed Feb 3, 2022
View reviewed changes
baseline-error-prone/build.gradle Outdated
'--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED'
Copy link
Contributor

@ferozco ferozco Feb 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just set this in gradle.properties? I think it will fix the java doc problem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not, I spent couple hours trying to make it work and nothing made sense. Since with toolchains with fork the jvm for compilation add this to gradle.properties doesn't fix compilation

robert3005
robert3005 commented Feb 3, 2022
View reviewed changes
...-java/src/test/groovy/com/palantir/baseline/BaselineErrorProneRefasterIntegrationTest.groovy
@@ -152,6 +152,10 @@ class BaselineErrorProneRefasterIntegrationTest extends AbstractPluginTest {
when:
buildFile << standardBuildFile
buildFile << '''
tasks.withType(JavaCompile) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unnecessary once mockito releases with bumped errorprone

robert3005
robert3005 commented Feb 3, 2022
View reviewed changes
gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineExactDependenciesTest.groovy Outdated
@@ -68,6 +68,9 @@ class BaselineExactDependenciesTest extends AbstractPluginTest {
mavenCentral()
mavenLocal() // for baseline-error-prone
}
tasks.withType(JavaCompile) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unnecessary once mockito releases with bumped errorprone

@policy-bot policy-bot bot requested a review from ferozco February 3, 2022 19:02
@schlosna schlosna mentioned this pull request Feb 3, 2022
Merged
@avisbal
Copy link

avisbal commented Feb 4, 2022

I'm seeing the following error in some excavators, would you mind updating AvoidNewHashMapInt as well? I'm guessing there was a break for MethodMatchers$ConstructorClassMatcher.withParameters(java.lang.String[]).

Caused by: java.lang.NoSuchMethodError: 'com.google.errorprone.matchers.method.MethodMatchers$ParameterMatcher com.google.errorprone.matchers.method.MethodMatchers$ConstructorClassMatcher.withParameters(java.lang.String[])'
	at com.palantir.baseline.errorprone.AvoidNewHashMapInt.<clinit>(AvoidNewHashMapInt.java:48)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at java.base/java.util.ServiceLoader$ProviderImpl.newInstance(ServiceLoader.java:780)
	... 49 more

@robert3005
Copy link
Contributor Author

robert3005 commented Feb 4, 2022 •
edited
Loading

I already have - the problem is that you have the rule compiled against 2.10.0 and you try to run it against 2.11.0. What you're seeing is a linking problem - if you'd recompile the method against new errorprone it will be fine since signature is api but not abi compatible

@gsheasby gsheasby mentioned this pull request Feb 22, 2022
Merged
@fawind fawind mentioned this pull request Feb 23, 2022
Merged
@carterkozak
opt out of mockito-error-prone
6f27804
@carterkozak
Fix BugPatternNaming
750190d 
```
./gradlew classes testClasses -PerrorProneApply=BugPatternNaming
./gradlew format
```
@carterkozak
drop mockito-error-prone
4be89dd
@carterkozak
remove test cruft
1d28d88
@bulldozer-bot bulldozer-bot bot merged commit 20c2a69 into develop Mar 28, 2022
@bulldozer-bot bulldozer-bot bot deleted the rk/bump-errorprone branch March 28, 2022 18:34
@svc-autorelease
Copy link
Collaborator

Released 4.82.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tpetracca tpetracca tpetracca approved these changes

@carterkozak carterkozak carterkozak approved these changes

@ferozco ferozco Awaiting requested review from ferozco

Assignees
No one assigned
Labels
autorelease merge when ready no changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@robert3005 @avisbal @svc-autorelease @tpetracca @carterkozak @ferozco @svc-excavator-bot