Skip to content

Commit

Permalink
Check for compilation errors in BugCheckerRefactoringTestHelper
Browse files Browse the repository at this point in the history 
MOE_MIGRATED_REVID=125743535
  • Loading branch information
@cushon
cushon committed Jun 24, 2016
1 parent b97df85 commit 0799c20
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 68 deletions.
70 changes: 35 additions & 35 deletions core/src/test/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
View file
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertAbout;
import static com.google.common.truth.Truth.assertThat;
import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource;
import static org.junit.Assert.fail;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -47,21 +48,20 @@
import java.util.List;
import java.util.Map;

import javax.tools.Diagnostic;
import javax.tools.DiagnosticCollector;
import javax.tools.JavaFileObject;

/**
* Compare a file transformed as suggested by {@link BugChecker} to an expected source.
*
* Inputs are a {@link BugChecker} instance, input file and expected file.
* <p>Inputs are a {@link BugChecker} instance, input file and expected file.
*
* @author kurs@google.com (Jan Kurs)
*/
public class BugCheckerRefactoringTestHelper {

/**
* Test mode for matching refactored source against expected source.
*/
/** Test mode for matching refactored source against expected source. */
public enum TestMode {
TEXT_MATCH {
@Override
Expand All @@ -85,7 +85,7 @@ abstract void verifyMatch(JavaFileObject refactoredSource, JavaFileObject expect
/**
* For checks that provide multiple possible fixes, chooses the one that will be applied for the
* test.
* */
*/
public interface FixChooser {
Fix choose(List<Fix> fixes);
}
Expand Down Expand Up @@ -147,21 +147,44 @@ private BugCheckerRefactoringTestHelper addInputAndOutput(

private void runTestOnPair(JavaFileObject input, JavaFileObject output, TestMode testMode)
throws IOException {
JavacTaskImpl task = createJavacTask();
JavacTool tool = JavacTool.create();
DiagnosticCollector<JavaFileObject> diagnosticsCollector = new DiagnosticCollector<>();
Context context = new Context();
context.put(ErrorProneOptions.class, ErrorProneOptions.empty());
JavacTaskImpl task =
(JavacTaskImpl)
tool.getTask(
CharStreams.nullWriter(),
fileManager,
diagnosticsCollector,
/*options=*/ ImmutableList.<String>of(),
/*classes=*/ null,
ImmutableList.copyOf(sources.keySet()),
context);
JCCompilationUnit tree = parseAndAnalyze(task, input);
JavaFileObject transformed = applyDiff(input, task, tree);

Iterable<Diagnostic<? extends JavaFileObject>> errorDiagnostics =
Iterables.filter(
diagnosticsCollector.getDiagnostics(),
new Predicate<Diagnostic<? extends JavaFileObject>>() {
@Override
public boolean apply(Diagnostic<? extends JavaFileObject> input) {
return input.getKind() == Diagnostic.Kind.ERROR;
}
});
if (!Iterables.isEmpty(errorDiagnostics)) {
fail("compilation failed unexpectedly: " + errorDiagnostics);
}
JavaFileObject transformed = applyDiff(input, context, tree);
testMode.verifyMatch(transformed, output);
}

private JavaFileObject applyDiff(
JavaFileObject sourceFileObject, JavacTaskImpl task, JCCompilationUnit tree)
throws IOException {
JavaFileObject sourceFileObject, Context context, JCCompilationUnit tree) throws IOException {
final DescriptionBasedDiff diff = DescriptionBasedDiff.create(tree);
transformer(refactoringBugChecker)
.apply(
new TreePath(tree),
task.getContext(),
context,
new DescriptionListener() {
@Override
public void onDescribed(Description description) {
Expand Down Expand Up @@ -197,35 +220,12 @@ public boolean apply(JCCompilationUnit compilation) {
}));
}

private JavacTaskImpl createJavacTask() {
JavacTool tool = JavacTool.create();
DiagnosticCollector<JavaFileObject> diagnosticsCollector = new DiagnosticCollector<>();
Context context = new Context();

context.put(ErrorProneOptions.class, ErrorProneOptions.empty());

JavacTaskImpl task =
(JavacTaskImpl)
tool.getTask(
CharStreams.nullWriter(),
fileManager,
diagnosticsCollector,
ImmutableList.<String>of(),
null,
ImmutableList.copyOf(sources.keySet()),
context);

return task;
}

private ErrorProneScannerTransformer transformer(BugChecker bugChecker) {
ErrorProneScanner scanner = new ErrorProneScanner(bugChecker);
return ErrorProneScannerTransformer.create(scanner);
}

/**
* To assert the proper {@code .addInput().addOutput()} chain.
*/
/** To assert the proper {@code .addInput().addOutput()} chain. */
public class ExpectOutput {
private final JavaFileObject input;

Expand Down
51 changes: 32 additions & 19 deletions core/src/test/java/com/google/errorprone/BugCheckerRefactoringTestHelperTest.java
View file
Expand Up @@ -40,11 +40,8 @@

import java.io.IOException;
import java.nio.file.FileAlreadyExistsException;
import java.util.NoSuchElementException;

/**
* Tests for {@link BugCheckerRefactoringTestHelper}.
*/
/** Tests for {@link BugCheckerRefactoringTestHelper}. */
@RunWith(JUnit4.class)
public class BugCheckerRefactoringTestHelperTest {

Expand Down Expand Up @@ -107,8 +104,6 @@ public void testReplaceFail() throws IOException {
.doTest();
}



@Test
public void testReplaceTextMatch() throws IOException {
helper
Expand All @@ -130,7 +125,7 @@ public void testReplaceTextMatch() throws IOException {
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test(expected = AssertionError.class)
public void testReplaceTextMatchFail() throws IOException {
helper
Expand All @@ -142,7 +137,8 @@ public void testReplaceTextMatchFail() throws IOException {
" return i;",
" }",
"}")
.addOutputLines("out/Test.java",
.addOutputLines(
"out/Test.java",
"public class Test {",
" public Object foo() {",
" Integer i = 1 + 2;",
Expand All @@ -151,12 +147,19 @@ public void testReplaceTextMatchFail() throws IOException {
"}")
.doTest(TestMode.TEXT_MATCH);
}
@Test(expected = NoSuchElementException.class)

@Test
public void compilationErrorFail() throws IOException {
helper
.addInputLines("syntax_error.java", "public clazz Bar { ! this should fail }")
.expectUnchanged()
.doTest();
try {
helper
.addInputLines("syntax_error.java", "public clazz Bar { ! this should fail }")
.expectUnchanged()
.doTest();
} catch (AssertionError e) {
assertThat(e.getMessage()).contains("compilation failed unexpectedly");
return;
}
fail("compilation succeeded unexpectedly");
}

@Test
Expand All @@ -168,9 +171,7 @@ public void testAnnotationFullName() throws IOException {
.addOutputLines("out/foo/Bar.java", "import bar.Foo;", "public class Bar {", "}")
.doTest(TestMode.TEXT_MATCH);
}
/**
* Mock {@link BugChecker} for testing only.
*/
/** Mock {@link BugChecker} for testing only. */
@BugPattern(
name = "ReturnNullRefactoring",
summary = "Mock refactoring that replaces all returns with 'return null;' statement.",
Expand All @@ -185,9 +186,7 @@ public Description matchReturn(ReturnTree tree, VisitorState state) {
return describeMatch(tree, SuggestedFix.replace(tree, "return null;"));
}
}
/**
* Mock {@link BugChecker} for testing only.
*/
/** Mock {@link BugChecker} for testing only. */
@BugPattern(
name = "RemoveAnnotationRefactoring",
summary = "Mock refactoring that removes all annotations declared in package bar ",
Expand Down Expand Up @@ -220,4 +219,18 @@ public void testFileAlreadyExists() throws IOException {
assertThat(e).hasMessage("Test.java");
}
}

@Test
public void compilationError() throws Exception {
try {
helper
.addInputLines("Test.java", "public class Test extends NoSuch {}")
.expectUnchanged()
.doTest();
} catch (AssertionError e) {
assertThat(e.getMessage()).contains("error: cannot find symbol");
return;
}
fail("compilation succeeded unexpectedly");
}
}
14 changes: 6 additions & 8 deletions core/src/test/java/com/google/errorprone/bugpatterns/ClassNewInstanceTest.java
View file
Expand Up @@ -254,7 +254,7 @@ public void negative() throws Exception {
.addInputLines(
"in/Test.java",
"class Test {",
" void f() {",
" void f() throws Exception {",
" getClass().getConstructor().newInstance();",
" }",
"}")
Expand Down Expand Up @@ -362,7 +362,7 @@ public void inCatch() throws Exception {
" void f() throws Exception {",
" try {",
" getClass().newInstance();",
" } catch (NoSuchMethodException e) {",
" } catch (InstantiationException e) {",
" getClass().newInstance();",
" }",
" }",
Expand Down Expand Up @@ -493,7 +493,6 @@ public void catchAndThrows() throws Exception {
" } catch (ReflectiveOperationException ex) {",
" return getClass().newInstance();",
" }",
" return null;",
" }",
"}")
.addOutputLines(
Expand All @@ -509,7 +508,6 @@ public void catchAndThrows() throws Exception {
" } catch (ReflectiveOperationException ex) {",
" return getClass().getConstructor().newInstance();",
" }",
" return null;",
" }",
"}")
.doTest();
Expand All @@ -523,7 +521,7 @@ public void mixedMulticatch() throws Exception {
"class Test {",
" void f() {",
" try {",
" return getClass().newInstance();",
" getClass().newInstance();",
" } catch (InstantiationException e) {",
" // InstantiationException",
" } catch (IllegalAccessException | NullPointerException e) {",
Expand All @@ -536,7 +534,7 @@ public void mixedMulticatch() throws Exception {
"class Test {",
" void f() {",
" try {",
" return getClass().getConstructor().newInstance();",
" getClass().getConstructor().newInstance();",
" } catch (InstantiationException e) {",
" // InstantiationException",
" } catch (ReflectiveOperationException | NullPointerException e) {",
Expand All @@ -555,7 +553,7 @@ public void freshVar() throws Exception {
"class Test {",
" void f(Exception e) {",
" try {",
" return getClass().newInstance();",
" getClass().newInstance();",
" } catch (InstantiationException e1) {",
" // one",
" } catch (IllegalAccessException e1) {",
Expand All @@ -568,7 +566,7 @@ public void freshVar() throws Exception {
"class Test {",
" void f(Exception e) {",
" try {",
" return getClass().getConstructor().newInstance();",
" getClass().getConstructor().newInstance();",
" } catch (InstantiationException e1) {",
" // one",
" } catch (IllegalAccessException e1) {",
Expand Down
12 changes: 6 additions & 6 deletions core/src/test/java/com/google/errorprone/bugpatterns/WildcardImportTest.java
View file
Expand Up @@ -463,12 +463,12 @@ public void qualifyMembersFix() throws Exception {
"in/Test.java",
"import static e.E.*;",
"public class Test {",
" E ex = {",
" Object[] ex = {",
" A, B, C, D, E, F, G, H, I, J,",
" K, L, M, N, O, P, Q, R, S, T,",
" U, V, W, X, Y, Z",
" };",
" boolean f(E e) {",
" boolean f(e.E e) {",
" switch (e) {",
" case A:",
" case E:",
Expand All @@ -478,19 +478,19 @@ public void qualifyMembersFix() throws Exception {
" return true;",
" default:",
" return false;",
" };",
" }",
" }",
"}")
.addOutputLines(
"out/Test.java",
"import e.E;",
"public class Test {",
" E ex = {",
" Object[] ex = {",
" E.A, E.B, E.C, E.D, E.E, E.F, E.G, E.H, E.I, E.J,",
" E.K, E.L, E.M, E.N, E.O, E.P, E.Q, E.R, E.S, E.T,",
" E.U, E.V, E.W, E.X, E.Y, E.Z",
" };",
" boolean f(E e) {",
" boolean f(e.E e) {",
" switch (e) {",
" case A:",
" case E:",
Expand All @@ -500,7 +500,7 @@ public void qualifyMembersFix() throws Exception {
" return true;",
" default:",
" return false;",
" };",
" }",
" }",
"}")
.doTest();
Expand Down
2 changes: 2 additions & 0 deletions ...om/google/errorprone/bugpatterns/inject/dagger/EmptySetMultibindingContributionsTest.java
View file
Expand Up @@ -77,6 +77,7 @@ public void elementsIntoSetMethod_emptySet() throws IOException {
"import dagger.Module;",
"import dagger.Provides;",
"import dagger.multibindings.ElementsIntoSet;",
"import java.util.Set;",
"@Module",
"class Test {",
" @Provides @ElementsIntoSet Set<?> provideEmpty() {",
Expand All @@ -89,6 +90,7 @@ public void elementsIntoSetMethod_emptySet() throws IOException {
"import dagger.Provides;",
"import dagger.multibindings.ElementsIntoSet;",
"import dagger.multibindings.Multibinds;",
"import java.util.Set;",
"@Module",
"abstract class Test {",
" @Multibinds abstract Set<?> provideEmpty();",
Expand Down
2 changes: 2 additions & 0 deletions .../errorprone/bugpatterns/inject/dagger/PrivateConstructorForNoninstantiableModuleTest.java
View file
Expand Up @@ -77,6 +77,7 @@ public void abstractClassWithStaticAndAbstractMethods() throws IOException {
"in/TestModule.java",
"import dagger.Binds;",
"import dagger.Module;",
"import dagger.Provides;",
"@Module abstract class TestModule {",
" @Provides static String provideString() { return \"\"; }",
" @Binds abstract Object bindObject(String string);",
Expand All @@ -87,6 +88,7 @@ public void abstractClassWithStaticAndAbstractMethods() throws IOException {
"out/TestModule.java", //
"import dagger.Binds;",
"import dagger.Module;",
"import dagger.Provides;",
"@Module abstract class TestModule {",
" @Provides static String provideString() { return \"\"; }",
" @Binds abstract Object bindObject(String string);",
Expand Down

0 comments on commit 0799c20

Please sign in to comment.