diff --git a/check_api/pom.xml b/check_api/pom.xml index 7675659b772..a78c013eee3 100644 --- a/check_api/pom.xml +++ b/check_api/pom.xml @@ -74,7 +74,7 @@ com.google.auto.value auto-value - 1.1 + ${autovalue.version} provided diff --git a/check_api/src/main/java/com/google/errorprone/BaseErrorProneCompiler.java b/check_api/src/main/java/com/google/errorprone/BaseErrorProneCompiler.java index 15a8c6074e4..1a0e5364347 100644 --- 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 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 index 821b9eee2c4..792dc268381 100644 --- 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> 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 checkers); abstract Builder inPlace(boolean inPlace); abstract Builder baseDirectory(String baseDirectory); + abstract Builder customRefactorer(Supplier 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 args) { } } else if (arg.startsWith(PATCH_CHECKS_PREFIX)) { String remaining = arg.substring(PATCH_CHECKS_PREFIX.length()); - Iterable 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 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 index 67ae93eaf18..f800d2fd08c 100644 --- 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 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 originalLines = LINE_SPLITTER.splitToList(oldSource); Patch diff = DiffUtils.diff(originalLines, LINE_SPLITTER.splitToList(newSource)); - String relativePath = relativize(update); + String relativePath = relativize(sourceFilePath); List 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 index ec534f3fb52..0c7343ec084 100644 --- 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 index 747542a392b..87962f3f3a4 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -117,7 +117,7 @@ com.google.auto.value auto-value - 1.1 + ${autovalue.version} provided @@ -289,6 +289,7 @@ Apache, BSD, and MIT are OK; others are not. --> com.google.errorprone:error_prone_annotation + com.google.errorprone:error_prone_check_api com.github.stephenc.jcip:jcip-annotations org.pcollections:pcollections com.google.guava:guava diff --git a/docgen/pom.xml b/docgen/pom.xml index 69b57ee4922..cb1c0d08cda 100644 --- a/docgen/pom.xml +++ b/docgen/pom.xml @@ -84,7 +84,7 @@ com.google.auto.value auto-value - 1.1 + ${autovalue.version} provided diff --git a/docgen_processor/pom.xml b/docgen_processor/pom.xml index 5cb25c4e56a..7fb2029fdeb 100644 --- a/docgen_processor/pom.xml +++ b/docgen_processor/pom.xml @@ -47,7 +47,7 @@ com.google.auto.value auto-value - 1.1 + ${autovalue.version} provided diff --git a/examples/maven/pom.xml b/examples/maven/pom.xml index 1b244eb7909..827796d6012 100644 --- a/examples/maven/pom.xml +++ b/examples/maven/pom.xml @@ -51,7 +51,7 @@ com.google.errorprone error_prone_core - 2.0.15 + 2.0.16-SNAPSHOT @@ -68,8 +68,8 @@ maven-compiler-plugin - - -XepPatch:IN_PLACE + -XepPatchLocation:${basedir} + -XepPatchChecks:DeadException,GetClassOnClass diff --git a/examples/maven/refaster-based-cleanup/pom.xml b/examples/maven/refaster-based-cleanup/pom.xml new file mode 100644 index 00000000000..da252b26b45 --- /dev/null +++ b/examples/maven/refaster-based-cleanup/pom.xml @@ -0,0 +1,50 @@ + + + + 4.0.0 + + com.example + refaster-cleanup + 1.0-SNAPSHOT + + + com.example + maven-plugin-example + 1.0-SNAPSHOT + + + + + fixerrors + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -XepPatchChecks:refaster:${basedir}/../../refaster/refactoring.out + -XepPatchLocation:${basedir} + + + + + + + + + diff --git a/examples/maven/refaster-based-cleanup/src/main/java/Main.java b/examples/maven/refaster-based-cleanup/src/main/java/Main.java new file mode 100644 index 00000000000..2c8cd799fdf --- /dev/null +++ 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!"); + } + } + } +} diff --git a/examples/refaster/README.md b/examples/refaster/README.md new file mode 100644 index 00000000000..bb22cc29922 --- /dev/null +++ b/examples/refaster/README.md @@ -0,0 +1,24 @@ +This is a Refaster template, allowing you to specify before-and-after versions +of java code. + +Compile this with: + +```shell +wget http://repo1.maven.org/maven2/com/google/errorprone/javac/9-dev-r3297-1/javac-9-dev-r3297-1.jar + +java -cp javac-9-dev-r3297-1.jar:../../refaster/target/error_prone_refaster-2.0.16-SNAPSHOT.jar \ + com.google.errorprone.refaster.RefasterRuleCompiler \ + StringLengthToEmpty.java --out `pwd`/refactoring.out +``` + +This will compile the Refaster template and emit a serialized file that +represents that template into refactoring.out. + +Then use the refactoring.out file in an Error Prone-based compile to refactor +according to this refactoring (the maven profile configured in this example +passes the correct arguments to the Error Prone compile): + +```shell +cd ../maven/refaster-based-cleanup +mvn clean compile -Pfixerrors +``` diff --git a/examples/refaster/StringLengthToEmpty.java b/examples/refaster/StringLengthToEmpty.java new file mode 100644 index 00000000000..c267f762d2c --- /dev/null +++ b/examples/refaster/StringLengthToEmpty.java @@ -0,0 +1,33 @@ +/* + * 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. + */ + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.AlsoNegation; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +/** Example template: Replaces calls of String.length() == 0 to String.isEmpty() */ +public class StringLengthToEmpty { + @BeforeTemplate + boolean toUtf8Length(String string) { + return string.length() == 0; + } + + @AfterTemplate + @AlsoNegation + boolean optimizedMethod(String string) { + return string.isEmpty(); + } +} diff --git a/pom.xml b/pom.xml index 7dde61548e6..376450ce31e 100644 --- a/pom.xml +++ b/pom.xml @@ -35,6 +35,7 @@ 20.0 0.30 9-dev-r3297-1 + 1.3 @@ -46,6 +47,7 @@ docgen docgen_processor ant + refaster diff --git a/refaster/pom.xml b/refaster/pom.xml new file mode 100644 index 00000000000..8c794d40a66 --- /dev/null +++ b/refaster/pom.xml @@ -0,0 +1,77 @@ + + + + + + error_prone_parent + com.google.errorprone + 2.0.16-SNAPSHOT + + 4.0.0 + + error_prone_refaster + Refaster rule compiler + + + + com.google.errorprone + error_prone_core + ${project.version} + compile + + + + com.google.errorprone + javac + ${javac.version} + + + + + + + org.apache.maven.plugins + maven-shade-plugin + + + package + + shade + + + false + + + + com.google.errorprone:error_prone_core + + + + + com.google.errorprone.refaster.RefasterRuleCompiler + + + + + + + + + diff --git a/core/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java b/refaster/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java similarity index 91% rename from core/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java rename to refaster/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java index 4026f675a76..04ff694739c 100644 --- a/core/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java +++ b/refaster/src/main/java/com/google/errorprone/refaster/RefasterRuleCompiler.java @@ -62,10 +62,13 @@ private Result run(String[] argv, Context context) { } try { - return new Main( - "RefasterRuleCompiler", - new PrintWriter(new OutputStreamWriter(System.err, StandardCharsets.UTF_8))) - .compile(argv, context); + Result compileResult = + new Main( + "RefasterRuleCompiler", + new PrintWriter(new OutputStreamWriter(System.err, StandardCharsets.UTF_8))) + .compile(argv, context); + System.err.flush(); + return compileResult; } catch (InvalidCommandLineOptionException e) { System.err.println(e.getMessage()); System.err.flush(); diff --git a/core/src/main/java/com/google/errorprone/refaster/RefasterRuleCompilerAnalyzer.java b/refaster/src/main/java/com/google/errorprone/refaster/RefasterRuleCompilerAnalyzer.java similarity index 100% rename from core/src/main/java/com/google/errorprone/refaster/RefasterRuleCompilerAnalyzer.java rename to refaster/src/main/java/com/google/errorprone/refaster/RefasterRuleCompilerAnalyzer.java diff --git a/test_helpers/pom.xml b/test_helpers/pom.xml index 5216d282c81..e548de265ec 100644 --- a/test_helpers/pom.xml +++ b/test_helpers/pom.xml @@ -74,7 +74,7 @@ com.google.auto.value auto-value - 1.1 + ${autovalue.version} provided