From ab96e340994bbc86ccb5f45a27984d9bbe7a0931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 14 Jan 2018 13:42:32 -0300 Subject: [PATCH 1/3] [java] Resolve NumberFormatException - InsufficientStringBufferDeclaration now supports all numbr formats (even octal and binary) - We move the logic of understanding the literal value to the literal node. Looking for other places where this logic is being replicated is pending --- .../pmd/lang/java/ast/ASTLiteral.java | 63 +++++++++++++++++++ ...sufficientStringBufferDeclarationRule.java | 7 +-- .../pmd/lang/java/ast/ASTLiteralTest.java | 40 ++++++++++++ .../InsufficientStringBufferDeclaration.xml | 19 +++++- 4 files changed, 121 insertions(+), 8 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLiteral.java index 964dc2789b..75bc981532 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLiteral.java @@ -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; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java index 91e0a91cf7..7933e24fe4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java @@ -273,11 +273,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; diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.java index f5738b386d..7fc3723187 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.java @@ -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(1234, literal.getValueAsInt()); + } + + @Test + public void testIntValueParsingBinary() { + ASTLiteral literal = new ASTLiteral(1); + literal.setIntLiteral(); + literal.setImage("0b0000_0010"); + literal.testingOnlySetBeginColumn(1); + literal.testingOnlySetEndColumn(7); + assertEquals(2, literal.getValueAsInt()); + } + + @Test + public void testIntValueParsingNegativeHexa() { + ASTLiteral literal = new ASTLiteral(1); + literal.setIntLiteral(); + literal.setImage("-0X0000_000f"); + literal.testingOnlySetBeginColumn(1); + literal.testingOnlySetEndColumn(7); + assertEquals(-15, 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(-3456.123456f, literal.getValueAsFloat(), 0); + } + @Test public void testStringUnicodeEscapesNotEscaped() { ASTLiteral literal = new ASTLiteral(1); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml index 1efccd82ee..1b00b27e2e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/InsufficientStringBufferDeclaration.xml @@ -310,7 +310,7 @@ public class Foo { 2 @@ -1059,6 +1059,19 @@ public final class test { "# Testing" + NEWLINE + "# More Contents" + NEWLINE); } +} + ]]> + + + + #841 InsufficientStringBufferDeclaration NumberFormatException + 0 + From 391c1e88421c68d71ea7786ffc6188cd98b73ecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 14 Jan 2018 16:15:22 -0300 Subject: [PATCH 2/3] Unify literal parsing logic --- ...sufficientStringBufferDeclarationRule.java | 5 ++- .../RedundantFieldInitializerRule.java | 41 ++----------------- 2 files changed, 7 insertions(+), 39 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java index 7933e24fe4..52e8019d92 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/InsufficientStringBufferDeclarationRule.java @@ -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(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java index 521ee44cae..9d8cfb9072 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/RedundantFieldInitializerRule.java @@ -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; - } } From fe53f8bb91d669cee4ef38b90761d371e0df0f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sun, 14 Jan 2018 16:17:11 -0300 Subject: [PATCH 3/3] Update changelog, refs #841 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 0ddcc0d529..dbc4514c5f 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -61,6 +61,8 @@ at . * [#817](https://github.com/pmd/pmd/issues/817): \[java] UnnecessaryModifierRule crashes on valid code * java-design * [#785](https://github.com/pmd/pmd/issues/785): \[java] NPE in DataClass rule +* java-performance + * [#841](https://github.com/pmd/pmd/issues/841): \[java] InsufficientStringBufferDeclaration NumberFormatException ### API Changes