Moe Sync

annotation/pom.xml

@@ -50,7 +50,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>0.44</version>
+      <version>0.45</version>
       <scope>test</scope>
     </dependency>
   </dependencies>

check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java

@@ -52,6 +52,7 @@ public class ErrorProneOptions {
   private static final String PATCH_OUTPUT_LOCATION = "-XepPatchLocation:";
   private static final String PATCH_IMPORT_ORDER_PREFIX = "-XepPatchImportOrder:";
   private static final String EXCLUDED_PATHS_PREFIX = "-XepExcludedPaths:";
+  private static final String IGNORE_LARGE_CODE_GENERATORS = "-XepIgnoreLargeCodeGenerators:";
 
   private static final String ERRORS_AS_WARNINGS_FLAG = "-XepAllErrorsAsWarnings";
   private static final String ENABLE_ALL_CHECKS = "-XepAllDisabledChecksAsWarnings";
@@ -165,6 +166,7 @@ final PatchingOptions build() {
   private final PatchingOptions patchingOptions;
   private final Pattern excludedPattern;
   private final boolean ignoreSuppressionAnnotations;
+  private final boolean ignoreLargeCodeGenerators;
 
   private ErrorProneOptions(
       ImmutableMap<String, Severity> severityMap,
@@ -178,7 +180,8 @@ private ErrorProneOptions(
       ErrorProneFlags flags,
       PatchingOptions patchingOptions,
       Pattern excludedPattern,
-      boolean ignoreSuppressionAnnotations) {
+      boolean ignoreSuppressionAnnotations,
+      boolean ignoreLargeCodeGenerators) {
     this.severityMap = severityMap;
     this.remainingArgs = remainingArgs;
     this.ignoreUnknownChecks = ignoreUnknownChecks;
@@ -191,6 +194,7 @@ private ErrorProneOptions(
     this.patchingOptions = patchingOptions;
     this.excludedPattern = excludedPattern;
     this.ignoreSuppressionAnnotations = ignoreSuppressionAnnotations;
+    this.ignoreLargeCodeGenerators = ignoreLargeCodeGenerators;
   }
 
   public String[] getRemainingArgs() {
@@ -221,6 +225,10 @@ public boolean isIgnoreSuppressionAnnotations() {
     return ignoreSuppressionAnnotations;
   }
 
+  public boolean ignoreLargeCodeGenerators() {
+    return ignoreLargeCodeGenerators;
+  }
+
   public ErrorProneFlags getFlags() {
     return flags;
   }
@@ -241,6 +249,7 @@ private static class Builder {
     private boolean disableAllChecks = false;
     private boolean isTestOnlyTarget = false;
     private boolean ignoreSuppressionAnnotations = false;
+    private boolean ignoreLargeCodeGenerators = true;
     private Map<String, Severity> severityMap = new HashMap<>();
     private final ErrorProneFlags.Builder flagsBuilder = ErrorProneFlags.builder();
     private final PatchingOptions.Builder patchingOptionsBuilder = PatchingOptions.builder();
@@ -299,6 +308,10 @@ public void setEnableAllChecksAsWarnings(boolean enableAllChecksAsWarnings) {
       this.enableAllChecksAsWarnings = enableAllChecksAsWarnings;
     }
 
+    public void setIgnoreLargeCodeGenerators(boolean ignoreLargeCodeGenerators) {
+      this.ignoreLargeCodeGenerators = ignoreLargeCodeGenerators;
+    }
+
     public void setDisableAllChecks(boolean disableAllChecks) {
       // Discard previously set severities so that the DisableAllChecks flag is position sensitive.
       severityMap.clear();
@@ -326,7 +339,8 @@ public ErrorProneOptions build(ImmutableList<String> remainingArgs) {
           flagsBuilder.build(),
           patchingOptionsBuilder.build(),
           excludedPattern,
-          ignoreSuppressionAnnotations);
+          ignoreSuppressionAnnotations,
+          ignoreLargeCodeGenerators);
     }
 
     public void setExcludedPattern(Pattern excludedPattern) {
@@ -426,6 +440,7 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
           } else if (arg.startsWith(EXCLUDED_PATHS_PREFIX)) {
             String pathRegex = arg.substring(EXCLUDED_PATHS_PREFIX.length());
             builder.setExcludedPattern(Pattern.compile(pathRegex));
+
           } else {
             remainingArgs.add(arg);
           }

check_api/src/main/java/com/google/errorprone/VisitorState.java

@@ -16,8 +16,10 @@
 
 package com.google.errorprone;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
-import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultiset;
 import com.google.errorprone.BugPattern.SeverityLevel;
@@ -62,10 +64,8 @@ public class VisitorState {
   private final StatisticsCollector statisticsCollector;
   private final Map<String, SeverityLevel> severityMap;
   private final ErrorProneOptions errorProneOptions;
-  private final Map<String, Optional<Type>> typeCache;
-  private final ErrorProneTimings timings;
+  private final SharedState sharedState;
   public final Context context;
-
   private final TreePath path;
   private final SuppressionInfo.SuppressedState suppressedState;
 
@@ -87,8 +87,8 @@ public static VisitorState createForUtilityPurposes(Context context) {
         // Can't use this VisitorState to report results, so no-op collector.
         StatisticsCollector.createNoOpCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   /**
@@ -104,8 +104,8 @@ public static VisitorState createForCustomFindingCollection(
         ErrorProneOptions.empty(),
         StatisticsCollector.createCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   /**
@@ -123,8 +123,8 @@ public static VisitorState createConfiguredForCompilation(
         errorProneOptions,
         StatisticsCollector.createCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   /**
@@ -143,8 +143,8 @@ public VisitorState(Context context) {
         // Can't use this VisitorState to report results, so no-op collector.
         StatisticsCollector.createNoOpCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   /**
@@ -162,8 +162,8 @@ public VisitorState(Context context, DescriptionListener listener) {
         ErrorProneOptions.empty(),
         StatisticsCollector.createCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   /**
@@ -184,8 +184,8 @@ public VisitorState(
         errorProneOptions,
         StatisticsCollector.createCollector(),
         null,
-        null,
-        SuppressedState.UNSUPPRESSED);
+        SuppressedState.UNSUPPRESSED,
+        null);
   }
 
   private VisitorState(
@@ -194,9 +194,9 @@ private VisitorState(
       Map<String, SeverityLevel> severityMap,
       ErrorProneOptions errorProneOptions,
       StatisticsCollector statisticsCollector,
-      Map<String, Optional<Type>> typeCache,
       TreePath path,
-      SuppressedState suppressedState) {
+      SuppressedState suppressedState,
+      SharedState sharedState) {
     this.context = context;
     this.descriptionListener = descriptionListener;
     this.severityMap = severityMap;
@@ -205,14 +205,7 @@ private VisitorState(
 
     this.suppressedState = suppressedState;
     this.path = path;
-    this.timings = ErrorProneTimings.instance(context);
-    this.typeCache =
-        typeCache != null
-            ? typeCache
-            // TODO(ronshapiro): should we presize this with a reasonable size? We can check for the
-            // smallest build and see how many types are loaded and use that. Or perhaps a heuristic
-            // based on number of files?
-            : new HashMap<>();
+    this.sharedState = sharedState != null ? sharedState : new SharedState(context);
   }
 
   public VisitorState withPath(TreePath path) {
@@ -222,9 +215,9 @@ public VisitorState withPath(TreePath path) {
         severityMap,
         errorProneOptions,
         statisticsCollector,
-        typeCache,
         path,
-        suppressedState);
+        suppressedState,
+        sharedState);
   }
 
   public VisitorState withPathAndSuppression(TreePath path, SuppressedState suppressedState) {
@@ -234,25 +227,29 @@ public VisitorState withPathAndSuppression(TreePath path, SuppressedState suppre
         severityMap,
         errorProneOptions,
         statisticsCollector,
-        typeCache,
         path,
-        suppressedState);
+        suppressedState,
+        sharedState);
   }
 
   public TreePath getPath() {
     return path;
   }
 
   public TreeMaker getTreeMaker() {
-    return TreeMaker.instance(context);
+    return sharedState.treeMaker;
   }
 
   public Types getTypes() {
-    return Types.instance(context);
+    return sharedState.types;
   }
 
   public Symtab getSymtab() {
-    return Symtab.instance(context);
+    return sharedState.symtab;
+  }
+
+  public Names getNames() {
+    return sharedState.names;
   }
 
   public NullnessAnalysis getNullnessAnalysis() {
@@ -315,7 +312,7 @@ public ImmutableMultiset<String> counters() {
   }
 
   public Name getName(String nameStr) {
-    return Names.instance(context).fromString(nameStr);
+    return getNames().fromString(nameStr);
   }
 
   /**
@@ -331,7 +328,8 @@ public Name getName(String nameStr) {
    */
   @Nullable
   public Type getTypeFromString(String typeStr) {
-    return typeCache
+    return sharedState
+        .typeCache
         .computeIfAbsent(typeStr, key -> Optional.fromNullable(getTypeFromStringInternal(key)))
         .orNull();
   }
@@ -360,12 +358,11 @@ private Type getTypeFromStringInternal(String typeStr) {
   public Symbol getSymbolFromString(String symStr) {
     symStr = inferBinaryName(symStr);
     Name name = getName(symStr);
-    Modules modules = Modules.instance(context);
-    boolean modular = modules.getDefaultModule() != getSymtab().noModule;
+    boolean modular = sharedState.modules.getDefaultModule() != getSymtab().noModule;
     if (!modular) {
       return getSymbolFromString(getSymtab().noModule, name);
     }
-    for (ModuleSymbol msym : Modules.instance(context).allModules()) {
+    for (ModuleSymbol msym : sharedState.modules.allModules()) {
       ClassSymbol result = getSymbolFromString(msym, name);
       if (result != null) {
         // TODO(cushon): the path where we iterate over all modules is probably slow.
@@ -401,19 +398,32 @@ public ClassSymbol getSymbolFromString(ModuleSymbol msym, Name name) {
   // TODO(cushon): consider migrating call sites to use binary names and removing this code.
   // (But then we'd probably want error handling for probably-incorrect canonical names,
   // so it may not end up being a performance win.)
-  private static String inferBinaryName(String classname) {
-    StringBuilder sb = new StringBuilder();
-    boolean first = true;
-    char sep = '.';
-    for (String bit : Splitter.on('.').split(classname)) {
-      if (!first) {
-        sb.append(sep);
-      }
-      sb.append(bit);
-      if (Character.isUpperCase(bit.charAt(0))) {
-        sep = '$';
+  @VisibleForTesting
+  static String inferBinaryName(String classname) {
+    int len = classname.length();
+    checkArgument(!classname.isEmpty(), "class name must be non-empty");
+    checkArgument(classname.charAt(len - 1) != '.', "invalid class name: %s", classname);
+
+    int lastPeriod = classname.lastIndexOf('.');
+    if (lastPeriod == -1) {
+      return classname; // top level class in default package
+    }
+    int secondToLastPeriod = classname.lastIndexOf('.', lastPeriod - 1);
+    if (secondToLastPeriod != -1
+        && !Character.isUpperCase(classname.charAt(secondToLastPeriod + 1))) {
+      return classname; // top level class
+    }
+
+    StringBuilder sb = new StringBuilder(len);
+    boolean foundUppercase = false;
+    for (int i = 0; i < len; i++) {
+      char c = classname.charAt(i);
+      foundUppercase = foundUppercase || Character.isUpperCase(c);
+      if (c == '.') {
+        sb.append(foundUppercase ? '$' : '.');
+      } else {
+        sb.append(c);
       }
-      first = false;
     }
     return sb.toString();
   }
@@ -605,6 +615,35 @@ public boolean isAndroidCompatible() {
 
   /** Returns a timing span for the given {@link Suppressible}. */
   public AutoCloseable timingSpan(Suppressible suppressible) {
-    return timings.span(suppressible);
+    return sharedState.timings.span(suppressible);
+  }
+
+  /**
+   * Instances that every {@link VisitorState} instance can share.
+   *
+   * <p>For the types that are typically stored in {@link Context}, caching the references over
+   * {@code SomeClass.instance(context)} has sizable performance improvements in aggregate.
+   */
+  private static final class SharedState {
+    private final Modules modules;
+    private final Names names;
+    private final Symtab symtab;
+    private final ErrorProneTimings timings;
+    private final Types types;
+    private final TreeMaker treeMaker;
+
+    // TODO(ronshapiro): should we presize this with a reasonable size? We can check for the
+    // smallest build and see how many types are loaded and use that. Or perhaps a heuristic
+    // based on number of files?
+    private final Map<String, Optional<Type>> typeCache = new HashMap<>();
+
+    SharedState(Context context) {
+      this.modules = Modules.instance(context);
+      this.names = Names.instance(context);
+      this.symtab = Symtab.instance(context);
+      this.timings = ErrorProneTimings.instance(context);
+      this.types = Types.instance(context);
+      this.treeMaker = TreeMaker.instance(context);
+    }
   }
 }

check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java

@@ -245,6 +245,10 @@ public boolean supportsSuppressWarnings() {
     return info.supportsSuppressWarnings();
   }
 
+  public boolean disableable() {
+    return info.disableable();
+  }
+
   @Override
   public Set<Class<? extends Annotation>> customSuppressionAnnotations() {
     return info.customSuppressionAnnotations();