From 74eba2968b020d8da2360a45c3406a5132eb6aee Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Thu, 28 Apr 2022 21:44:34 +0200 Subject: [PATCH] [MNG-7464] Warn about using read-only parameters for Mojo in configuration (cherry picked from commit 3dd0afd89772929fa89db974d5bc432846f170de) --- ...bstractMavenPluginParametersValidator.java | 153 ++++++++++++++++++ .../internal/DefaultMavenPluginManager.java | 9 +- .../internal/DeprecatedPluginValidator.java | 84 ++-------- .../ReadOnlyPluginParametersValidator.java | 85 ++++++++++ 4 files changed, 261 insertions(+), 70 deletions(-) create mode 100644 maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java create mode 100644 maven-core/src/main/java/org/apache/maven/plugin/internal/ReadOnlyPluginParametersValidator.java diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java new file mode 100644 index 0000000000..3fe62a894f --- /dev/null +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java @@ -0,0 +1,153 @@ +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 java.util.Arrays; +import java.util.List; + +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; + +/** + * Common implementations for plugin parameters configuration validation. + * + * @author Slawomir Jaranowski + */ +abstract class AbstractMavenPluginParametersValidator implements MavenPluginConfigurationValidator +{ + + // 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 IGNORED_PROPERTY_VALUES = Arrays.asList( + "basedir", + "executedProject", + "localRepository", + "mojo", + "mojoExecution", + "plugin", + "project", + "reactorProjects", + "session", + "settings" + ); + + private static final List IGNORED_PROPERTY_PREFIX = Arrays.asList( + "mojo.", + "pom.", + "plugin.", + "project.", + "session.", + "settings." + ); + + protected abstract Logger getLogger(); + + protected 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; + } + + if ( isIgnoredProperty( strValue ) ) + { + return false; + } + + // for declaration like @Parameter( property = "config.property" ) + // the value will contain ${config.property} + + try + { + return expressionEvaluator.evaluate( strValue ) != null; + } + catch ( ExpressionEvaluationException e ) + { + // not important + // will be reported during Mojo fields populate + } + + // fallback - in case of error in expressionEvaluator + return false; + } + + private static boolean isIgnoredProperty( String strValue ) + { + 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 ); + } + + protected abstract String getParameterLogReason( Parameter parameter ); + + protected void logParameter( 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( " " ) + .warning( getParameterLogReason( parameter ) ); + + 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 8f00f16594..a69c88eac6 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 @@ -126,7 +126,7 @@ public class DefaultMavenPluginManager * same class realm is used to load build extensions and load mojos for extensions=true plugins. *

* Note: This is part of internal implementation and may be changed or removed without notice - * + * * @since 3.3.0 */ public static final String KEY_EXTENSIONS_REALMS = DefaultMavenPluginManager.class.getName() + "/extensionsRealms"; @@ -165,7 +165,7 @@ public class DefaultMavenPluginManager private PluginArtifactsCache pluginArtifactsCache; @Requirement - private MavenPluginConfigurationValidator configurationValidator; + private List configurationValidators; private ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder(); @@ -612,7 +612,10 @@ public class DefaultMavenPluginManager ExpressionEvaluator expressionEvaluator = new PluginParameterExpressionEvaluator( session, mojoExecution ); - configurationValidator.validate( mojoDescriptor, pomConfiguration, expressionEvaluator ); + for ( MavenPluginConfigurationValidator validator: configurationValidators ) + { + 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 4c21f80e3e..5f4af1a9c7 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 @@ -21,10 +21,8 @@ package org.apache.maven.plugin.internal; 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.annotations.Component; -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; @@ -37,10 +35,22 @@ import org.slf4j.LoggerFactory; */ @Component( role = MavenPluginConfigurationValidator.class ) -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, @@ -67,55 +77,18 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator .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 ); + logParameter( 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 - } - - // fallback - in case of error in expressionEvaluator - return false; - } - private void logDeprecatedMojo( MojoDescriptor mojoDescriptor ) { String message = MessageUtils.buffer() @@ -127,27 +100,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 0000000000..3165b426e8 --- /dev/null +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/ReadOnlyPluginParametersValidator.java @@ -0,0 +1,85 @@ +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 a 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, must not be used in configuration"; + } + + @Override + public void validate( MojoDescriptor mojoDescriptor, PlexusConfiguration pomConfiguration, + ExpressionEvaluator expressionEvaluator ) + { + if ( !LOGGER.isWarnEnabled() ) + { + return; + } + + if ( mojoDescriptor.getParameters() == null ) + { + 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 ); + } + } +}