Skip to content

Fix #13068: make PluginDependenciesResolver methods default - #13069

Merged
ascheman merged 1 commit into
apache:maven-4.0.xfrom
aschemaven:bugfix/40x-plugin-dependencies-resolver-defaults
Sep 8, 2026
Merged

Fix #13068: make PluginDependenciesResolver methods default#13069
ascheman merged 1 commit into
apache:maven-4.0.xfrom
aschemaven:bugfix/40x-plugin-dependencies-resolver-defaults

Conversation

@ascheman

@ascheman ascheman commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #13068.

4.0.0-rc-6 added resolveCoreExtensionAndFlatten and resolvePluginAndFlatten to the internal PluginDependenciesResolver as abstract methods (#12335). Implementations that live out of tree — IntelliJ IDEA, Eclipse m2e, NetBeans all have one — are compiled against a different Maven than they run on, so the first plugin resolution dies with AbstractMethodError.

This is the fix @gnodet announced for rc-7 in reply to the rc-6 vote.

Change

Both methods become default and delegate to the pre-existing resolvePlugin, which DefaultPluginDependenciesResolver already treats as an alias — there, resolvePlugin delegates to resolvePluginAndFlatten.

resolvePlugin stays abstract on purpose: giving it a default too would let an implementation that overrides neither method recurse infinitely. There is an @implSpec note warning against delegating back.

The default for resolveCoreExtensionAndFlatten is documented as best effort — a dedicated implementation additionally reads the extension's artifact descriptor to apply relocations and run MavenPluginDependenciesValidator. If you would rather not guess there, I am happy to make it throw UnsupportedOperationException; say the word.

Tests

UnitPluginDependenciesResolverDefaultMethodsTest:

  • a resolver implementing exactly the pre-forward-port method set; its compiling is the compatibility assertion
  • both defaults asserted to delegate to resolvePlugin with the expected arguments
  • a Method.isDefault() guard, so re-abstracting either method fails the build

IntegrationMavenITgh13068LegacyPluginDependenciesResolverTest: builds a core extension that overrides PluginDependenciesResolver with only the pre-rc-6 method set, deliberately compiled against maven-core:4.0.0-rc-5, installs it, and runs a build that uses it via .mvn/extensions.xml. It asserts an error-free build and two marker lines, so it cannot pass without actually exercising the extension.

Both were verified red without the change. Reverting only the interface gives:

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0 <<< FAILURE!
Caused by: java.lang.AbstractMethodError: Receiver class
  org.apache.maven.its.gh13068.LegacyPluginDependenciesResolver does not define or inherit
  an implementation of the resolved method 'abstract ... resolvePluginAndFlatten(...)'
  at DefaultMavenPluginManager.createPluginRealm(DefaultMavenPluginManager.java:421)

Real-world verification

@chrisdutz confirmed that a maven-4.0.x build carrying this patch imports Apache PLC4X in IntelliJ IDEA cleanly, where stock rc-6 produces a cascade of errors. PLC4X is affected via resolveCoreExtensionAndFlatten because it declares core extensions in .mvn/extensions.xml.

Worth noting the JetBrains side does not make this unnecessary: resolveCoreExtensionAndFlatten is implemented only on their master and is absent from IntelliJ IDEA 2026.2.2, the current release.

Note on the base branch

This targets maven-4.0.x because that is where the regression shipped and where rc-7 / GA come from. Happy to open the equivalent against master first if you prefer that order — the change applies unmodified, the baselines are identical.

@ascheman ascheman added this to the 4.0.0-rc-7 milestone Sep 7, 2026
@ascheman ascheman added the bug Something isn't working label Sep 7, 2026
@ascheman
ascheman requested review from cstamas and gnodet and a lite review from Copilot September 7, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped to restoring binary compatibility, uses safe delegation consistent with existing DefaultPluginDependenciesResolver behavior, and is protected by both unit and integration regression tests.

Pull request overview

This PR restores binary compatibility for out-of-tree PluginDependenciesResolver implementations by changing the two methods introduced as abstract in 4.0.0-rc-6 (resolveCoreExtensionAndFlatten, resolvePluginAndFlatten) into default methods that delegate to the pre-existing resolvePlugin, preventing AbstractMethodError in IDE embedders compiled against older Maven versions.

Changes:

  • Make PluginDependenciesResolver.resolveCoreExtensionAndFlatten(...) and resolvePluginAndFlatten(...) default methods delegating to resolvePlugin(...), with @implSpec guidance to avoid recursion and clarify best-effort behavior.
  • Add unit coverage to lock in the “must remain default methods” contract and verify delegation behavior.
  • Add an integration test that compiles a legacy resolver against maven-core:4.0.0-rc-5 and verifies it runs cleanly on newer Maven via a core extension.
File summaries
File Description
impl/maven-core/src/main/java/org/apache/maven/plugin/internal/PluginDependenciesResolver.java Turns the two newly-added abstract methods into default methods delegating to resolvePlugin to preserve binary compatibility.
impl/maven-core/src/test/java/org/apache/maven/plugin/internal/PluginDependenciesResolverDefaultMethodsTest.java Verifies delegation and asserts the methods are truly default (guards against regression).
its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh13068LegacyPluginDependenciesResolverTest.java End-to-end regression test proving legacy compiled resolver works without AbstractMethodError.
its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java Ensures the new integration test is included/ordered in the core IT suite.
its/core-it-suite/src/test/resources/gh-13068-legacy-plugin-dependencies-resolver/** Adds the test fixture projects (legacy extension + client using .mvn/extensions.xml).
Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

resolveCoreExtensionAndFlatten and resolvePluginAndFlatten were added as
abstract methods in 4.0.0-rc-6, forward-porting the Maven 3.10.0 changes
(apache#12335). The interface is documented as internal, but it is the only
hook Maven offers for influencing plugin resolution, and every major
Java IDE overrides it: IntelliJ IDEA, Eclipse m2e and NetBeans. Those
implementations live out of tree and are compiled against one Maven
while running on another, so adding abstract methods turns plugin
resolution into an AbstractMethodError.

Both methods now default to the pre-existing resolvePlugin, which
DefaultPluginDependenciesResolver already treats as an alias -- there,
resolvePlugin delegates to resolvePluginAndFlatten. resolvePlugin itself
deliberately stays abstract: a default there would let an implementation
overriding neither method recurse infinitely.

The default for resolveCoreExtensionAndFlatten is best effort. A
dedicated implementation additionally reads the extension's artifact
descriptor to apply relocations and run MavenPluginDependenciesValidator.

Verified against the class shipped in IntelliJ IDEA 2026.2.2, which
implements resolvePluginAndFlatten but not resolveCoreExtensionAndFlatten
and therefore still fails on stock rc-6.

Adds a unit test pinning the compatibility contract and a core IT that
builds an extension against maven-core 4.0.0-rc-5 -- the last release
before the methods existed -- and runs a build through it.

Verified by Christofer Dutz (@chrisdutz) on Apache PLC4X: a build
carrying this change imports cleanly in IntelliJ IDEA, where stock rc-6
fails.

Closes apache#13068

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W2HnhKt7MJWYsSSVUmPrvU
@ascheman
ascheman force-pushed the bugfix/40x-plugin-dependencies-resolver-defaults branch from e191d1b to d90273d Compare September 7, 2026 19:51

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fix is correct and the test coverage is exemplary.

ASK (change-type signals detected):

  1. New default methods on a public interface — binary compat contract, delegation safety
  2. Delegation cycle risk — mutual delegation between resolvePluginresolvePluginAndFlatten
  3. Test adequacy — does the IT fixture actually exercise the default method path, or does it accidentally trigger a full override?

NARROW / READ / DECIDE:

Default method delegation chain — non-issue. The chain from BootstrapCoreExtensionManager through a legacy impl is: resolveCoreExtensionAndFlatten default → resolvePlugin (legacy impl overrides, delegates to DefaultPluginDependenciesResolver.resolvePlugin) → resolvePluginAndFlatten (in DefaultPluginDependenciesResolver, not the interface default) → resolveInternal. No recursion. null artifact is handled at line 255 of DefaultPluginDependenciesResolver. The DefaultMavenPluginManager calls resolvePluginAndFlatten directly — for a legacy impl that path hits the interface default → resolvePlugin → legacy impl → delegate. Also clean.

Delegation cycle warning — non-issue. The warning on resolvePlugin ("must not delegate to resolvePluginAndFlatten without also overriding it") is correct. An impl that does delegate there would recurse, and the warning catches it. The warning in the <p> tag on a @Deprecated method is slightly easy to miss, but the PR body documents the intent clearly and DefaultPluginDependenciesResolver sets the example.

Best-effort resolveCoreExtensionAndFlatten default — non-issue. The default skips the artifact descriptor read (relocations, MavenPluginDependenciesValidator). This is acknowledged in the Javadoc and confirmed real-world by @chrisdutz — IntelliJ IDEA 2026.2.2 works with this. The tradeoff (silent downgrade vs. UnsupportedOperationException) favors leniency for a compat shim.

IT fixture integrity — non-issue. The LegacyPluginDependenciesResolver implements resolvePlugin (which logs the marker line), not the newer methods. When DefaultMavenPluginManager calls resolvePluginAndFlatten on it, the call routes through the interface default → resolvePlugin → legacy impl logs the marker. The verifyTextInLog assertion on "[gh-13068] resolvePlugin reached for maven-clean-plugin" cannot pass without the extension being active and the default method being traversed. The test cannot false-pass.

Static analysis: ast-grep flagged a broad-exception catch in TestSuiteOrdering.java — pre-existing pattern, not introduced by this PR (the PR only adds a single addTestSuite(...) line). Semgrep: no findings.

Solid PR. The compat regression is real, the fix is minimal, and both the unit test (compile-time contract + reflection guard) and the IT (cross-version linkage + execution proof) are doing genuine work.


This review was generated by an AI agent, Hermès, on behalf of @gnodet.

@ascheman
ascheman merged commit 349395c into apache:maven-4.0.x Sep 8, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants