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

Upgrade error_prone_annotations to 2.3.4 #6574

Merged
merged 30 commits into from
Jan 3, 2020
Merged

Upgrade error_prone_annotations to 2.3.4 #6574

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0417f50
error_prone_annotation 2.3.4
suztomo Dec 27, 2019
c534c13
error_prone_annotations not from perfmark
suztomo Dec 27, 2019
f4e6c09
error prone core 2.3.3
suztomo Dec 27, 2019
484531b
errorprone core 2.3.4
suztomo Dec 27, 2019
e7f9c6a
Declaring errorprone to replace perfmark's dependency
suztomo Dec 27, 2019
d6b4454
Merge remote-tracking branch 'origin/master' into error_prone_annotat…
suztomo Dec 30, 2019
f7fa412
Merge remote-tracking branch 'origin/master' into error_prone_annotat…
suztomo Dec 30, 2019
ef4264e
IO_GRPC_GRPC_JAVA_ARTIFACTS
suztomo Dec 30, 2019
9e1c3a5
SuppressWarnings("UnnecessaryAnonymousClass")
suztomo Dec 30, 2019
51175e4
FormatMethod
suztomo Dec 30, 2019
f26fe6d
grpc-netty: @SuppressWarnings("UnnecessaryAnonymousClass")
suztomo Dec 30, 2019
61fb9f2
Removed unused argument
suztomo Dec 30, 2019
af940dc
grpc-interop-testing: SuppressWarnings("UnnecessaryAnonymousClass")
suztomo Dec 30, 2019
5e13810
SuppressWarnings("LiteProtoToString")
suztomo Dec 31, 2019
ab08958
SameNameButDifferent
suztomo Dec 31, 2019
6c98862
SuppressWarnings("UnnecessaryAnonymousClass") at build.gradle
suztomo Dec 31, 2019
5a1fa57
ManagedChannelImplTest for SameNameButDifferent
suztomo Dec 31, 2019
6c20c9d
Revert unnecessary change
suztomo Dec 31, 2019
211c2c2
ManagedChannelImplTest2 for SameNameButDifferent
suztomo Dec 31, 2019
7488e21
compileJmhJava with errorprone config
suztomo Dec 31, 2019
5bd6d01
OkHttpClientTransportTest for SameNameButDifferent
suztomo Dec 31, 2019
6fb9bac
SuppressWarning(PreferJavaTimeOverload)
suztomo Dec 31, 2019
fb4d7a6
SuppressWarnings("PreferJavaTimeOverload") at build.gradle
suztomo Dec 31, 2019
028bb6e
SuppressWarnings("GuardedBy")
suztomo Dec 31, 2019
a2f4a83
BinaryLogProviderTest for SameNameButDifferent
suztomo Dec 31, 2019
0a2a76b
prefer our version to perfmark's 2.3.3
suztomo Dec 31, 2019
749b67f
Fixed 'FormatMethod from an indirect dependency'
suztomo Jan 2, 2020
ef42e8b
SuppressWarnings("GuardedBy")
suztomo Jan 2, 2020
dbcdc4c
Merge remote-tracking branch 'origin/master' into error_prone_annotat…
suztomo Jan 3, 2020
10ec0cf
FakeNameResolverFactory2
suztomo Jan 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ repositories {
}

