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]

Reply via email to