Preserve dependency scope in plugin artifacts - fix #12497

[slawekjaranowski]

What changed:

• Added RepositoryUtils.toArtifact(ArtifactResult) to convert resolved artifacts and copy dependency metadata from - Made RepositoryUtils.toArtifact(Dependency) public (was package-private).
• Added private DefaultMavenPluginManager.toMavenArtifacts(DependencyResult) that iterates over dependency graph nodes instead of raw artifact results.
• Used toMavenArtifacts for both plugin realm and extension artifact resolution.

Why:

• PluginDescriptor.getArtifacts() returned artifacts with null scope in Maven 3.10.0-rc-1.
• Restoring scope/optional metadata brings behavior back in line with 3.9.x and avoids regressions in plugins that depend on artifact scope.

Analogous fix was applied to Maven 4 in #1928

fix #12497

Following this checklist to help us incorporate your
contribution quickly and easily:

• Your pull request should address just one issue, without pulling in other changes.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Each commit in the pull request should have a meaningful subject line and body.
  Note that commits might be squashed by a maintainer on merge.
• Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  This may not always be possible but is a best-practice.
• Run mvn verify to make sure basic checks pass.
  A more thorough check will be performed on your pull request automatically.
• You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
• In any other case, please file an Apache Individual Contributor License Agreement.

[gnodet]

AI Review — PR #12503

Verdict: ✅ APPROVE

Summary: This PR correctly fixes a regression in 3.10.0-rc-1 where PluginDescriptor.getArtifacts() returned artifacts with null scope. The new RepositoryUtils.toArtifact(ArtifactResult) overload navigates the ArtifactResult → ArtifactRequest → DependencyNode → Dependency chain to recover scope and optional metadata. The approach is minimal, null-safe via Optional, and follows existing codebase patterns.

Findings

Important — CI Spotless failure
RepositoryUtils.java line 177: Spotless rejects the space before the closing ) — } ); should be });. All three build jobs fail at spotless:check. Running mvn spotless:apply -B in maven-core will fix it.

// Current (fails Spotless):
                } );
// Fix:
                });

Suggestion — Javadoc
The new method only has @since 3.10.0. A brief description would help future readers:

/**
 * Converts a resolved {@link ArtifactResult} to a Maven Artifact,
 * preserving dependency scope and optional flag from the resolution
 * request's dependency node.
 *
 * @since 3.10.0
 */

Suggestion — Test coverage
No test for the new toArtifact(ArtifactResult) overload. A test verifying scope/optional preservation through the ArtifactResult → DependencyNode → Dependency chain would help prevent this regression from resurfacing.

What I Appreciate

• Follows existing patterns precisely — the private toArtifact(Dependency) already does setScope()/setOptional(), and this recovers the same info from the ArtifactResult path.
• Null-safety via Optional.ofNullable(...).map(...).map(...).ifPresent(...) — if any link is absent, the artifact retains its prior state gracefully.
• The method reference change in DefaultMavenPluginManager is clean and unambiguous.
• Backward compatible — the root plugin artifact with null scope gets a no-op setScope(null).

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet

[slawekjaranowski]

@gnodet @cstamas I have checked a master branch if is affected, and make this change similar to #1928

so please review again

[gnodet]

Re-review: updated approach (force-push e2b4130)

The approach was reworked since my previous review — this version is a significant improvement.

What changed

Previous approach used Optional.ofNullable(r.getDependency()).map(Dependency::getScope) to patch the scope from ArtifactResult into the converted artifact.

New approach shifts to getDependencyNodeResults() (from Resolver 2.0.21, now available after #12504 merged) and converts directly from Dependency via the existing toArtifact(Dependency) method, which already correctly sets scope via result.setScope(dependency.getScope()).

What this PR does well

• Uses the natural API path — getDependencyNodeResults() gives access to Dependency objects with their scopes intact, eliminating the need for workaround extraction
• DRY — both createPluginRealm and resolveExtensionArtifacts now share toMavenArtifacts(DependencyResult)
• Clean API addition — toArtifact(Dependency) promoted from private to public with proper Javadoc and @since 3.10.0
• Correct filter — n.getArtifact().getPath() != null is a direct check for resolved artifacts, equivalent to the previous ArtifactResult::isResolved

Checklist

Check	Status
Tests	✓ Existing IT from previous version still applies
Docs	✓ Javadoc on newly-public method
Public API	✓ Addition only (private → public), @since 3.10.0
CI	⏳ Builds passing, integration tests pending

LGTM — cleaner than the previous version. ✅

---

Reviewed with Claude Code on behalf of Guillaume Nodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.