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.

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

Update minimum Error Prone version and Guava version #843

Merged
merged 7 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ jobs:
include:
- os: ubuntu-latest
java: 11
epVersion: 2.4.0
epVersion: 2.10.0
- os: ubuntu-latest
java: 17
epVersion: 2.4.0
epVersion: 2.10.0
- os: macos-latest
java: 11
epVersion: 2.22.0
Expand Down
34 changes: 10 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ NullAway is *fast*. It is built as a plugin to [Error Prone](http://errorprone.

### Overview

NullAway requires that you build your code with [Error Prone](http://errorprone.info), version 2.4.0 or higher. See the [Error Prone documentation](http://errorprone.info/docs/installation) for instructions on getting started with Error Prone and integration with your build system. The instructions below assume you are using Gradle; see [the docs](https://github.com/uber/NullAway/wiki/Configuration#other-build-systems) for discussion of other build systems.
NullAway requires that you build your code with [Error Prone](http://errorprone.info), version 2.10.0 or higher. See the [Error Prone documentation](http://errorprone.info/docs/installation) for instructions on getting started with Error Prone and integration with your build system. The instructions below assume you are using Gradle; see [the docs](https://github.com/uber/NullAway/wiki/Configuration#other-build-systems) for discussion of other build systems.

### Gradle

Expand All @@ -19,19 +19,18 @@ To integrate NullAway into your non-Android Java project, add the following to y
```gradle
plugins {
// we assume you are already using the Java plugin
id "net.ltgt.errorprone" version "0.6"
id "net.ltgt.errorprone" version "<plugin version>"
}

dependencies {
annotationProcessor "com.uber.nullaway:nullaway:0.10.13"
errorprone "com.uber.nullaway:nullaway:<NullAway version>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to remove version numbers from here or just adding the version of the plugin to the set of things to update when releasing? Either way, releasing instructions might need updating too!

Copy link
Collaborator Author

@msridhar msridhar Oct 8, 2023

Choose a reason for hiding this comment

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

I think we should be consistent and not have the version here. It hinders copy-paste-ability, but before we were just updating the NullAway version and not versions of other related plugins, and stuff got out of date. So I propose leaving the version out here and updating the releasing instructions. WDYT?

Copy link
Collaborator

@lazaroclapp lazaroclapp Oct 8, 2023

Choose a reason for hiding this comment

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

Fine by me. Another option is to have some script to update the readme that fetches these from our own configuration, or add steps to the release instructions, but that is a bit of work and might not be super reliable anyways (will break if the README ever changes shape significantly). Let's go with these placeholders for now, then.


// Optional, some source of nullability annotations.
// Not required on Android if you use the support
// library nullability annotations.
compileOnly "com.google.code.findbugs:jsr305:3.0.2"

errorprone "com.google.errorprone:error_prone_core:2.4.0"
errorproneJavac "com.google.errorprone:javac:9+181-r4173-1"
errorprone "com.google.errorprone:error_prone_core:<Error Prone version>"
}

import net.ltgt.gradle.errorprone.CheckSeverity
Expand All @@ -47,23 +46,11 @@ tasks.withType(JavaCompile) {
}
```

Let's walk through this script step by step. The `plugins` section pulls in the [Gradle Error Prone plugin](https://github.com/tbroyer/gradle-errorprone-plugin) for Error Prone integration. If you are using the older `apply plugin` syntax instead of a `plugins` block, the following is equivalent:
```gradle
buildscript {
repositories {
gradlePluginPortal()
}
dependencies {
classpath "net.ltgt.gradle:gradle-errorprone-plugin:0.6"
}
}

apply plugin: 'net.ltgt.errorprone'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I presume this is no longer supported?

Copy link
Collaborator Author

@msridhar msridhar Oct 8, 2023

Choose a reason for hiding this comment

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

So I was following the style of other READMEs I've seen recently, including the Gradle Error Prone plugin. None of them even document this possibility anymore; it was outdated years ago. I think it's not worth the space here.

```
Let's walk through this script step by step. The `plugins` section pulls in the [Gradle Error Prone plugin](https://github.com/tbroyer/gradle-errorprone-plugin) for Error Prone integration.

In `dependencies`, the `annotationProcessor` line loads NullAway, and the `compileOnly` line loads a [JSR 305](https://jcp.org/en/jsr/detail?id=305) library which provides a suitable `@Nullable` annotation (`javax.annotation.Nullable`). NullAway allows for any `@Nullable` annotation to be used, so, e.g., `@Nullable` from the Android Support Library or JetBrains annotations is also fine. The `errorprone` line ensures that a compatible version of Error Prone is used, and the `errorproneJavac` line is needed for JDK 8 compatibility.
In `dependencies`, the first `errorprone` line loads NullAway, and the `compileOnly` line loads a [JSR 305](https://jcp.org/en/jsr/detail?id=305) library which provides a suitable `@Nullable` annotation (`javax.annotation.Nullable`). NullAway allows for any `@Nullable` annotation to be used, so, e.g., `@Nullable` from the Android Support Library or JetBrains annotations is also fine. The second `errorprone` line sets the version of Error Prone is used.

Finally, in the `tasks.withType(JavaCompile)` section, we pass some configuration options to NullAway. First `check("NullAway", CheckSeverity.ERROR)` sets NullAway issues to the error level (it's equivalent to the `-Xep:NullAway:ERROR` standard Error Prone argument); by default NullAway emits warnings. Then, `option("NullAway:AnnotatedPackages", "com.uber")` (equivalent to the `-XepOpt:NullAway:AnnotatedPackages=com.uber` standard Error Prone argument), tells NullAway that source code in packages under the `com.uber` namespace should be checked for null dereferences and proper usage of `@Nullable` annotations, and that class files in these packages should be assumed to have correct usage of `@Nullable` (see [the docs](https://github.com/uber/NullAway/wiki/Configuration) for more detail). NullAway requires at least the `AnnotatedPackages` configuration argument to run, in order to distinguish between annotated and unannotated code. See [the configuration docs](https://github.com/uber/NullAway/wiki/Configuration) for other useful configuration options.
Finally, in the `tasks.withType(JavaCompile)` section, we pass some configuration options to NullAway. First `check("NullAway", CheckSeverity.ERROR)` sets NullAway issues to the error level (it's equivalent to the `-Xep:NullAway:ERROR` standard Error Prone argument); by default NullAway emits warnings. Then, `option("NullAway:AnnotatedPackages", "com.uber")` (equivalent to the `-XepOpt:NullAway:AnnotatedPackages=com.uber` standard Error Prone argument) tells NullAway that source code in packages under the `com.uber` namespace should be checked for null dereferences and proper usage of `@Nullable` annotations, and that class files in these packages should be assumed to have correct usage of `@Nullable` (see [the docs](https://github.com/uber/NullAway/wiki/Configuration) for more detail). NullAway requires at least the `AnnotatedPackages` configuration argument to run, in order to distinguish between annotated and unannotated code. See [the configuration docs](https://github.com/uber/NullAway/wiki/Configuration) for other useful configuration options. For even simpler configuration of NullAway options, use the [Gradle NullAway plugin](https://github.com/tbroyer/gradle-nullaway-plugin).

We recommend addressing all the issues that Error Prone reports, particularly those reported as errors (rather than warnings). But, if you'd like to try out NullAway without running other Error Prone checks, you can use `options.errorprone.disableAllChecks` (equivalent to passing `"-XepDisableAllChecks"` to the compiler, before the NullAway-specific arguments).

Expand All @@ -75,12 +62,11 @@ The configuration for an Android project is very similar to the Java case, with

```gradle
dependencies {
annotationProcessor "com.uber.nullaway:nullaway:0.10.13"
errorprone "com.google.errorprone:error_prone_core:2.4.0"
errorproneJavac "com.google.errorprone:javac:9+181-r4173-1"
errorprone "com.uber.nullaway:nullaway:<NullAway version>"
errorprone "com.google.errorprone:error_prone_core:<Error Prone version>"
}
```
A complete Android `build.gradle` example is [here](https://gist.github.com/msridhar/6cacd429567f1d1ad9a278e06809601c). Also see our [sample app](https://github.com/uber/NullAway/blob/master/sample-app/). (The sample app's [`build.gradle`](https://github.com/uber/NullAway/blob/master/sample-app/) is not suitable for direct copy-pasting, as some configuration is inherited from the top-level `build.gradle`.)
For a more complete example see our [sample app](https://github.com/uber/NullAway/blob/master/sample-app/). (The sample app's [`build.gradle`](https://github.com/uber/NullAway/blob/master/sample-app/) is not suitable for direct copy-pasting, as some configuration is inherited from the top-level `build.gradle`.)

#### Annotation Processors / Generated Code

Expand Down
4 changes: 2 additions & 2 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import org.gradle.util.VersionNumber
*/

// The oldest version of Error Prone that we support running on
def oldestErrorProneVersion = "2.4.0"
def oldestErrorProneVersion = "2.10.0"
// Latest released Error Prone version that we've tested with
def latestErrorProneVersion = "2.22.0"
// Default to using latest tested Error Prone version
Expand Down Expand Up @@ -72,7 +72,7 @@ def build = [
errorProneJavac : "com.google.errorprone:javac:9+181-r4173-1",
errorProneTestHelpers : "com.google.errorprone:error_prone_test_helpers:${versions.errorProneApi}",
checkerDataflow : "org.checkerframework:dataflow-nullaway:${versions.checkerFramework}",
guava : "com.google.guava:guava:24.1.1-jre",
guava : "com.google.guava:guava:30.1-jre",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will need some internal testing before release. I do believe we have a later version of Guava in the annotation processor path for at least Java monorepo, but we need to test this for both internal monorepos.

javaxValidation : "javax.validation:validation-api:2.0.1.Final",
jspecify : "org.jspecify:jspecify:0.3.0",
jsr305Annotations : "com.google.code.findbugs:jsr305:3.0.2",
Expand Down
2 changes: 1 addition & 1 deletion guava-recent-unit-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ dependencies {
def jdk8Test = tasks.register("testJdk8", Test) {
onlyIf {
// Only if we are using a version of Error Prone compatible with JDK 8
deps.versions.errorProneApi == "2.4.0"
deps.versions.errorProneApi == "2.10.0"
}

javaLauncher = javaToolchains.launcherFor {
Expand Down
13 changes: 1 addition & 12 deletions nullaway/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,6 @@ javadoc {
failOnError = false
}


test {
if (deps.versions.errorProneApi == "2.4.0" && JavaVersion.current() >= JavaVersion.VERSION_17) {
// This test does not pass on JDK 17 with Error Prone 2.4.0 due to a Mockito incompatibility. Skip it (the
// test passes with more recent Error Prone versions on JDK 17)
filter {
excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.suggestNullableArgumentOnBytecodeNoFileInfo"
}
}
}

apply plugin: 'com.vanniktech.maven.publish'

// These --add-exports arguments are required when targeting JDK 11+ since Error Prone and NullAway access a bunch of
Expand All @@ -107,7 +96,7 @@ apply plugin: 'com.vanniktech.maven.publish'
def jdk8Test = tasks.register("testJdk8", Test) {
onlyIf {
// Only if we are using a version of Error Prone compatible with JDK 8
deps.versions.errorProneApi == "2.4.0"
deps.versions.errorProneApi == "2.10.0"
}

javaLauncher = javaToolchains.launcherFor {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
package com.uber.nullaway;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.ErrorProneFlags;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand All @@ -36,54 +34,21 @@ public class NullAwayAutoSuggestNoCastTest {

@Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();

private ErrorProneFlags flagsWithAutoFixSuppressionComment;

private ErrorProneFlags flagsNoAutoFixSuppressionComment;

@Before
public void setup() {
// With AutoFixSuppressionComment
ErrorProneFlags.Builder b = ErrorProneFlags.builder();
b.putFlag("NullAway:AnnotatedPackages", "com.uber,com.ubercab,io.reactivex");
b.putFlag("NullAway:SuggestSuppressions", "true");
b.putFlag("NullAway:AutoFixSuppressionComment", "PR #000000");
flagsWithAutoFixSuppressionComment = b.build();
// Without AutoFixSuppressionComment
b = ErrorProneFlags.builder();
b.putFlag("NullAway:AnnotatedPackages", "com.uber,com.ubercab,io.reactivex");
b.putFlag("NullAway:SuggestSuppressions", "true");
flagsNoAutoFixSuppressionComment = b.build();
}

// In EP 2.6.0 the newInstance() method we use below is deprecated. We cannot currently address
// the warning since the replacement method was only added in EP 2.5.1, and we still want to
// support EP 2.4.0. So, we suppress the warning for now
@SuppressWarnings("deprecation")
private BugCheckerRefactoringTestHelper makeTestHelperWithSuppressionComment() {
return BugCheckerRefactoringTestHelper.newInstance(
new NullAway(flagsWithAutoFixSuppressionComment), getClass())
return BugCheckerRefactoringTestHelper.newInstance(NullAway.class, getClass())
.setArgs(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
// the remaining args are not needed right now, but they will be necessary when we
// switch to the more modern newInstance() API
"-XepOpt:NullAway:AnnotatedPackages=com.uber,com.ubercab,io.reactivex",
"-XepOpt:NullAway:SuggestSuppressions=true",
"-XepOpt:NullAway:AutoFixSuppressionComment=PR #000000");
}

// In EP 2.6.0 the newInstance() method we use below is deprecated. We cannot currently address
// the warning since the replacement method was only added in EP 2.5.1, and we still want to
// support EP 2.4.0. So, we suppress the warning for now
@SuppressWarnings("deprecation")
private BugCheckerRefactoringTestHelper makeTestHelper() {
return BugCheckerRefactoringTestHelper.newInstance(
new NullAway(flagsNoAutoFixSuppressionComment), getClass())
return BugCheckerRefactoringTestHelper.newInstance(NullAway.class, getClass())
.setArgs(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
// the remaining args are not needed right now, but they will be necessary when we
// switch to the more modern newInstance() API
"-XepOpt:NullAway:AnnotatedPackages=com.uber,com.ubercab,io.reactivex",
"-XepOpt:NullAway:SuggestSuppressions=true");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@
import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.ErrorProneFlags;
import com.sun.source.tree.Tree;
import com.uber.nullaway.testlibrarymodels.TestLibraryModels;
import java.io.IOException;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand All @@ -41,30 +39,13 @@ public class NullAwayAutoSuggestTest {

@Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();

private ErrorProneFlags flags;

@Before
public void setup() {
ErrorProneFlags.Builder b = ErrorProneFlags.builder();
b.putFlag("NullAway:AnnotatedPackages", "com.uber,com.ubercab,io.reactivex");
b.putFlag("NullAway:CastToNonNullMethod", "com.uber.nullaway.testdata.Util.castToNonNull");
b.putFlag("NullAway:SuggestSuppressions", "true");
flags = b.build();
}

// In EP 2.6.0 the newInstance() method we use below is deprecated. We cannot currently address
// the warning since the replacement method was only added in EP 2.5.1, and we still want to
// support EP 2.4.0. So, we suppress the warning for now
@SuppressWarnings("deprecation")
private BugCheckerRefactoringTestHelper makeTestHelper() {
return BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass())
return BugCheckerRefactoringTestHelper.newInstance(NullAway.class, getClass())
.setArgs(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-processorpath",
TestLibraryModels.class.getProtectionDomain().getCodeSource().getLocation().getPath(),
// the remaining args are not needed right now, but they will be necessary when we
// switch to the more modern newInstance() API
"-XepOpt:NullAway:AnnotatedPackages=com.uber,com.ubercab,io.reactivex",
"-XepOpt:NullAway:CastToNonNullMethod=com.uber.nullaway.testdata.Util.castToNonNull",
"-XepOpt:NullAway:SuggestSuppressions=true");
Expand Down