Merge branch 'master' of github.com:pmd/pmd into issue-827

This commit is contained in:
Clément Fournier
2018-01-17 00:15:13 +01:00
25 changed files with 393 additions and 67 deletions

View File

@ -96,6 +96,69 @@ public class ASTLiteral extends AbstractJavaTypeNode {
}
return false;
}
private String stripIntValue() {
String image = getImage().toLowerCase().replaceAll("_", "");
boolean isNegative = false;
if (image.charAt(0) == '-') {
isNegative = true;
image = image.substring(1);
}
if (image.endsWith("l")) {
image = image.substring(0, image.length() - 1);
}
// ignore base prefix if any
if (image.charAt(0) == '0' && image.length() > 1) {
if (image.charAt(1) == 'x' || image.charAt(1) == 'b') {
image = image.substring(2);
} else {
image = image.substring(1);
}
}
if (isNegative) {
return "-" + image;
}
return image;
}
private String stripFloatValue() {
return getImage().toLowerCase().replaceAll("_", "");
}
private int getIntBase() {
final String image = getImage().toLowerCase();
final int offset = image.charAt(0) == '-' ? 1 : 0;
if (image.startsWith("0x", offset)) {
return 16;
}
if (image.startsWith("0b", offset)) {
return 2;
}
if (image.startsWith("0", offset) && image.length() > 1) {
return 8;
}
return 10;
}
public int getValueAsInt() {
return (int) getValueAsLong(); // the downcast allows to parse 0x80000000+ numbers as negative instead of a NumberFormatException
}
public long getValueAsLong() {
return Long.parseLong(stripIntValue(), getIntBase());
}
public float getValueAsFloat() {
return Float.parseFloat(stripFloatValue());
}
public double getValueAsDouble() {
return Double.parseDouble(stripFloatValue());
}
public void setCharLiteral() {
this.isChar = true;

View File

@ -35,6 +35,10 @@ import net.sourceforge.pmd.lang.symboltable.Scope;
*/
public class ForLoopCanBeForeachRule extends AbstractJavaRule {
public ForLoopCanBeForeachRule() {
addRuleChainVisit(ASTForStatement.class);
}
@Override
public Object visit(ASTForStatement node, Object data) {
@ -43,13 +47,13 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
final ASTExpression guardCondition = node.getFirstChildOfType(ASTExpression.class);
if (init == null && update == null || guardCondition == null) {
return super.visit(node, data);
return data;
}
Entry<VariableNameDeclaration, List<NameOccurrence>> indexDecl = getIndexVarDeclaration(init, update);
if (indexDecl == null) {
return super.visit(node, data);
return data;
}
@ -67,7 +71,7 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
if (occurrences == null || !"int".equals(index.getTypeImage()) || !indexStartsAtZero(index)) {
return super.visit(node, data);
return data;
}
@ -76,14 +80,14 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
if (!isForUpdateSimpleEnough(update, itName) || iterableName == null) {
return super.visit(node, data);
return data;
}
Entry<VariableNameDeclaration, List<NameOccurrence>> iterableInfo = findDeclaration(iterableName, node.getScope());
VariableNameDeclaration iterableDeclaration = iterableInfo == null ? null : iterableInfo.getKey();
if (iterableDeclaration == null) {
return super.visit(node, data);
return data;
}
if (iterableDeclaration.isArray() && isReplaceableArrayLoop(node, occurrences, iterableDeclaration)) {
@ -94,7 +98,7 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
addViolation(data, node);
}
return super.visit(node, data);
return data;
}
@ -240,8 +244,13 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule {
return null;
}
String name = initializer.getFirstDescendantOfType(ASTName.class)
.getImage();
ASTName nameNode = initializer.getFirstDescendantOfType(ASTName.class);
if (nameNode == null) {
// TODO : This can happen if we are calling a local / statically imported method that returns the iterable - currently unhandled
return null;
}
String name = nameNode.getImage();
int dotIndex = name.indexOf('.');
if (dotIndex > 0) {

View File

@ -150,9 +150,11 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
private boolean isSafeVarargs(final ASTMethodDeclaration node) {
for (final ASTAnnotation annotation : node.jjtGetParent().findChildrenOfType(ASTAnnotation.class)) {
final Node childAnnotation = annotation.jjtGetChild(0);
if (childAnnotation instanceof ASTMarkerAnnotation
&& SafeVarargs.class.isAssignableFrom(((ASTMarkerAnnotation) childAnnotation).getType())) {
return true;
if (childAnnotation instanceof ASTMarkerAnnotation) {
final ASTMarkerAnnotation marker = (ASTMarkerAnnotation) childAnnotation;
if (marker.getType() != null && SafeVarargs.class.isAssignableFrom(marker.getType())) {
return true;
}
}
}

View File

@ -199,16 +199,17 @@ public class InsufficientStringBufferDeclarationRule extends AbstractJavaRule {
anticipatedLength += str.length() - 2;
} else if (literal.isCharLiteral()) {
anticipatedLength += 1;
} else if (literal.isIntLiteral() && str.startsWith("0x")) {
} else if (literal.isIntLiteral()) {
// but only if we are not inside a cast expression
Node parentNode = literal.jjtGetParent().jjtGetParent().jjtGetParent();
if (parentNode instanceof ASTCastExpression
&& ((ASTCastExpression) parentNode).getType() == char.class) {
anticipatedLength += 1;
} else {
// any number, regardless of the base will be converted to base 10
// e.g. 0xdeadbeef -> will be converted to a
// base 10 integer string: 3735928559
anticipatedLength += String.valueOf(Long.parseLong(str.substring(2), 16)).length();
anticipatedLength += String.valueOf(literal.getValueAsLong()).length();
}
} else {
anticipatedLength += str.length();
@ -273,11 +274,8 @@ public class InsufficientStringBufferDeclarationRule extends AbstractJavaRule {
// characters
// don't add the constructor's length
iConstructorLength = 14 + str.length();
} else if (literal.isIntLiteral() && str.startsWith("0x")) {
// bug 3516101 - the string could be a hex number
iConstructorLength = Integer.parseInt(str.substring(2), 16);
} else {
iConstructorLength = Integer.parseInt(str);
} else if (literal.isIntLiteral()) {
iConstructorLength = literal.getValueAsInt();
}
} else {
iConstructorLength = -1;

View File

@ -4,8 +4,6 @@
package net.sourceforge.pmd.lang.java.rule.performance;
import java.math.BigInteger;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTCastExpression;
@ -83,21 +81,13 @@ public class RedundantFieldInitializerRule extends AbstractJavaRule {
// code.
Number value = -1;
if (literal.isIntLiteral()) {
value = parseInteger(literal.getImage());
value = literal.getValueAsInt();
} else if (literal.isLongLiteral()) {
String s = literal.getImage();
// remove the ending "l" or "L" for long
// values
s = s.substring(0, s.length() - 1);
value = parseInteger(s);
value = literal.getValueAsLong();
} else if (literal.isFloatLiteral()) {
String s = literal.getImage();
// remove the ending "f" or "F" for float
// values
s = s.substring(0, s.length() - 1);
value = Float.valueOf(s.replaceAll("_", ""));
value = literal.getValueAsFloat();
} else if (literal.isDoubleLiteral()) {
value = Double.valueOf(literal.getImage().replaceAll("_", ""));
value = literal.getValueAsDouble();
} else if (literal.isCharLiteral()) {
value = (int) literal.getImage().charAt(1);
}
@ -152,27 +142,4 @@ public class RedundantFieldInitializerRule extends AbstractJavaRule {
private void addViolation(Object data, ASTVariableDeclarator variableDeclarator) {
super.addViolation(data, variableDeclarator, variableDeclarator.jjtGetChild(0).getImage());
}
private Number parseInteger(String s) {
boolean negative = false;
String number = s;
if (number.charAt(0) == '-') {
negative = true;
number = number.substring(1);
}
BigInteger result;
if (number.startsWith("0x") || number.startsWith("0X")) {
result = new BigInteger(number.substring(2).replaceAll("_", ""), 16);
} else if (number.startsWith("0b") || number.startsWith("0B")) {
result = new BigInteger(number.substring(2).replaceAll("_", ""), 8);
} else if (number.charAt(0) == '0' && number.length() > 1) {
result = new BigInteger(number.substring(1).replaceAll("_", ""), 8);
} else {
result = new BigInteger(number.replaceAll("_", ""));
}
if (negative) {
result = result.negate();
}
return result;
}
}

View File

@ -572,7 +572,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter {
}
} catch (final NoSuchFieldException ignored) {
// swallow
} catch (final NoClassDefFoundError e) {
} catch (final LinkageError e) {
if (LOG.isLoggable(Level.WARNING)) {
LOG.log(Level.WARNING, "Error during type resolution due to: " + e);
}
// TODO : report a missing class once we start doing that...
return null;
}

View File

@ -158,4 +158,6 @@ public abstract class JavaTypeDefinition implements TypeDefinition {
public abstract JavaTypeDefinition getJavaType(int index);
public abstract int getJavaTypeCount();
protected abstract String shallowString();
}

View File

@ -241,9 +241,23 @@ import java.util.logging.Logger;
@Override
public String toString() {
final StringBuilder sb = new StringBuilder("JavaTypeDefinition [clazz=").append(clazz)
.append(", definitionType=").append(getDefinitionType())
.append(", genericArgs=[");
for (final JavaTypeDefinition jtd : genericArgs) {
sb.append(jtd.shallowString()).append(", ");
}
return sb.replace(sb.length() - 3, sb.length() - 1, "]") // last comma to bracket
.append(", isGeneric=").append(isGeneric)
.append("]\n").toString();
}
@Override
public String shallowString() {
return new StringBuilder("JavaTypeDefinition [clazz=").append(clazz)
.append(", definitionType=").append(getDefinitionType())
.append(", genericArgs=").append(genericArgs)
.append(", isGeneric=").append(isGeneric)
.append("]\n").toString();
}

View File

@ -111,15 +111,20 @@ import java.util.Set;
public String toString() {
StringBuilder builder = new StringBuilder()
.append("JavaTypeDefinition ")
.append(getDefinitionType().toString())
.append(getDefinitionType())
.append(" [")
.append(typeList[0]);
for (int index = 1; index < typeList.length; ++index) {
builder.append(" && ");
builder.append(typeList[index]);
builder.append(" && ")
.append(typeList[index]);
}
return builder.append("]").toString();
}
@Override
protected String shallowString() {
return toString();
}
@Override
public boolean equals(Object obj) {

View File

@ -59,6 +59,46 @@ public class ASTLiteralTest {
assertTrue((literals.iterator().next()).isCharLiteral());
}
@Test
public void testIntValueParsing() {
ASTLiteral literal = new ASTLiteral(1);
literal.setIntLiteral();
literal.setImage("1___234");
literal.testingOnlySetBeginColumn(1);
literal.testingOnlySetEndColumn(7);
assertEquals(1___234, literal.getValueAsInt());
}
@Test
public void testIntValueParsingBinary() {
ASTLiteral literal = new ASTLiteral(1);
literal.setIntLiteral();
literal.setImage("0b0000_0010");
literal.testingOnlySetBeginColumn(1);
literal.testingOnlySetEndColumn(7);
assertEquals(0b0000_0010, literal.getValueAsInt());
}
@Test
public void testIntValueParsingNegativeHexa() {
ASTLiteral literal = new ASTLiteral(1);
literal.setIntLiteral();
literal.setImage("-0X0000_000f");
literal.testingOnlySetBeginColumn(1);
literal.testingOnlySetEndColumn(7);
assertEquals(-0X0000_000f, literal.getValueAsInt());
}
@Test
public void testFloatValueParsingNegative() {
ASTLiteral literal = new ASTLiteral(1);
literal.setIntLiteral();
literal.setImage("-3_456.123_456");
literal.testingOnlySetBeginColumn(1);
literal.testingOnlySetEndColumn(7);
assertEquals(-3_456.123_456f, literal.getValueAsFloat(), 0);
}
@Test
public void testStringUnicodeEscapesNotEscaped() {
ASTLiteral literal = new ASTLiteral(1);

View File

@ -102,6 +102,7 @@ import net.sourceforge.pmd.typeresolution.testdata.MethodThirdPhase;
import net.sourceforge.pmd.typeresolution.testdata.NestedAnonymousClass;
import net.sourceforge.pmd.typeresolution.testdata.Operators;
import net.sourceforge.pmd.typeresolution.testdata.OverloadedMethodsUsage;
import net.sourceforge.pmd.typeresolution.testdata.PmdStackOverflow;
import net.sourceforge.pmd.typeresolution.testdata.Promotion;
import net.sourceforge.pmd.typeresolution.testdata.SubTypeUsage;
import net.sourceforge.pmd.typeresolution.testdata.SuperExpression;
@ -123,6 +124,12 @@ import net.sourceforge.pmd.typeresolution.testdata.dummytypes.SuperClassB2;
public class ClassTypeResolverTest {
@Test
public void stackOverflowTest() {
// See #831 https://github.com/pmd/pmd/issues/831 - [java] StackOverflow in JavaTypeDefinitionSimple.toString
parseAndTypeResolveForClass15(PmdStackOverflow.class);
}
@Test
public void testClassNameExists() {
ClassTypeResolver classTypeResolver = new ClassTypeResolver();

View File

@ -0,0 +1,47 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.typeresolution.testdata;
public class PmdStackOverflow {
public void shouldThrowStackOverfloError() {
MessageBuilder messageBuilder = new MessageBuilderA();
// Works
PartBuilder partBuilder = messageBuilder.newComponent();
messageBuilder.addComponent(partBuilder.withSomeValue("ABC"));
// Does not work
messageBuilder.addComponent(messageBuilder.newComponent().withSomeValue("ABC"));
}
}
abstract class MessageBuilder<T extends MessageBuilder, U extends PartBuilder<U>> {
public abstract U newComponent();
public T addComponent(U ignore) {
return (T) this;
}
}
class MessageBuilderA extends MessageBuilder<MessageBuilderA, PartBuilderA> {
@Override
public PartBuilderA newComponent() {
return new PartBuilderA();
}
}
class PartBuilder<T extends PartBuilder> {
public T withSomeValue(String ignore) {
return (T) this;
}
}
class PartBuilderA extends PartBuilder<PartBuilderA> {
}

View File

@ -266,5 +266,25 @@
]]></code>
</test-code>
<test-code>
<description>Iterating on this object NPE, refs #800</description>
<expected-problems>0</expected-problems> <!-- FIXME should be 1 -->
<code><![CDATA[
import java.util.Iterator;
class Foo<T> implements Iterable<T> {
@Override
public Iterator<T> iterator() {
return null;
}
private void fofo() {
for (Iterator<T> it = this.iterator(); it.hasNext();) {
T item = it.next();
}
}
}
]]></code>
</test-code>
</test-data>

View File

@ -585,6 +585,16 @@ public class Foo {
// do something on fw
}
}
}
]]></code>
</test-code>
<test-code>
<description>#817 [java] UnnecessaryModifierRule crashes on valid code</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
class Foo {
@Bar
final void method() { }
}
]]></code>
</test-code>

View File

@ -310,7 +310,7 @@ public class Foo {
</test-code>
<test-code>
<description><![CDATA[
15, Append int, incorrect presize
15, Append long, incorrect presize
]]></description>
<expected-problems>2</expected-problems>
<code><![CDATA[
@ -318,11 +318,11 @@ public class Foo {
private static org.apache.log4j.Logger logger = org.apache.log4j.Logger.getLogger(Foo.class);
public void bar() {
StringBuffer sb = new StringBuffer();
sb.append(12345678901234567890);
sb.append(12345678901234567890L);
}
public void bar2() {
StringBuilder sb = new StringBuilder();
sb.append(12345678901234567890);
sb.append(12345678901234567890L);
}
}
]]></code>
@ -1059,6 +1059,19 @@ public final class test {
"# Testing" + NEWLINE +
"# More Contents" + NEWLINE);
}
}
]]></code>
</test-code>
<test-code>
<description>#841 InsufficientStringBufferDeclaration NumberFormatException</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Test {
public static void main(final String ... args) {
StringBuilder report = new StringBuilder(10_000).append("test");
}
}
]]></code>
</test-code>