You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "michael-o (via GitHub)" <gi...@apache.org> on 2023/04/04 18:54:45 UTC
[GitHub] [maven] michael-o commented on a diff in pull request #1079: [MNG-7754] Improvement and extension of plugin validation
michael-o commented on code in PR #1079:
URL: https://github.com/apache/maven/pull/1079#discussion_r1157634508
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginDependenciesValidator.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Service responsible for validating plugin dependencies.
+ *
+ * @since 3.9.2
+ */
+abstract class AbstractMavenPluginDependenciesValidator implements MavenPluginDependenciesValidator {
+
+ protected final PluginValidationManager pluginValidationManager;
+
+ protected AbstractMavenPluginDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ this.pluginValidationManager = requireNonNull(pluginValidationManager);
+ }
+
+ @Override
+ public void validate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ if (mojoDescriptor.getPluginDescriptor() != null
Review Comment:
Can they be null?! Weird
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -103,6 +107,19 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
+ if (result.getDependencies() != null) {
+ for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
+ if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
+ && "maven-compat".equals(dependency.getArtifact().getArtifactId())
+ && !JavaScopes.TEST.equals(dependency.getScope())) {
Review Comment:
Does provided make sense?
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java:
##########
@@ -540,6 +548,18 @@ public <T> T getConfiguredMojo(Class<T> mojoInterface, MavenSession session, Moj
((Mojo) mojo).setLog(new DefaultLog(mojoLogger));
}
+ if (mojo instanceof Contextualizable) {
+ pluginValidationManager.reportPluginMojoValidationIssue(
+ session,
+ mojoDescriptor,
+ mojo.getClass(),
+ "Implements `Contextualizable` interface from Plexus Container, that is EOL.");
Review Comment:
...Container, which is EOL.
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java:
##########
@@ -94,19 +98,19 @@ protected boolean isIgnoredProperty(String strValue) {
protected abstract String getParameterLogReason(Parameter parameter);
- protected void logParameter(Parameter parameter) {
- MessageBuilder messageBuilder = MessageUtils.buffer()
- .warning("Parameter '")
- .warning(parameter.getName())
- .warning('\'');
+ protected String formatParameter(Parameter parameter) {
Review Comment:
Attention: The `MessageBuilder` is here on purpose to highlight stuff. Why is it gone?
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java:
##########
@@ -40,7 +43,7 @@ class DeprecatedCoreExpressionValidator extends AbstractMavenPluginParametersVal
private static final HashMap<String, String> DEPRECATED_CORE_PARAMETERS;
private static final String ARTIFACT_REPOSITORY_REASON =
- "Avoid use of ArtifactRepository type. If you need access to local repository, switch to '${repositorySystemSession}' expression and get LRM from it instead.";
+ "ArtifactRepository type is deprecated and it's use in Mojos should be avoided.";
Review Comment:
its use in mojos...
"it's == it is" which is not the same as 'its'
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/Maven2DependenciesValidator.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.codehaus.plexus.component.repository.ComponentDependency;
+
+/**
+ * Detects Maven2 plugins.
+ *
+ * @since 3.9.2
+ */
+@Singleton
+@Named
+class Maven2DependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ Maven2DependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ Set<String> maven2Versions = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ .filter(d -> "org.apache.maven".equals(d.getGroupId()))
+ .filter(d -> !"maven-archiver".equals(d.getArtifactId()))
+ .map(ComponentDependency::getVersion)
+ .filter(v -> v.startsWith("2."))
+ .collect(Collectors.toSet());
+
+ if (!maven2Versions.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ mavenSession, mojoDescriptor, "Plugin is a Maven 2.x plugin, will be not supported in Maven 4.x");
Review Comment:
, which will not
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * 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 javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.io.File;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.AbstractMavenLifecycleParticipant;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.InputLocation;
+import org.apache.maven.model.Plugin;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.apache.maven.plugin.descriptor.PluginDescriptor;
+import org.apache.maven.project.MavenProject;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+@Named
+public final class DefaultPluginValidationManager extends AbstractMavenLifecycleParticipant
+ implements PluginValidationManager {
+
+ private static final String ISSUES_KEY = DefaultPluginValidationManager.class.getName() + ".issues";
+
+ private static final String MAVEN_PLUGIN_VALIDATION_ENABLED_KEY = "maven.plugin.validation.enabled";
Review Comment:
I think we can drop the `.enabled` since our user properties are boolean by default if no value is provided: `-Dmaven.plugin.validation`, at least other components don't use `.enabled` explicitly.
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenMixedDependenciesValidator.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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 javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+import org.codehaus.plexus.component.repository.ComponentDependency;
+
+/**
+ * Detects mixed Maven versions in plugins.
+ *
+ * @since 3.9.2
+ */
+@Singleton
+@Named
+class MavenMixedDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ MavenMixedDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ Set<String> mavenVersions = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ .filter(d -> "org.apache.maven".equals(d.getGroupId()))
+ .filter(d -> !"maven-archiver".equals(d.getArtifactId()))
+ .map(ComponentDependency::getVersion)
+ .collect(Collectors.toSet());
+
+ if (mavenVersions.size() > 1) {
+ pluginValidationManager.reportPluginValidationIssue(
+ mavenSession, mojoDescriptor, "Plugin mixes multiple Maven versions: " + mavenVersions);
+ }
Review Comment:
How to understand this message?
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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 javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+
+/**
+ * Detects Plexus Container Default in plugins.
+ *
+ * @since 3.9.2
+ */
+@Singleton
+@Named
+class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ PlexusContainerDefaultDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+
+ if (pcdPresent) {
+ pluginValidationManager.reportPluginValidationIssue(
+ mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, that is EOL");
Review Comment:
..., which is...
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org