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 19, 2018
1 parent 1e7496c commit a3f3e96
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.

### CHANGED
* Allow parallel workspace builds in Eclipse with Spotbugs installed
* 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))

## 3.1.8 - 2018-10-16

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,118 @@
/*
* 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.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;

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.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 Logger LOGGER = Logger.getLogger(InefficientReplaceAll.class.getName());

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 final Pattern REGEX_CHARS_PATTERN = Pattern.compile("[" + Pattern.quote("-.\\[]{}()<>*+=?^$|") + "]");

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) {
LOGGER.log(Level.FINE, "------------------- Analyzing {0} ----------------", obj.getName());
super.visit(obj);
}

@Override
public void visit(Code obj) {
super.visit(obj);
bugAccumulator.reportAccumulatedBugs();

}

@Override
public void sawOpcode(int seen) {
LOGGER.log(Level.FINE, "Opcode: {0}", 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()) {
Optional<String> firstArgument = getFirstArgument();
if (firstArgument.isPresent() && !REGEX_CHARS_PATTERN.matcher(firstArgument.get()).find()) {
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 Optional<String> getFirstArgument() {
Object value = getStack().getStackItem(getNumberArguments(getMethodDescriptorOperand().getSignature()) - 1)
.getConstant();
return Optional.ofNullable(Objects.toString(value, null));
}

/**
* @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;
}
}
66 changes: 66 additions & 0 deletions spotbugsTestCases/src/java/ghIssues/Issue705.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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 replacement = "replacement";
String target = "target";
target = target.replaceAll(".", replacement);
target = target.replaceAll("\\", replacement);
target = target.replaceAll("[", replacement);
target = target.replaceAll("]", replacement);
target = target.replaceAll("{", replacement);
target = target.replaceAll("}", replacement);
target = target.replaceAll("(", replacement);
target = target.replaceAll(")", replacement);
target = target.replaceAll("<", replacement);
target = target.replaceAll(">", replacement);
target = target.replaceAll("*", replacement);
target = target.replaceAll("+", replacement);
target = target.replaceAll("-", replacement);
target = target.replaceAll("=", replacement);
target = target.replaceAll("?", replacement);
target = target.replaceAll("^", replacement);
target = target.replaceAll("$", replacement);
target = target.replaceAll("|", replacement);
return target;
}

public static String nonBugNull() {
String replaceable = null;
String replacement = "replacement";
String target = "target";
return target.replaceAll(replaceable, replacement);
}
}

0 comments on commit a3f3e96

Please sign in to comment.