[java] Fix #5046 - FPs in LocalVariableCouldBeFinal (#5191)

Merge pull request #5191 from oowekyala:issue5046-localVariableCouldBeFinal-fp-catch
This commit is contained in:
Andreas Dangel 2024-09-26 15:21:51 +02:00
commit 4930e98706
No known key found for this signature in database
GPG Key ID: 93450DF2DF9A3FA3
13 changed files with 84 additions and 18 deletions

View File

@ -27,6 +27,8 @@ This is a {{ site.pmd.release_type }} release.
* [#5222](https://github.com/pmd/pmd/issues/5222): \[core] RuleReference/RuleSetWriter don't handle changed default property values correctly
* java
* [#5190](https://github.com/pmd/pmd/issues/5190): \[java] NPE in type inference
* java-codestyle
* [#5046](https://github.com/pmd/pmd/issues/5046): \[java] LocalVariableCouldBeFinal false positive with try/catch
* java-errorprone
* [#5068](https://github.com/pmd/pmd/issues/5068): \[java] MissingStaticMethodInNonInstantiatableClass: false positive with builder pattern
* [#5207](https://github.com/pmd/pmd/issues/5207): \[java] CheckSkipResult: false positve for a private method `void skip(int)` in a subclass of FilterInputStream

View File

@ -636,7 +636,16 @@ public final class DataflowPass {
SpanInfo exceptionalState = null;
int i = 0;
for (ASTCatchClause catchClause : node.getCatchClauses()) {
SpanInfo current = acceptOpt(catchClause, catchSpans.get(i));
/*
Note: here we absorb the end state of the body, which is not necessary.
We do that to conform to the language's definition of "effective-finality",
which is more conservative than needed. Doing this fixes FPs in LocalVariableCouldBeFinal
at the cost of some FNs in UnusedAssignment.
*/
SpanInfo catchSpan = catchSpans.get(i);
catchSpan.absorb(bodyState);
SpanInfo current = acceptOpt(catchClause, catchSpan);
exceptionalState = current.absorb(exceptionalState);
i++;
}

View File

@ -0,0 +1,32 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule;
import org.junit.platform.suite.api.SelectClasses;
import org.junit.platform.suite.api.Suite;
import net.sourceforge.pmd.lang.java.rule.bestpractices.UnusedAssignmentTest;
import net.sourceforge.pmd.lang.java.rule.codestyle.LocalVariableCouldBeFinalTest;
import net.sourceforge.pmd.lang.java.rule.design.AvoidThrowingNullPointerExceptionTest;
import net.sourceforge.pmd.lang.java.rule.design.ImmutableFieldTest;
import net.sourceforge.pmd.lang.java.rule.design.LawOfDemeterTest;
import net.sourceforge.pmd.lang.java.rule.design.SingularFieldTest;
import net.sourceforge.pmd.lang.java.rule.errorprone.ImplicitSwitchFallThroughTest;
import net.sourceforge.pmd.lang.java.rule.errorprone.InvalidLogMessageFormatTest;
@Suite
@SelectClasses({
LocalVariableCouldBeFinalTest.class,
ImmutableFieldTest.class,
UnusedAssignmentTest.class,
LawOfDemeterTest.class,
SingularFieldTest.class,
ImplicitSwitchFallThroughTest.class,
InvalidLogMessageFormatTest.class,
AvoidThrowingNullPointerExceptionTest.class
})
public class AllDataflowRuleTests {
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.test.PmdRuleTst;
class UnusedAssignmentTest extends PmdRuleTst {
public class UnusedAssignmentTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import net.sourceforge.pmd.test.PmdRuleTst;
class LocalVariableCouldBeFinalTest extends PmdRuleTst {
public class LocalVariableCouldBeFinalTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.test.PmdRuleTst;
class AvoidThrowingNullPointerExceptionTest extends PmdRuleTst {
public class AvoidThrowingNullPointerExceptionTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.test.PmdRuleTst;
class ImmutableFieldTest extends PmdRuleTst {
public class ImmutableFieldTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.test.PmdRuleTst;
class LawOfDemeterTest extends PmdRuleTst {
public class LawOfDemeterTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.test.PmdRuleTst;
class SingularFieldTest extends PmdRuleTst {
public class SingularFieldTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
import net.sourceforge.pmd.test.PmdRuleTst;
class ImplicitSwitchFallThroughTest extends PmdRuleTst {
public class ImplicitSwitchFallThroughTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
import net.sourceforge.pmd.test.PmdRuleTst;
class InvalidLogMessageFormatTest extends PmdRuleTst {
public class InvalidLogMessageFormatTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -1199,11 +1199,12 @@ public class Foo {
<test-code>
<description>Definitions in try block reach catch blocks through method calls</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>4,7</expected-linenumbers>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<expected-messages>
<message>The initializer for variable 'halfway' is never used (overwritten on line 7)</message>
<message>The value assigned to variable 'halfway' is never used</message>
<!-- This was disabled because it causes FPs in LocalVariableCouldBeFinal. -->
<!-- <message>The value assigned to variable 'halfway' is never used</message>-->
</expected-messages>
<code><![CDATA[
public class Foo {
@ -1251,11 +1252,13 @@ public class Foo {
</test-code>
<test-code>
<description>Definitions in try block reach catch blocks through method calls 3</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<expected-messages>
<message>The value assigned to variable 'halfway' is never used</message>
</expected-messages>
<!-- This was disabled because it causes FPs in LocalVariableCouldBeFinal. -->
<expected-problems>0</expected-problems>
<!-- <expected-problems>1</expected-problems>-->
<!-- <expected-linenumbers>8</expected-linenumbers>-->
<!-- <expected-messages>-->
<!-- <message>The value assigned to variable 'halfway' is never used</message>-->
<!-- </expected-messages>-->
<code><![CDATA[
public class Foo {
@ -1328,7 +1331,7 @@ public class Foo {
<expected-linenumbers>5,7,9</expected-linenumbers>
<expected-messages>
<message>The initializer for variable 'a' is never used (overwritten on lines 7, 9 and 11)</message>
<message>The value assigned to variable 'a' is never used (overwritten on line 11)</message>
<message>The value assigned to variable 'a' is never used (overwritten on lines 9 and 11)</message>
<message>The value assigned to variable 'a' is never used (overwritten on line 11)</message>
</expected-messages>
<code><![CDATA[

View File

@ -386,6 +386,26 @@ public class ClassWithLambda {
ek[114] |= 32; ek[123] |= 2;
}
}
}
]]></code>
</test-code>
<test-code>
<description> [java] LocalVariableCouldBeFinal false positive with try/catch #5046 </description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.Optional;
import foo.FileWatcher;
public class ClassWithLambda {
private Optional<FileWatcher> createFileWatcher() {
Optional<FileWatcher> optionalFileWatcher; // false positive in PMD 7.2.0, cannot be final
try {
optionalFileWatcher = Optional.of(new FileWatcher());
} catch (final IOException e) {
optionalFileWatcher = Optional.empty();
}
return optionalFileWatcher;
}
}
]]></code>
</test-code>