gnodet-bot commented on code in PR #13636: URL: https://github.com/apache/maven/pull/13636#discussion_r4318289494
########## impl/maven-core/src/main/java/org/apache/maven/plugin/internal/V4MojoConfigurator.java: ########## @@ -0,0 +1,226 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.internal; + +import java.io.File; +import java.lang.reflect.Array; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.RecordComponent; +import java.lang.reflect.Type; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; + +import org.apache.maven.api.xml.XmlNode; +import org.apache.maven.plugin.PluginConfigurationException; +import org.apache.maven.plugin.descriptor.PluginDescriptor; +import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.component.configurator.expression.TypeAwareExpressionEvaluator; + +/** Configures a Maven 4 mojo directly from the public Maven XML model. */ +public final class V4MojoConfigurator { + private final ExpressionEvaluator evaluator; + private final ClassLoader classLoader; + private final PluginDescriptor pluginDescriptor; + + public V4MojoConfigurator( + ExpressionEvaluator evaluator, ClassLoader classLoader, PluginDescriptor pluginDescriptor) { + this.evaluator = evaluator; + this.classLoader = classLoader; + this.pluginDescriptor = pluginDescriptor; + } + + public void configure(Object mojo, XmlNode configuration) throws PluginConfigurationException { + if (configuration == null) { + return; + } + for (XmlNode child : configuration.children()) { + try { + set(mojo, child.name(), child); + } catch (Exception e) { + throw new PluginConfigurationException( + pluginDescriptor, + "Unable to configure '" + child.name() + "' on " + + mojo.getClass().getName(), + e); + } + } + } + + private void set(Object bean, String name, XmlNode node) throws Exception { + String title = Character.toUpperCase(name.charAt(0)) + name.substring(1); + for (Method method : bean.getClass().getMethods()) { + if ((method.getName().equals("set" + title) || method.getName().equals("add" + title)) + && method.getParameterCount() == 1) { + method.invoke(bean, convert(node, method.getGenericParameterTypes()[0])); + return; + } + } + for (Class<?> type = bean.getClass(); type != null; type = type.getSuperclass()) { + try { + Field field = type.getDeclaredField(name); + field.setAccessible(true); + field.set(bean, convert(node, field.getGenericType())); + return; + } catch (NoSuchFieldException ignored) { + // Continue through the class hierarchy. + } + } + throw new NoSuchFieldException(name); + } + + private Object convert(XmlNode node, Type target) throws Exception { + Class<?> type = rawType(target); + if (type == XmlNode.class) { + return node; + } + if (type == String.class || type == Object.class) { + return evaluate(node.value(), type); + } + if (type == Path.class || type == File.class) { + Object evaluated = evaluate(node.value(), String.class); + if (evaluated == null) return null; + String value = String.valueOf(evaluated); + Path path = Paths.get(value.replace('/', File.separatorChar)); + if (evaluator instanceof TypeAwareExpressionEvaluator typed) { + path = typed.alignToBaseDirectory(path.toFile()).toPath(); + } + return type == File.class ? path.toFile() : path; + } + if (type == boolean.class || type == Boolean.class) + return Boolean.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == int.class || type == Integer.class) + return Integer.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == long.class || type == Long.class) return Long.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == double.class || type == Double.class) + return Double.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == float.class || type == Float.class) + return Float.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == short.class || type == Short.class) + return Short.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == byte.class || type == Byte.class) return Byte.valueOf(String.valueOf(evaluate(node.value(), type))); + if (type == char.class || type == Character.class) + return String.valueOf(evaluate(node.value(), type)).charAt(0); + if (type.isEnum()) { + @SuppressWarnings({"rawtypes", "unchecked"}) + Object value = Enum.valueOf((Class) type, String.valueOf(evaluate(node.value(), type))); + return value; + } + if (type.isArray()) { + List<XmlNode> children = node.children(); + Object array = Array.newInstance(type.getComponentType(), children.size()); + for (int i = 0; i < children.size(); i++) + Array.set(array, i, convert(children.get(i), type.getComponentType())); + return array; + } + if (Collection.class.isAssignableFrom(type)) { + Type itemType = target instanceof ParameterizedType p ? p.getActualTypeArguments()[0] : Object.class; + Collection<Object> values = + Set.class.isAssignableFrom(type) ? new java.util.LinkedHashSet<>() : new ArrayList<>(); + for (XmlNode child : node.children()) values.add(convert(child, itemType)); + return values; + } + if (Map.class.isAssignableFrom(type)) { + Type valueType = target instanceof ParameterizedType p ? p.getActualTypeArguments()[1] : Object.class; + if (Properties.class.isAssignableFrom(type)) { + Properties properties = new Properties(); + for (XmlNode child : node.children()) { + properties.put(child.name(), convert(child, valueType)); + } + return properties; + } + Map<String, Object> values = new LinkedHashMap<>(); + for (XmlNode child : node.children()) values.put(child.name(), convert(child, valueType)); + return values; + } + String implementation = node.attribute("implementation"); + if (implementation != null && !implementation.isEmpty()) { + try { + type = Class.forName(implementation, true, classLoader); + } catch (ClassNotFoundException e) { + if (!type.isSealed()) throw e; + String hint = implementation; + List<Class<?>> matches = java.util.Arrays.stream(type.getPermittedSubclasses()) + .filter(candidate -> + hint.equals(candidate.getSimpleName()) || hint.equals(candidate.getCanonicalName())) + .toList(); + if (matches.size() != 1) throw e; + type = matches.get(0); + } + } + Object value = node.value(); + if (value == null || value.isEmpty()) { + value = node.attribute("default-value"); + } + if (value != null && node.children().isEmpty()) { + Method setter = findDefaultSetter(type); + if (setter != null) { + Object instance = type.getDeclaredConstructor().newInstance(); + setter.invoke( + instance, convert(XmlNode.newInstance("value", value), setter.getGenericParameterTypes()[0])); + return instance; + } + } + if (type.isRecord()) { + RecordComponent[] components = type.getRecordComponents(); + Class<?>[] parameterTypes = new Class<?>[components.length]; + Object[] arguments = new Object[components.length]; + for (int i = 0; i < components.length; i++) { + RecordComponent component = components[i]; + parameterTypes[i] = component.getType(); + XmlNode child = node.child(component.getName()); + arguments[i] = child == null ? null : convert(child, component.getGenericType()); + } + var constructor = type.getDeclaredConstructor(parameterTypes); + constructor.setAccessible(true); + return constructor.newInstance(arguments); + } + Object instance = type.getDeclaredConstructor().newInstance(); Review Comment: ⚠️ **Package-private class instantiation fails without `setAccessible(true)`** This is the same bug fixed by #13470 in `EnhancedConfigurationConverter`: `getDeclaredConstructor().newInstance()` throws `IllegalAccessException` for package-private configuration classes (their no-arg constructor is not accessible from this module). The record path just above (line 198–200) already does it correctly. Same applies to line 182 (`findDefaultSetter` path). Suggest extracting a private helper (as done in #13470): ```suggestion Object instance = instantiate(type); ``` With the helper added to the class: ```java private static Object instantiate(Class<?> type) throws ReflectiveOperationException { var ctor = type.getDeclaredConstructor(); if (!ctor.canAccess(null)) { ctor.setAccessible(true); } return ctor.newInstance(); } ``` Then apply the same fix to line 182 as well. -- 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]
