Merge pull request #306 from nebula-plugins/bypassed-forces-works-wit…

…h-groovy-2.4

bypassed-forces rule works with groovy 2.4.15

src/main/groovy/com/netflix/nebula/lint/rule/dependency/BypassedForcesRule.groovy

@@ -14,12 +14,15 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression
 import org.codehaus.groovy.ast.stmt.BlockStatement
 import org.gradle.api.Project
 import org.gradle.api.artifacts.Configuration
+import org.gradle.api.artifacts.ModuleVersionIdentifier
 import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.DefaultVersionComparator
 import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.DefaultVersionSelectorScheme
 import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.strategy.VersionSelector
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
+import java.util.stream.Collectors
+
 class BypassedForcesRule extends GradleLintRule implements GradleModelAware {
     String description = 'remove bypassed forces and strict constraints. Works for static and ranged declarations'
     Map<String, Collection<ForcedDependency>> forcedDependenciesPerProject = new HashMap<String, Collection<ForcedDependency>>()
@@ -102,7 +105,7 @@ class BypassedForcesRule extends GradleLintRule implements GradleModelAware {
         bypassedForces
                 .groupBy { it.dep }
                 .each { dep, bypassedForcesByDep ->
-                    Collection<String> projectNames = bypassedForcesByDep.collect { it.projectName }
+                    Collection<String> projectNames = bypassedForcesByDep.stream().map { bypassedForce -> bypassedForce.projectName }.collect(Collectors.toList())
                     BypassedForce exemplarBypassedForce = bypassedForcesByDep.first()
                     def updatedMessage = exemplarBypassedForce.forcedDependency.message + " for the affected project(s): ${projectNames.sort().join(', ')}"
                     addBuildLintViolation(updatedMessage, exemplarBypassedForce.forcedDependency.forceExpression)
@@ -151,37 +154,48 @@ class BypassedForcesRule extends GradleLintRule implements GradleModelAware {
         Collection<ForcedDependency> dependenciesWithUnusedForces = new ArrayList<ForcedDependency>()
 
         // inspect direct and transitive dependencies
-        configuration.resolvedConfiguration.resolvedArtifacts
-                .collect { it.moduleVersion.id }
-                .unique { it.module.toString() }
+        Collection<ModuleVersionIdentifier> uniqueModuleIdentifiers = configuration.resolvedConfiguration.resolvedArtifacts
+                .stream()
+                .map {artifact -> artifact.moduleVersion }
+                .map { resolvedModuleVersion -> resolvedModuleVersion.id }
+                .collect(Collectors.toList())
+                .unique()
+
+        uniqueModuleIdentifiers
                 .each { resolvedDep ->
                     forcedDeps.each { forcedDependency ->
-                        if (forcedDependency.dep.group == resolvedDep.module.group && forcedDependency.dep.name == resolvedDep.name) {
-                            def expectedVersion = ''
-                            if (forcedDependency.dep.version != null) {
-                                expectedVersion = forcedDependency.dep.version
-                            }
-                            if (!forcedDependency.strictVersion.isEmpty()) {
-                                expectedVersion = forcedDependency.strictVersion
-                            }
-
-                            VersionSelector versionSelector = VERSION_SCHEME.parseSelector(expectedVersion)
-                            if (!expectedVersion.startsWith('$')
-                                    && !expectedVersion.toString().contains(".+")
-                                    && !expectedVersion.toString().contains("latest")
-                                    && !versionSelector.accept(resolvedDep.version)) {
-
-                                forcedDependency.resolvedConfigurations.add(configuration)
-                                forcedDependency.message = "The ${forcedDependency.forceType} has been bypassed.\nRemove or update this value"
-                                dependenciesWithUnusedForces.add(forcedDependency)
-                            }
-                        }
+                        dependenciesWithUnusedForces.addAll(handleEachResolvedDependencyForcedDependency(forcedDependency, resolvedDep, configuration))
                     }
                 }
 
         return dependenciesWithUnusedForces
     }
 
+    private Collection<ForcedDependency> handleEachResolvedDependencyForcedDependency(ForcedDependency forcedDependency, ModuleVersionIdentifier resolvedDep, Configuration configuration) {
+        Collection<ForcedDependency> dependenciesWithUnusedForces = new ArrayList<ForcedDependency>()
+        if (forcedDependency.dep.group == resolvedDep.module.group && forcedDependency.dep.name == resolvedDep.name) {
+            def expectedVersion = ''
+            if (forcedDependency.dep.version != null) {
+                expectedVersion = forcedDependency.dep.version
+            }
+            if (!forcedDependency.strictVersion.isEmpty()) {
+                expectedVersion = forcedDependency.strictVersion
+            }
+
+            VersionSelector versionSelector = VERSION_SCHEME.parseSelector(expectedVersion)
+            if (!expectedVersion.startsWith('$')
+                    && !expectedVersion.toString().contains(".+")
+                    && !expectedVersion.toString().contains("latest")
+                    && !versionSelector.accept(resolvedDep.version)) {
+
+                forcedDependency.resolvedConfigurations.add(configuration)
+                forcedDependency.message = "The ${forcedDependency.forceType} has been bypassed.\nRemove or update this value"
+                dependenciesWithUnusedForces.add(forcedDependency)
+            }
+        }
+        return dependenciesWithUnusedForces
+    }
+
     private static Boolean closureContainsForce(ClosureExpression expr) {
         return expr.code.statements.any {
             (it.expression instanceof BinaryExpression) &&

src/test/groovy/com/netflix/nebula/lint/rule/dependency/BypassedForcesWithResolutionRulesSpec.groovy

@@ -31,7 +31,6 @@ class BypassedForcesWithResolutionRulesSpec extends IntegrationTestKitSpec {
     def setup() {
         setupDependenciesAndRules()
         debug = true
-        keepFiles = true
     }
 
     @Unroll
@@ -216,6 +215,21 @@ class BypassedForcesWithResolutionRulesSpec extends IntegrationTestKitSpec {
         'variable'       | '\${testNebulaVersion}' | true
     }
 
+    def 'works with Groovy 2.4.x - direct dependency force not honored'() {
+        def coreAlignment = true
+        setupSingleProjectBuildFileForGradle4_10_2()
+        buildFile << directDependencyForceNotHonoredDependenciesBlock()
+
+        when:
+        def tasks = ['dependencyInsight', '--dependency', 'test.nebula', '--warning-mode', 'none', "-Dnebula.features.coreAlignmentSupport=$coreAlignment"]
+        tasks += 'fixGradleLint'
+        def results = runTasks(*tasks)
+
+        then:
+        // substitution rule to a known-good-version was the primary contributor; force to a bad version was a secondary contributor
+        assertDirectDependencyAndResolutionStrategyForceNotHonored(results.output)
+    }
+
     @Unroll
     def 'resolution strategy force is honored - force to good version while substitution is triggered by a transitive dependency | core alignment #coreAlignment'() {
         setupSingleProjectBuildFile()
@@ -425,6 +439,28 @@ class BypassedForcesWithResolutionRulesSpec extends IntegrationTestKitSpec {
 test.nebula:a:1.2.0\n""")
     }
 
+    def 'works with Groovy 2.4.x - resolution strategy force not honored'() {
+        def coreAlignment = true
+        setupSingleProjectBuildFileForGradle4_10_2()
+        buildFile << """\
+            ${resolutionStrategyForceNotHonoredConfigurationsBlock()}
+            dependencies {
+                implementation 'test.nebula:a:1.2.0' // bad version
+                implementation 'test.nebula:b:1.0.0' // added for alignment
+                implementation 'test.nebula:c:1.0.0' // added for alignment
+            }
+        """.stripIndent()
+
+        when:
+        def tasks = ['dependencyInsight', '--dependency', 'test.nebula', '--warning-mode', 'none', "-Dnebula.features.coreAlignmentSupport=$coreAlignment"]
+        tasks += 'fixGradleLint'
+        def results = runTasks(*tasks)
+
+        then:
+        // substitution rule to a known-good-version was the primary contributor; force to a bad version was a secondary contributor
+        assertDirectDependencyAndResolutionStrategyForceNotHonored(results.output)
+    }
+
     @Unroll
     def 'handles multiple forces in one statement | core alignment #coreAlignment'() {
         setupSingleProjectBuildFile()
@@ -769,6 +805,20 @@ test.nebula:a:1.2.0\n""")
         'variable'       | '\${testNebulaVersion}' | true
     }
 
+    def 'works with Groovy 2.4.x - dependency with strict version declaration not honored'() {
+        def coreAlignment = true
+        setupSingleProjectBuildFileForGradle4_10_2()
+        buildFile << strictVersionsDeclarationNotHonoredDependenciesBlock()
+
+        when:
+        def tasks = ['dependencyInsight', '--dependency', 'test.nebula', "-Dnebula.features.coreAlignmentSupport=$coreAlignment"]
+        tasks += 'fixGradleLint'
+        def results = runTasks(*tasks)
+
+        then:
+        results.output.contains('BUILD SUCCESSFUL')
+    }
+
     @Unroll
     def 'dependency constraint with strict version declaration honored | core alignment #coreAlignment'() {
         setupSingleProjectBuildFile()
@@ -1077,6 +1127,34 @@ test.nebula:a:1.3.0\n""")
         'variable'       | '\${testNebulaVersion}' | true
     }
 
+    def 'works with Groovy 2.4.x - dependency constraint with strict version declaration not honored'() {
+        def coreAlignment = true
+        setupSingleProjectBuildFileForGradle4_10_2()
+        buildFile << """\
+            dependencies {
+                ${dependencyConstraintWithStrictVersionDeclarationNotHonoredDependenciesBlock()}                
+                implementation 'test.brings-a:a:1.0.0' // added for alignment
+                implementation 'test.brings-b:b:1.0.0' // added for alignment
+                implementation 'test.brings-c:c:1.0.0' // added for alignment
+            }
+        """.stripIndent()
+
+        def graph = new DependencyGraphBuilder()
+                .addModule(new ModuleBuilder('test.brings-b:b:1.0.0').addDependency('test.nebula:b:1.0.0').build())
+                .addModule(new ModuleBuilder('test.brings-a:a:1.0.0').addDependency('test.nebula:a:1.0.0').build())
+                .addModule(new ModuleBuilder('test.brings-c:c:1.0.0').addDependency('test.nebula:c:1.0.0').build())
+                .build()
+        new GradleDependencyGenerator(graph, "${projectDir}/testrepogen").generateTestMavenRepo()
+
+        when:
+        def tasks = ['dependencyInsight', '--dependency', 'test.nebula', "-Dnebula.features.coreAlignmentSupport=$coreAlignment"]
+        tasks += 'fixGradleLint'
+        def results = runTasks(*tasks)
+
+        then:
+        results.output.contains('BUILD SUCCESSFUL')
+    }
+
     @Unroll
     def 'ignores buildscript dependencies for #type'() {
         buildFile << """\
@@ -1190,6 +1268,26 @@ test.nebula:a:1.3.0\n""")
             """.stripIndent()
     }
 
+    private void setupSingleProjectBuildFileForGradle4_10_2() {
+        gradleVersion = '4.10.2' // with groovy 2.4.15
+
+        buildFile << """
+            plugins {
+                id 'java'
+                id "nebula.resolution-rules" version "6.0.5" 
+                id 'nebula.lint'
+            }
+
+            dependencies {
+                resolutionRules files('$rulesJsonFile')
+            }
+            repositories {
+                maven { url '${mavenrepo.absolutePath}' }
+            }
+            gradleLint.rules = ['bypassed-forces']
+            """
+    }
+
     private void setupMultiProjectBuildFile() {
         definePluginOutsideOfPluginBlock = true