Skip to content

Commit

Permalink
[MNG-7464] Warn about using read-only parameters for Mojo in configur…
Browse files Browse the repository at this point in the history
…ation

(cherry picked from commit 3dd0afd)
  • Loading branch information
slawekjaranowski committed May 6, 2022
1 parent 229cfc8 commit 74eba29
Show file tree
Hide file tree
Showing 4 changed files with 261 additions and 70 deletions.
@@ -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<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.",
"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() );
}
}
Expand Up @@ -126,7 +126,7 @@ public class DefaultMavenPluginManager
* same class realm is used to load build extensions and load mojos for extensions=true plugins.
* </p>
* <strong>Note:</strong> 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";
Expand Down Expand Up @@ -165,7 +165,7 @@ public class DefaultMavenPluginManager
private PluginArtifactsCache pluginArtifactsCache;

@Requirement
private MavenPluginConfigurationValidator configurationValidator;
private List<MavenPluginConfigurationValidator> configurationValidators;

private ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder();

Expand Down Expand Up @@ -612,7 +612,10 @@ else if ( cause instanceof LinkageError )

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 );
Expand Down
Expand Up @@ -21,10 +21,8 @@

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;
Expand All @@ -37,10 +35,22 @@
*/

@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,
Expand All @@ -67,53 +77,16 @@ public void validate( MojoDescriptor mojoDescriptor,
.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;
logParameter( parameter );
}

// 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 )
Expand All @@ -127,27 +100,4 @@ private void logDeprecatedMojo( MojoDescriptor mojoDescriptor )

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() );
}
}
@@ -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 );
}
}
}

0 comments on commit 74eba29

Please sign in to comment.