ascopes opened a new issue, #11378:
URL: https://github.com/apache/maven/issues/11378
### New feature, improvement proposal
Hello.
Proposing that given Maven 4 will baseline to Java 17, sealed classes get a
simplified interface for implementation discovery during the parameter
convertion process.
## Aims
- Reduce boilerplate for users.
- Allow polymorphic conversion of sealed types using intrinsics
- Allow further documentation generation automatically from sealed types.
## Sealed type handling
Effectively, suppose I have the following Mojo:
```java
@Mojo(...)
public class MyMojo extends ... {
@Parameter
ThirdPartyArtifact artifact;
...
}
```
Now, let's assume `ThirdPartyArtifact` is sealed:
```java
public sealed interface ThirdPartyArtifact {
public final class MavenArtifact implements ThirdPartyArtifact { ... }
public final class SystemPathArtifact implements ThirdPartyArtifact { ... }
public final class LocalPathArtifact implements ThirdPartyArtifact { ... }
public final class RemoteUrlArtifact implements ThirdPartyArtifact { ... }
}
```
Currently, my understanding is that the user would have to provide the
canonical class name and package to the `implementation` attribute in their XML
configuration, to avoid ambiguity.
Given the types are sealed, we should be able to query
`Class#getPermittedSubclasses()` to get all implementations. This would in
theory allow us to perform this class lookup based on just the class simple
name, allowing a user to simplify configuration from...
```xml
<configuration>
<artifact
implementation="io.github.ascopes.myplugin.models.MavenArtifact.SystemPathArtifact">
<name>gcc</name>
</artifact>
</configuration>
```
...to just...
```xml
<configuration>
<artifact implementation="SystemPathArtifact">
<name>gcc</name>
</artifact>
</configuration>
```
...which is far less verbose and far more user-friendly.
## Possible implementation
A new converter could be added to the Plexus context by default with the
following implementation:
```java
public class SealedTypeConverter extends AbstractBasicConverter {
@Override
public boolean canConvert(Class<?> cls) {
return cls.isSealed();
}
@Override
public Object fromConfiguration(
ConverterLookup lookup,
PlexusConfiguration configuration,
Class<?> type,
Class<?> enclosingType,
ClassLoader classLoader,
ExpressionEvaluator evaluator
) throws ComponentConfigurationException {
if (configuration.getChildCount() == 0) {
return null;
}
var hint = configuration.getAttribute("implementation");
var implementationClass = Stream
.of(type.getPermittedSubclasses())
.sorted(Class::getName)
.filter(cls -> getPermittedClassNames(cls)
.anyMatch(name -> name.equals(hint))
.findFirst();
if (implementationClass.isNotPresent()) {
throw new ComponentConfigurationException(...);
}
return converterLookup.lookupConverterFromType(implementationClass.get())
.fromConfiguration(...);
}
private Stream<String> getPermittedClassNames(Class<?> type) {
var canonicalNames = Stream.of(
type.getName(),
type.getSimpleName()
);
var customHints = ofNullable(type.getAnnotation(SealedTypeHint.class)
.map(SealedTypeHint::value)
.stream()
.flatMap(Stream::of);
return Stream.concat(canonicalNames, customHints);
}
}
## Addition of metadata to `@Parameter`
It would be sensible to add an attribute to `@Parameter` to allow the
specification of the default implementation type to consider if information is
not provided. I'd envison this being allowed for both sealed and non-sealed
types, for example:
```java
@interface Parameter {
...
Class<?> defaultImplementation() default void.class;
}
```
```java
class MyMojo extends ... {
@Parameter(defaultImplementation = MavenArtifact.class)
ThirdPartyArtifact artifact;
// Also useful for cases where code is generated via an annotation
processor.
// Consider the immutables library:
@Parameter(defaultImplementation = MutableSourceListing.class)
SourceListing source;
}
```
## Caveats and possible improvements
A caveat to this would be if there is more than one implementor with the
same name. Aside from being a very poor design decision, this could be worked
around by either raising a warning/exception, or providing the developer with a
way of "tagging" implementation names to the sealed class implementations via
an annotation.
The annotation for doing this could
be the following:
```java
@Retention(RUNTIME)
@Target({TYPE, RECORD})
public @interface SealedTypeHint {
String[] value();
}
```
...this would allow defining my example from earlier like so:
```java
public sealed interface ThirdPartyArtifact {
@SealedTypeHint("maven")
public final class MavenArtifact implements ThirdPartyArtifact { ... }
@SealedTypeHint("system-path")
public final class SystemPathArtifact implements ThirdPartyArtifact { ... }
@SealedTypeHint("local-path")
public final class LocalPathArtifact implements ThirdPartyArtifact { ... }
@SealedTypeHint("remote")
public final class RemoteUrlArtifact implements ThirdPartyArtifact { ... }
}
```
...enabling the XML example to be just...
```xml
<configuration>
<artifact implementation="system-path">
<name>gcc</name>
</artifact>
</configuration>
```
One other caveat may be the use of `non-sealed` implementation types.
However, in this case, the default behaviour of `implementation` should still
be allowed for backwards compatibility.
### Further possible improvements
- Records could be supported for deserialization via Plexus to allow
developers to create concise parameter definitions.
- Documentation could be generated from the knowledge of the sealed types in
`maven-plugin-plugin` to avoid developers having to manually document supported
types.
- The `maven-plugin-plugin` could configure the generated XSD schema to have
knowledge of the known permitted types for the parameter, allowing IDEs to
provide clear linting and autocomplete for polymorphic types without
having to perform expensive classpath analysis of the plugin and dependency
tree.
- This implementation would be compatible with other schema formats like
JsonSchema, should Maven decide to allow non-XML configuration in the future.
- The `@SealedTypeHint` _in theory_ could be changed to be just a regular
`@TypeHint` or leverage similar Java EE/Jakarta EE metadata annotations for the
same purpose. This would potentially require costly classpath scanning though
which may hinder startup execution times for plugins. In theory though this
information may be possible to collect when `maven-plugin-plugin` is run.
## Workarounds
Right now in Maven 3, parameters have to have a clear type or rely on the
user giving a fully qualified canonical class name. This contributes to the
outdated archetype that Maven is overly verbose for new users, which can hinder
adoption.
Polymorphic types have to be documented explicitly by the developer rather
than being generated from JavaDocs when a type is sealed, which leads to
documentation mistakes and inconsistencies. Many plugins just do not document
this sort of thing properly at all, requiring users of plugins to have to read
the source code to have any idea how to configure this sort of thing properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
