[GitHub] [maven] slawekjaranowski commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data

["slawekjaranowski (via GitHub)" <gi...@apache.org>]

slawekjaranowski commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201834506


##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ 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())) {
-                        pluginValidationManager.reportPluginValidationIssue(
-                                session,
-                                pluginArtifact,
-                                "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
-                    }
-                }
-
-                Set<String> mavenArtifacts = result.getDependencies().stream()
-                        .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
-                        .map(org.eclipse.aether.graph.Dependency::getArtifact)
-                        .filter(a -> "org.apache.maven".equals(a.getGroupId()))
-                        .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
-                                a.getGroupId() + ":" + a.getArtifactId()))
-                        .filter(a -> a.getVersion().startsWith("3."))
-                        .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
-                        .collect(Collectors.toSet());
-
-                if (!mavenArtifacts.isEmpty()) {
-                    pluginValidationManager.reportPluginValidationIssue(
-                            session,
-                            pluginArtifact,
-                            "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
-                }
+            for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+                dependenciesValidator.validate(session, pluginArtifact, result);

Review Comment:
   I think that dependency list returned by `repoSystem.readArtifactDescriptor` is different than `repoSystem.collectDependencies`
   
   But for building plugin classloder we use output of `resolveInternal` method ... so maybe we should check dependencies returned by `repoSystem.collectDependencies`
   
   



-- 
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