Merge branch 'pr-2761' into master

[java] Fix #2759, UnusedAssignment false positive #2761

docs/pages/release_notes.md

@@ -20,6 +20,7 @@ This is a {{ site.pmd.release_type }} release.
     * [#2708](https://github.com/pmd/pmd/issues/2708): \[java] False positive FinalFieldCouldBeStatic when using lombok Builder.Default
     * [#2738](https://github.com/pmd/pmd/issues/2738): \[java] Custom rule with @ExhaustiveEnumSwitch throws NPE
     * [#2756](https://github.com/pmd/pmd/issues/2756): \[java] TypeTestUtil fails with NPE for anonymous class
+    * [#2759](https://github.com/pmd/pmd/issues/2759): \[java] False positive in UnusedAssignment
     * [#2767](https://github.com/pmd/pmd/issues/2767): \[java] IndexOutOfBoundsException when parsing an initializer BlockStatement
 
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java

@@ -895,11 +895,6 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
             // For the record this has problems with call chains with side effects, like
             //  a.foo(a = 2).bar(a = 3);
 
-            if (!state.isInTryBlock()) {
-                return;
-            }
-            // otherwise any method/ctor call may throw
-
             // In 7.0, with the precise type/overload resolution, we
             // could only target methods that throw checked exceptions
             // (unless some catch block catches an unchecked exceptions)
@@ -907,7 +902,7 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
                 if (child instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) child).isArguments()
                     || child instanceof ASTPrimarySuffix && child.getNumChildren() > 0 && child.getChild(0) instanceof ASTAllocationExpression
                     || child instanceof ASTPrimaryPrefix && child.getNumChildren() > 0 && child.getChild(0) instanceof ASTAllocationExpression) {
-                    state.abruptCompletionByThrow(true);
+                    state.abruptCompletionByThrow(true); // this is a noop if we're outside a try block that has catch/finally
                 }
             }
         }
@@ -1385,11 +1380,6 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
             return this;
         }
 
-        private boolean isInTryBlock() {
-            // ignore resources, once they're initialized everything's fine in the block
-            return !myCatches.isEmpty() || myFinally != null;
-        }
-
         SpanInfo absorb(SpanInfo other) {
             // Merge reaching defs of the other scope into this
             // This is used to join paths after the control flow has forked

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

@@ -3159,4 +3159,29 @@ public class UnusedAssignmentNative {
             ]]></code>
     </test-code>
 
+    <test-code>
+        <description>False positive with method that may throw in forked state (the if state) #2759</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+            class Test {
+                int a() {
+                    int a = 10;
+                    while (a > 0) {
+                        a--;
+                        try {
+                            if (dummy) {
+                                return somethingThatCanThrowRandomly(1);
+                            } else {
+                                return somethingThatCanThrowRandomly(2);
+                            }
+                        } catch (RuntimeException e) {
+                            // retry
+                        }
+                    }
+                    return 0;
+                }
+            }
+            ]]></code>
+    </test-code>
+
 </test-data>