From 9e8258716abebba049ac32d731d144c5f2b01225 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Mon, 12 Sep 2022 11:26:19 +0200 Subject: [PATCH] Cleanup PR --- pom.xml | 5 +- refaster-rule-selector/pom.xml | 5 + .../selector/DefaultRuleSelectorFactory.java | 4 +- .../selector/SmartRefasterRuleSelector.java | 16 ++ .../errorprone/refaster/runner/Refaster.java | 210 ------------------ .../errorprone/refaster/runner/Util.java | 102 --------- 6 files changed, 25 insertions(+), 317 deletions(-) delete mode 100644 refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Util.java diff --git a/pom.xml b/pom.xml index 1e2f2cdb85..98ee437365 100644 --- a/pom.xml +++ b/pom.xml @@ -145,7 +145,7 @@ 1.0.1 1.9 ${version.error-prone-orig} - v2.14.0-picnic-3 + v${version.error-prone-orig}-picnic-3 2.14.0 0.1.15 1.0 @@ -1572,9 +1572,6 @@ -XepOpt:NullAway:AssertsEnabled=true -XepOpt:NullAway:CheckOptionalEmptiness=true -XepOpt:Nullness:Conservative=false - - -Xep:VoidMissingNullable:OFF - -Xep:ReturnMissingNullable:OFF ${error-prone.patch-args} ${error-prone.self-check-args} diff --git a/refaster-rule-selector/pom.xml b/refaster-rule-selector/pom.xml index 171620ea3f..11b14df85a 100644 --- a/refaster-rule-selector/pom.xml +++ b/refaster-rule-selector/pom.xml @@ -35,6 +35,11 @@ auto-value-annotations ${version.auto-value} + + com.google.code.findbugs + jsr305 + provided + com.google.guava guava diff --git a/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/DefaultRuleSelectorFactory.java b/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/DefaultRuleSelectorFactory.java index 1931e03b22..4d2a25742a 100644 --- a/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/DefaultRuleSelectorFactory.java +++ b/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/DefaultRuleSelectorFactory.java @@ -36,7 +36,9 @@ public boolean isClassPathCompatible(ClassLoader classLoader) { Class.forName( "com.google.errorprone.ErrorProneOptions", /* initialize= */ false, classLoader); } catch (ClassNotFoundException e) { - throw new IllegalStateException("Cannot load `com.google.errorprone.ErrorProneOptions`", e); + return false; + // throw new IllegalStateException("Cannot load + // `com.google.errorprone.ErrorProneOptions`", e); } return Arrays.stream(clazz.getDeclaredMethods()) diff --git a/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/SmartRefasterRuleSelector.java b/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/SmartRefasterRuleSelector.java index 6d90241a67..71cb0c226d 100644 --- a/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/SmartRefasterRuleSelector.java +++ b/refaster-rule-selector/src/main/java/tech/picnic/errorprone/rule/selector/SmartRefasterRuleSelector.java @@ -32,6 +32,7 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Set; +import javax.annotation.Nullable; /** XXX: Write this */ @AutoService(RefasterRuleSelector.class) @@ -88,6 +89,7 @@ private static ImmutableSet> extractTemplateIdentifie // XXX: Make the scanner static, then make also its helper methods static. new TreeScanner>>() { + @Nullable @Override public Void visitIdentifier(IdentifierTree node, List> identifierCombinations) { // XXX: Also include the package name if not `java.lang`; it must be present. @@ -117,6 +119,7 @@ private String getSimpleName(String fcqn) { return index < 0 ? fcqn : fcqn.substring(index + 1); } + @Nullable @Override public Void visitMemberReference( MemberReferenceTree node, List> identifierCombinations) { @@ -126,6 +129,7 @@ public Void visitMemberReference( return null; } + @Nullable @Override public Void visitMemberSelect( MemberSelectTree node, List> identifierCombinations) { @@ -135,12 +139,14 @@ public Void visitMemberSelect( return null; } + @Nullable @Override public Void visitAssignment(AssignmentTree node, List> identifierCombinations) { registerOperator(node, identifierCombinations); return super.visitAssignment(node, identifierCombinations); } + @Nullable @Override public Void visitCompoundAssignment( CompoundAssignmentTree node, List> identifierCombinations) { @@ -148,12 +154,14 @@ public Void visitCompoundAssignment( return super.visitCompoundAssignment(node, identifierCombinations); } + @Nullable @Override public Void visitUnary(UnaryTree node, List> identifierCombinations) { registerOperator(node, identifierCombinations); return super.visitUnary(node, identifierCombinations); } + @Nullable @Override public Void visitBinary(BinaryTree node, List> identifierCombinations) { registerOperator(node, identifierCombinations); @@ -165,6 +173,7 @@ private void registerOperator(ExpressionTree node, List> identifierC identifierCombinations.forEach(ids -> ids.add(treeKindToString(node.getKind()))); } + @Nullable @Override public Void visitOther(Tree node, List> identifierCombinations) { if (node instanceof UAnyOf) { @@ -199,12 +208,14 @@ private ImmutableSortedSet extractSourceIdentifiers(Tree tree) { // XXX: Make the scanner static. new TreeScanner>() { + @Nullable @Override public Void visitIdentifier(IdentifierTree node, Set identifiers) { identifiers.add(node.getName().toString()); return null; } + @Nullable @Override public Void visitMemberReference(MemberReferenceTree node, Set identifiers) { super.visitMemberReference(node, identifiers); @@ -212,6 +223,7 @@ public Void visitMemberReference(MemberReferenceTree node, Set identifie return null; } + @Nullable @Override public Void visitMemberSelect(MemberSelectTree node, Set identifiers) { super.visitMemberSelect(node, identifiers); @@ -219,24 +231,28 @@ public Void visitMemberSelect(MemberSelectTree node, Set identifiers) { return null; } + @Nullable @Override public Void visitAssignment(AssignmentTree node, Set identifiers) { registerOperator(node, identifiers); return super.visitAssignment(node, identifiers); } + @Nullable @Override public Void visitCompoundAssignment(CompoundAssignmentTree node, Set identifiers) { registerOperator(node, identifiers); return super.visitCompoundAssignment(node, identifiers); } + @Nullable @Override public Void visitUnary(UnaryTree node, Set identifiers) { registerOperator(node, identifiers); return super.visitUnary(node, identifiers); } + @Nullable @Override public Void visitBinary(BinaryTree node, Set identifiers) { registerOperator(node, identifiers); diff --git a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java index 224d3dc9d1..ccc47de156 100644 --- a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java +++ b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java @@ -79,11 +79,7 @@ public Refaster(ErrorProneFlags flags) { @CanIgnoreReturnValue @Override - @SuppressWarnings("UnusedVariable") public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) { - // XXX: Inline this variable. - // Set> candidateRules = getCandidateRefasterRules(tree); - DefaultRuleSelectorFactory ruleSelectorFactory = new DefaultRuleSelectorFactory(); RefasterRuleSelector selector = ruleSelectorFactory.createRefasterRuleSelector( @@ -112,15 +108,6 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s return Description.NO_MATCH; } - // XXX: Here and below: drop redundant `Refaster` from method names? - // private Set> getCandidateRefasterRules(CompilationUnitTree tree) { - // Set> candidateRules = newSetFromMap(new IdentityHashMap<>()); - // refasterRules.collectCandidateTemplates( - // extractSourceIdentifiers(tree).asList(), candidateRules::add); - // - // return candidateRules; - // } - private static List> getRefasterRules(ErrorProneFlags flags) { CodeTransformer compositeCodeTransformer = createCompositeCodeTransformer(flags); @@ -215,201 +202,4 @@ private static ImmutableList filterCodeTransformers( .map(Map.Entry::getValue) .collect(toImmutableList()); } - - // XXX: Decompose `RefasterRule`s such that each has exactly one `@BeforeTemplate`. - // private static ImmutableSet> extractTemplateIdentifiers( - // RefasterRule refasterRule) { - // ImmutableSet.Builder> results = ImmutableSet.builder(); - // - // for (Object template : refasterRule.beforeTemplates()) { - // if (template instanceof ExpressionTemplate) { - // UExpression expr = ((ExpressionTemplate) template).expression(); - // results.addAll(extractTemplateIdentifiers(ImmutableList.of(expr))); - // } else if (template instanceof BlockTemplate) { - // ImmutableList statements = ((BlockTemplate) template).templateStatements(); - // results.addAll(extractTemplateIdentifiers(statements)); - // } else { - // throw new IllegalStateException( - // String.format("Unexpected template type '%s'", template.getClass())); - // } - // } - // - // return results.build(); - // } - // - // // XXX: Consider interning the strings (once a benchmark is in place). - // private static ImmutableSet> extractTemplateIdentifiers( - // ImmutableList trees) { - // List> identifierCombinations = new ArrayList<>(); - // identifierCombinations.add(new HashSet<>()); - // - // // XXX: Make the scanner static, then make also its helper methods static. - // new TreeScanner>>() { - // @Override - // public Void visitIdentifier(IdentifierTree node, List> identifierCombinations) - // { - // // XXX: Also include the package name if not `java.lang`; it must be present. - // if (node instanceof UClassIdent) { - // for (Set ids : identifierCombinations) { - // ids.add(getSimpleName(((UClassIdent) node).getTopLevelClass())); - // ids.add(getIdentifier(node)); - // } - // } else if (node instanceof UStaticIdent) { - // UClassIdent subNode = ((UStaticIdent) node).classIdent(); - // for (Set ids : identifierCombinations) { - // ids.add(getSimpleName(subNode.getTopLevelClass())); - // ids.add(getIdentifier(subNode)); - // ids.add(node.getName().toString()); - // } - // } - // - // return null; - // } - // - // private String getIdentifier(IdentifierTree tree) { - // return getSimpleName(tree.getName().toString()); - // } - // - // private String getSimpleName(String fcqn) { - // int index = fcqn.lastIndexOf('.'); - // return index < 0 ? fcqn : fcqn.substring(index + 1); - // } - // - // @Override - // public Void visitMemberReference( - // MemberReferenceTree node, List> identifierCombinations) { - // super.visitMemberReference(node, identifierCombinations); - // String id = node.getName().toString(); - // identifierCombinations.forEach(ids -> ids.add(id)); - // return null; - // } - // - // @Override - // public Void visitMemberSelect( - // MemberSelectTree node, List> identifierCombinations) { - // super.visitMemberSelect(node, identifierCombinations); - // String id = node.getIdentifier().toString(); - // identifierCombinations.forEach(ids -> ids.add(id)); - // return null; - // } - // - // @Override - // public Void visitAssignment(AssignmentTree node, List> identifierCombinations) - // { - // registerOperator(node, identifierCombinations); - // return super.visitAssignment(node, identifierCombinations); - // } - // - // @Override - // public Void visitCompoundAssignment( - // CompoundAssignmentTree node, List> identifierCombinations) { - // registerOperator(node, identifierCombinations); - // return super.visitCompoundAssignment(node, identifierCombinations); - // } - // - // @Override - // public Void visitUnary(UnaryTree node, List> identifierCombinations) { - // registerOperator(node, identifierCombinations); - // return super.visitUnary(node, identifierCombinations); - // } - // - // @Override - // public Void visitBinary(BinaryTree node, List> identifierCombinations) { - // registerOperator(node, identifierCombinations); - // return super.visitBinary(node, identifierCombinations); - // } - // - // // XXX: Rename! - // private void registerOperator(ExpressionTree node, List> - // identifierCombinations) { - // identifierCombinations.forEach(ids -> ids.add(Util.treeKindToString(node.getKind()))); - // } - // - // @Override - // public Void visitOther(Tree node, List> identifierCombinations) { - // if (node instanceof UAnyOf) { - // List> base = copy(identifierCombinations); - // identifierCombinations.clear(); - // - // for (UExpression expr : ((UAnyOf) node).expressions()) { - // List> branch = copy(base); - // scan(expr, branch); - // identifierCombinations.addAll(branch); - // } - // } - // - // return null; - // } - // - // private List> copy(List> identifierCombinations) { - // return identifierCombinations.stream() - // .map(HashSet::new) - // .collect(toCollection(ArrayList::new)); - // } - // }.scan(trees, identifierCombinations); - // - // return identifierCombinations.stream() - // .map(ImmutableSortedSet::copyOf) - // .collect(toImmutableSet()); - // } - // - // // XXX: Consider interning! - // private static ImmutableSortedSet extractSourceIdentifiers(Tree tree) { - // Set identifiers = new HashSet<>(); - // - // // XXX: Make the scanner static. - // new TreeScanner>() { - // @Override - // public Void visitIdentifier(IdentifierTree node, Set identifiers) { - // identifiers.add(node.getName().toString()); - // return null; - // } - // - // @Override - // public Void visitMemberReference(MemberReferenceTree node, Set identifiers) { - // super.visitMemberReference(node, identifiers); - // identifiers.add(node.getName().toString()); - // return null; - // } - // - // @Override - // public Void visitMemberSelect(MemberSelectTree node, Set identifiers) { - // super.visitMemberSelect(node, identifiers); - // identifiers.add(node.getIdentifier().toString()); - // return null; - // } - // - // @Override - // public Void visitAssignment(AssignmentTree node, Set identifiers) { - // registerOperator(node, identifiers); - // return super.visitAssignment(node, identifiers); - // } - // - // @Override - // public Void visitCompoundAssignment(CompoundAssignmentTree node, Set identifiers) - // { - // registerOperator(node, identifiers); - // return super.visitCompoundAssignment(node, identifiers); - // } - // - // @Override - // public Void visitUnary(UnaryTree node, Set identifiers) { - // registerOperator(node, identifiers); - // return super.visitUnary(node, identifiers); - // } - // - // @Override - // public Void visitBinary(BinaryTree node, Set identifiers) { - // registerOperator(node, identifiers); - // return super.visitBinary(node, identifiers); - // } - // - // // XXX: Rename! - // private void registerOperator(ExpressionTree node, Set identifiers) { - // identifiers.add(Util.treeKindToString(node.getKind())); - // } - // }.scan(tree, identifiers); - // - // return ImmutableSortedSet.copyOf(identifiers); - // } } diff --git a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Util.java b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Util.java deleted file mode 100644 index ff0295f9b8..0000000000 --- a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Util.java +++ /dev/null @@ -1,102 +0,0 @@ -package tech.picnic.errorprone.refaster.runner; - -import com.sun.source.tree.Tree; - -// XXX: Drop this in favor of adding it to the class where it's needed? -/** Fix a comment here later. */ -public final class Util { - private Util() {} - - /** - * Returns a unique string representation of the given {@link Tree.Kind}. - * - * @return A string representation of the operator, if known - * @throws IllegalArgumentException If the given input is not supported. - */ - // XXX: Extend list to cover remaining cases; at least for any `Kind` that may appear in a - // Refaster template. - static String treeKindToString(Tree.Kind kind) { - switch (kind) { - case ASSIGNMENT: - return "="; - case POSTFIX_INCREMENT: - return "x++"; - case PREFIX_INCREMENT: - return "++x"; - case POSTFIX_DECREMENT: - return "x--"; - case PREFIX_DECREMENT: - return "--x"; - case UNARY_PLUS: - return "+x"; - case UNARY_MINUS: - return "-x"; - case BITWISE_COMPLEMENT: - return "~"; - case LOGICAL_COMPLEMENT: - return "!"; - case MULTIPLY: - return "*"; - case DIVIDE: - return "/"; - case REMAINDER: - return "%"; - case PLUS: - return "+"; - case MINUS: - return "-"; - case LEFT_SHIFT: - return "<<"; - case RIGHT_SHIFT: - return ">>"; - case UNSIGNED_RIGHT_SHIFT: - return ">>>"; - case LESS_THAN: - return "<"; - case GREATER_THAN: - return ">"; - case LESS_THAN_EQUAL: - return "<="; - case GREATER_THAN_EQUAL: - return ">="; - case EQUAL_TO: - return "=="; - case NOT_EQUAL_TO: - return "!="; - case AND: - return "&"; - case XOR: - return "^"; - case OR: - return "|"; - case CONDITIONAL_AND: - return "&&"; - case CONDITIONAL_OR: - return "||"; - case MULTIPLY_ASSIGNMENT: - return "*="; - case DIVIDE_ASSIGNMENT: - return "/="; - case REMAINDER_ASSIGNMENT: - return "%="; - case PLUS_ASSIGNMENT: - return "+="; - case MINUS_ASSIGNMENT: - return "-="; - case LEFT_SHIFT_ASSIGNMENT: - return "<<="; - case RIGHT_SHIFT_ASSIGNMENT: - return ">>="; - case UNSIGNED_RIGHT_SHIFT_ASSIGNMENT: - return ">>>="; - case AND_ASSIGNMENT: - return "&="; - case XOR_ASSIGNMENT: - return "^="; - case OR_ASSIGNMENT: - return "|="; - default: - throw new IllegalStateException("Cannot convert Tree.Kind to a String: " + kind); - } - } -}