Skip to content

Commit

Permalink
Apply suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
rickie committed Mar 28, 2022
1 parent 2033055 commit bb49a9e
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,14 +28,15 @@
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;
import java.util.ArrayList;
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;

Expand All @@ -50,8 +52,6 @@ public final class RefasterValidateTests extends BugChecker implements Compilati
private static final long serialVersionUID = 1L;
private static final Supplier<ImmutableListMultimap<String, CodeTransformer>>
ALL_CODE_TRANSFORMERS = Suppliers.memoize(CodeTransformers::loadAllCodeTransformers);
private static final Collector<CharSequence, ?, String> LIST_COLLECTOR =
joining("\n- ", "\n- ", "\n");

private final ImmutableSet<String> templateNamesFromClassPath;
private final RefasterCheck delegate;
Expand Down Expand Up @@ -87,13 +87,15 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
ImmutableRangeMap<Integer, String> matchesRangeMap =
buildRangeMapForMatches(matches, compilationUnit.endPositions);

ImmutableSet<String> templatesWithoutMatch = getTemplateNamesWithoutMatch(matches);
ImmutableSet<String> 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);
}

Expand All @@ -104,21 +106,21 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
return Description.NO_MATCH;
}

private ImmutableSet<String> getTemplateNamesWithoutMatch(List<Description> matches) {
ImmutableSet<String> templateNamesOfMatches =
matches.stream()
.map(description -> description.checkName)
.map(RefasterValidateTests::getNameFromFQCN)
.collect(toImmutableSet());

return Sets.difference(templateNamesFromClassPath, templateNamesOfMatches).immutableCopy();
private ImmutableSet<String> getTemplateNamesWithoutMatch(
ImmutableCollection<String> 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<String> 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<Integer, String> buildRangeMapForMatches(
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ final class RefasterCollectionTestUtilTest {
MatchInWrongMethodTemplates.class,
MethodNameWithNumberTemplates.class,
MissingTestAndWrongTestTemplates.class,
PartialTestMatchTemplates.class,
TemplateWithoutTestTemplates.class
})
void verifyRefasterTemplateCollections(Class<?> clazz) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*/
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Original file line number Diff line number Diff line change
@@ -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("");
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

0 comments on commit bb49a9e

Please sign in to comment.