From 83f8562dcd7aa311c9d0983ed9023b6079eabb78 Mon Sep 17 00:00:00 2001 From: Lasse Lindqvist Date: Thu, 8 Nov 2018 22:10:49 +0200 Subject: [PATCH] Implement issue #705 Implement new detector IRA_INEFFICIENT_REPLACEALL. Finds occurrences of replaceAll(String regex, String replacement) without any special regex characters. --- CHANGELOG.md | 1 + .../edu/umd/cs/findbugs/ba/Issue705Test.java | 41 +++++ spotbugs/etc/findbugs.xml | 4 + spotbugs/etc/messages.xml | 20 +++ .../detect/InefficientReplaceAll.java | 143 ++++++++++++++++++ .../src/java/ghIssues/Issue705.java | 42 +++++ 6 files changed, 251 insertions(+) create mode 100644 spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue705Test.java create mode 100644 spotbugs/src/main/java/edu/umd/cs/findbugs/detect/InefficientReplaceAll.java create mode 100644 spotbugsTestCases/src/java/ghIssues/Issue705.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b2872193fc3..3b8f4cff133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. * Fix some out-of-bounds reports from LGTM * Update asm to 7.0 for better Java 11 support ([#785](https://github.com/spotbugs/spotbugs/pull/785)) * Ignore @FXML annotated fields in UR\_UNIT\_READ ([#702](https://github.com/spotbugs/spotbugs/issues/702)) +* Add new detector IRA\_INEFFICIENT\_REPLACEALL for detecting usage of String.replaceAll where no regex is being used ([#705](https://github.com/spotbugs/spotbugs/issues/705)) ### CHANGED * Allow parallel workspace builds in Eclipse with Spotbugs installed diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue705Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue705Test.java new file mode 100644 index 00000000000..1f1b7dab67c --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue705Test.java @@ -0,0 +1,41 @@ +/* + * Contributions to SpotBugs + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package edu.umd.cs.findbugs.ba; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.junit.Assert.assertThat; + +import org.junit.Test; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.SortedBugCollection; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; + +public class Issue705Test extends AbstractIntegrationTest { + + @Test + public void test() { + performAnalysis("ghIssues/Issue705.class"); + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .bugType("IRA_INEFFICIENT_REPLACEALL").build(); + SortedBugCollection bugCollection = (SortedBugCollection) getBugCollection(); + assertThat(bugCollection, containsExactly(1, bugTypeMatcher)); + } + +} diff --git a/spotbugs/etc/findbugs.xml b/spotbugs/etc/findbugs.xml index 379674b7d65..bdb8c9b2854 100644 --- a/spotbugs/etc/findbugs.xml +++ b/spotbugs/etc/findbugs.xml @@ -524,6 +524,8 @@ reports="IIO_INEFFICIENT_INDEX_OF,IIO_INEFFICIENT_LAST_INDEX_OF"/> + + diff --git a/spotbugs/etc/messages.xml b/spotbugs/etc/messages.xml index ef00d7a374b..5351fec6a9c 100644 --- a/spotbugs/etc/messages.xml +++ b/spotbugs/etc/messages.xml @@ -1156,6 +1156,14 @@ A fast detector. using the toArray() method that takes a prototype array, passing an array argument which is zero-length.

+]]> + + + +
+ This detector finds occurrences of replaceAll(String regex, String replacement) without any special regex characters. +

]]>
@@ -6407,6 +6415,18 @@ If the array passed in is big enough to store all of the elements of the collection, then it is populated and returned directly. This avoids the need to create a second array (by reflection) to return as the result.

