From 5cb7088581f5ab0d764572d95fdc7daed2c2be21 Mon Sep 17 00:00:00 2001 From: Tom Copeland Date: Thu, 3 Aug 2006 14:48:57 +0000 Subject: [PATCH] Fixed bug 1531593 - UnnecessaryConversionTemporary no longer reports false positives when toString() is invoked inside the call to 'new Long/Integer/etc()'. git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/trunk@4480 51baf565-9d33-0410-a72c-fc3788e3496d --- pmd/etc/changelog.txt | 1 + .../pmd/rules/UnnecessaryTemporariesTest.java | 28 ++++++++++++------- .../rules/UnnecessaryConversionTemporary.java | 13 +++++---- pmd/xdocs/credits.xml | 1 + 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index e007a89dfa..9cd2d602e4 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -14,6 +14,7 @@ Fixed bug 1522054 - BooleanInstantiation now detects instantiations inside metho Fixed bug 1522056 - UseStringBufferForStringAppends now flags appends which occur in static initializers and constructors Fixed bug 1526530 - SingularField now finds fields which are hidden at the method or static level Fixed bug 1529805 - UnusedModifier no longer throws NPEs on JDK 1.5 enums. +Fixed bug 1531593 - UnnecessaryConversionTemporary no longer reports false positives when toString() is invoked inside the call to 'new Long/Integer/etc()'. Fixed a bug in AvoidProtectedFieldInFinalClass - it no longer reports false positives for protected fields in inner classes. Fixed a bug in the C++ grammar - the tokenizer now properly recognizes macro definitions which are followed by a multiline comment. Implemented RFE 1501850 - UnusedFormalParameter now catches cases where a parameter is assigned to but not used. diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryTemporariesTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryTemporariesTest.java index ec79f96a70..2cb047daad 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryTemporariesTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryTemporariesTest.java @@ -19,18 +19,26 @@ public class UnnecessaryTemporariesTest extends SimpleAggregatorTst { public void testAll() { runTests(new TestDescriptor[]{ new TestDescriptor(TEST1, "all glommed together", 6, rule), + new TestDescriptor(TEST2, "called on String", 0, rule), }); } private static final String TEST1 = - " public class Foo {" + PMD.EOL + - " void method (int x) {" + PMD.EOL + - " new Integer(x).toString(); " + PMD.EOL + - " new Long(x).toString(); " + PMD.EOL + - " new Float(x).toString(); " + PMD.EOL + - " new Byte((byte)x).toString(); " + PMD.EOL + - " new Double(x).toString(); " + PMD.EOL + - " new Short((short)x).toString(); " + PMD.EOL + - " }" + PMD.EOL + - " }"; + " public class Foo {" + PMD.EOL + + " void method (int x) {" + PMD.EOL + + " new Integer(x).toString(); " + PMD.EOL + + " new Long(x).toString(); " + PMD.EOL + + " new Float(x).toString(); " + PMD.EOL + + " new Byte((byte)x).toString(); " + PMD.EOL + + " new Double(x).toString(); " + PMD.EOL + + " new Short((short)x).toString(); " + PMD.EOL + + " }" + PMD.EOL + + " }"; + + private static final String TEST2 = + " public class Foo {" + PMD.EOL + + " Long method (Foo foo) {" + PMD.EOL + + " return new Long(foo.get().toString()); " + PMD.EOL + + " }" + PMD.EOL + + " }"; } diff --git a/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConversionTemporary.java b/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConversionTemporary.java index a725c73aff..cedd5b429b 100644 --- a/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConversionTemporary.java +++ b/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConversionTemporary.java @@ -17,6 +17,7 @@ import java.util.Set; public class UnnecessaryConversionTemporary extends AbstractRule implements Rule { private boolean inPrimaryExpressionContext; + private ASTPrimaryExpression primary; private boolean usingPrimitiveWrapperAllocation; private Set primitiveWrappers = new HashSet(); @@ -36,6 +37,7 @@ public class UnnecessaryConversionTemporary extends AbstractRule implements Rule } // TODO... hmmm... is this inPrimaryExpressionContext gibberish necessary? inPrimaryExpressionContext = true; + primary = node; super.visit(node, data); inPrimaryExpressionContext = false; usingPrimitiveWrapperAllocation = false; @@ -54,11 +56,12 @@ public class UnnecessaryConversionTemporary extends AbstractRule implements Rule } public Object visit(ASTPrimarySuffix node, Object data) { - if (!inPrimaryExpressionContext || !usingPrimitiveWrapperAllocation) { - return super.visit(node, data); - } - if (node.getImage() != null && node.getImage().equals("toString")) { - addViolation(data, node); + if (inPrimaryExpressionContext && usingPrimitiveWrapperAllocation) { + if (node.getImage() != null && node.getImage().equals("toString")) { + if (node.jjtGetParent() == primary) { + addViolation(data, node); + } + } } return super.visit(node, data); } diff --git a/pmd/xdocs/credits.xml b/pmd/xdocs/credits.xml index ff19f56668..4612170024 100644 --- a/pmd/xdocs/credits.xml +++ b/pmd/xdocs/credits.xml @@ -48,6 +48,7 @@