Bug 565695 - Cleanup for regex should not change 1-char non-regexps
- change PatternCleanUp to not create a pattern for a String split
if the pattern is a single-char non-meta character or is an
escaped character that is not alpha-numeric since the String
split() method fast-paths these without using patterns
Change-Id: I87af41904bc87f02cd8e4d4349221cb87f8cc058
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
index f549de6..8bf62be 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
@@ -7597,7 +7597,7 @@
+ " return dateText1 + dateText2;\n" //
+ " }\n" //
+ "\n" //
- + " public String usePatternForSplit(String speech1, String speech2) {\n" //
+ + " public String usePatternForSplit1(String speech1, String speech2) {\n" //
+ " // Keep this comment\n" //
+ " String line= \"\\\\r?\\\\n\";\n" //
+ "\n" //
@@ -7609,6 +7609,30 @@
+ " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ " }\n" //
+ "\n" //
+ + " public String usePatternForSplit2(String speech1, String speech2) {\n" //
+ + " // Keep this comment\n" //
+ + " String line= \".\";\n" //
+ + "\n" //
+ + " // Keep this comment too\n" //
+ + " String[] phrases1= speech1.split(line);\n" //
+ + " // Keep this comment also\n" //
+ + " String[] phrases2= speech2.split(line, 123);\n" //
+ + "\n" //
+ + " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String usePatternForSplit3(String speech1, String speech2) {\n" //
+ + " // Keep this comment\n" //
+ + " String line= \"\\\\a\";\n" //
+ + "\n" //
+ + " // Keep this comment too\n" //
+ + " String[] phrases1= speech1.split(line);\n" //
+ + " // Keep this comment also\n" //
+ + " String[] phrases2= speech2.split(line, 123);\n" //
+ + "\n" //
+ + " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ + " }\n" //
+ + "\n" //
+ " public String usePatternForLocalVariableOnly(String date1, String date2, String date3) {\n" //
+ " String dateText1= date1.replaceFirst(dateValidation, \"0000-00-00\");\n" //
+ " // Keep this comment\n" //
@@ -7672,7 +7696,7 @@
+ " return dateText1 + dateText2;\n" //
+ " }\n" //
+ "\n" //
- + " public String usePatternForSplit(String speech1, String speech2) {\n" //
+ + " public String usePatternForSplit1(String speech1, String speech2) {\n" //
+ " // Keep this comment\n" //
+ " Pattern line= Pattern.compile(\"\\\\r?\\\\n\");\n" //
+ "\n" //
@@ -7684,6 +7708,30 @@
+ " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ " }\n" //
+ "\n" //
+ + " public String usePatternForSplit2(String speech1, String speech2) {\n" //
+ + " // Keep this comment\n" //
+ + " Pattern line= Pattern.compile(\".\");\n" //
+ + "\n" //
+ + " // Keep this comment too\n" //
+ + " String[] phrases1= line.split(speech1);\n" //
+ + " // Keep this comment also\n" //
+ + " String[] phrases2= line.split(speech2, 123);\n" //
+ + "\n" //
+ + " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public String usePatternForSplit3(String speech1, String speech2) {\n" //
+ + " // Keep this comment\n" //
+ + " Pattern line= Pattern.compile(\"\\\\a\");\n" //
+ + "\n" //
+ + " // Keep this comment too\n" //
+ + " String[] phrases1= line.split(speech1);\n" //
+ + " // Keep this comment also\n" //
+ + " String[] phrases2= line.split(speech2, 123);\n" //
+ + "\n" //
+ + " return Arrays.toString(phrases1) + Arrays.toString(phrases2);\n" //
+ + " }\n" //
+ + "\n" //
+ " public String usePatternForLocalVariableOnly(String date1, String date2, String date3) {\n" //
+ " String dateText1= date1.replaceFirst(dateValidation, \"0000-00-00\");\n" //
+ " // Keep this comment\n" //
@@ -7758,6 +7806,20 @@
+ "\n" //
+ " return dateText1 + dateText2;\n" //
+ " }\n" //
+ + " public String doNotUsePatternOnSimpleSplit1(String speech1, String speech2) {\n" //
+ + " String line= \"a\";\n" //
+ + "\n" //
+ + " String[] phrases1= speech1.split(line);\n" //
+ + " String[] phrases2= speech2.split(line, 1);\n" //
+ + " return phrases1[0] + phrases2[0];\n" //
+ + " }\n" //
+ + " public String doNotUsePatternOnSimpleSplit2(String speech1, String speech2) {\n" //
+ + " String line= \"\\\\;\";\n" //
+ + "\n" //
+ + " String[] phrases1= speech1.split(line1);\n" //
+ + " String[] phrases2= speech2.split(line1, 1);\n" //
+ + " return phrases1[0] + phrases2[0];\n" //
+ + " }\n" //
+ "}\n";
ICompilationUnit cu= pack1.createCompilationUnit("E1.java", sample, false, null);
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/PatternCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/PatternCleanUp.java
index 04e54ad..fb961c5 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/PatternCleanUp.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/PatternCleanUp.java
@@ -25,15 +25,18 @@
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.dom.AST;
+import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.ASTVisitor;
import org.eclipse.jdt.core.dom.Block;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.Expression;
+import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
+import org.eclipse.jdt.core.dom.StringLiteral;
import org.eclipse.jdt.core.dom.Type;
import org.eclipse.jdt.core.dom.VariableDeclarationExpression;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
@@ -186,7 +189,7 @@
if (writes.size() == 1 && reads.size() > 1) {
for (SimpleName simpleName : reads) {
- if (!isRegExUse(simpleName)) {
+ if (!isRegExUse(simpleName, initializer)) {
return true;
}
}
@@ -200,7 +203,7 @@
return true;
}
- private boolean isRegExUse(final SimpleName simpleName) {
+ private boolean isRegExUse(final SimpleName simpleName, final Expression initializer) {
if (simpleName.getParent() instanceof MethodInvocation) {
MethodInvocation methodInvocation= (MethodInvocation) simpleName.getParent();
@@ -209,9 +212,48 @@
&& simpleName.equals(methodInvocation.arguments().get(0))) {
if (ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, MATCHES_METHOD, STRING_CLASS_NAME)
|| ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, REPLACE_ALL_METHOD, STRING_CLASS_NAME, STRING_CLASS_NAME)
- || ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, REPLACE_FIRST_METHOD, STRING_CLASS_NAME, STRING_CLASS_NAME)
- || ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, SPLIT_METHOD, STRING_CLASS_NAME)
+ || ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, REPLACE_FIRST_METHOD, STRING_CLASS_NAME, STRING_CLASS_NAME)) {
+ return true;
+ } else if (ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, SPLIT_METHOD, STRING_CLASS_NAME)
|| ASTNodes.usesGivenSignature(methodInvocation, STRING_CLASS_NAME, SPLIT_METHOD, STRING_CLASS_NAME, int.class.getCanonicalName())) {
+ String value= null;
+ Expression expression= ASTNodes.getUnparenthesedExpression(initializer);
+
+ if (expression instanceof StringLiteral) {
+ StringLiteral literal= (StringLiteral) expression;
+ value= literal.getLiteralValue();
+ } else if (expression instanceof Name) {
+ Name name= (Name) expression;
+ IBinding binding= name.resolveBinding();
+
+ if (binding instanceof IVariableBinding) {
+ ASTNode aSTNode= ASTNodes.findDeclaration(binding, startNode);
+
+ if (aSTNode instanceof VariableDeclarationFragment) {
+ Expression exp= ((VariableDeclarationFragment) aSTNode).getInitializer();
+
+ if (exp != null) {
+ exp= ASTNodes.getUnparenthesedExpression(exp);
+
+ if (exp instanceof StringLiteral) {
+ value= ((StringLiteral) exp).getLiteralValue();
+ }
+ }
+ }
+ }
+ }
+
+ if (value != null) {
+ // Don't compile pattern for single character that doesn't use regex meta-chars
+ // and don't compile pattern for double character where first char is back-slash
+ // and second character isn't alpha-numeric
+ if (value.length() == 1) {
+ return ".$|()[{^?*+\\".contains(value); //$NON-NLS-1$
+ } else if (value.length() == 2 && value.charAt(0) == '\\') {
+ return Character.isLetterOrDigit(value.charAt(1));
+ }
+ }
+
return true;
}
}
