GROOVY-11770: StackOverflowError processing generics for kubernetes-c…

[paulk-asert]

…lient library (cycle guard instead of try/catch)

Caveat: the cycle guard uses object identity. If the recursion ever creates fresh ClassNode instances at each level for the same logical pair, identity won't catch it and we'd be back to SOE territory, except now without the catch as a backstop.

We could consider keeping the SOE catch around the recursive call as a final safety net (with a comment like "should be unreachable; cycle guard handles known cases").

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.1396%. Comparing base (5a41b8f) to head (539926d).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
.../codehaus/groovy/ast/tools/WideningCategories.java	91.6667%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2498        +/-   ##
==================================================
+ Coverage     67.1374%   67.1396%   +0.0021%     
- Complexity      31622      31629         +7     
==================================================
  Files            1451       1451                
  Lines          122565     122582        +17     
  Branches        22007      22010         +3     
==================================================
+ Hits            82287      82301        +14     
  Misses          33199      33199                
- Partials         7079       7082         +3     

Files with missing lines	Coverage Δ
.../codehaus/groovy/ast/tools/WideningCategories.java	83.5206% <91.6667%> (+1.1206%)	⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Addresses GROOVY-11770 by preventing StackOverflowError during generics LUB (lowest-upper-bound) computation (notably seen with kubernetes client types) via an explicit recursion/cycle guard rather than relying on catching SOE.

Changes:

• Add a new STC regression test that exercises an F-bounded generic LUB cycle through a wrapper type.
• Introduce a LowestUpperBoundContext to track in-flight (t1, t2) LUB computations and break recursion cycles during LUB parameterization.
• Refactor lowestUpperBound(ClassNode, ClassNode) to route through a context-aware private overload.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/test/groovy/groovy/transform/stc/GenericsSTCTest.groovy	Adds GROOVY-11770 regression test intended to ensure LUB computation does not overflow the stack.
src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java	Adds identity-based cycle guard context for recursive LUB parameterization; removes SOE catch.

[Copilot]

The comment says “structural cycle”, but the guard currently detects cycles using identity (TypePair uses ==). Either adjust the wording to reflect identity-based detection, or change the keying strategy to something structural/stable so the comment matches the behavior.

Suggested change

	// GROOVY-11770: structural cycle (e.g. LUB(B, D) where B extends A<W<B>>, D extends A<W<D>>)
	// GROOVY-11770: recursion guard for an already-expanding type pair (e.g. LUB(B, D) where B extends A<W<B>>, D extends A<W<D>>)

[Copilot]

This test comment says the cycle is “caught by structural cycle guard”, but the new guard in WideningCategories is identity-based (and may not be structural if new ClassNode instances are created). Suggest rewording the comment to match the implementation (e.g., “caught by in-flight LUB pair guard”).

Suggested change

	// which needs LUB(B, D) again. Caught by structural cycle guard.
	// which needs LUB(B, D) again. Caught by the in-flight LUB pair guard.

[Copilot]

LowestUpperBoundContext uses identity to track in-flight (t1, t2) pairs. However, generics rewriting utilities (e.g., correctToGenericsSpecRecurse/makeClassSafe0) can produce fresh ClassNode instances for the same logical type, which can defeat identity-based cycle detection; with the StackOverflowError catch removed, a missed cycle could reintroduce a hard SOE. Consider keeping the SOE catch as a last-resort backstop, or normalizing the tracked pair to a stable representation (redirect + generics signature) so logically-equal nodes match.

Suggested change

	basicType = lowestUpperBound(ctx, t1, t2);
	basicType = lowestUpperBound(ctx, t1, t2);
	} catch (StackOverflowError ignore) {
	// Some generics-rewriting paths can create fresh ClassNode instances for the same
	// logical type, which may defeat identity-based cycle tracking in the context.
	// Fall back rather than allowing an unbounded recursive expansion to overflow.
	basicType = fallback;