dependencies {
errorprone 'com.google.errorprone:error_prone_core:2.3.3'
errorprone 'com.google.errorprone:error_prone_core:2.3.4'
errorproneJavac 'com.google.errorprone:javac:9+181-r4173-1'

implementation 'io.grpc:grpc-core:1.27.0-SNAPSHOT' // CURRENT_GRPC_VERSION
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ subprojects {
libraries = [
android_annotations: "com.google.android:annotations:4.1.1.4",
animalsniffer_annotations: "org.codehaus.mojo:animal-sniffer-annotations:1.18",
errorprone: "com.google.errorprone:error_prone_annotations:2.3.3",
errorprone: "com.google.errorprone:error_prone_annotations:2.3.4",
gson: "com.google.code.gson:gson:2.8.6",
guava: "com.google.guava:guava:${guavaVersion}",
hpack: 'com.twitter:hpack:0.10.1',
Expand Down Expand Up @@ -263,7 +263,7 @@ subprojects {

if (rootProject.properties.get('errorProne', true)) {
dependencies {
errorprone 'com.google.errorprone:error_prone_core:2.3.3'
errorprone 'com.google.errorprone:error_prone_core:2.3.4'
errorproneJavac 'com.google.errorprone:javac:9+181-r4173-1'

annotationProcessor 'com.google.guava:guava-beta-checker:1.0'
Expand Down
5 changes: 4 additions & 1 deletion core/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ dependencies {
compile project(':grpc-api'),
libraries.gson,
libraries.android_annotations,
libraries.perfmark
libraries.errorprone // to avoid using perfmark's dependency
compile (libraries.perfmark) {
exclude group: 'com.google.errorprone', module: 'error_prone_annotations'
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gradle complained perfmark's errorprone annotation 2.3.3.

https://travis-ci.org/grpc/grpc-java/jobs/630091143?utm_medium=notification&utm_source=github_status

Copy link
Member

Choose a reason for hiding this comment

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

Yes. You did the appropriate thing here, although we normally put a comment saying we are using version X instead of version Y (so that in the future we can tell if the exclude is old and should be removed).

Our build enforces dependency convergence because I've not found a better approach to detecting requireUpperBoundDeps violations from within Gradle (because they aren't a problem in Gradle...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated comment as "prefer our version to perfmark's 2.3.3".

compile (libraries.opencensus_api) {
// prefer our own versions instead of opencensus-api's dependency
exclude group: 'com.google.code.findbugs', module: 'jsr305'
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class ProxyDetectorImpl implements ProxyDetector {
// $ sudo tail -f /var/log/squid/access.log

private static final Logger log = Logger.getLogger(ProxyDetectorImpl.class.getName());

@SuppressWarnings("UnnecessaryAnonymousClass") // grpc-java targets Java 7 (no method references)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Travis CI complained: https://travis-ci.org/grpc/grpc-java/jobs/631065748?utm_medium=notification&utm_source=github_status

> Task :grpc-core:compileJava
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:125: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final AuthenticationProvider DEFAULT_AUTHENTICATOR = new AuthenticationProvider() {
                                              ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  PasswordAuthentication defaultAuthenticator('?
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:143: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final Supplier<ProxySelector> DEFAULT_PROXY_SELECTOR =
                                               ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  ProxySelector defaultProxySelector() {'?
error: warnings found and -Werror specified
1 error
2 warnings

Copy link
Member

Choose a reason for hiding this comment

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

Suppressing this looks appropriate. There's a reasonable number of these. In the future if there are lots of a particular failure that doesn't make sense like this you can disable the check altogether. See what we've done for JdkObsolete in the main build.gradle for an example of that. In this case it's on the border of silencing them globally or in each file (so what you've done is fine).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the guidance. I choose to suppress UnnecessaryAnonymousClass at build.gradle because the decision of the suppression is project level (choosing Java 7 compatible source; no method references).

private static final AuthenticationProvider DEFAULT_AUTHENTICATOR = new AuthenticationProvider() {
@Override
public PasswordAuthentication requestPasswordAuthentication(
Expand All @@ -140,6 +142,8 @@ public PasswordAuthentication requestPasswordAuthentication(
host, addr, port, protocol, prompt, scheme, url, Authenticator.RequestorType.PROXY);
}
};

@SuppressWarnings("UnnecessaryAnonymousClass") // grpc-java targets Java 7 (no method references)
private static final Supplier<ProxySelector> DEFAULT_PROXY_SELECTOR =
new Supplier<ProxySelector>() {
@Override
Expand Down
2 changes: 1 addition & 1 deletion cronet/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ android {
}

dependencies {
errorprone 'com.google.errorprone:error_prone_core:2.3.3'
errorprone 'com.google.errorprone:error_prone_core:2.3.4'
errorproneJavac 'com.google.errorprone:javac:9+181-r4173-1'

implementation 'io.grpc:grpc-core:1.27.0-SNAPSHOT' // CURRENT_GRPC_VERSION
Expand Down
2 changes: 1 addition & 1 deletion examples/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
<dependency>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_annotations</artifactId>
<version>2.3.3</version> <!-- prefer to use 2.3.3 or later -->
<version>2.3.4</version> <!-- prefer to use 2.3.3 or later -->
</dependency>
<dependency>
<groupId>junit</groupId>
Expand Down
6 changes: 3 additions & 3 deletions repositories.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [
"com.google.auth:google-auth-library-oauth2-http:0.19.0",
"com.google.code.findbugs:jsr305:3.0.2",
"com.google.code.gson:gson:jar:2.8.6",
"com.google.errorprone:error_prone_annotations:2.3.3",
"com.google.errorprone:error_prone_annotations:2.3.4",
"com.google.guava:failureaccess:1.0.1",
"com.google.guava:guava:28.1-android",
"com.google.j2objc:j2objc-annotations:1.3",
Expand Down Expand Up @@ -226,9 +226,9 @@ def com_google_code_gson_gson():
def com_google_errorprone_error_prone_annotations():
jvm_maven_import_external(
name = "com_google_errorprone_error_prone_annotations",
artifact = "com.google.errorprone:error_prone_annotations:2.3.3",
artifact = "com.google.errorprone:error_prone_annotations:2.3.4",
server_urls = ["https://repo.maven.apache.org/maven2/"],
artifact_sha256 = "ec59f1b702d9afc09e8c3929f5c42777dec623a6ea2731ac694332c7d7680f5a",
artifact_sha256 = "baf7d6ea97ce606c53e11b6854ba5f2ce7ef5c24dddf0afa18d1260bd25b002c",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

suztomo@suxtomo24:~/grpc-java$ shasum -a 256 ~/Downloads/error_prone_annotations-2.3.4.jar 
baf7d6ea97ce606c53e11b6854ba5f2ce7ef5c24dddf0afa18d1260bd25b002c  /usr/local/google/home/suztomo/Downloads/error_prone_annotations-2.3.4.jar

licenses = ["notice"], # Apache 2.0
)

Expand Down