fix(apt): remove incorrect 'new QClass()' suggestion from circular Q-class warning

[o54711254]

Background

In #1739 I added compile-time detection for circular Q-class references that can
cause class initialization deadlocks. The warning message suggests three
workarounds:

To avoid deadlock, consider:
  (1) Removing the bidirectional association on one side.
  (2) Pre-initializing Q-classes in a single thread before handling requests (e.g. via @PostConstruct).
  (3) Using 'new QClass("alias")' instead of static field access in your repositories.

Suggestion (3) does not work. This PR removes it.

Why (3) does not work

Creating an instance of a class triggers class initialization (JLS 12.4.1),
exactly as accessing a static field does. So new QOrder("alias") still runs
QOrder.<clinit>, which initializes the static field, whose constructor
creates the other Q-class — the same chain that causes the deadlock.

Given a bidirectional @OneToOne between Order and Payment, the generated
class looks like this:

public class QOrder extends EntityPathBase<Order> {
    private static final PathInits INITS = PathInits.DIRECT2;
    public static final QOrder order = new QOrder("order");

    public final QPayment payment;

    public QOrder(Class<? extends Order> type, PathMetadata metadata, PathInits inits) {
        super(type, metadata, inits);
        this.payment = inits.isInitialized("payment")
            ? new QPayment(forProperty("payment"), inits.get("payment"))
            : null;
    }
}

new QOrder("alias")
→ triggers QOrder.<clinit>
→ initializes the static field order
→ its constructor creates QPayment
→ triggers QPayment.<clinit>

Avoiding static field access does not avoid static field initialization.

Reproduction

Two threads, each entering only via new:

CountDownLatch start = new CountDownLatch(1);

Thread a = new Thread(() -> {
    awaitQuietly(start);
    new QOrder("a");
    System.out.println("A done");
});
Thread b = new Thread(() -> {
    awaitQuietly(start);
    new QPayment("b");
    System.out.println("B done");
});

a.start();
b.start();
start.countDown();

a.join(5000);
b.join(5000);

System.out.println("A alive: " + a.isAlive());
System.out.println("B alive: " + b.isAlive());

Output:

A alive: true
B alive: true

Neither thread completed — both were blocked on class initialization monitors.

Change

Removes suggestion (3) from the warning message. (1) and (2) remain valid and
are unaffected. The detection logic itself is unchanged.

Follow-up to #1739. Sorry for the incorrect guidance in the original PR.

[velo]

Code quality review

The diagnosis is right — new QCustomer("alias") triggers QCustomer.<clinit> exactly like static field access does (JLS 12.4.1), so it never dodged the deadlock. The deadlock mechanism itself checks out: DefaultEntitySerializer.introDefaultInstance (DefaultEntitySerializer.java:425) emits public static final QOrder order = new QOrder("order"), and initEntityField (DefaultEntitySerializer.java:344) emits new QCustomer(forProperty("customer"), ...) inside that constructor. Two threads, two classes, cross-<clinit> wait.

But I think deletion is the wrong move here, and the surrounding code has defects that undercut the PR's own goal.

1. This deletes a mitigation the processor already knows how to enable

AbstractQuerydslProcessor.java:218 guards the whole detector on serializerConfig.createDefaultVariable(). That flag is the switch that removes the deadlock: it is the sole gate on emitting the static default instance (DefaultEntitySerializer.java:367), and it is a documented, first-class APT option — querydsl.createDefaultVariable (APTOptions.java:24, docs/guides/code-generation.md:101).

So suggestion (3) wasn't wrong, it was half-written. new QClass("alias") alone does nothing; -Aquerydsl.createDefaultVariable=false plus new QClass("alias") is a complete and correct fix — and it's cheaper than both surviving suggestions, which each ask the user to restructure something real (their domain model, or their application bootstrap). The processor checks this flag two lines before building the message and then declines to mention it.

Suggest restoring (3) in its complete form:

  (3) Generating Q-classes without the static default variable
      (-Aquerydsl.createDefaultVariable=false) and using 'new QClass("alias")' instead.

2. The warning also prints incorrect cycles

dfs (AbstractQuerydslProcessor.java:745-748) emits the entire traversal path, not the cycle. For A → B → C → B, the reported "cycle" is A → B → C → B — A is not part of the cycle.

Neither test covers this: circularQClassReference_producesWarning and the three-entity test both use cycles rooted at the DFS entry node, and both assert only hadWarningContaining("Circular Q-class references detected") — the header, never the payload.

The fix is one line: slice the path from the neighbour's position instead of copying it whole. If the point of this PR is "the warning tells users something untrue", the untrue cycle list belongs in scope.

3. Detection is enabled by HashMap iteration order

var serializerConfig = conf.getSerializerConfig(context.entityTypes.values().iterator().next());
if (serializerConfig.createDefaultVariable()) {

getSerializerConfig is per-type / per-package (DefaultConfiguration.java:227), and context.entityTypes is a plain HashMap (Context.java:37). Whether the detector runs at all is decided by whichever entity happens to hash first. Anyone using @Config at type or package scope gets nondeterministic warnings.

The deadlock is a property of individual types, so the config check belongs inside the graph walk rather than being sampled once globally: only types whose own config has createDefaultVariable() can participate in a static-init cycle.

4. Structural: this feature doesn't belong in this file

AbstractQuerydslProcessor is 767 lines of APT orchestration. detectCircularQClassReferences + dfs is ~65 lines of pure EntityType-graph analysis with zero APT coupling except the final printMessage. And dfs threads six parameters of mutable state through a recursion (typeMap, path, inStack, globalVisited, detectedCycles), which is the usual signal that a collaborator object is missing.

Extracting a QClassCycleDetector that takes the entity map and returns List<List<String>> would leave the processor doing formatting and printMessage and nothing else. The payoff isn't tidiness — it's that the detector becomes unit-testable on plain EntityType fixtures instead of only through full javac compile tests. That test gap is precisely why items 2 and 3 are uncovered today: you can't cheaply assert cycle contents or per-type config behaviour through CompilationSubject. The six params collapse to visit(EntityType).

While in there: the six StringBuilder.append calls building the message want to be a text block — this diff left an orphaned message.append( wrap behind.

---

Suggested path: fold 1–3 into this PR (they're all "the warning is wrong", same theme, ~15 lines), and do the extraction in 4 first so 2 and 3 can actually be tested.

[o54711254]

Thanks for the detailed review — addressed all four points:

(1) Restored suggestion (3) with the required flag — added back verbatim as you suggested.

(2) Cycle vs entry path — sliceCycleFrom now slices from the back-edge target using path.indexOf(entry) instead of copying the whole DFS path. The path now stores EntityType instances, while traversal identity uses full names, so entities sharing a simple name aren't conflated.

(3) Nondeterministic detection — removed the sample-one-entity gate. Each entity is now filtered individually by its own createDefaultVariable config before the walk, and the detector iterates starts sorted by full name so cycle report order is stable regardless of map ordering. Added a unit test that shuffles the input map and asserts stable output.

(4) Extract detector — pulled ~65 lines into QClassCycleDetector with 8 unit tests on plain fixtures. The message.append( StringBuilder chain is now a single text block with %s interpolation for the cycle list — output is byte-for-byte identical.