[java] Fix PrimitiveWrapperInstantion false negative with new Boolean
Fixes #3595
This commit is contained in:
@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
### Fixed Issues
|
||||
|
||||
* java-bestpractices
|
||||
* [#3595](https://github.com/pmd/pmd/issues/3595): \[java] PrimitiveWrapperInstantiation: no violation on 'new Boolean(val)'
|
||||
|
||||
### API Changes
|
||||
|
||||
### External Contributions
|
||||
|
@ -74,7 +74,8 @@ public class PrimitiveWrapperInstantiationRule extends AbstractJavaRule {
|
||||
if (arguments == null || arguments.size() != 1) {
|
||||
return;
|
||||
}
|
||||
String messagePart = node instanceof ASTAllocationExpression
|
||||
boolean isNewBoolean = node instanceof ASTAllocationExpression;
|
||||
String messagePart = isNewBoolean
|
||||
? "Do not use `new Boolean"
|
||||
: "Do not use `Boolean.valueOf";
|
||||
ASTLiteral stringLiteral = getFirstArgStringLiteralOrNull(arguments);
|
||||
@ -84,6 +85,8 @@ public class PrimitiveWrapperInstantiationRule extends AbstractJavaRule {
|
||||
addViolationWithMessage(data, node, messagePart + "(\"true\")`, prefer `Boolean.TRUE`");
|
||||
} else if (stringLiteral.hasImageEqualTo("\"false\"")) {
|
||||
addViolationWithMessage(data, node, messagePart + "(\"false\")`, prefer `Boolean.FALSE`");
|
||||
} else {
|
||||
addViolationWithMessage(data, node, messagePart + "(\"...\")`, prefer `Boolean.valueOf`");
|
||||
}
|
||||
} else if (boolLiteral != null) {
|
||||
if (boolLiteral.isTrue()) {
|
||||
@ -91,6 +94,9 @@ public class PrimitiveWrapperInstantiationRule extends AbstractJavaRule {
|
||||
} else {
|
||||
addViolationWithMessage(data, node, messagePart + "(false)`, prefer `Boolean.FALSE`");
|
||||
}
|
||||
} else if (isNewBoolean) {
|
||||
// any argument with "new Boolean", might be a variable access
|
||||
addViolationWithMessage(data, node, messagePart + "(...)`, prefer `Boolean.valueOf`");
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -382,4 +382,25 @@ public class Foo {
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>[java] PrimitiveWrapperInstantiation: no violation on 'new Boolean(val)' #3595</description>
|
||||
<expected-problems>2</expected-problems>
|
||||
<expected-linenumbers>5,6</expected-linenumbers>
|
||||
<expected-messages>
|
||||
<message>Do not use `new Boolean(...)`, prefer `Boolean.valueOf`</message>
|
||||
<message>Do not use `new Boolean("...")`, prefer `Boolean.valueOf`</message>
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class SomeClass {
|
||||
private Boolean bar;
|
||||
|
||||
public void method(String s) {
|
||||
this.bar = new Boolean(s); //violation for the BooleanInstantiation
|
||||
this.bar = new Boolean("some arbitrary string is just false"); //violation
|
||||
this.bar = Boolean.valueOf(s); //use this instead of Boolean#new
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
</test-data>
|
||||
|
@ -157,6 +157,23 @@ public class Foo {
|
||||
public void bar() {
|
||||
Object o = new @Interned MyObject();
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>[java] PrimitiveWrapperInstantiation: no violation on 'new Boolean(val)' #3595</description>
|
||||
<expected-problems>2</expected-problems>
|
||||
<expected-linenumbers>5,6</expected-linenumbers>
|
||||
<code><![CDATA[
|
||||
public class SomeClass {
|
||||
private Boolean bar;
|
||||
|
||||
public void method(String s) {
|
||||
this.bar = new Boolean(s); //violation for the BooleanInstantiation
|
||||
this.bar = new Boolean("some arbitrary string is just false"); //violation
|
||||
this.bar = Boolean.valueOf(s); //use this instead of Boolean#new
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
Reference in New Issue
Block a user