+]]> + + + + Method uses replaceAll(String regex, String replacement) without any special regex characters + {1} uses replaceAll(String regex, String replacement) without any special regex characters +
+ This method uses replaceAll(String regex, String replacement) without any special regex characters. +It is more efficient to use +myString.replace(CharSequence target, CharSequence replacement) +if there is no need to use regular expressions.

]]>
diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/InefficientReplaceAll.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/InefficientReplaceAll.java new file mode 100644 index 00000000000..63ca06860de --- /dev/null +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/InefficientReplaceAll.java @@ -0,0 +1,143 @@ +/* + * Contributions to SpotBugs + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package edu.umd.cs.findbugs.detect; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.bcel.Const; +import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.Method; + +import edu.umd.cs.findbugs.BugAccumulator; +import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugReporter; +import edu.umd.cs.findbugs.StatelessDetector; +import edu.umd.cs.findbugs.SystemProperties; +import edu.umd.cs.findbugs.ba.ClassContext; +import edu.umd.cs.findbugs.bcel.OpcodeStackDetector; +import edu.umd.cs.findbugs.classfile.MethodDescriptor; + +/** + * Find occurrences of replaceAll(String regex, String replacement) without any special regex characters. + * + * @author Lasse Lindqvist + */ +public class InefficientReplaceAll extends OpcodeStackDetector implements StatelessDetector { + private static final boolean DEBUG = SystemProperties.getBoolean("ira.debug"); + + private static final List methods = Collections.singletonList(new MethodDescriptor("", "replaceAll", + "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;")); + + private final BugAccumulator bugAccumulator; + + private static List regexCharList = new ArrayList<>(); + static { + regexCharList.add("."); + regexCharList.add("\\"); + regexCharList.add("["); + regexCharList.add("]"); + regexCharList.add("{"); + regexCharList.add("}"); + regexCharList.add("("); + regexCharList.add(")"); + regexCharList.add("<"); + regexCharList.add(">"); + regexCharList.add("*"); + regexCharList.add("+"); + regexCharList.add("-"); + regexCharList.add("="); + regexCharList.add("?"); + regexCharList.add("^"); + regexCharList.add("$"); + regexCharList.add("|"); + } + + public InefficientReplaceAll(BugReporter bugReporter) { + this.bugAccumulator = new BugAccumulator(bugReporter); + } + + @Override + public void visitClassContext(ClassContext classContext) { + if (hasInterestingMethod(classContext.getJavaClass().getConstantPool(), methods)) { + classContext.getJavaClass().accept(this); + } + } + + @Override + public void visit(Method obj) { + if (DEBUG) { + System.out.println("------------------- Analyzing " + obj.getName() + " ----------------"); + } + super.visit(obj); + } + + @Override + public void visit(Code obj) { + super.visit(obj); + bugAccumulator.reportAccumulatedBugs(); + + } + + @Override + public void sawOpcode(int seen) { + if (DEBUG) { + System.out.println("Opcode: " + Const.getOpcodeName(seen)); + } + if (((seen == Const.INVOKEVIRTUAL) || (seen == Const.INVOKEINTERFACE)) && ("replaceAll".equals(getNameConstantOperand())) + && ("(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;".equals(getSigConstantOperand())) + && hasConstantArguments()) { + String firstArgument = getFirstArgument(); + boolean found = false; + for (String regexChar : regexCharList) { + if (firstArgument.contains(String.valueOf(regexChar))) { + found = true; + break; + } + } + if (!found) { + bugAccumulator.accumulateBug( + new BugInstance(this, "IRA_INEFFICIENT_REPLACEALL", LOW_PRIORITY).addClassAndMethod(this), this); + } + } + } + + /** + * @return first argument of the called method if it's a constant + */ + private String getFirstArgument() { + Object value = getStack().getStackItem(getNumberArguments(getMethodDescriptorOperand().getSignature()) - 1) + .getConstant(); + return value == null ? null : value.toString(); + } + + /** + * @return true if only constants are passed to the called method + */ + private boolean hasConstantArguments() { + int nArgs = getNumberArguments(getMethodDescriptorOperand().getSignature()); + for (int i = 0; i < nArgs; i++) { + if (getStack().getStackItem(i).getConstant() == null) { + return false; + } + } + return true; + } +} + diff --git a/spotbugsTestCases/src/java/ghIssues/Issue705.java b/spotbugsTestCases/src/java/ghIssues/Issue705.java new file mode 100644 index 00000000000..341dd713f56 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/Issue705.java @@ -0,0 +1,42 @@ +/* + * Contributions to SpotBugs + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package ghIssues; + +public class Issue705 { + + public static String bug() { + String replaceable = "replaceable"; + String replacement = "replacement"; + String target = "target"; + return target.replaceAll(replaceable, replacement); + } + + public static String nonBug() { + String replaceable = "(replaceable)"; + String replacement = "replacement"; + String target = "target"; + return target.replaceAll(replaceable, replacement); + } + + public static String nonBug2() { + String replaceable = "(.*replaceable)"; + String replacement = "replacement"; + String target = "target"; + return target.replaceAll(replaceable, replacement); + } +}