Skip to content

GROOVY-11929: drop abstract methods from methodsForSuper if no MOP match#2472

Merged
eric-milles merged 1 commit into
masterfrom
GROOVY-11929
Apr 20, 2026
Merged

GROOVY-11929: drop abstract methods from methodsForSuper if no MOP match#2472
eric-milles merged 1 commit into
masterfrom
GROOVY-11929

Conversation

@eric-milles
Copy link
Copy Markdown
Member

@eric-milles eric-milles commented Apr 18, 2026

Java classes as well as Groovy classes with special super circumstances (see GROOVY-11929) have abstract methods in the methodsForSuper index not replaced by MOP methods. These can be discarded, since they are not callable. This is an extension of the culling of bridge and some other methods from this index.

In the test example, the index for EntityDaoImpl contains an entry save: ["save(T) from AbstractDao", "save(Entity) from EntityDao"] before MetaClassImpl#replaceWithMOPCalls runs. After it runs the index contains save: ["super$2$save(Object) from EntityDaoImpl", "save(Entity) from EntityDao"]. "save(Entity)" is abstract and is not replaced by a MOP method (due to the parameter matching). So discard it to prevent the runtime error.

@eric-milles eric-milles requested a review from paulk-asert April 18, 2026 00:34
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.1426%. Comparing base (9aa5d93) to head (d003017).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...haus/groovy/runtime/metaclass/MetaMethodIndex.java 75.0000% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##             master      #2472         +/-   ##
=================================================
+ Coverage          0   67.1426%   +67.1426%     
- Complexity        0      30913      +30913     
=================================================
  Files             0       1436       +1436     
  Lines             0     119842     +119842     
  Branches          0      21239      +21239     
=================================================
+ Hits              0      80465      +80465     
- Misses            0      32675      +32675     
- Partials          0       6702       +6702     
Files with missing lines Coverage Δ
src/main/java/groovy/lang/MetaClassImpl.java 76.9744% <100.0000%> (ø)
...haus/groovy/runtime/metaclass/MetaMethodIndex.java 93.4959% <75.0000%> (ø)

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes GROOVY-11929 by ensuring methodsForSuper does not retain abstract methods that are not replaced by MOP “super$N$…” methods, preventing runtime failures when resolving super calls in certain generic/interface override scenarios.

Changes:

  • Add regression test covering a generic DAO + interface override scenario where an abstract method could remain in methodsForSuper.
  • Extend MetaClassImpl#replaceWithMOPCalls culling logic to remove abstract methods from methodsForSuper when no MOP match exists.
  • Minor refactor/clarification in MetaMethodIndex#isOverridden condition structure (no behavioral change).

Reviewed changes

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

File Description
src/test/groovy/bugs/Groovy11929.groovy New regression test reproducing the problematic super.save(...) resolution scenario.
src/main/java/org/codehaus/groovy/runtime/metaclass/MetaMethodIndex.java Small readability-only tweak to override decision logic.
src/main/java/groovy/lang/MetaClassImpl.java Drops non-replaced abstract methods from methodsForSuper during MOP replacement to avoid selecting non-invokable methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@eric-milles eric-milles merged commit e8d46af into master Apr 20, 2026
49 checks passed
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.

4 participants