Exclude direct test-scope dependencies when building dependency graph

Fixes #185

src/it/projects/flatten-dependency-all-both-test-and-transitive/pom.xml

@@ -0,0 +1,35 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.codehaus.mojo.flatten.its</groupId>
+  <artifactId>flatten-dependency-all-both-test-and-transitive</artifactId>
+  <version>0.0.1-SNAPSHOT</version>
+
+  <build>
+    <defaultGoal>verify</defaultGoal>
+    <plugins>
+      <plugin>
+        <groupId>org.codehaus.mojo</groupId>
+        <artifactId>flatten-maven-plugin</artifactId>
+        <configuration>
+          <flattenMode>oss</flattenMode>
+          <flattenDependencyMode>all</flattenDependencyMode>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.codehaus.mojo.flatten.its</groupId>
+      <artifactId>core</artifactId>
+      <!-- This artifact depends on dep:3.2.1 with compile scope -->
+      <version>3.2.1</version>
+    </dependency>
+    <dependency>
+      <groupId>org.codehaus.mojo.flatten.its</groupId>
+      <artifactId>dep</artifactId>
+      <version>3.2.1</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+</project>

src/it/projects/flatten-dependency-all-both-test-and-transitive/verify.groovy

@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+File originalPom = new File( basedir, 'pom.xml' )
+assert originalPom.exists()
+
+def originalProject = new XmlSlurper().parse( originalPom )
+assert 2 ==  originalProject.dependencies.dependency.size()
+assert "dep" ==  originalProject.dependencies.dependency[1].artifactId.text()
+assert "3.2.1" ==  originalProject.dependencies.dependency[1].version.text()
+assert "test" ==  originalProject.dependencies.dependency[1].scope.text()
+
+File flattenedPom = new File( basedir, '.flattened-pom.xml' )
+assert flattenedPom.exists()
+
+def flattenedProject = new XmlSlurper().parse( flattenedPom )
+
+// core and dep should be there. It's because while the test-scope dep (the
+// direct dependency), core declares dep as compile-scope (default) dependency.
+assert 2 ==  flattenedProject.dependencies.dependency.size()
+
+assert "core" ==  flattenedProject.dependencies.dependency[0].artifactId.text()
+assert "3.2.1" ==  flattenedProject.dependencies.dependency[0].version.text()
+assert "compile" ==  flattenedProject.dependencies.dependency[0].scope.text()
+
+// The flattened pom.xml should declare the dep under core as compile scope.
+// It's ok to ignore the one in the test-scope dependency.
+assert "dep" ==  flattenedProject.dependencies.dependency[1].artifactId.text()
+assert "3.2.1" ==  flattenedProject.dependencies.dependency[1].version.text()
+assert "compile" ==  flattenedProject.dependencies.dependency[1].scope.text()

src/main/java/org/codehaus/mojo/flatten/FlattenMojo.java

@@ -87,6 +87,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Properties;
@@ -1095,11 +1096,11 @@ private void createFlattenedDependenciesDirect( List<Dependency> projectDependen
      * The collected dependencies are stored in order, so that the leaf dependencies are prioritized in front of direct dependencies.
      * In addition, every non-leaf dependencies will exclude its own direct dependency, since all transitive dependencies
      * will be collected.
-	 *
+     *
      * Transitive dependencies are all going to be collected and become a direct dependency. Maven should already resolve
      * versions properly because now the transitive dependencies are closer to the artifact. However, when this artifact is
      * being consumed, Maven Enforcer Convergence rule will fail because there may be multiple versions for the same transitive dependency.
-	 *
+     *
      * Typically, exclusion can be done by using the wildcard. However, a known Maven issue prevents convergence enforcer from
      * working properly w/ wildcard exclusions. Thus, this will exclude each dependencies explicitly rather than using the wildcard.
      *
@@ -1117,7 +1118,7 @@ private void createFlattenedDependenciesAll( List<Dependency> projectDependencie
         final Artifact projectArtifact = this.project.getArtifact();
 
         ProjectBuildingRequest buildingRequest = new DefaultProjectBuildingRequest( session.getProjectBuildingRequest() );
-        buildingRequest.setProject( project );
+        buildingRequest.setProject( cloneProjectWithoutTestDependencies( project ) );
 
         final DependencyNode dependencyNode = this.dependencyGraphBuilder.buildDependencyGraph( buildingRequest, null);
 
@@ -1206,6 +1207,43 @@ private void createFlattenedDependenciesAll( List<Dependency> projectDependencie
         }
     }
 
+    /**
+     * Returns a cloned project that does not have direct test-scope dependencies.
+     *
+     * Test-scope project dependencies may hinder transitive dependencies by marking them as 'omitted for duplicate' when
+     * building dependency tree. This was a problem when the transitive dependency is actually needed by another non-test dependency
+     * of the project (See https://github.com/mojohaus/flatten-maven-plugin/issues/185). To avoid this interference of
+     * test-scope project dependencies, this plugin builds a dependency tree of the project without direct, test-scope dependencies.
+     *
+     * Removal of test scope dependencies is safe because these dependencies do not appear in library users' class path in
+     * any case.
+     *
+     * @param project is the original project to clone.
+     * @return a cloned project without direct test-scope dependencies.
+     */
+    private static MavenProject cloneProjectWithoutTestDependencies( MavenProject project )
+    {
+        final Set<String> testScopeProjectDependencyKeys = new HashSet<>();
+        for ( Dependency projectDependency : project.getDependencies() )
+        {
+            if ( "test".equals( projectDependency.getScope() ) )
+            {
+                testScopeProjectDependencyKeys.add( projectDependency.getManagementKey() );
+            }
+        }
+        // LinkedHashSet preserves the order.
+        final Set<Artifact> dependencyArtifactsWithoutTest = new LinkedHashSet<>( project.getDependencyArtifacts() );
+        dependencyArtifactsWithoutTest.removeIf(artifact -> {
+            // The same logic as org.apache.maven.model.Dependency.getManagementKey()
+            String managementKey = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getType()
+                + ( artifact.getClassifier() != null ? ":" + artifact.getClassifier() : "" );
+            return testScopeProjectDependencyKeys.contains( managementKey );
+        });
+        final MavenProject projectWithoutTestScopeDeps = project.clone();
+        projectWithoutTestScopeDeps.setDependencyArtifacts( dependencyArtifactsWithoutTest );
+        return projectWithoutTestScopeDeps;
+    }
+
     /**
      * Collects the resolved {@link Dependency dependencies} from the given <code>effectiveModel</code>.
      *