Initial version of an open source refaster, with an example in the RE…

…ADME.md. Incidental changes performed to get there * Shade in check_api to the core jar to allow end users to not need check_api if they have core * Per feedback, no longer fail the compile when refactorings are performed. RELNOTES: Barebones implementation of Refaster, a tool allowing for semantic refactorings of codebases using before-and-after templates. MOE_MIGRATED_REVID=142680811
google · Dec 22, 2016 · 19024d0 · 19024d0
1 parent fe41858
commit 19024d0
Show file tree

Hide file tree

Showing 18 changed files with 312 additions and 54 deletions.
diff --git a/check_api/pom.xml b/check_api/pom.xml
@@ -74,7 +74,7 @@
       <!-- Apache 2.0 -->
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>1.1</version>
+      <version>${autovalue.version}</version>
       <scope>provided</scope>
     </dependency>
     <dependency>

diff --git a/check_api/src/main/java/com/google/errorprone/BaseErrorProneCompiler.java b/check_api/src/main/java/com/google/errorprone/BaseErrorProneCompiler.java
@@ -37,6 +37,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import javax.annotation.Nullable;
 import javax.annotation.processing.Processor;
 import javax.tools.DiagnosticListener;
@@ -178,28 +179,34 @@ private String[] prepareCompilation(String[] argv, Context context)
     setupMessageBundle(context);
     ErrorProneAnalyzer analyzer;
     if (epOptions.patchingOptions().doRefactor()) {
-      RefactoringCollection refactoringCollection;
-      if (epOptions.patchingOptions().inPlace()) {
-        refactoringCollection = RefactoringCollection.refactorInPlace();
-      } else {
-        refactoringCollection =
-            RefactoringCollection.refactorToPatchFile(epOptions.patchingOptions().baseDirectory());
-      }
-
-
-      ScannerSupplier toUse = scannerSupplier;
-      if (!epOptions.patchingOptions().namedCheckers().isEmpty()) {
-        toUse =
-            scannerSupplier.filter(
-                bci -> epOptions.patchingOptions().namedCheckers().contains(bci.canonicalName()));
-      }
+      RefactoringCollection refactoringCollection =
+          epOptions.patchingOptions().inPlace()
+              ? RefactoringCollection.refactorInPlace()
+              : RefactoringCollection.refactorToPatchFile(
+                  epOptions.patchingOptions().baseDirectory());
+
+      // Refaster refactorer or using builtin checks
+      CodeTransformer codeTransformer =
+          epOptions
+              .patchingOptions()
+              .customRefactorer()
+              .or(
+                  () -> {
+                    ScannerSupplier toUse = scannerSupplier;
+                    Set<String> namedCheckers = epOptions.patchingOptions().namedCheckers();
+                    if (!namedCheckers.isEmpty()) {
+                      toUse =
+                          scannerSupplier.filter(
+                              bci -> namedCheckers.contains(bci.canonicalName()));
+                    }
+                    return ErrorProneScannerTransformer.create(
+                        toUse.applyOverrides(epOptions).get());
+                  })
+              .get();
 
       analyzer =
           ErrorProneAnalyzer.createWithCustomDescriptionListener(
-              ErrorProneScannerTransformer.create(toUse.applyOverrides(epOptions).get()),
-              epOptions,
-              context,
-              refactoringCollection);
+              codeTransformer, epOptions, context, refactoringCollection);
       context.put(RefactoringCollection.class, refactoringCollection);
     } else {
       analyzer = ErrorProneAnalyzer.createByScanningForPlugins(scannerSupplier, epOptions, context);
@@ -274,19 +281,11 @@ private Result wrapPotentialRefactoringCall(
     // Attempt the refactor
     try {
       RefactoringResult refactoringResult = refactoringCollection.applyChanges();
-      switch (refactoringResult.type()) {
-        case NO_CHANGES:
-          return original;
-        case CHANGED:
-          errOutput.println(refactoringResult.message());
-          errOutput.flush();
-          // Changes were made to the code, so we want to fail the compile phase. (This is to remind
-          // end users that the compiled code does not contained the refactored code, and that they
-          // should re-compile after inspecting the differences).
-          return Result.ERROR;
-        default:
-          throw new AssertionError("Unexpected RefactoringResult: " + refactoringResult);
+      if (refactoringResult.type() == RefactoringCollection.RefactoringResultType.CHANGED) {
+        errOutput.println(refactoringResult.message());
+        errOutput.flush();
       }
+      return original;
     } catch (Exception e) {
       errOutput.append(e.getMessage());
       errOutput.flush();

diff --git a/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java b/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java
@@ -17,11 +17,18 @@
 package com.google.errorprone;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
+import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -88,6 +95,8 @@ final boolean doRefactor() {
 
     abstract String baseDirectory();
 
+    abstract Optional<Supplier<CodeTransformer>> customRefactorer();
+
     static Builder builder() {
       return new AutoValue_ErrorProneOptions_PatchingOptions.Builder()
           .baseDirectory("")
@@ -97,20 +106,25 @@ static Builder builder() {
 
     @AutoValue.Builder
     abstract static class Builder {
+
       abstract Builder namedCheckers(Set<String> checkers);
 
       abstract Builder inPlace(boolean inPlace);
 
       abstract Builder baseDirectory(String baseDirectory);
 
+      abstract Builder customRefactorer(Supplier<CodeTransformer> refactorer);
+
       abstract PatchingOptions autoBuild();
 
       final PatchingOptions build() {
 
         PatchingOptions patchingOptions = autoBuild();
 
-        // If anything is specified, then checkers and output must be set.
-        if (!patchingOptions.namedCheckers().isEmpty() ^ patchingOptions.doRefactor()) {
+        // If anything is specified, then (checkers or refaster) and output must be set.
+        if ((!patchingOptions.namedCheckers().isEmpty()
+                || patchingOptions.customRefactorer().isPresent())
+            ^ patchingOptions.doRefactor()) {
           throw new InvalidCommandLineOptionException(
               "-XepPatchChecks and -XepPatchLocation must be specified together");
         }
@@ -267,8 +281,25 @@ public static ErrorProneOptions processArgs(Iterable<String> args) {
             }
           } else if (arg.startsWith(PATCH_CHECKS_PREFIX)) {
             String remaining = arg.substring(PATCH_CHECKS_PREFIX.length());
-            Iterable<String> checks = Splitter.on(',').trimResults().split(remaining);
-            builder.patchingOptionsBuilder().namedCheckers(ImmutableSet.copyOf(checks));
+            if (remaining.startsWith("refaster:")) {
+              // Refaster rule, load from InputStream at file
+              builder
+                  .patchingOptionsBuilder()
+                  .customRefactorer(
+                      () -> {
+                        String path = remaining.substring("refaster:".length());
+                        try (InputStream in =
+                                Files.newInputStream(FileSystems.getDefault().getPath(path));
+                            ObjectInputStream ois = new ObjectInputStream(in)) {
+                          return (CodeTransformer) ois.readObject();
+                        } catch (IOException | ClassNotFoundException e) {
+                          throw new RuntimeException("Can't load Refaster rule from " + path, e);
+                        }
+                      });
+            } else {
+              Iterable<String> checks = Splitter.on(',').trimResults().split(remaining);
+              builder.patchingOptionsBuilder().namedCheckers(ImmutableSet.copyOf(checks));
+            }
           } else {
             outputArgs.add(arg);
           }

diff --git a/check_api/src/main/java/com/google/errorprone/apply/PatchFileDestination.java b/check_api/src/main/java/com/google/errorprone/apply/PatchFileDestination.java
@@ -42,7 +42,7 @@ public final class PatchFileDestination implements FileDestination {
 
   private final Path baseDir;
   private final Path rootPath;
-  // Path -> Unified Diff String, sorted by path
+  // Path -> Unified Diff, sorted by path
   private final Map<String, String> diffByFile = new TreeMap<>();
 
   public PatchFileDestination(Path baseDir, Path rootPath) {
@@ -52,24 +52,24 @@ public PatchFileDestination(Path baseDir, Path rootPath) {
 
   @Override
   public void writeFile(SourceFile update) throws IOException {
-    Path originalFilePath = rootPath.resolve(update.getPath());
-    String oldSource = new String(Files.readAllBytes(originalFilePath), UTF_8);
+    Path sourceFilePath = rootPath.resolve(update.getPath());
+    String oldSource = new String(Files.readAllBytes(sourceFilePath), UTF_8);
     String newSource = update.getSourceText();
     if (!oldSource.equals(newSource)) {
       List<String> originalLines = LINE_SPLITTER.splitToList(oldSource);
 
       Patch<String> diff = DiffUtils.diff(originalLines, LINE_SPLITTER.splitToList(newSource));
-      String relativePath = relativize(update);
+      String relativePath = relativize(sourceFilePath);
       List<String> unifiedDiff =
           DiffUtils.generateUnifiedDiff(relativePath, relativePath, originalLines, diff, 2);
 
       String diffString = Joiner.on("\n").join(unifiedDiff);
-      diffByFile.put(originalFilePath.toString(), diffString);
+      diffByFile.put(sourceFilePath.toString(), diffString);
     }
   }
 
-  private String relativize(SourceFile update) {
-    return baseDir.relativize(rootPath.resolve(update.getPath())).toString();
+  private String relativize(Path sourceFilePath) {
+    return baseDir.relativize(sourceFilePath).toString();
   }
 
   public String patchFile() {

diff --git a/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java b/check_api/src/test/java/com/google/errorprone/ErrorProneOptionsTest.java
@@ -132,6 +132,7 @@ public void recognizesPatch() {
     assertThat(options.patchingOptions().inPlace()).isTrue();
     assertThat(options.patchingOptions().namedCheckers())
         .containsExactly("MissingOverride", "FooBar");
+    assertThat(options.patchingOptions().customRefactorer()).isAbsent();
 
     options =
         ErrorProneOptions.processArgs(
@@ -143,6 +144,7 @@ public void recognizesPatch() {
     assertThat(options.patchingOptions().baseDirectory()).isEqualTo("/some/base/dir");
     assertThat(options.patchingOptions().namedCheckers())
         .containsExactly("MissingOverride", "FooBar");
+    assertThat(options.patchingOptions().customRefactorer()).isAbsent();
 
     options = ErrorProneOptions.processArgs(new String[] {});
     assertThat(options.patchingOptions().doRefactor()).isFalse();
@@ -158,4 +160,14 @@ public void throwsExceptionWithBadPatchArgs() {
         () ->
             ErrorProneOptions.processArgs(new String[] {"-XepPatchChecks:FooBar,MissingOverride"}));
   }
+
+  @Test
+  public void recognizesRefaster() {
+    ErrorProneOptions options =
+        ErrorProneOptions.processArgs(
+            new String[] {"-XepPatchChecks:refaster:/foo/bar", "-XepPatchLocation:IN_PLACE"});
+    assertThat(options.patchingOptions().doRefactor()).isTrue();
+    assertThat(options.patchingOptions().inPlace()).isTrue();
+    assertThat(options.patchingOptions().customRefactorer()).isPresent();
+  }
 }
diff --git a/core/pom.xml b/core/pom.xml
@@ -117,7 +117,7 @@
       <!-- Apache 2.0 -->
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>1.1</version>
+      <version>${autovalue.version}</version>
       <scope>provided</scope>
     </dependency>
     <dependency>
@@ -289,6 +289,7 @@
                      Apache, BSD, and MIT are OK; others are not. -->
                 <includes>
                   <include>com.google.errorprone:error_prone_annotation</include>
+                  <include>com.google.errorprone:error_prone_check_api</include>
                   <include>com.github.stephenc.jcip:jcip-annotations</include>
                   <include>org.pcollections:pcollections</include>
                   <include>com.google.guava:guava</include>

diff --git a/docgen/pom.xml b/docgen/pom.xml
@@ -84,7 +84,7 @@
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>1.1</version>
+      <version>${autovalue.version}</version>
       <scope>provided</scope>
     </dependency>
     <dependency>

diff --git a/docgen_processor/pom.xml b/docgen_processor/pom.xml
@@ -47,7 +47,7 @@
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
-      <version>1.1</version>
+      <version>${autovalue.version}</version>
       <scope>provided</scope>
     </dependency>
     <dependency>

diff --git a/examples/maven/pom.xml b/examples/maven/pom.xml
@@ -51,7 +51,7 @@
           <dependency>
             <groupId>com.google.errorprone</groupId>
             <artifactId>error_prone_core</artifactId>
-            <version>2.0.15</version>
+            <version>2.0.16-SNAPSHOT</version>
           </dependency>
         </dependencies>
       </plugin>
@@ -68,8 +68,8 @@
             <artifactId>maven-compiler-plugin</artifactId>
             <configuration>
               <compilerArgs combine.children="append">
-                <!--<compilerArg>-XepPatch:${basedir}</compilerArg>-->
-                <compilerArg>-XepPatch:IN_PLACE</compilerArg>
+                <compilerArg>-XepPatchLocation:${basedir}</compilerArg>
+                <compilerArg>-XepPatchChecks:DeadException,GetClassOnClass</compilerArg>
               </compilerArgs>
             </configuration>
           </plugin>

diff --git a/examples/maven/refaster-based-cleanup/pom.xml b/examples/maven/refaster-based-cleanup/pom.xml
@@ -0,0 +1,50 @@
+<!--
+  Copyright 2016 Google Inc. All Rights Reserved.
+
+  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.
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>com.example</groupId>
+  <artifactId>refaster-cleanup</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <parent>
+    <groupId>com.example</groupId>
+    <artifactId>maven-plugin-example</artifactId>
+    <version>1.0-SNAPSHOT</version>
+  </parent>
+
+  <profiles>
+    <profile>
+      <id>fixerrors</id>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-compiler-plugin</artifactId>
+            <configuration>
+              <compilerArgs combine.children="override">
+                <compilerArg>-XepPatchChecks:refaster:${basedir}/../../refaster/refactoring.out</compilerArg>
+                <compilerArg>-XepPatchLocation:${basedir}</compilerArg>
+              </compilerArgs>
+            </configuration>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
+  </profiles>
+
+</project>
diff --git a/examples/maven/refaster-based-cleanup/src/main/java/Main.java b/examples/maven/refaster-based-cleanup/src/main/java/Main.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2016 Google Inc. All Rights Reserved.
+ *
+ * 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.
+ */
+
+/** Example class containing a String call to be replaced. */
+public class Main {
+  public static void main(String[] args) {
+    for (String arg : args) {
+      if (arg.length() == 0) {
+        System.out.println("Empty!");
+      }
+    }
+  }
+}