[MNG-7464] Warn about using read-only parameters for Mojo in configuration

maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java

@@ -0,0 +1,152 @@
+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<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.",
+        "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() );
+    }
+}

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> configurationValidators;
 
     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> configurationValidators )
     {
         this.container = container;
         this.classRealmManager = classRealmManager;
@@ -172,7 +172,7 @@ public class DefaultMavenPluginManager
         this.pluginVersionResolver = pluginVersionResolver;
         this.pluginArtifactsCache = pluginArtifactsCache;
         this.pluginValidator = pluginValidator;
-        this.configurationValidator = configurationValidator;
+        this.configurationValidators = configurationValidators;
     }
 
     public synchronized PluginDescriptor getPluginDescriptor( Plugin plugin, List<RemoteRepository> repositories,
@@ -606,7 +606,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 );

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,18 +68,13 @@ 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,
+    private void checkParameter( Parameter parameter,
                                  PlexusConfiguration pomConfiguration,
                                  ExpressionEvaluator expressionEvaluator )
     {
@@ -77,47 +82,10 @@ class DeprecatedPluginValidator implements MavenPluginConfigurationValidator
 
         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()
@@ -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() );
-    }
 }

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 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;
+        }
+
+        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 );
+        }
+    }
+}