Skip to content

Commit

Permalink
Implement issue spotbugs#705
Browse files Browse the repository at this point in the history
Implement new detector IRA_INEFFICIENT_REPLACEALL. Finds occurrences of replaceAll(String regex, String replacement) without any special regex characters.
  • Loading branch information
lasselindqvist committed Nov 10, 2018
1 parent 64fe232 commit 83f8562
Show file tree
Hide file tree
Showing 6 changed files with 251 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}

}
4 changes: 4 additions & 0 deletions spotbugs/etc/findbugs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@
reports="IIO_INEFFICIENT_INDEX_OF,IIO_INEFFICIENT_LAST_INDEX_OF"/>
<Detector class="edu.umd.cs.findbugs.detect.InefficientToArray" speed="fast" disabled="true"
reports="ITA_INEFFICIENT_TO_ARRAY"/>
<Detector class="edu.umd.cs.findbugs.detect.InefficientReplaceAll" speed="fast" disabled="true"
reports="IRA_INEFFICIENT_REPLACEALL"/>
<Detector class="edu.umd.cs.findbugs.detect.InvalidJUnitTest" speed="fast"
reports="IJU_SETUP_NO_SUPER,IJU_TEARDOWN_NO_SUPER,IJU_SUITE_NOT_STATIC,IJU_NO_TESTS,IJU_BAD_SUITE_METHOD"/>
<Detector class="edu.umd.cs.findbugs.detect.BadlyOverriddenAdapter" speed="fast"
Expand Down Expand Up @@ -1089,6 +1091,8 @@
experimental="true"/>
<BugPattern abbrev="ITA" type="ITA_INEFFICIENT_TO_ARRAY" category="PERFORMANCE"
experimental="true"/>
<BugPattern abbrev="IRA" type="IRA_INEFFICIENT_REPLACEALL" category="PERFORMANCE"
experimental="true"/>
<BugPattern abbrev="IJU" type="IJU_ASSERT_METHOD_INVOKED_FROM_RUN_METHOD"
category="CORRECTNESS"/>
<BugPattern abbrev="IJU" type="IJU_BAD_SUITE_METHOD" category="CORRECTNESS"/>
Expand Down
20 changes: 20 additions & 0 deletions spotbugs/etc/messages.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,14 @@ A fast detector.
using the toArray() method that takes a prototype array, passing
an array argument which is zero-length.
</p>
]]>
</Details>
</Detector>
<Detector class="edu.umd.cs.findbugs.detect.InefficientReplaceAll">
<Details>
<![CDATA[
<p> This detector finds occurrences of replaceAll(String regex, String replacement) without any special regex characters.
</p>
]]>
</Details>
</Detector>
Expand Down Expand Up @@ -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.</p>
]]>
</Details>
</BugPattern>
<BugPattern type="IRA_INEFFICIENT_REPLACEALL">
<ShortDescription>Method uses replaceAll(String regex, String replacement) without any special regex characters</ShortDescription>
<LongDescription>{1} uses replaceAll(String regex, String replacement) without any special regex characters</LongDescription>
<Details>
<![CDATA[
<p> This method uses replaceAll(String regex, String replacement) without any special regex characters.
It is more efficient to use
<code>myString.replace(CharSequence target, CharSequence replacement)</code>
if there is no need to use regular expressions.</p>
]]>
</Details>
</BugPattern>
Expand Down
Original file line number Diff line number Diff line change
@@ -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<MethodDescriptor> methods = Collections.singletonList(new MethodDescriptor("", "replaceAll",
"(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"));

private final BugAccumulator bugAccumulator;

private static List<String> 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;
}
}

42 changes: 42 additions & 0 deletions spotbugsTestCases/src/java/ghIssues/Issue705.java
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 83f8562

Please sign in to comment.