Skip to content

UseEnumSetOf introduces circular class-init ClassCastException #1157

Description

@protocol7

Here's a fun edge case, feel free to ignore :)

What version of OpenRewrite are you using?

  • org.openrewrite.recipe:rewrite-migrate-java:3.38.0
  • org.openrewrite:rewrite-core:8.85.7
  • org.openrewrite:rewrite-java:8.85.7

What is the smallest, simplest way to reproduce the problem?

UseEnumSetOf rewrites Set.of() to EnumSet.noneOf(E.class) without checking whether E's own constants have a circular class-initialization dependency back on the class that declares the field being rewritten. When that's the case, the resulting code compiles fine but can throw ClassCastException at runtime depending on which class happens to get touched first.

@Test
void useEnumSetOfIntroducesCircularStaticInitClassCastException() {
  rewriteRun(
      spec -> spec.recipe(new UseEnumSetOf(true)),
      // language=java
      java(
          """
          package com.helloworld;

          import java.util.Set;

          public enum ConfigType {
            ENUM,
            BOOLEAN,
            INTEGER;

            public static final Set<ConfigProperty> SUBSCRIBE_TO_ALL = Set.<ConfigProperty>of();
          }
          """,
          spec -> spec.markers(javaVersion(25))),
      // language=java
      java(
          """
          package com.helloworld;

          import static com.helloworld.ConfigType.BOOLEAN;
          import static com.helloworld.ConfigType.ENUM;
          import static com.helloworld.ConfigType.INTEGER;

          public enum ConfigProperty {
            TRIAL_TYPE(ENUM),
            IS_TRIAL_ENABLED(BOOLEAN),
            MIN_ACCOUNT_AGE_MINUTES(INTEGER);

            ConfigProperty(final ConfigType type) {}
          }
          """,
          spec -> spec.markers(javaVersion(25))));
}

ConfigProperty's constants are constructed with arguments (ENUM, BOOLEAN, INTEGER) that are static fields of ConfigType, and ConfigType separately declares a field of type Set<ConfigProperty>.

What did you see instead?

The recipe rewrites ConfigType's field:

 package com.helloworld;

+import java.util.EnumSet;
 import java.util.Set;

 public enum ConfigType {
   ENUM,
   BOOLEAN,
   INTEGER;

-  public static final Set<ConfigProperty> SUBSCRIBE_TO_ALL = Set.<ConfigProperty>of();
+  public static final Set<ConfigProperty> SUBSCRIBE_TO_ALL = EnumSet.noneOf(ConfigProperty.class);
 }

This compiles cleanly. But if anything touches ConfigProperty before ConfigType (e.g. a caller that only needs ConfigProperty.TRIAL_TYPE, or class-loading order in a larger app), it throws at runtime (reproduced by hand-compiling the rewritten output with javac/java 17.0.19, outside the recipe test harness):

Exception in thread "main" java.lang.ExceptionInInitializerError
	at com.helloworld.ConfigProperty.<clinit>(ConfigProperty.java:8)
	at com.helloworld.Main.main(Main.java:5)
Caused by: java.lang.ClassCastException: class com.helloworld.ConfigProperty not an enum
	at java.base/java.util.EnumSet.noneOf(EnumSet.java:113)
	at com.helloworld.ConfigType.<clinit>(ConfigType.java:12)
	... 2 more

ConfigProperty genuinely is an enum — the message is misleading. What happens: ConfigProperty.<clinit> starts constructing TRIAL_TYPE, which needs ConfigType.ENUM, which triggers ConfigType.<clinit> (nested, same thread). ConfigType.<clinit> finishes its own constants and reaches SUBSCRIBE_TO_ALL = EnumSet.noneOf(ConfigProperty.class), which reflectively invokes ConfigProperty.values() via Class.getEnumConstantsShared(). But ConfigProperty's own $VALUES array hasn't been assigned yet — its <clinit> is still stuck constructing the very first constant, several frames up the stack. values() throws NullPointerException; getEnumConstantsShared() swallows InvocationTargetException/etc. and returns null; EnumSet.noneOf sees null and throws ClassCastException("... not an enum") instead of propagating the real cause.

We hit this for real in an internal enum pair analogous to the above (one enum's constants carrying a "type" from a sibling enum that also holds an EnumSet.noneOf(...) field) during a batch run of migration recipes, and had to revert the change by hand.

What did you expect to see?

Either:

  1. UseEnumSetOf detects that E's own class-initialization can re-enter the class declaring the field (directly, or via a static import used in one of E's constant constructor arguments) and skips the rewrite, leaving Set.of()/Collections.emptySet() in place, or
  2. At minimum, the recipe should not apply when E's constants reference static state on another enum that isn't yet guaranteed to be initialized — this is inherently unsafe for any pair of enums with a two-way static dependency, regardless of which one happens to load first in a given run.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions