Skip to content

GROOVY-12188: accept covariant override of abstract method regardless… - #2733

Closed
paulk-asert wants to merge 1 commit into
apache:masterfrom
paulk-asert:groovy12188
Closed

GROOVY-12188: accept covariant override of abstract method regardless…#2733
paulk-asert wants to merge 1 commit into
apache:masterfrom
paulk-asert:groovy12188

Conversation

@paulk-asert

Copy link
Copy Markdown
Contributor

… of classgen order

Since GROOVY-10687, a top-level class whose inner classes contain a closure is generated after its nest members, which can place it after its own subclass in the classgen order. The abstract-method check in ClassCompletionVerifier relied on the covariant bridge method that Verifier adds to the declaring class, so verifying a subclass first raised a spurious "same name but different return type" error. Recognise a covariant override directly instead; genuinely incompatible return types are still rejected by Verifier's covariant-method check.

… of classgen order

Since GROOVY-10687, a top-level class whose inner classes contain a closure is generated after its nest members, which can place it after its own subclass in the classgen order. The abstract-method check in ClassCompletionVerifier relied on the covariant bridge method that Verifier adds to the declaring class, so verifying a subclass first raised a spurious "same name but different return type" error.
Recognise a covariant override directly instead; genuinely incompatible return types are still rejected by Verifier's covariant-method check.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.2743%. Comparing base (3240fcf) to head (3334e37).

Files with missing lines Patch % Lines
...ehaus/groovy/classgen/ClassCompletionVerifier.java 0.0000% 2 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2733        +/-   ##
==================================================
+ Coverage     69.2732%   69.2743%   +0.0012%     
- Complexity      34769      34771         +2     
==================================================
  Files            1542       1542                
  Lines          130456     130461         +5     
  Branches        23786      23790         +4     
==================================================
+ Hits            90371      90376         +5     
  Misses          31962      31962                
  Partials         8123       8123                
Files with missing lines Coverage Δ
...ehaus/groovy/classgen/ClassCompletionVerifier.java 84.2199% <0.0000%> (-0.7533%) ⬇️

... and 7 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.

@testlens-app

testlens-app Bot commented Jul 24, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 3334e37
▶️ Tests: 105866 executed
⚪️ Checks: 31/31 completed


Learn more about TestLens at testlens.app.

@paulk-asert

Copy link
Copy Markdown
Contributor Author

Cherry-picked manually onto the GROOVY_5_0_X branch as the minimal fix. For master, see #2734 which should be a better long-term fix. If no impacts from the long-term fix are noticed on master, it would be a candidate for back-porting to GROOVY_5_0_X.

GROOVY_5_0_X: 09a2d2f
Master: 361a02e

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants