Merge a0c93aa into ec6b218

gradle/dependencies.gradle

@@ -73,6 +73,7 @@ def test = [
     rxjava2                 : "io.reactivex.rxjava2:rxjava:2.1.2",
     commonsLang3            : "org.apache.commons:commons-lang3:3.8.1",
     commonsLang             : "commons-lang:commons-lang:2.6",
+    lombok                  : "org.projectlombok:lombok:1.18.12",
 ]
 
 ext.deps = [

nullaway/build.gradle

@@ -60,6 +60,7 @@ dependencies {
     testCompile deps.test.commonsLang
     testCompile deps.test.commonsLang3
     testCompile project(":test-library-models")
+    testCompile deps.test.lombok
 }
 
 // We include and shade the checker framework jars into the NullAway jar, as we may have custom

nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java

@@ -104,6 +104,8 @@ public abstract class AbstractConfig implements Config {
   protected String jarInferRegexStripModelJarName;
   protected String jarInferRegexStripCodeJarName;
 
+  protected boolean tryHandleLombok;
+
   protected String errorURL;
 
   protected static Pattern getPackagePattern(Set<String> packagePrefixes) {
@@ -288,4 +290,9 @@ public boolean treatGeneratedAsUnannotated() {
   public boolean acknowledgeAndroidRecent() {
     return acknowledgeAndroidRecent;
   }
+
+  @Override
+  public boolean tryHandleLombok() {
+    return tryHandleLombok;
+  }
 }

nullaway/src/main/java/com/uber/nullaway/Config.java

@@ -234,4 +234,11 @@ public interface Config {
    *     similarly for {@code @RecentlyNonNull}
    */
   boolean acknowledgeAndroidRecent();
+
+  /**
+   * Try to handle lombok-using code on a best-effort basis.
+   *
+   * @return true if the experimental lombok code handler should be turned on.
+   */
+  boolean tryHandleLombok();
 }

nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java

@@ -174,4 +174,9 @@ public boolean treatGeneratedAsUnannotated() {
   public boolean acknowledgeAndroidRecent() {
     throw new IllegalStateException(error_msg);
   }
+
+  @Override
+  public boolean tryHandleLombok() {
+    throw new IllegalStateException(error_msg);
+  }
 }

nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java

@@ -51,6 +51,7 @@
 import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
 import com.sun.tools.javac.util.DiagnosticSource;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
+import com.uber.nullaway.handlers.Handler;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -63,15 +64,17 @@
 public class ErrorBuilder {
 
   private final Config config;
+  private final Handler handler;
 
   /** Checker name that can be used to suppress the warnings. */
   private final String suppressionName;
 
   /** Additional identifiers for this check, to be checked for in @SuppressWarnings annotations. */
   private final Set<String> allNames;
 
-  ErrorBuilder(Config config, String suppressionName, Set<String> allNames) {
+  ErrorBuilder(Config config, Handler handler, String suppressionName, Set<String> allNames) {
     this.config = config;
+    this.handler = handler;
     this.suppressionName = suppressionName;
     this.allNames = allNames;
   }
@@ -120,6 +123,10 @@ public Description createErrorDescription(
       return Description.NO_MATCH;
     }
 
+    if (!handler.onPreErrorReporting(errorMessage, state)) {
+      return Description.NO_MATCH;
+    }
+
     if (config.suggestSuppressions() && suggestTree != null) {
       builder = addSuggestedSuppression(errorMessage, suggestTree, builder);
     }

nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java

@@ -65,6 +65,8 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
   /** --- JarInfer configs --- */
   static final String FL_JI_ENABLED = EP_FL_NAMESPACE + ":JarInferEnabled";
 
+  static final String FL_TRY_HANDLE_LOMBOK = EP_FL_NAMESPACE + ":TryHandleLombok";
+
   static final String FL_JI_USE_RETURN = EP_FL_NAMESPACE + ":JarInferUseReturnAnnotations";
 
   static final String FL_JI_REGEX_MODEL_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripModelJar";
@@ -107,6 +109,8 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
           "org.junit.jupiter.api.BeforeAll",
           "org.junit.jupiter.api.BeforeEach"); // + Anything with @Initializer as its "simple name"
 
+  static final ImmutableSet<String> DEFAULT_EXTERNAL_INIT_ANNOT = ImmutableSet.of("lombok.Builder");
+
   static final ImmutableSet<String> DEFAULT_EXCLUDED_FIELD_ANNOT =
       ImmutableSet.of(
           "javax.inject.Inject", // no explicit initialization when there is dependency injection
@@ -136,7 +140,8 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
     excludedClassAnnotations = getFlagStringSet(flags, FL_CLASS_ANNOTATIONS_TO_EXCLUDE);
     initializerAnnotations =
         getFlagStringSet(flags, FL_INITIALIZER_ANNOT, DEFAULT_INITIALIZER_ANNOT);
-    externalInitAnnotations = getFlagStringSet(flags, FL_EXTERNAL_INIT_ANNOT);
+    externalInitAnnotations =
+        getFlagStringSet(flags, FL_EXTERNAL_INIT_ANNOT, DEFAULT_EXTERNAL_INIT_ANNOT);
     isExhaustiveOverride = flags.getBoolean(FL_EXHAUSTIVE_OVERRIDE).orElse(false);
     isSuggestSuppressions = flags.getBoolean(FL_SUGGEST_SUPPRESSIONS).orElse(false);
     isAcknowledgeRestrictive = flags.getBoolean(FL_ACKNOWLEDGE_RESTRICTIVE).orElse(false);
@@ -168,6 +173,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig {
     // the analyzed classes.
     jarInferRegexStripModelJarName = flags.get(FL_JI_REGEX_MODEL_PATH).orElse(BASENAME_REGEX);
     jarInferRegexStripCodeJarName = flags.get(FL_JI_REGEX_CODE_PATH).orElse(BASENAME_REGEX);
+    tryHandleLombok = flags.getBoolean(FL_TRY_HANDLE_LOMBOK).orElse(false);
     errorURL = flags.get(FL_ERROR_URL).orElse(DEFAULT_URL);
     if (acknowledgeAndroidRecent && !isAcknowledgeRestrictive) {
       throw new IllegalStateException(

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -231,14 +231,14 @@ public NullAway() {
     config = new DummyOptionsConfig();
     handler = Handlers.buildEmpty();
     nonAnnotatedMethod = this::isMethodUnannotated;
-    errorBuilder = new ErrorBuilder(config, "", ImmutableSet.of());
+    errorBuilder = new ErrorBuilder(config, handler, "", ImmutableSet.of());
   }
 
   public NullAway(ErrorProneFlags flags) {
     config = new ErrorProneCLIFlagsConfig(flags);
     handler = Handlers.buildDefault(config);
     nonAnnotatedMethod = this::isMethodUnannotated;
-    errorBuilder = new ErrorBuilder(config, canonicalName(), allNames());
+    errorBuilder = new ErrorBuilder(config, handler, canonicalName(), allNames());
     // workaround for Checker Framework static state bug;
     // See https://github.com/typetools/checker-framework/issues/1482
     AnnotationUtils.clear();

nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java

@@ -180,4 +180,9 @@ public Optional<ErrorMessage> onExpressionDereference(
   public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) {
     return false;
   }
+
+  @Override
+  public boolean onPreErrorReporting(ErrorMessage errorMessage, VisitorState state) {
+    return true;
+  }
 }

nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java

@@ -230,4 +230,10 @@ public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState s
     }
     return shouldFilter;
   }
+
+  @Override
+  public boolean onPreErrorReporting(ErrorMessage errorMessage, VisitorState state) {
+    // Return true if and only if all handlers return true
+    return handlers.stream().allMatch(h -> h.onPreErrorReporting(errorMessage, state));
+  }
 }

nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java

@@ -297,6 +297,17 @@ Optional<ErrorMessage> onExpressionDereference(
    */
   boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state);
 
+  /**
+   * Called right before reporting a NullAway error.
+   *
+   * <p>This callback point allows handlers to suppress errors.
+   *
+   * @param errorMessage The error being reported
+   * @param state The current visitor state.
+   * @return true if the error should be reported (default), false to suppress the error.
+   */
+  boolean onPreErrorReporting(ErrorMessage errorMessage, VisitorState state);
+
   /**
    * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate
    * their knowledge of the method return nullability to the the rest of NullAway.

nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java

@@ -59,6 +59,9 @@ public static Handler buildDefault(Config config) {
     if (config.checkOptionalEmptiness()) {
       handlerListBuilder.add(new OptionalEmptinessHandler(config, methodNameUtil));
     }
+    if (config.tryHandleLombok()) {
+      handlerListBuilder.add(new LombokHandler());
+    }
     return new CompositeHandler(handlerListBuilder.build());
   }
 

nullaway/src/main/java/com/uber/nullaway/handlers/LombokHandler.java

@@ -0,0 +1,53 @@
+package com.uber.nullaway.handlers;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import com.sun.tools.javac.code.Symbol;
+import com.uber.nullaway.ErrorMessage;
+
+public class LombokHandler extends BaseNoOpHandler {
+
+  /**
+   * Find out if we are inside the FooBuilder generated by lombok as an inner class for `@Builder
+   * Foo`.
+   *
+   * @param state the Visitor State
+   * @return if we are inside FooBuilder for some `@Builder Foo`.
+   */
+  private static boolean isInsideLombokBuilder(VisitorState state) {
+    // First, get the immediate containing class (candidate FooBuilder)
+    TreePath path = state.getPath();
+    while (!path.getLeaf().getKind().equals(Tree.Kind.CLASS)) {
+      path = path.getParentPath();
+    }
+    ClassTree classTree = (ClassTree) path.getLeaf();
+    // Check the name first since that is fast
+    if (!classTree.getSimpleName().toString().endsWith("Builder")) {
+      return false;
+    }
+    // Now check that it's a nested class
+    Symbol.ClassSymbol classSymbol = ASTHelpers.getSymbol(classTree);
+    if (!classSymbol.getNestingKind().isNested()) {
+      return false;
+    }
+    // Traverse to finding the containing class (candidate `@Builder Foo`)
+    path = path.getParentPath();
+    while (!path.getLeaf().getKind().equals(Tree.Kind.CLASS)) {
+      path = path.getParentPath();
+    }
+    ClassTree containingClass = (ClassTree) path.getLeaf();
+    // Check for the @Builder annotation
+    return ASTHelpers.getSymbol(containingClass)
+        .getAnnotationMirrors()
+        .stream()
+        .anyMatch(a -> a.getAnnotationType().toString().equals("lombok.Builder"));
+  }
+
+  @Override
+  public boolean onPreErrorReporting(ErrorMessage errorMessage, VisitorState state) {
+    return !isInsideLombokBuilder(state);
+  }
+}

nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java

@@ -95,6 +95,11 @@ public void coreNullabilitySkipClass() {
         .doTest();
   }
 
+  @Test
+  public void lombokSupportTesting() {
+    compilationHelper.addSourceFile("lombok/LombokBuilderInit.java").doTest();
+  }
+
   @Test
   public void skipClass() {
     compilationHelper

nullaway/src/test/resources/com/uber/nullaway/testdata/lombok/LombokBuilderInit.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2020 Uber Technologies, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.uber.nullaway.testdata.lombok;
+
+import javax.annotation.Nullable;
+import lombok.Builder;
+
+@Builder
+public class LombokBuilderInit {
+  private String field;
+  @Builder.Default private String fieldWithDefault = "Default";
+  @Nullable private String nullableField;
+}

settings.gradle

@@ -18,6 +18,7 @@ include ':sample-library-model'
 include ':sample'
 include ':sample-app'
 include ':test-java-lib'
+include ':test-java-lib-lombok'
 include ':test-library-models'
 include ':compile-bench'
 include ':jar-infer:android-jarinfer-models-sdk28'

test-java-lib-lombok/build.gradle

@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2017. Uber Technologies
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import net.ltgt.gradle.errorprone.CheckSeverity
+
+plugins {
+  id "java"
+}
+
+sourceCompatibility = "1.8"
+targetCompatibility = "1.8"
+
+dependencies {
+    annotationProcessor project(path: ":nullaway", configuration: "shadow")
+    annotationProcessor deps.test.lombok
+
+    compileOnly deps.build.jsr305Annotations
+    compileOnly deps.build.javaxValidation
+    compileOnly deps.test.cfQual
+    compileOnly deps.test.lombok
+}
+
+tasks.withType(JavaCompile) {
+  if (!name.toLowerCase().contains("test")) {
+    options.errorprone {
+      check("NullAway", CheckSeverity.ERROR)
+      check("UnusedVariable", CheckSeverity.OFF) // We are not the only checker that fails on Lombok
+      option("NullAway:AnnotatedPackages", "com.uber")
+      option("NullAway:UnannotatedSubPackages", "com.uber.lib.unannotated")
+      option("NullAway:TryHandleLombok", "true")
+    }
+  }
+}

test-java-lib-lombok/src/main/java/com/uber/lombok/LombokBuilderInit.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2020 Uber Technologies, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.uber.lombok;
+
+import javax.annotation.Nullable;
+import lombok.Builder;
+
+@Builder
+public class LombokBuilderInit {
+  private String field;
+  @Builder.Default private String fieldWithDefault = "Default";
+  @Nullable private String nullableField;
+}