From bb49a9ede0364cf87ab550c2d9c56d0baeb3faae Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 28 Mar 2022 15:24:07 +0200 Subject: [PATCH] Apply suggestions --- .../refaster/test/RefasterValidateTests.java | 47 ++++++++++--------- .../test/PartialTestMatchTemplates.java | 32 +++++++++++++ .../test/RefasterCollectionTestUtilTest.java | 1 + ...MatchInWrongMethodTemplatesTestOutput.java | 11 +++-- ...ngTestAndWrongTestTemplatesTestOutput.java | 18 +++---- .../PartialTestMatchTemplatesTestInput.java | 9 ++++ .../PartialTestMatchTemplatesTestOutput.java | 13 +++++ ...emplateWithoutTestTemplatesTestOutput.java | 9 ++-- 8 files changed, 101 insertions(+), 39 deletions(-) create mode 100644 refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplates.java create mode 100644 refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestInput.java create mode 100644 refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestOutput.java diff --git a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterValidateTests.java b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterValidateTests.java index 9804779ba98..d679a21f935 100644 --- a/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterValidateTests.java +++ b/refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterValidateTests.java @@ -6,6 +6,7 @@ import static tech.picnic.errorprone.refaster.runner.RefasterCheck.INCLUDED_TEMPLATES_PATTERN_FLAG; import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableRangeMap; @@ -27,6 +28,7 @@ import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.LineMap; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; import com.sun.source.util.TreeScanner; import com.sun.tools.javac.tree.EndPosTable; import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; @@ -34,7 +36,7 @@ import java.util.List; import java.util.Set; import java.util.function.Supplier; -import java.util.stream.Collector; +import java.util.stream.Stream; import tech.picnic.errorprone.refaster.runner.CodeTransformers; import tech.picnic.errorprone.refaster.runner.RefasterCheck; @@ -50,8 +52,6 @@ public final class RefasterValidateTests extends BugChecker implements Compilati private static final long serialVersionUID = 1L; private static final Supplier> ALL_CODE_TRANSFORMERS = Suppliers.memoize(CodeTransformers::loadAllCodeTransformers); - private static final Collector LIST_COLLECTOR = - joining("\n- ", "\n- ", "\n"); private final ImmutableSet templateNamesFromClassPath; private final RefasterCheck delegate; @@ -87,13 +87,15 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s ImmutableRangeMap matchesRangeMap = buildRangeMapForMatches(matches, compilationUnit.endPositions); - ImmutableSet templatesWithoutMatch = getTemplateNamesWithoutMatch(matches); + ImmutableSet templatesWithoutMatch = + getTemplateNamesWithoutMatch(matchesRangeMap.asMapOfRanges().values()); if (!templatesWithoutMatch.isEmpty()) { - appendCommentToCompilationUnit( + addCommentToTreeInOutputFile( + tree, String.format( "Did not encounter a test in `%s` for the following template(s)", getNameFromFQCN(compilationUnit.sourcefile.getName().replace(".java", ""))), - templatesWithoutMatch.stream().collect(LIST_COLLECTOR), + templatesWithoutMatch.stream(), state); } @@ -104,21 +106,21 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s return Description.NO_MATCH; } - private ImmutableSet getTemplateNamesWithoutMatch(List matches) { - ImmutableSet templateNamesOfMatches = - matches.stream() - .map(description -> description.checkName) - .map(RefasterValidateTests::getNameFromFQCN) - .collect(toImmutableSet()); - - return Sets.difference(templateNamesFromClassPath, templateNamesOfMatches).immutableCopy(); + private ImmutableSet getTemplateNamesWithoutMatch( + ImmutableCollection templateNamesOfMatches) { + return Sets.difference(templateNamesFromClassPath, ImmutableSet.copyOf(templateNamesOfMatches)) + .immutableCopy(); } - private void appendCommentToCompilationUnit(String message, String list, VisitorState state) { - String comment = String.format("\n/* %s:%s*/", message, list); - CompilationUnitTree compilationUnit = state.getPath().getCompilationUnit(); - state.reportMatch( - describeMatch(compilationUnit, SuggestedFix.postfixWith(compilationUnit, comment))); + private void addCommentToTreeInOutputFile( + Tree tree, String message, Stream violations, VisitorState state) { + String listOfViolations = violations.collect(joining("\n* - ", "\n* - ", "\n")); + String comment = String.format("/*\n* %s:%s*/\n", message, listOfViolations); + SuggestedFix fixWithComment = + tree instanceof MethodTree + ? SuggestedFix.prefixWith(tree, comment) + : SuggestedFix.postfixWith(tree, "\n" + comment); + state.reportMatch(describeMatch(tree, fixWithComment)); } private static ImmutableRangeMap buildRangeMapForMatches( @@ -162,18 +164,19 @@ public Void visitMethod(MethodTree tree, VisitorState state) { boolean correctTemplatesMatchedInMethod = matchesInCurrentMethod.asMapOfRanges().values().stream().allMatch(methodName::equals); if (!correctTemplatesMatchedInMethod) { - appendCommentToCompilationUnit( + addCommentToTreeInOutputFile( + tree, String.format( "The following matches unexpectedly occurred in method `%s`", tree.getName()), matchesRangeMap.asMapOfRanges().entrySet().stream() + .filter(e -> !e.getValue().equals(methodName)) .map( e -> String.format( "Template `%s` matches on line %s, while it should match in a method named `test%s`.", e.getValue(), lineMap.getLineNumber(e.getKey().lowerEndpoint()), - e.getValue())) - .collect(LIST_COLLECTOR), + e.getValue())), state); } return super.visitMethod(tree, state); diff --git a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplates.java b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplates.java new file mode 100644 index 00000000000..cfe971ba359 --- /dev/null +++ b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplates.java @@ -0,0 +1,32 @@ +package tech.picnic.errorprone.refaster.test; + +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; + +public class PartialTestMatchTemplates { + private PartialTestMatchTemplates() {} + + static final class StringIsEmpty { + @BeforeTemplate + boolean before(String string) { + return string.equals(""); + } + + @AfterTemplate + boolean after(String string) { + return string.isEmpty(); + } + } + + static final class StringEquals { + @BeforeTemplate + boolean before(String string1, String string2) { + return string1 == string2; + } + + @AfterTemplate + boolean after(String string1, String string2) { + return string1.equals(string2); + } + } +} diff --git a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/RefasterCollectionTestUtilTest.java b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/RefasterCollectionTestUtilTest.java index fd8b7773a6b..c49c7c4e050 100644 --- a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/RefasterCollectionTestUtilTest.java +++ b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/RefasterCollectionTestUtilTest.java @@ -11,6 +11,7 @@ final class RefasterCollectionTestUtilTest { MatchInWrongMethodTemplates.class, MethodNameWithNumberTemplates.class, MissingTestAndWrongTestTemplates.class, + PartialTestMatchTemplates.class, TemplateWithoutTestTemplates.class }) void verifyRefasterTemplateCollections(Class clazz) { diff --git a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MatchInWrongMethodTemplatesTestOutput.java b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MatchInWrongMethodTemplatesTestOutput.java index 14f84ad9431..7f693ab7e9e 100644 --- a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MatchInWrongMethodTemplatesTestOutput.java +++ b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MatchInWrongMethodTemplatesTestOutput.java @@ -2,14 +2,15 @@ /** Code to test the Refaster templates from `MatchInWrongMethodTemplates`. */ final class MatchInWrongMethodTemplatesTest implements RefasterTemplateTestCase { + /* + * The following matches unexpectedly occurred in method `testWrongName`: + * - Template `StringIsEmpty` matches on line 6, while it should match in a method named `testStringIsEmpty`. + * - Template `StringIsEmpty` matches on line 7, while it should match in a method named `testStringIsEmpty`. + * - Template `StringIsEmpty` matches on line 8, while it should match in a method named `testStringIsEmpty`. + */ boolean testWrongName() { "foo".isEmpty(); "bar".isEmpty(); return "baz".isEmpty(); } } -/* The following matches unexpectedly occurred in method `testWrongName`: -- Template `StringIsEmpty` matches on line 6, while it should match in a method named `testStringIsEmpty`. -- Template `StringIsEmpty` matches on line 7, while it should match in a method named `testStringIsEmpty`. -- Template `StringIsEmpty` matches on line 8, while it should match in a method named `testStringIsEmpty`. -*/ diff --git a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MissingTestAndWrongTestTemplatesTestOutput.java b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MissingTestAndWrongTestTemplatesTestOutput.java index 4af5cd857ec..e2034752458 100644 --- a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MissingTestAndWrongTestTemplatesTestOutput.java +++ b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/MissingTestAndWrongTestTemplatesTestOutput.java @@ -2,17 +2,19 @@ /** Code to test the Refaster templates from `MissingTestAndWrongTestTemplates`. */ final class MissingTestAndWrongTestTemplatesTest implements RefasterTemplateTestCase { + /* + * The following matches unexpectedly occurred in method `testWrongName`: + * - Template `StringIsEmpty` matches on line 6, while it should match in a method named `testStringIsEmpty`. + * - Template `StringIsEmpty` matches on line 7, while it should match in a method named `testStringIsEmpty`. + * - Template `StringIsEmpty` matches on line 8, while it should match in a method named `testStringIsEmpty`. + */ boolean testWrongName() { "foo".isEmpty(); "bar".isEmpty(); return "baz".isEmpty(); } } -/* Did not encounter a test in `MissingTestAndWrongTestTemplatesTestInput` for the following template(s): -- TemplateWithoutTest -*/ -/* The following matches unexpectedly occurred in method `testWrongName`: -- Template `StringIsEmpty` matches on line 6, while it should match in a method named `testStringIsEmpty`. -- Template `StringIsEmpty` matches on line 7, while it should match in a method named `testStringIsEmpty`. -- Template `StringIsEmpty` matches on line 8, while it should match in a method named `testStringIsEmpty`. -*/ +/* + * Did not encounter a test in `MissingTestAndWrongTestTemplatesTestInput` for the following template(s): + * - TemplateWithoutTest + */ diff --git a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestInput.java b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestInput.java new file mode 100644 index 00000000000..072ff0bcc9d --- /dev/null +++ b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestInput.java @@ -0,0 +1,9 @@ +package tech.picnic.errorprone.refaster.test; + +/** Code to test the Refaster templates from {@link PartialTestMatchTemplates}. */ +final class PartialTestMatchTemplatesTest implements RefasterTemplateTestCase { + boolean testStringIsEmpty() { + boolean b = "foo" == ""; + return "bar".equals(""); + } +} diff --git a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestOutput.java b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestOutput.java new file mode 100644 index 00000000000..7171cf89429 --- /dev/null +++ b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/PartialTestMatchTemplatesTestOutput.java @@ -0,0 +1,13 @@ +package tech.picnic.errorprone.refaster.test; + +/** Code to test the Refaster templates from {@link PartialTestMatchTemplates}. */ +final class PartialTestMatchTemplatesTest implements RefasterTemplateTestCase { + /* + * The following matches unexpectedly occurred in method `testStringIsEmpty`: + * - Template `StringEquals` matches on line 6, while it should match in a method named `testStringEquals`. + */ + boolean testStringIsEmpty() { + boolean b = "foo".equals(""); + return "bar".isEmpty(); + } +} diff --git a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/TemplateWithoutTestTemplatesTestOutput.java b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/TemplateWithoutTestTemplatesTestOutput.java index 2a39707b48a..92a8486e469 100644 --- a/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/TemplateWithoutTestTemplatesTestOutput.java +++ b/refaster-test-support/src/test/resources/tech/picnic/errorprone/refaster/test/TemplateWithoutTestTemplatesTestOutput.java @@ -6,7 +6,8 @@ boolean testStringIsEmpty() { return "foo".isEmpty(); } } -/* Did not encounter a test in `TemplateWithoutTestTemplatesTestInput` for the following template(s): -- AnotherTemplateWithoutTest -- TemplateWithoutTest -*/ +/* + * Did not encounter a test in `TemplateWithoutTestTemplatesTestInput` for the following template(s): + * - AnotherTemplateWithoutTest + * - TemplateWithoutTest + */