Upgrade to Error Prone 2.3.3 (#295)

Resolves #286. Also, we separate error prone API version from error prone build version Most of the noise in this diff falls under a few categories: - UnusedVariable check - Strong enforcement of immutables (no mixing, using them in autovalue) - Use Types.isSameType() for type comparisons (instead of .equals()) - Use state.getSourceForNode() for representing trees rather than Tree#toString() - Use fluent APIs for assertions in tests
uber · Apr 8, 2019 · 26b3f55 · 26b3f55
1 parent e529f53
commit 26b3f55
Show file tree

Hide file tree

Showing 14 changed files with 114 additions and 116 deletions.
diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -16,7 +16,8 @@
 
 def versions = [
     checkerFramework       : "2.5.5",
-    errorProne             : "2.3.2",
+    errorProne             : "2.3.3", // The version of error prone running on this project
+    errorProneApi          : "2.3.1", // The version of error prone built against and shipped
     support                : "27.1.1",
     wala                   : "1.5.0-uber.1",
     commonscli             : "1.4",
@@ -30,7 +31,7 @@ def apt = [
 ]
 
 def build = [
-    errorProneCheckApi      : "com.google.errorprone:error_prone_check_api:${versions.errorProne}",
+    errorProneCheckApi      : "com.google.errorprone:error_prone_check_api:${versions.errorProneApi}",
     errorProneCore          : "com.google.errorprone:error_prone_core:${versions.errorProne}",
     errorProneJavac         : "com.google.errorprone:javac:9+181-r4173-1",
     errorProneTestHelpers   : "com.google.errorprone:error_prone_test_helpers:${versions.errorProne}",

diff --git a/...infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.java b/...infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParams.java
@@ -58,9 +58,6 @@ private static void LOG(boolean cond, String tag, String msg) {
   private final ControlFlowGraph<SSAInstruction, ISSABasicBlock> cfg;
   private PrunedCFG<SSAInstruction, ISSABasicBlock> prunedCFG;
 
-  // used to resolve references to fields in putstatic instructions
-  private final IClassHierarchy cha;
-
   /** List of null test APIs and the parameter position. */
   private static final ImmutableMap<String, Integer> NULL_TEST_APIS =
       new ImmutableMap.Builder<String, Integer>()
@@ -95,7 +92,6 @@ public DefinitelyDerefedParams(
     this.method = method;
     this.ir = ir;
     this.cfg = cfg;
-    this.cha = cha;
     prunedCFG = null;
   }
 

diff --git a/...jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/...jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java
@@ -145,7 +145,6 @@ public static Result run(String inPaths, String pkgName, String outPath, boolean
     DEBUG = dbg;
     VERBOSE = vbs;
     long start = System.currentTimeMillis();
-    String firstInPath = inPaths.split(",")[0];
     Set<String> setInPaths = new HashSet<>(Arrays.asList(inPaths.split(",")));
     for (String inPath : setInPaths) {
       InputStream jarIS = null;

diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
@@ -88,12 +88,10 @@ private void testTemplate(
   /**
    * Run a unit test with a specified jar file.
    *
-   * @param testName An useful name for the unit test.
    * @param pkg Qualified package name.
    * @param jarPath Path to the target jar file.
    */
   private void testJARTemplate(
-      String testName,
       String pkg, // in dot syntax
       String jarPath // in dot syntax
       ) throws Exception {
@@ -224,7 +222,6 @@ public void toyNonStatic() throws Exception {
   @Test
   public void toyJAR() throws Exception {
     testJARTemplate(
-        "toyJAR",
         "com.uber.nullaway.jarinfer.toys.unannotated",
         "../test-java-lib-jarinfer/build/libs/test-java-lib-jarinfer.jar");
   }

diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
@@ -124,7 +124,7 @@ public NullnessHint onDataflowVisitMethodInvocation(
     Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
     Preconditions.checkNotNull(callee);
     setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, context);
-    setConditionalArgumentNullness(thenUpdates, elseUpdates, node.getArguments(), callee, context);
+    setConditionalArgumentNullness(elseUpdates, node.getArguments(), callee, context);
     if (getOptLibraryModels(context).hasNonNullReturn(callee, types)) {
       return NullnessHint.FORCE_NONNULL;
     } else if (getOptLibraryModels(context).hasNullableReturn(callee, types)) {
@@ -135,7 +135,6 @@ public NullnessHint onDataflowVisitMethodInvocation(
   }
 
   private void setConditionalArgumentNullness(
-      AccessPathNullnessPropagation.Updates thenUpdates,
       AccessPathNullnessPropagation.Updates elseUpdates,
       List<Node> arguments,
       Symbol.MethodSymbol callee,

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/RxNullabilityPropagator.java b/nullaway/src/main/java/com/uber/nullaway/handlers/RxNullabilityPropagator.java
@@ -234,7 +234,6 @@ public void onMatchMethodInvocation(
     Type receiverType = ASTHelpers.getReceiverType(tree);
     for (StreamTypeRecord streamType : RX_MODELS) {
       if (streamType.matchesType(receiverType, state)) {
-        String methodName = methodSymbol.toString();
         // Build observable call chain
         buildObservableCallChain(tree);
 
@@ -262,7 +261,7 @@ public void onMatchMethodInvocation(
             // Ensure that this `new B() ...` has a custom class body, otherwise, we skip for now.
             if (annonClassBody != null) {
               MaplikeMethodRecord methodRecord = streamType.getMaplikeMethodRecord(methodSymbol);
-              handleMapAnonClass(methodRecord, tree, annonClassBody, state);
+              handleMapAnonClass(methodRecord, tree, annonClassBody);
             }
           } else if (argTree instanceof LambdaExpressionTree) {
             observableCallToInnerMethodOrLambda.put(tree, (LambdaExpressionTree) argTree);
@@ -298,7 +297,6 @@ private void handleChainFromFilter(
       outerCallInChain = observableOuterCallInChain.get(outerCallInChain);
       // Check for a map method (which might be a pass-through method or the first method after a
       // pass-through chain)
-      MethodInvocationTree mapCallsite = observableOuterCallInChain.get(observableDotFilter);
       if (observableCallToInnerMethodOrLambda.containsKey(outerCallInChain)) {
         // Update mapToFilterMap
         Symbol.MethodSymbol mapMethod = ASTHelpers.getSymbol(outerCallInChain);
@@ -341,8 +339,7 @@ private void handleFilterLambda(
   private void handleMapAnonClass(
       MaplikeMethodRecord methodRecord,
       MethodInvocationTree observableDotMap,
-      ClassTree annonClassBody,
-      VisitorState state) {
+      ClassTree annonClassBody) {
     for (Tree t : annonClassBody.getMembers()) {
       if (t instanceof MethodTree
           && ((MethodTree) t).getName().toString().equals(methodRecord.getInnerMethodName())) {

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAndroidTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAndroidTest.java
@@ -17,6 +17,7 @@ public class NullAwayAndroidTest {
 
   private CompilationTestHelper compilationHelper;
 
+  @SuppressWarnings("CheckReturnValue")
   @Before
   public void setup() {
     compilationHelper = CompilationTestHelper.newInstance(NullAway.class, getClass());
@@ -174,6 +175,7 @@ public void androidxActivitySuccess() {
   }
 
   /** Initialises the default android classes that are commonly used. */
+  @SuppressWarnings("CheckReturnValue")
   private void initialiseAndroidCoreClasses() {
     compilationHelper
         .addSourceFile("androidstubs/core/Context.java")

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java
@@ -60,8 +60,8 @@ public void suggestSuppressionWithComment() {
     BugCheckerRefactoringTestHelper bcr =
         BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "class Test {",
@@ -76,8 +76,8 @@ public void suggestSuppressionWithComment() {
             "  @SuppressWarnings(\"NullAway\") /* PR #000000 */ Object test1() {",
             "    return null;",
             "  }",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); // Yes we can!
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); // Yes we can!
   }
 
   @Test
@@ -86,8 +86,8 @@ public void suggestSuppressionWithoutComment() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "class Test {",
@@ -102,8 +102,8 @@ public void suggestSuppressionWithoutComment() {
             "  @SuppressWarnings(\"NullAway\") Object test1() {",
             "    return null;",
             "  }",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -112,8 +112,8 @@ public void suggestSuppressionFieldLambdaDeref() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -135,8 +135,8 @@ public void suggestSuppressionFieldLambdaDeref() {
             "    () -> {",
             "      foo.toString();",
             "    };",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -145,8 +145,8 @@ public void suggestSuppressionFieldLambdaUnbox() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -170,8 +170,8 @@ public void suggestSuppressionFieldLambdaUnbox() {
             "    () -> {",
             "      id(foo);",
             "    };",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -180,8 +180,8 @@ public void suggestSuppressionFieldLambdaAssignment() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -205,8 +205,8 @@ public void suggestSuppressionFieldLambdaAssignment() {
             "      @SuppressWarnings(\"NullAway\")",
             "      int x = foo + 1;",
             "    };",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -215,8 +215,8 @@ public void suggestLambdaAssignInMethod() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -245,8 +245,8 @@ public void suggestLambdaAssignInMethod() {
             "    @SuppressWarnings(\"NullAway\")",
             "    java.util.function.Function<Object,Integer> g = (x) -> { return foo + 1; };",
             "  }",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -255,8 +255,8 @@ public void suppressMethodRefOverrideParam() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -284,8 +284,8 @@ public void suppressMethodRefOverrideParam() {
             "  static void bar() {",
             "    callFoo(Test::biz);",
             "  }",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 
   @Test
@@ -294,8 +294,8 @@ public void suppressMethodRefOverrideReturn() {
         BugCheckerRefactoringTestHelper.newInstance(
             new NullAway(flagsNoAutoFixSuppressionComment), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -325,7 +325,7 @@ public void suppressMethodRefOverrideReturn() {
             "  static void bar() {",
             "    callFoo(Test::biz);",
             "  }",
-            "}");
-    bcr.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+            "}")
+        .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
   }
 }
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java
@@ -53,8 +53,8 @@ public void correctCastToNonNull() throws IOException {
     BugCheckerRefactoringTestHelper bcr =
         BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -64,17 +64,17 @@ public void correctCastToNonNull() throws IOException {
             "    return castToNonNull(o);",
             "  }",
             "}")
-        .expectUnchanged();
-    bcr.doTest();
+        .expectUnchanged()
+        .doTest();
   }
 
   @Test
   public void suggestCastToNonNull() throws IOException {
     BugCheckerRefactoringTestHelper bcr =
         BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -92,17 +92,17 @@ public void suggestCastToNonNull() throws IOException {
             "  Object test1(@Nullable Object o) {",
             "    return castToNonNull(o);",
             "  }",
-            "}");
-    bcr.doTest();
+            "}")
+        .doTest();
   }
 
   @Test
   public void removeUnnecessaryCastToNonNull() throws IOException {
     BugCheckerRefactoringTestHelper bcr =
         BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -121,17 +121,17 @@ public void removeUnnecessaryCastToNonNull() throws IOException {
             "  Object test1(Object o) {",
             "    return o;",
             "  }",
-            "}");
-    bcr.doTest();
+            "}")
+        .doTest();
   }
 
   @Test
   public void suggestSuppressionOnMethodRef() throws IOException {
     BugCheckerRefactoringTestHelper bcr =
         BugCheckerRefactoringTestHelper.newInstance(new NullAway(flags), getClass());
 
-    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath());
-    bcr.addInputLines(
+    bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath())
+        .addInputLines(
             "Test.java",
             "package com.uber;",
             "import javax.annotation.Nullable;",
@@ -166,7 +166,7 @@ public void suggestSuppressionOnMethodRef() throws IOException {
             "  @SuppressWarnings(\"NullAway\") void test() throws Exception {",
             "    takesNonNull(execute(Test::doReturnNullable));",
             "  }",
-            "}");
-    bcr.doTest();
+            "}")
+        .doTest();
   }
 }