hiifong/pmd
1
0 Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Fixed handling of escape characters in UseIndexOfChar and AppendCharacterWithChar

Browse Source 
problems discovered while refactoring detection of single char in String
literals, now done by isSingleCharacterStringLiteral() method in ASTLiteral.


git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@6421 51baf565-9d33-0410-a72c-fc3788e3496d
This commit is contained in:
Xavier Le Vourch committed 2008-08-28 20:25:16 +00:00
1 parent 742beba5b3
commit 8f4f95ddff
7 files changed
+193 -85
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

pmd/etc/changelog.txt
+1
View File
@@ -371,6 +371,7 @@ Java grammar enhanced to include AnnotationMethodDeclaration as parent node of m
JavaCC generated artifacts updated to JavaCC 4.1d1.
All comment types are now stored in ASTCompilationUnit, not just formal ones
Fixed false negative in UselessOverridingMethod
Fixed handling of escape characters in UseIndexOfChar and AppendCharacterWithChar
New rules:
pmd/regress/test/net/sourceforge/pmd/lang/java/rule/strings/xml/AppendCharacterWithChar.xml
+47
View File
@@ -95,6 +95,26 @@ public class Foo {
</test-code>
<test-code>
<description><![CDATA[
concatenates all escaped characters
]]></description>
<expected-problems>8</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(StringBuffer sb) {
sb.append("\n");
sb.append("\t");
sb.append("\b");
sb.append("\r");
sb.append("\f");
sb.append("\\");
sb.append("\'");
sb.append("\"");
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
concatenates a single upper case
]]></description>
<expected-problems>1</expected-problems>
@@ -142,6 +162,33 @@ public class Foo {
public void bar(StringBuffer sb) {
sb.append("/t");
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
a single octal character
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(StringBuffer sb) {
sb.append("\12");
sb.append("\123");
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
octal character in longer string
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(StringBuffer sb) {
sb.append("\1234");
}
}
]]></code>
</test-code>
pmd/regress/test/net/sourceforge/pmd/lang/java/rule/strings/xml/UseIndexOfChar.xml
+48
View File
@@ -81,6 +81,54 @@ public class Foo {
String x = "hello world";
if (x.indexOf("e" + "o") == -1) {}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
all escaped characters
]]></description>
<expected-problems>8</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(String x) {
if (x.indexOf("\n") == -1) {}
if (x.indexOf("\t") == -1) {}
if (x.indexOf("\b") == -1) {}
if (x.indexOf("\r") == -1) {}
if (x.indexOf("\f") == -1) {}
if (x.indexOf("\\") == -1) {}
if (x.indexOf("\'") == -1) {}
if (x.indexOf("\"") == -1) {}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
a single octal character
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(String x) {
if (x.indexOf("\12") == -1) {}
if (x.indexOf("\123") == -1) {}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
octal character in longer string
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(String x) {
if (x.indexOf("\1234") == -1) {}
}
}
]]></code>
</test-code>
pmd/src/net/sourceforge/pmd/lang/java/ast/ASTLiteral.java
+48 -15
View File
@@ -2,13 +2,15 @@
package net.sourceforge.pmd.lang.java.ast;
import java.util.regex.Pattern;
public class ASTLiteral extends AbstractJavaTypeNode {
private boolean isInt;
private boolean isFloat;
private boolean isChar;
private boolean isString;
private boolean isInt;
private boolean isFloat;
private boolean isChar;
private boolean isString;
public ASTLiteral(int id) {
super(id);
}
@@ -20,35 +22,66 @@ public class ASTLiteral extends AbstractJavaTypeNode {
/**
* Accept the visitor. *
*/
@Override
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}
public void setIntLiteral() {
this.isInt = true;
this.isInt = true;
}
public boolean isIntLiteral() {
return isInt;
return isInt;
}
public void setFloatLiteral() {
this.isFloat = true;
this.isFloat = true;
}
public boolean isFloatLiteral() {
return isFloat;
return isFloat;
}
public void setCharLiteral() {
this.isChar = true;
this.isChar = true;
}
public boolean isCharLiteral() {
return isChar;
return isChar;
}
public void setStringLiteral() {
this.isString = true;
this.isString = true;
}
public boolean isStringLiteral() {
return isString;
return isString;
}
/**
* Returns true if this is a String literal with only one character.
* Handles octal and escape characters.
*
* @return true is this is a String literal with only one character
*/
public boolean isSingleCharacterStringLiteral() {
if (isString) {
String image = getImage();
int length = image.length();
if (length == 3) {
return true;
} else if (image.charAt(1) == '\\') {
return SINGLE_CHAR_ESCAPE_PATTERN.matcher(image).matches();
}
}
return false;
}
/**
* Pattern used to detect a single escaped character or octal character in a String.
*/
private static final Pattern SINGLE_CHAR_ESCAPE_PATTERN = Pattern
.compile("^\"\\\\(([ntbrf\\\\'\\\"])|([0-7][0-7]?)|([0-3][0-7][0-7]))\"");
}
pmd/src/net/sourceforge/pmd/lang/java/rule/AbstractPoorMethodCall.java
+39 -50
View File
@@ -15,13 +15,14 @@ import net.sourceforge.pmd.lang.java.symboltable.NameOccurrence;
* efficient/modern ways of implementing the same function.
*
* Concrete subclasses are expected to provide the name of the target class and an
* array of method names that we are looking for. We then pass judgement on any literal
* array of method names that we are looking for. We then pass judgment on any literal
* arguments we find in the subclass as well.
*
* @author Brian Remedios
* @version $Revision$
*/
public abstract class AbstractPoorMethodCall extends AbstractJavaRule {
//FIXME not sure the abstraction is generic enough to be reused as is.
/**
* The name of the type the method will be invoked against.
@@ -38,15 +39,13 @@ public abstract class AbstractPoorMethodCall extends AbstractJavaRule {
protected abstract String[] methodNames();
/**
* Returns whether the string argument at the stated position being sent to
* the method is ok or not. Return true if you want to record the method call
* as a violation, false otherwise.
* Returns whether the node being sent to the method is OK or not. Return
* true if you want to record the method call as a violation.
*
* @param argIndex int
* @param arg String
* @param arg the node to inspect
* @return boolean
*/
protected abstract boolean isViolationArgument(int argIndex, String arg);
protected abstract boolean isViolationArgument(Node arg);
/**
* Returns whether the name occurrence is one of the method calls
@@ -57,29 +56,19 @@ public abstract class AbstractPoorMethodCall extends AbstractJavaRule {
*/
private boolean isNotedMethod(NameOccurrence occurrence) {
if (occurrence == null) {
return false;
}
if (occurrence == null) {
return false;
}
String methodCall = occurrence.getImage();
String[] methodNames = methodNames();
String methodCall = occurrence.getImage();
String[] methodNames = methodNames();
for (String element : methodNames) {
if (methodCall.indexOf(element) != -1) {
return true;
}
}
return false;
}
/**
* Returns whether the value argument is a single character string.
*
* @param value String
* @return boolean
*/
public static boolean isSingleCharAsString(String value) {
return value.length() == 3 && value.charAt(0) == '\"';
for (String element : methodNames) {
if (methodCall.indexOf(element) != -1) {
return true;
}
}
return false;
}
/**
@@ -91,28 +80,28 @@ public abstract class AbstractPoorMethodCall extends AbstractJavaRule {
*/
@Override
public Object visit(ASTVariableDeclaratorId node, Object data) {
if (!node.getNameDeclaration().getTypeImage().equals(targetTypename())) {
return data;
}
if (!node.getNameDeclaration().getTypeImage().equals(targetTypename())) {
return data;
}
for (NameOccurrence occ : node.getUsages()) {
if (isNotedMethod(occ.getNameForWhichThisIsAQualifier())) {
Node parent = occ.getLocation().jjtGetParent().jjtGetParent();
if (parent instanceof ASTPrimaryExpression) {
// bail out if it's something like indexOf("a" + "b")
if (parent.hasDescendantOfType(ASTAdditiveExpression.class)) {
return data;
}
List<ASTLiteral> literals = parent.findDescendantsOfType(ASTLiteral.class);
for (int l = 0; l < literals.size(); l++) {
ASTLiteral literal = literals.get(l);
if (isViolationArgument(l, literal.getImage())) {
addViolation(data, occ.getLocation());
}
}
}
}
}
return data;
for (NameOccurrence occ : node.getUsages()) {
if (isNotedMethod(occ.getNameForWhichThisIsAQualifier())) {
Node parent = occ.getLocation().jjtGetParent().jjtGetParent();
if (parent instanceof ASTPrimaryExpression) {
// bail out if it's something like indexOf("a" + "b")
if (parent.hasDescendantOfType(ASTAdditiveExpression.class)) {
return data;
}
List<ASTLiteral> literals = parent.findDescendantsOfType(ASTLiteral.class);
for (int l = 0; l < literals.size(); l++) {
ASTLiteral literal = literals.get(l);
if (isViolationArgument(literal)) {
addViolation(data, occ.getLocation());
}
}
}
}
}
return data;
}
}
pmd/src/net/sourceforge/pmd/lang/java/rule/strings/AppendCharacterWithCharRule.java
+2 -12
View File
@@ -7,9 +7,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import java.util.regex.Pattern;
import java.util.regex.Matcher;
/**
* This rule finds the following:
* <p/>
@@ -23,21 +20,14 @@ import java.util.regex.Matcher;
*/
public class AppendCharacterWithCharRule extends AbstractJavaRule {
private static final Pattern REGEX = Pattern.compile("\"[\\\\]?[\\s\\S]\"");
@Override
public Object visit(ASTLiteral node, Object data) {
ASTBlockStatement bs = node.getFirstParentOfType(ASTBlockStatement.class);
if (bs == null) {
return data;
}
String str = node.getImage();
if (str == null || str.length() < 3 || str.length() > 4) {
return data;
}
Matcher matcher = REGEX.matcher(str);
if (matcher.find()) {
if (node.isSingleCharacterStringLiteral()) {
if (!InefficientStringBufferingRule.isInStringBufferOperation(node, 8, "append")) {
return data;
}
pmd/src/net/sourceforge/pmd/lang/java/rule/strings/UseIndexOfCharRule.java
+8 -8
View File
@@ -1,5 +1,7 @@
package net.sourceforge.pmd.lang.java.rule.strings;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.rule.AbstractPoorMethodCall;
/**
@@ -13,7 +15,8 @@ public class UseIndexOfCharRule extends AbstractPoorMethodCall {
* Method targetTypeName.
* @return String
*/
protected String targetTypename() {
@Override
protected String targetTypename() {
return TARGET_TYPE_NAME;
}
@@ -21,19 +24,16 @@ public class UseIndexOfCharRule extends AbstractPoorMethodCall {
* Method methodNames.
* @return String[]
*/
@Override
protected String[] methodNames() {
return METHOD_NAMES;
}
/**
* Method isViolationArgument.
* @param argIndex int
* @param arg String
* @return boolean
* {@inheritDoc}
*/
protected boolean isViolationArgument(int argIndex, String arg) {
return isSingleCharAsString(arg);
protected boolean isViolationArgument(Node arg) {
return ((ASTLiteral) arg).isSingleCharacterStringLiteral();
}
}