[java] LooseCoupling - false-positive for Stack, if implementation methods of stack actually used

[Wolf2323]

Affects PMD Version: 7.1.0

Rule: LooseCoupling

Please provide the rule name and a link to the rule documentation: https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#loosecoupling

Description: LooseCoupling detects the Stack, even if implementation methods are used. Previously in PMD 6 this was not an issue, now with PMD 7 it is.

Code Sample demonstrating the issue:

    public void updateConflictStateWithFlatSourceTraces() {
        final Set<? extends RecursionGuard.Traceable> traces = tracesSupplier.get();
        final Set<RecursionGuard.Traceable> result = new HashSet<>();
        final Stack<RecursionGuard.Traceable> stack = new Stack<>();
        stack.addAll(traces);
        while (!stack.isEmpty()) {
            final RecursionGuard.Traceable trace = stack.pop();
            if (!result.add(trace)) {
                continue;
            }
            trace.getSourceTraces().forEach(stack::push);
        }
        updateConflictState(result);
    }

Expected outcome: As the usage of stack methods can be seen, this should not be an violation

[INFO] PMD Failure: works.reload.relogic.recursion.RecursionFinder:164 Rule:LooseCoupling Priority:3 Avoid using implementation types like 'Stack'; use the interface instead.

Running PMD through: Maven

[jsotuyod]

Thanks for the report. This is effectively a FP.

Having said that, rather than Stack (which extends Vector and is therefore synchronized) you may want to use a Deque.

[adangel]

See also #4622 . However, the suggested fix wouldn't exclude this case, as Stack is a generic class. In terms of the rule "LooseCouling" we just might end up excluding Stack completely.

However, as @jsotuyod mentioned, Stack extends Vector, and for that we have a separate rule ReplaceVectorWithList. We might need to add a new rule "ReplaceStackWithDeque". Note: Stack and Vector are from java 1.0 and predate the Java Collections Framework. Vector is considered legacy implementation, and so can be Stack.

[Wolf2323]

In general this sounds good for me. I now already use Deque.

I only want to add one more information that may can have an impact on this. We also use org.json.simple.JSONObject.

public class JSONObject extends HashMap implements Map, JSONAware, JSONStreamAware {

and there we also have a violation for calling writeJSONString, and there is no replacement for us. I supressed the violation in this case.

In general i think every lib can have classes that violate this rule, but it should be fine if implementation methods are called and therefor Map is not a replacement. But i can completely understand if only java cases are covered and not every possible lib.