New rule to handle isEnum usage

Hi, a little while a go I found some odd behaviour in the Neo4j OGM, and raised an issue there: https://github.com/neo4j/neo4j-ogm/issues/643 this led the developer there to post this into twitter https://twitter.com/rotnroll666/status/1146316196876378112
Basically if you use isEnum to check if something is an enum it might not always do what you expect, specifically in the case when the enum has an instance method then isEnum is false.

I had a look, but didn’t see a rule for this currently, would it be a useful thing to catch/flag? Seems its caught out a few projects from the twitter. Hopefully the commit that was used to fix the issue in the Neo4j OGM project provides guidance on what the code should be.

Thanks,

Matt

Hi Matthew,

Interesting bug indeed. Thanks for sharing it.
I understand why isEnum cannot be used in this case (JDK-6708424 ticket).
We can create a rule detecting calls to isEnum() on any object of type Class<?>:

Class<?> clazz = ...;
clazz.isEnum(); // Noncompliant

The solution we would suggest is however not totally clear to me. When I look at the pull request in Neo4J OGM I see

    public static boolean isEnum(Class<?> clazz) {

        return clazz.isEnum() || Enum.class.isAssignableFrom(clazz);
    }

Why use
clazz.isEnum() || Enum.class.isAssignableFrom(clazz)
and not just
Enum.class.isAssignableFrom(clazz)?

In which case is Enum.class.isAssignableFrom(clazz) false and clazz.isEnum() true?
Or is it for performance reasons?

Cheers,
Nicolas

1 Like

Hi,

I had a dig around in the Spring issue that was mentioned in the twitter replies, and in https://github.com/spring-projects/spring-data-mongodb/blob/master/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java it looks like they just moved to using the isAssignableFrom method. I’m not sure why both were included, but I think/it looks like just using isAssignableFrom should be fine.

Cheers,

Matt

1 Like