[java] [7.0-rc1] UnusedAssignment for used field

[ben-manes]

Affects PMD Version:
7.0-rc1 when upgrading from 6.55

Rule:
UnusedAssignment

Description:
A class field is not the same as a method variable as it is in scope for other method calls. This also only restores the value when an unhandled error occurs to rollback the state, so the detection is misunderstanding the behavior.

Code Sample demonstrating the issue:

public void advance(BoundedLocalCache<K, V> cache, long currentTimeNanos) {
  long previousTimeNanos = nanos;
  nanos = currentTimeNanos;

  try {
    // ...
  } catch (Throwable t) {
    nanos = previousTimeNanos;
    throw t;
  }
}

Expected outcome:
PMD reports a violation but that should not apply to a class field.

#	File	Line	Problem
1	TimerWheel.java	90	The value assigned to field 'nanos' is never used (overwritten on line 111)

Running PMD through: Gradle
This requires workaround in gradle/gradle#24502

[harbulot]

Not sure if I should open a different issue, but I've noticed a very similar false-positive for UnusedAssigment in cases where it assumes that something won't be null in the control flow that follows (no exception catching involved).

Here is a full example (slightly contrived), using maven-pmd-plugin version 3.20.1-pmd-7-SNAPSHOT configured with PMD 7.0.0-rc2:

package com.example;

import java.util.Iterator;
import java.util.List;
import java.util.Arrays;

public class Example {    
    private int testResult;
    private List<Integer> testList;
    
    public int getTestResult() {
        return testResult;
    }
    
    public List<Integer> getTestList() {
        return testList;
    }
    
    public void setTestList(List<Integer> testList) {
        this.testList = testList;
    }
    
    public void run() {
        testResult = -1; // This is flagged as UnusedAssigment
        if (testList == null) {
            return;
        }
        
        int i = 0;
        Iterator<Integer> it = testList.iterator();
        while(it.hasNext()) {
            Integer value = it.next();
            if (value != null && value > 10) {
                testResult = i;
                break;
            }
            i++;
        }
    }
}

This flags the assignment at line 24 (testResult = -1;) as UnusedAssigment. That's of course not necessarily the case if testList is indeed null.

I've also seen this false positive in a piece of code that goes through a conditional while loop in a similar way, along these lines:

// Sample data structure
public class Node {
    String value;
    Node nextNode;
}

public class Example {
    private String matchingEntry;
    
    private String getMatchingEntry() { return matchingEntry; }
    
    private void findPosition() {
        matchingEntry = null; // This is flagged as UnusedAssigment, even if it doesn't know the result of loadNodeChain()
        
        Node head = loadNodeChain(); // May or may not be null...
        while (head != null) {
            if (head.getValue() != null && head.getValue().startsWith("a")) {
                matchingEntry = head.getValue();
                break;
            }
            head = head.nextNode();
        }
    }
}