Skip to content

Commit 4905909

Browse files
committed
Fix UnusedAssignment false positive with unconditional if(true) overwrite
- Fix the false positive in DataflowPass where assignments in unconditional if(true) blocks were not properly marked as overwriting previous assignments - Update test expectations to reflect the fix - Addresses issue pmd#6518
1 parent 4339980 commit 4905909

2 files changed

Lines changed: 105 additions & 8 deletions

File tree

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/DataflowPass.java

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -481,17 +481,30 @@ public SpanInfo visit(ASTConditionalExpression node, SpanInfo data) {
481481
return makeConditional(data, node.getCondition(), node.getThenBranch(), node.getElseBranch());
482482
}
483483

484-
SpanInfo makeConditional(SpanInfo before, ASTExpression condition, JavaNode thenBranch, JavaNode elseBranch) {
485-
SpanInfo thenState = before.fork();
486-
SpanInfo elseState = elseBranch != null ? before.fork() : before;
484+
SpanInfo makeConditional(SpanInfo before, ASTExpression condition, JavaNode thenBranch, JavaNode elseBranch) {
485+
// Handle literal boolean conditions specially
486+
if (JavaAstUtils.isBooleanLiteral(condition, true)) {
487+
// if (true): evaluate only condition and then-branch
488+
SpanInfo state = acceptOpt(condition, before);
489+
state = acceptOpt(thenBranch, state);
490+
return state;
491+
} else if (JavaAstUtils.isBooleanLiteral(condition, false)) {
492+
// if (false): evaluate only condition and else-branch
493+
SpanInfo state = acceptOpt(condition, before);
494+
state = acceptOpt(elseBranch, state);
495+
return state;
496+
}
497+
498+
SpanInfo thenState = before.fork();
499+
SpanInfo elseState = elseBranch != null ? before.fork() : before;
487500

488-
linkConditional(before, condition, thenState, elseState, true);
501+
linkConditional(before, condition, thenState, elseState, true);
489502

490-
thenState = acceptOpt(thenBranch, thenState);
491-
elseState = acceptOpt(elseBranch, elseState);
503+
thenState = acceptOpt(thenBranch, thenState);
504+
elseState = acceptOpt(elseBranch, elseState);
492505

493-
return elseState.absorb(thenState);
494-
}
506+
return elseState.absorb(thenState);
507+
}
495508

496509
/*
497510
* This recursive procedure translates shortcut conditionals

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedAssignment.xml

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4020,4 +4020,88 @@ class Lock {
40204020
}
40214021
]]></code>
40224022
</test-code>
4023+
4024+
<test-code>
4025+
<description>[java] UnusedAssignment: fix false negative with unconditional if(true) overwrite #6518</description>
4026+
<expected-problems>1</expected-problems>
4027+
<expected-linenumbers>3</expected-linenumbers>
4028+
<expected-messages>
4029+
<message>The initializer for variable 'x' is never used (overwritten on line 5)</message>
4030+
</expected-messages>
4031+
<code><![CDATA[
4032+
public class Test {
4033+
void test() {
4034+
int x = 0; // should be reported as unused
4035+
if (true) {
4036+
x = 1;
4037+
}
4038+
System.out.println(x);
4039+
}
4040+
}
4041+
]]></code>
4042+
</test-code>
4043+
4044+
<test-code>
4045+
<description>[java] UnusedAssignment: if(false) case</description>
4046+
<expected-problems>1</expected-problems>
4047+
<expected-linenumbers>3</expected-linenumbers>
4048+
<expected-messages>
4049+
<message>The initializer for variable 'x' is never used</message>
4050+
</expected-messages>
4051+
<code><![CDATA[
4052+
public class Test {
4053+
void test() {
4054+
int x = 0; // should be reported as unused
4055+
if (false) {
4056+
x = 1;
4057+
}
4058+
System.out.println(x);
4059+
}
4060+
}
4061+
]]></code>
4062+
</test-code>
4063+
4064+
<test-code>
4065+
<description>[java] UnusedAssignment: if(true) with else branch</description>
4066+
<expected-problems>1</expected-problems>
4067+
<expected-linenumbers>3</expected-linenumbers>
4068+
<expected-messages>
4069+
<message>The initializer for variable 'x' is never used (overwritten on line 5)</message>
4070+
</expected-messages>
4071+
<code><![CDATA[
4072+
public class Test {
4073+
void test() {
4074+
int x = 0; // should be reported as unused
4075+
if (true) {
4076+
x = 1;
4077+
} else {
4078+
x = 2;
4079+
}
4080+
System.out.println(x);
4081+
}
4082+
}
4083+
]]></code>
4084+
</test-code>
4085+
4086+
<test-code>
4087+
<description>[java] UnusedAssignment: if(false) with else branch</description>
4088+
<expected-problems>1</expected-problems>
4089+
<expected-linenumbers>3</expected-linenumbers>
4090+
<expected-messages>
4091+
<message>The initializer for variable 'x' is never used (overwritten on line 7)</message>
4092+
</expected-messages>
4093+
<code><![CDATA[
4094+
public class Test {
4095+
void test() {
4096+
int x = 0; // should be reported as unused
4097+
if (false) {
4098+
x = 1;
4099+
} else {
4100+
x = 2;
4101+
}
4102+
System.out.println(x);
4103+
}
4104+
}
4105+
]]></code>
4106+
</test-code>
40234107
</test-data>

0 commit comments

Comments
 (0)