Illegal reflective access to OptionalDouble.isPresent

[lukeu]

Similarly to #214 and friends, this time for java.util.OptionalDouble (running EV 3.1.12 and Java 11):

Example class to test:

final class MyClass {
    final OptionalDouble measure;

    MyClass(OptionalDouble measure) {
        this.measure = measure;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) {
            return true;
        }
        if (!(obj instanceof MyClass)) {
            return false;
        }
        return Objects.equals(measure, ((MyClass) obj).measure);
    }

    @Override
    public int hashCode() {
        return Objects.hash(measure);
    }
}

(If you search/replace OptionalDouble with String then the warning is not emitted.)

I guess something similar to the existing Prefab value for Optional might do the trick?

OptionalInt and OptionalLong might also want similar treatment.

[lukeu]

Now that I've found out how to get a complete list, I've found the following in our build as well:

javax.swing.tree.DefaultMutableTreeNode (various fields)
java.util.EventObject.source
java.awt.Font (various fields)
java.beans.ChangeListenerMap.map
java.beans.PropertyChangeSupport.map
java.beans.PropertyChangeSupport.source
java.util.AbstractList.modCount

Should I try to put together a complete example that demonstrates all of these?

[lukeu]

I managed to put together an extended example of all but the last. (Still not sure where AbstractList is coming in, and it's taking a long time to try and track down.)

Anyway, here's the extended example for the rest:

import java.awt.Font;
import java.beans.PropertyChangeSupport;
import java.util.Arrays;
import java.util.EventObject;
import java.util.Objects;
import java.util.OptionalDouble;
import java.util.OptionalInt;
import java.util.OptionalLong;
import javax.swing.tree.DefaultMutableTreeNode;
import com.google.common.collect.ImmutableList;

final class MyClass {
    final OptionalDouble optionalD;
    final OptionalInt optionalI;
    final OptionalLong optionalL;
    final DefaultMutableTreeNode node;
    final EventObject eventObj;
    final PropertyChangeSupport changeSupport = new PropertyChangeSupport(this);
    final Font font = Font.getFont(Font.MONOSPACED);
    final ImmutableList<String> strings;

    public MyClass(OptionalDouble optionalD, OptionalInt optionalI, OptionalLong optionalL,
            DefaultMutableTreeNode node, EventObject eventObj, ImmutableList<String> strings) {
        this.optionalD = optionalD;
        this.optionalI = optionalI;
        this.optionalL = optionalL;
        this.node = node;
        this.eventObj = eventObj;
        this.strings = strings;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) {
            return true;
        }
        if (!(obj instanceof MyClass)) {
            return false;
        }
        MyClass that = (MyClass) obj;
        return Arrays.equals(fieldValues(), that.fieldValues());
    }

    @Override
    public int hashCode() {
        return Objects.hash(fieldValues());
    }
    
    private Object[] fieldValues() {
        return new Object[] {
                optionalD,
                optionalI,
                optionalL,
                node,
                eventObj,
                strings,
        };
    }
}

With passing test:

import java.awt.Font;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.junit.jupiter.api.Test;

class MyClassTest {

    @Test
    void test() {
        Font font1 = new Font(Font.SANS_SERIF, Font.BOLD, 25);
        Font font2 = new Font(Font.SANS_SERIF, Font.BOLD, 50);

        EqualsVerifier.forClass(MyClass.class)
                .withIgnoredFields("changeSupport", "font")
                .withPrefabValues(Font.class, font1, font2)
                .verify();
    }
}

[jqno]

Thanks for the elaborate issue description, this is really helpful!

Your analysis is correct, prefab values should be added for these types. If you're interested in making a PR, the places where you should look are JavaApiPrefabValues and JavaApiClassesTest.

If not, I'll probably be able to do this some time next week.

[jqno]

I'm adding prefab values for OptionalDouble, OptionalInt, OptionalLong, EventObject, Font and DefaultMutableTreeNode.

In your report, that leaves PropertyChangeSupport and ImmutableList. Regarding PropertyChangeSupport, that doesn't seem to be something one would include in equals. In fact, the equals method you posted doesn't include it either. Or am I missing something here?

ImmutableList already has a built-in prefab value, so I'm surprised that it would cause issues here. Is this something that you can help me reproduce?

[lukeu]

Regarding PropertyChangeSupport, that doesn't seem to be something one would include in equals.

Indeed: note this line in the test:

   .withIgnoredFields("changeSupport", "font")

It seems that if these 2 classes are fields in the tested class, involved in equals or not, then the warning is emitted. PCS is something that might be included on mutable data-classes to notify observers of state changes.

ImmutableList already has a built-in prefab value, so I'm surprised that it would cause issues here.

D'oh! sorry, I must have been testing theories about the warning on AbstractList and forgot to omit that code.

[jqno]

Ah, I see! Thanks.
I'll add something for PropertyChangeSupport as well, then.

[lukeu]

Hi, many thanks for this!

I've found the offending test regarding AbstractList (with the help of some gradle magic). It turns out we have a class that extends it:

final class MyCustomList extends AbstractList<Element> {
     ...
}

I tried Prefab values (in the test for the class that included MyCustomList) wondering if that would make the warning go away, but that didn't seem to. I'd be curious if you know any tricks for this(?)

Either way I'm happy for it not to block this issue being closed.

(Update: in fact I've embarked on a large refactoring to try and make that class implement just Iterable<Element> instead. Wish me luck ;-))

[jqno]

I've just released a new version (3.1.13) with the new prefab values.

I remember I've looked into supporting custom classes that implement AbstractList at some point, and that I ran into some issues and eventually gave up... Still, I wish you much luck with your refactoring 💪