scijava / scijava-common

A plugin framework and application container with built-in extensibility mechanism :electric_plug:
BSD 2-Clause "Simplified" License
87 stars 52 forks source link

Possible issue with use of equals() in scijava.util.LastRecentlyUsed.java #453

Closed scuniff closed 1 year ago

scuniff commented 1 year ago

The Static Code Analyzer (Spot Bugs) flagged the following as a possible issue.

In scijava.util.LastRecentlyUsed.java (https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/util/LastRecentlyUsed.java)

The equals() method is comparing an Object(newValue) to an array(previous).

public boolean replace(final int index, T newValue) {
    final Object previousValue = get(index);
    if (previousValue == null) {
        throw new IllegalArgumentException("No current entry at position " +
            index);
    }
    if (newValue.equals(previous)) return false;

Was previousValue meant to be used? Should it be this:

    if (newValue.equals(previousValue)) return false;

Description from Spot Bugs:

Bug kind and pattern: EC - EC_ARRAY_AND_NONARRAY

equals() used to compare array and nonarray This method invokes the .equals(Object o) to compare an array and a reference that doesn't seem to be an array. If things being compared are of different types, they are guaranteed to be unequal and the comparison is almost certainly an error. Even if they are both arrays, the equals() method on arrays only determines if the two arrays are the same object. To compare the contents of the arrays, use java.util.Arrays.equals(Object[], Object[]).

Bug kind and pattern: EC - EC_ARRAY_AND_NONARRAY

ctrueden commented 1 year ago

Thanks for the report, @scuniff!