From 3e14f54f8e3951e337bc5cf65ee49927c9c6d18f Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Tue, 23 Mar 2021 16:07:34 -0700 Subject: [PATCH] Generalize `ConstantPatternCompile` PiperOrigin-RevId: 364667499 --- .../bugpatterns/ConstantPatternCompile.java | 103 ++++++++---------- 1 file changed, 47 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java index 6946b896bc7..279843e5c28 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java @@ -16,33 +16,27 @@ package com.google.errorprone.bugpatterns; +import static com.google.common.base.CaseFormat.LOWER_CAMEL; +import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.fixes.SuggestedFixes.renameVariableUsages; +import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.staticMethod; -import static com.google.errorprone.util.ASTHelpers.getStartPosition; -import static com.google.errorprone.util.ASTHelpers.getType; -import com.google.common.base.CaseFormat; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; -import com.google.errorprone.matchers.Matchers; -import com.google.errorprone.suppliers.Supplier; -import com.google.errorprone.suppliers.Suppliers; import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; -import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.MethodSymbol; -import com.sun.tools.javac.code.Type; import javax.lang.model.element.Modifier; import javax.lang.model.element.NestingKind; @@ -58,64 +52,61 @@ severity = SUGGESTION) public final class ConstantPatternCompile extends BugChecker implements VariableTreeMatcher { - private static final String PATTERN_CLASS = "java.util.regex.Pattern"; - private static final Supplier PATTERN_TYPE = Suppliers.typeFromString(PATTERN_CLASS); - private static final Matcher PATTERN_COMPILE_CHECK = - Matchers.anyOf(staticMethod().onClass(PATTERN_CLASS).named("compile")); + staticMethod() + .onClassAny( + "java.util.regex.Pattern" + ) + .named("compile"); @Override public Description matchVariable(VariableTree tree, VisitorState state) { - Tree parent = state.getPath().getParentPath().getLeaf(); - if (parent instanceof ClassTree) { - return Description.NO_MATCH; - } - if (!ASTHelpers.isSameType(getType(tree.getType()), PATTERN_TYPE.get(state), state)) { - return Description.NO_MATCH; - } - if (!(tree.getInitializer() instanceof MethodInvocationTree)) { - return Description.NO_MATCH; + ExpressionTree initializer = tree.getInitializer(); + if (!PATTERN_COMPILE_CHECK.matches(initializer, state)) { + return NO_MATCH; } - MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree.getInitializer(); - if (!PATTERN_COMPILE_CHECK.matches(methodInvocationTree, state)) { - return Description.NO_MATCH; - } - if (!methodInvocationTree.getArguments().stream() - .allMatch(ConstantPatternCompile::isArgStaticAndConstant)) { - return Description.NO_MATCH; + if (!((MethodInvocationTree) initializer) + .getArguments().stream().allMatch(ConstantPatternCompile::isArgStaticAndConstant)) { + return NO_MATCH; } MethodTree outerMethodTree = ASTHelpers.findEnclosingNode(state.getPath(), MethodTree.class); if (outerMethodTree == null) { - return Description.NO_MATCH; + return NO_MATCH; + } + Symbol sym = ASTHelpers.getSymbol(tree); + if (sym == null) { + return NO_MATCH; + } + switch (sym.getKind()) { + case RESOURCE_VARIABLE: + return describeMatch(tree); + case LOCAL_VARIABLE: + return describeMatch(tree, fixLocal(tree, outerMethodTree, state)); + default: + return NO_MATCH; } - MethodSymbol sym = ASTHelpers.getSymbol(outerMethodTree); + } + + private static SuggestedFix fixLocal( + VariableTree tree, MethodTree outerMethodTree, VisitorState state) { + MethodSymbol methodSymbol = ASTHelpers.getSymbol(outerMethodTree); boolean canUseStatic = - (sym != null && sym.owner.enclClass().getNestingKind() == NestingKind.TOP_LEVEL) + (methodSymbol != null + && methodSymbol.owner.enclClass().getNestingKind() == NestingKind.TOP_LEVEL) || outerMethodTree.getModifiers().getFlags().contains(Modifier.STATIC); - - String originalVariableTreeString = state.getSourceForNode(tree); - String varName = tree.getName().toString(); - String upperUnderscoreVarName = CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, varName); - - int typeEndPos = state.getEndPosition(tree.getType()); - // handle implicit lambda parameter types - int searchOffset = typeEndPos == -1 ? 0 : (typeEndPos - getStartPosition(tree)); - int pos = state.getSourceForNode(tree).indexOf(varName, searchOffset); - String variableReplacedString = - new StringBuilder(originalVariableTreeString) - .replace(pos, pos + varName.length(), upperUnderscoreVarName) - .toString(); - - String modifiers = "private " + (canUseStatic ? "static " : "") + "final "; - String variableTreeString = "\n" + modifiers + variableReplacedString + "\n"; - - SuggestedFix fix = - SuggestedFix.builder() - .merge(renameVariableUsages(tree, upperUnderscoreVarName, state)) - .postfixWith(outerMethodTree, variableTreeString) - .delete(tree) - .build(); - return describeMatch(tree, fix); + String newName = LOWER_CAMEL.to(UPPER_UNDERSCORE, tree.getName().toString()); + String replacement = + String.format( + "private %s final %s %s = %s;", + canUseStatic ? "static " : "", + state.getSourceForNode(tree.getType()), + newName, + state.getSourceForNode(tree.getInitializer())); + return SuggestedFix.builder() + .merge(renameVariableUsages(tree, newName, state)) + .postfixWith(outerMethodTree, replacement) + .delete(tree) + .build(); } private static boolean isArgStaticAndConstant(ExpressionTree arg) {