This is an automated email from the ASF dual-hosted git repository. sjaranowski pushed a commit to branch MNG-7464 in repository https://gitbox.apache.org/repos/asf/maven.git
commit 6ae2a0efbd39e12d6e67a1ce39806ab62eeac3b6 Author: Slawomir Jaranowski <[email protected]> AuthorDate: Thu Apr 28 21:44:34 2022 +0200 [MNG-7464] Warn about using read-only parameters for Mojo in configuration --- ...=> AbstractMavenPluginParametersValidator.java} | 122 ++++++++++----------- .../plugin/internal/DefaultMavenPluginManager.java | 9 +- .../plugin/internal/DeprecatedPluginValidator.java | 89 +++------------ .../ReadOnlyPluginParametersValidator.java | 80 ++++++++++++++ 4 files changed, 163 insertions(+), 137 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java similarity index 54% copy from maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java copy to maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java index 3139f5b84..e25bcb7f0 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java @@ -19,10 +19,9 @@ package org.apache.maven.plugin.internal; * under the License. */ -import javax.inject.Named; -import javax.inject.Singleton; +import java.util.Arrays; +import java.util.List; -import org.apache.maven.plugin.descriptor.MojoDescriptor; import org.apache.maven.plugin.descriptor.Parameter; import org.apache.maven.shared.utils.logging.MessageBuilder; import org.apache.maven.shared.utils.logging.MessageUtils; @@ -30,59 +29,45 @@ import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluatio import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; import org.codehaus.plexus.configuration.PlexusConfiguration; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** - * Print warnings if deprecated mojo or parameters of plugin are used in configuration. + * Common implementations for plugin parameters configuration validation. * * @author Slawomir Jaranowski */ -@Named -@Singleton -class DeprecatedPluginValidator implements MavenPluginConfigurationValidator +abstract class AbstractMavenPluginParametersValidator implements MavenPluginConfigurationValidator { - private static final Logger LOGGER = LoggerFactory.getLogger( DeprecatedPluginValidator.class ); - @Override - public void validate( MojoDescriptor mojoDescriptor, - PlexusConfiguration pomConfiguration, - ExpressionEvaluator expressionEvaluator ) - { - if ( !LOGGER.isWarnEnabled() ) - { - return; - } - - if ( mojoDescriptor.getDeprecated() != null ) - { - logDeprecatedMojo( mojoDescriptor ); - } - - if ( mojoDescriptor.getParameters() == null ) - { - return; - } - - mojoDescriptor.getParameters().stream() - .filter( parameter -> parameter.getDeprecated() != null ) - .filter( Parameter::isEditable ) - .forEach( parameter -> checkParameter( parameter, pomConfiguration, expressionEvaluator ) ); - } - - private static void checkParameter( Parameter parameter, - PlexusConfiguration pomConfiguration, - ExpressionEvaluator expressionEvaluator ) - { - PlexusConfiguration config = pomConfiguration.getChild( parameter.getName(), false ); - - if ( isValueSet( config, expressionEvaluator ) ) - { - logDeprecatedParameter( parameter ); - } - } - - private static boolean isValueSet( PlexusConfiguration config, - ExpressionEvaluator expressionEvaluator ) + // plugin author can provide @Parameter( property = "session" ) in this case property will always evaluate + // so, we need ignore those + + // source org.apache.maven.plugin.PluginParameterExpressionEvaluator + private static final List<String> IGNORED_PROPERTY_VALUES = Arrays.asList( + "basedir", + "executedProject", + "localRepository", + "mojo", + "mojoExecution", + "plugin", + "project", + "reactorProjects", + "session", + "settings" + ); + + private static final List<String> IGNORED_PROPERTY_PREFIX = Arrays.asList( + "mojo.", + "plugin.", + "pom.", + "project.", + "session.", + "settings." + ); + + protected abstract Logger getLogger(); + + protected static boolean isValueSet( PlexusConfiguration config, + ExpressionEvaluator expressionEvaluator ) { if ( config == null ) { @@ -102,8 +87,14 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator return false; } + if ( isIgnoredProperty( strValue ) ) + { + return false; + } + // for declaration like @Parameter( property = "config.property" ) // the value will contains ${config.property} + try { return expressionEvaluator.evaluate( strValue ) != null; @@ -118,19 +109,26 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator return false; } - private void logDeprecatedMojo( MojoDescriptor mojoDescriptor ) + private static boolean isIgnoredProperty( String strValue ) { - String message = MessageUtils.buffer() - .warning( "Goal '" ) - .warning( mojoDescriptor.getGoal() ) - .warning( "' is deprecated: " ) - .warning( mojoDescriptor.getDeprecated() ) - .toString(); - - LOGGER.warn( message ); + if ( !strValue.startsWith( "${" ) ) + { + return false; + } + + String propertyName = strValue.replace( "${", "" ).replace( "}", "" ); + + if ( IGNORED_PROPERTY_VALUES.contains( propertyName ) ) + { + return true; + } + + return IGNORED_PROPERTY_PREFIX.stream().anyMatch( propertyName::startsWith ); } - private static void logDeprecatedParameter( Parameter parameter ) + protected abstract String getParameterLogReason( Parameter parameter ); + + protected void logParameter( Parameter parameter ) { MessageBuilder messageBuilder = MessageUtils.buffer() .warning( "Parameter '" ) @@ -147,9 +145,9 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator } messageBuilder - .warning( " is deprecated: " ) - .warning( parameter.getDeprecated() ); + .warning( " " ) + .warning( getParameterLogReason( parameter ) ); - LOGGER.warn( messageBuilder.toString() ); + getLogger().warn( messageBuilder.toString() ); } } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index 51c49c84f..4a30f99e9 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -143,7 +143,7 @@ public class DefaultMavenPluginManager private PluginVersionResolver pluginVersionResolver; private PluginArtifactsCache pluginArtifactsCache; private MavenPluginValidator pluginValidator; - private MavenPluginConfigurationValidator configurationValidator; + private List<MavenPluginConfigurationValidator> configurationValidator; private final ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder(); private final PluginDescriptorBuilder builder = new PluginDescriptorBuilder(); @@ -160,7 +160,7 @@ public class DefaultMavenPluginManager PluginVersionResolver pluginVersionResolver, PluginArtifactsCache pluginArtifactsCache, MavenPluginValidator pluginValidator, - MavenPluginConfigurationValidator configurationValidator ) + List<MavenPluginConfigurationValidator> configurationValidator ) { this.container = container; this.classRealmManager = classRealmManager; @@ -606,7 +606,10 @@ public class DefaultMavenPluginManager ExpressionEvaluator expressionEvaluator = new PluginParameterExpressionEvaluator( session, mojoExecution ); - configurationValidator.validate( mojoDescriptor, pomConfiguration, expressionEvaluator ); + for ( MavenPluginConfigurationValidator validator: configurationValidator ) + { + validator.validate( mojoDescriptor, pomConfiguration, expressionEvaluator ); + } populateMojoExecutionFields( mojo, mojoExecution.getExecutionId(), mojoDescriptor, pluginRealm, pomConfiguration, expressionEvaluator ); diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java index 3139f5b84..2f1291dd4 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedPluginValidator.java @@ -24,9 +24,7 @@ import javax.inject.Singleton; import org.apache.maven.plugin.descriptor.MojoDescriptor; import org.apache.maven.plugin.descriptor.Parameter; -import org.apache.maven.shared.utils.logging.MessageBuilder; import org.apache.maven.shared.utils.logging.MessageUtils; -import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException; import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; import org.codehaus.plexus.configuration.PlexusConfiguration; import org.slf4j.Logger; @@ -39,10 +37,22 @@ import org.slf4j.LoggerFactory; */ @Named @Singleton -class DeprecatedPluginValidator implements MavenPluginConfigurationValidator +class DeprecatedPluginValidator extends AbstractMavenPluginParametersValidator { private static final Logger LOGGER = LoggerFactory.getLogger( DeprecatedPluginValidator.class ); + @Override + protected Logger getLogger() + { + return LOGGER; + } + + @Override + protected String getParameterLogReason( Parameter parameter ) + { + return "is deprecated: " + parameter.getDeprecated(); + } + @Override public void validate( MojoDescriptor mojoDescriptor, PlexusConfiguration pomConfiguration, @@ -58,64 +68,22 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator logDeprecatedMojo( mojoDescriptor ); } - if ( mojoDescriptor.getParameters() == null ) - { - return; - } - mojoDescriptor.getParameters().stream() .filter( parameter -> parameter.getDeprecated() != null ) .filter( Parameter::isEditable ) .forEach( parameter -> checkParameter( parameter, pomConfiguration, expressionEvaluator ) ); } - private static void checkParameter( Parameter parameter, - PlexusConfiguration pomConfiguration, - ExpressionEvaluator expressionEvaluator ) + private void checkParameter( Parameter parameter, + PlexusConfiguration pomConfiguration, + ExpressionEvaluator expressionEvaluator ) { PlexusConfiguration config = pomConfiguration.getChild( parameter.getName(), false ); if ( isValueSet( config, expressionEvaluator ) ) { - logDeprecatedParameter( parameter ); - } - } - - private static boolean isValueSet( PlexusConfiguration config, - ExpressionEvaluator expressionEvaluator ) - { - if ( config == null ) - { - return false; - } - - // there are sub items ... so configuration is declared - if ( config.getChildCount() > 0 ) - { - return true; - } - - String strValue = config.getValue(); - - if ( strValue == null || strValue.isEmpty() ) - { - return false; - } - - // for declaration like @Parameter( property = "config.property" ) - // the value will contains ${config.property} - try - { - return expressionEvaluator.evaluate( strValue ) != null; - } - catch ( ExpressionEvaluationException e ) - { - // not important - // will be reported during Mojo fields populate + logParameter( parameter ); } - - // fallback - in case of error in expressionEvaluator - return false; } private void logDeprecatedMojo( MojoDescriptor mojoDescriptor ) @@ -129,27 +97,4 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator LOGGER.warn( message ); } - - private static void logDeprecatedParameter( Parameter parameter ) - { - MessageBuilder messageBuilder = MessageUtils.buffer() - .warning( "Parameter '" ) - .warning( parameter.getName() ) - .warning( '\'' ); - - if ( parameter.getExpression() != null ) - { - String userProperty = parameter.getExpression().replace( "${", "'" ).replace( '}', '\'' ); - messageBuilder - .warning( " (user property " ) - .warning( userProperty ) - .warning( ")" ); - } - - messageBuilder - .warning( " is deprecated: " ) - .warning( parameter.getDeprecated() ); - - LOGGER.warn( messageBuilder.toString() ); - } } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/ReadOnlyPluginParametersValidator.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/ReadOnlyPluginParametersValidator.java new file mode 100644 index 000000000..acff281f2 --- /dev/null +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/ReadOnlyPluginParametersValidator.java @@ -0,0 +1,80 @@ +package org.apache.maven.plugin.internal; + +/* + * 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. + */ + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.apache.maven.plugin.descriptor.MojoDescriptor; +import org.apache.maven.plugin.descriptor.Parameter; +import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.configuration.PlexusConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Print warnings if read-only parameters of plugin are used in configuration. + * + * @author Slawomir Jaranowski + */ +@Named +@Singleton +public class ReadOnlyPluginParametersValidator extends AbstractMavenPluginParametersValidator +{ + private static final Logger LOGGER = LoggerFactory.getLogger( ReadOnlyPluginParametersValidator.class ); + + @Override + protected Logger getLogger() + { + return LOGGER; + } + + @Override + protected String getParameterLogReason( Parameter parameter ) + { + return "is read-only, should not be used in configuration"; + } + + @Override + public void validate( MojoDescriptor mojoDescriptor, PlexusConfiguration pomConfiguration, + ExpressionEvaluator expressionEvaluator ) + { + if ( !LOGGER.isWarnEnabled() ) + { + return; + } + + mojoDescriptor.getParameters().stream() + .filter( parameter -> !parameter.isEditable() ) + .forEach( parameter -> checkParameter( parameter, pomConfiguration, expressionEvaluator ) ); + } + + protected void checkParameter( Parameter parameter, + PlexusConfiguration pomConfiguration, + ExpressionEvaluator expressionEvaluator ) + { + PlexusConfiguration config = pomConfiguration.getChild( parameter.getName(), false ); + + if ( isValueSet( config, expressionEvaluator ) ) + { + logParameter( parameter ); + } + } +}
