Skip to content

Support sealed parameter implementation hints#12117

Open
Will-thom wants to merge 1 commit into
apache:masterfrom
Will-thom:gh-11378-sealed-parameter-config
Open

Support sealed parameter implementation hints#12117
Will-thom wants to merge 1 commit into
apache:masterfrom
Will-thom:gh-11378-sealed-parameter-config

Conversation

@Will-thom
Copy link
Copy Markdown

@Will-thom Will-thom commented May 19, 2026

Fixes #11378

Summary

When a plugin parameter type is a sealed interface/class, users currently must provide the fully qualified class name in the implementation attribute. This PR allows using the simple class name instead, by scanning Class.getPermittedSubclasses() when standard Plexus class resolution fails.

For example, users can now write:

<artifact implementation="LocalArtifact">
  <name>local</name>
</artifact>

instead of:

<artifact implementation="com.example.myplugin.LocalArtifact">
  <name>local</name>
</artifact>

Fully qualified names continue to work as before.

Changes

  • Add resolveClassForImplementationHint() wrapper in EnhancedConfigurationConverter that falls back to sealed type resolution when standard Plexus resolution fails
  • Handle ambiguous simple names (multiple permitted subclasses sharing the same simple name) with a clear error message
  • Unit tests in DefaultBeanConfiguratorTest covering simple name, fully qualified name, and missing subclass scenarios
  • Integration test with a Maven 4 plugin exercising sealed parameter configuration end-to-end

Copy link
Copy Markdown
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

Claude Code on behalf of Guillaume Nodet

Review: GH-11378 Support sealed parameter implementation hints

Overview

This PR adds sealed-class awareness to Maven 4's plugin parameter configuration. When a plugin parameter type is a sealed interface/class, users can now use simple class names (e.g., implementation="LocalArtifact") instead of fully qualified names in their POM configuration. The implementation wraps the existing Plexus getClassForImplementationHint call — on failure, it falls back to scanning Class.getPermittedSubclasses() for a match. Includes unit tests and a Maven 4 plugin integration test.

Verdict

Looks good overall — minor issues to address. The approach is sound: backward-compatible, minimally invasive, and properly scoped.

Issues

Minor — Missing cause in ambiguity exception

In getPermittedSubclass(), the "not found" error path includes the original cause exception, but the "ambiguous" path does not. This loses diagnostic context:

// "not found" — includes cause
throw new ComponentConfigurationException(configuration,
    "Cannot find permitted subclass '...'", cause);

// "ambiguous" — missing cause
throw new ComponentConfigurationException(configuration,
    "Implementation hint '...' is ambiguous...");

Should add cause to the ambiguity exception for consistency.

Minor — Odd control flow: sort before isEmpty check

if (matches.size() == 1) {
    return matches.get(0);
}
matches.sort(Comparator.comparing(Class::getName));
if (matches.isEmpty()) {
    throw ...;
}

Sorting an empty list is harmless, but checking isEmpty() before sorting reads more naturally and makes the intent clearer.

Minor — No test for the ambiguity case

The unit tests cover simple name resolution, fully qualified name, and missing class — but there's no test for the ambiguity path (two permitted subclasses with the same simple name). This code path exists and has specific error formatting with sorted class names, so it should be verified.

Nit — Brittle error message assertion

testSealedTypeImplementationHintMustMatchPermittedSubclass asserts e.getMessage() against an exact string. If BeanConfigurationException wraps the cause message differently across versions, this could break. Consider using a contains check instead.

Suggestions (non-blocking)

  • Recursive sealed hierarchies: getPermittedSubclasses() only returns direct permits. If a hierarchy has intermediate sealed types (A permits B where B is sealed and B permits C), C won't be found from A. Fine for initial implementation but worth noting for future work.

  • IT test plugin pom.xml uses <maven.version>4.1.0-SNAPSHOT</maven.version> — correct for development against master but will need updating if the SNAPSHOT version changes before merge.

  • The issue mentions several follow-up features (@SealedTypeHint annotation, defaultImplementation on @Parameter, XSD generation). These are clearly out of scope for this PR, which is the right call.

@gnodet gnodet changed the title GH-11378 Support sealed parameter implementation hints Support sealed parameter implementation hints May 20, 2026
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.

Maven 4: first class support for sealed types in parameter configuration.

2 participants