gnodet-bot commented on code in PR #13636:
URL: https://github.com/apache/maven/pull/13636#discussion_r4483645780
##########
impl/maven-testing/src/main/java/org/apache/maven/testing/plugin/MojoExtension.java:
##########
@@ -311,9 +310,8 @@ public Object resolveParameter(ParameterContext
parameterContext, ExtensionConte
ExpressionEvaluator evaluator = new WrapEvaluator(
getInjector(), new
PluginParameterExpressionEvaluatorV4(session, project, mojoExecution));
- EnhancedComponentConfigurator configurator = new
EnhancedComponentConfigurator();
- configurator.configureComponent(
- mojo, new XmlPlexusConfiguration(pluginConfiguration),
evaluator, null, null);
+ new V4MojoConfigurator(evaluator,
mojo.getClass().getClassLoader(), null)
Review Comment:
❌ **Compile error: type mismatch — `ExpressionEvaluator` is not
`MojoExpressionEvaluator`.**
`evaluator` is declared as `ExpressionEvaluator` (the Plexus base
interface), but `V4MojoConfigurator` constructor takes
`MojoExpressionEvaluator` (the new package-private interface). Java is
nominally typed — `WrapEvaluator implements TypeAwareExpressionEvaluator` does
not satisfy `MojoExpressionEvaluator`.
Since `PlexusMojoExpressionEvaluatorAdapter` is package-private to
`o.a.m.plugin.internal`, it cannot be used directly from `maven-testing`. Two
options:
1. **Add a package-accessible factory method** (simplest):
```suggestion
new V4MojoConfigurator(
V4MojoConfigurator.fromPlexus((TypeAwareExpressionEvaluator) evaluator),
mojo.getClass().getClassLoader(), null)
```
with a package-private static `fromPlexus(TypeAwareExpressionEvaluator)`
on `V4MojoConfigurator` that returns a `PlexusMojoExpressionEvaluatorAdapter`.
2. **Make `PlexusMojoExpressionEvaluatorAdapter` package-accessible** (allow
use from sibling modules).
Either way this will not compile as written.
##########
impl/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -823,6 +827,22 @@ private void populateMojoExecutionFields(
}
try {
+ if ("enhanced".equals(configuratorId)) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Configuring mojo execution '" +
mojoDescriptor.getId() + ':' + executionId
+ + "' with Maven XML configurator -->");
+ }
+ new V4MojoConfigurator(
+ new PlexusMojoExpressionEvaluatorAdapter(
+ (TypeAwareExpressionEvaluator)
expressionEvaluator),
Review Comment:
⚠️ **Unchecked cast — deferred `ClassCastException` if a non-`TypeAware`
evaluator is passed.**
`populateMojoExecutionFields` accepts `ExpressionEvaluator` (the base Plexus
interface), but this branch unconditionally casts it to
`TypeAwareExpressionEvaluator`. Both current concrete implementations
(`PluginParameterExpressionEvaluator` and
`PluginParameterExpressionEvaluatorV4`) implement
`TypeAwareExpressionEvaluator`, so it works today — but the cast is fragile and
will throw an opaque `ClassCastException` at the point of `evaluate()` if any
other evaluator is ever passed.
```suggestion
if (!(expressionEvaluator instanceof
TypeAwareExpressionEvaluator tae)) {
throw new PluginConfigurationException(
mojoDescriptor.getPluginDescriptor(),
"Mojo configurator 'enhanced' requires a
TypeAwareExpressionEvaluator but got: "
+
expressionEvaluator.getClass().getName());
}
new V4MojoConfigurator(
new
PlexusMojoExpressionEvaluatorAdapter(tae),
pluginRealm,
mojoDescriptor.getPluginDescriptor())
.configure(mojo, configurationDom);
```
--
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]