From e3e1e3b6bc6a598f4ae1e9a7cb4471a52b721699 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Wed, 24 Sep 2025 16:48:40 -0700 Subject: [PATCH] Fix IllegalResolutionException in UnusedDependencyExcludeRule for Gradle 9.x --- .../UnusedDependencyExcludeRule.groovy | 41 ++-- .../UnusedDependencyExcludeRuleSpec.groovy | 206 +++++++++++++++++- 2 files changed, 221 insertions(+), 26 deletions(-) diff --git a/src/main/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRule.groovy b/src/main/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRule.groovy index bf1e175b..89db5925 100644 --- a/src/main/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRule.groovy +++ b/src/main/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRule.groovy @@ -20,8 +20,6 @@ import com.netflix.nebula.lint.rule.GradleAstUtil import com.netflix.nebula.lint.rule.GradleDependency import com.netflix.nebula.lint.rule.ModelAwareGradleLintRule import org.codehaus.groovy.ast.expr.MethodCallExpression -import org.gradle.api.artifacts.Configuration -import org.gradle.api.specs.Specs class UnusedDependencyExcludeRule extends ModelAwareGradleLintRule { String description = 'excludes that have no effect on the classpath should be removed for clarity' @@ -51,28 +49,29 @@ class UnusedDependencyExcludeRule extends ModelAwareGradleLintRule { } private boolean isExcludeUnnecessary(String group, String name) { - // Since Gradle does not expose any information about which excludes were effective, we will create a new configuration - // lintExcludeConf, add the dependency and resolve it. - Configuration lintExcludeConf = project.configurations.create("lintExcludes") - project.dependencies.add(lintExcludeConf.name, "$dependency.group:$dependency.name:$dependency.version") + // Use detached configurations instead of project configurations to avoid threading issues + // in Gradle 9.x which requires exclusive locks for configuration resolution + def detachedConf = project.configurations.detachedConfiguration( + project.dependencies.create("$dependency.group:$dependency.name:$dependency.version") + ) - // If we find a dependency in the transitive closure of this special conf, then we can infer that the exclusion is - // doing something. Note that all*.exclude style exclusions are applied to all of the configurations at the time - // of project evaluation, but not lintExcludeConf. + // This is thread-safe and doesn't require exclusive locks + def resolutionResult = detachedConf.incoming.resolutionResult + def excludeIsInTransitiveClosure = false - def deps = lintExcludeConf.resolvedConfiguration.lenientConfiguration.getFirstLevelModuleDependencies() - while(!deps.isEmpty() && !excludeIsInTransitiveClosure) { - deps = deps.collect { d -> - if((!group || d.moduleGroup == group) && (!name || d.moduleName == name)) { - excludeIsInTransitiveClosure = true - } - d.children + + def allComponents = resolutionResult.allComponents + + for (component in allComponents) { + def moduleVersion = component.moduleVersion + if (moduleVersion && + (!group || moduleVersion.group == group) && + (!name || moduleVersion.name == name)) { + excludeIsInTransitiveClosure = true + break } - .flatten() } - - project.configurations.remove(lintExcludeConf) - - !excludeIsInTransitiveClosure + + return !excludeIsInTransitiveClosure } } diff --git a/src/test/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRuleSpec.groovy b/src/test/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRuleSpec.groovy index 9bb58335..e68c5ebd 100644 --- a/src/test/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRuleSpec.groovy +++ b/src/test/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyExcludeRuleSpec.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2015-2019 Netflix, Inc. + * Copyright 2015-2025 Netflix, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,7 +28,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec { def 'unused exclude violates'() { when: - // trivial case: no dependencies project.buildFile << """ apply plugin: 'java' dependencies { @@ -58,7 +57,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec { def 'unused exclude violates - api configuration'() { when: - // trivial case: no dependencies project.buildFile << """ apply plugin: 'java-library' dependencies { @@ -89,7 +87,7 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec { def 'exclude matching a transitive dependency does not violate'() { when: - // trivial case: no dependencies + project.buildFile << """ apply plugin: 'java' dependencies { @@ -120,7 +118,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec { @Issue('#57') def 'exclude on a dependency that is unresolvable is considered unapplicable'() { when: - // trivial case: no dependencies project.buildFile << """ apply plugin: 'java' dependencies { @@ -147,4 +144,203 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec { then: results.violates() } + + def 'detects multiple unused excludes on same dependency'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava', module: 'guava' + exclude group: 'com.fasterxml.jackson.core', module: 'jackson-core' + exclude group: 'commons-lang', module: 'commons-lang' + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava', module: 'guava' + exclude group: 'com.fasterxml.jackson.core', module: 'jackson-core' + exclude group: 'commons-lang', module: 'commons-lang' + } + } + } + + def results = runRulesAgainst(rule) + + then: + results.violates() + + results.violations.size() == 2 + results.violations.any { it.message.contains('the excluded dependency is not a transitive') && it.sourceLine.contains('com.google.guava') } + results.violations.any { it.message.contains('the excluded dependency is not a transitive') && it.sourceLine.contains('com.fasterxml.jackson.core') } + !results.violations.any { it.sourceLine.contains('commons-lang') && it.sourceLine.contains('commons-lang') } + } + + def 'works with testImplementation configuration'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + testImplementation('junit:junit:4.13.2') { + exclude group: 'fake.group', module: 'fake-module' + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + testImplementation('junit:junit:4.13.2') { + exclude group: 'fake.group', module: 'fake-module' + } + } + } + + def results = runRulesAgainst(rule) + + then: + results.violates() + results.violations[0].message.contains('the excluded dependency is not a transitive') + results.violations[0].sourceLine.contains('fake.group') && results.violations[0].sourceLine.contains('fake-module') + } + + def 'handles exclude with only group specified'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava' + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava' + } + } + } + + def results = runRulesAgainst(rule) + + then: + results.violates() + results.violations[0].message.contains('the excluded dependency is not a transitive') + } + + def 'handles exclude with only module specified'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude module: 'guava' + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude module: 'guava' + } + } + } + + def results = runRulesAgainst(rule) + + then: + results.violates() + results.violations[0].message.contains('the excluded dependency is not a transitive') + } + + @Issue('Gradle 9.x compatibility - detached configurations') + def 'rule creates and cleans up configurations properly'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava', module: 'guava' + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + implementation('commons-configuration:commons-configuration:1.10') { + exclude group: 'com.google.guava', module: 'guava' + } + } + } + + def configurationsBefore = project.configurations.collect { it.name } + def results = runRulesAgainst(rule) + def configurationsAfter = project.configurations.collect { it.name } + + then: + results.violates() + configurationsBefore == configurationsAfter + !configurationsAfter.any { it.contains('lintExcludes') } + } + + def 'handles complex dependency with multiple transitives'() { + when: + project.buildFile << """ + apply plugin: 'java' + dependencies { + implementation('org.springframework:spring-web:5.3.0') { + exclude group: 'org.springframework', module: 'spring-core' // valid + exclude group: 'fake.spring', module: 'fake-spring-core' // invalid + } + } + """ + + project.with { + apply plugin: 'java' + repositories { + mavenCentral() + } + dependencies { + implementation('org.springframework:spring-web:5.3.0') { + exclude group: 'org.springframework', module: 'spring-core' + exclude group: 'fake.spring', module: 'fake-spring-core' + } + } + } + + def results = runRulesAgainst(rule) + + then: + results.violates() + + results.violations.size() == 1 + results.violations[0].message.contains('the excluded dependency is not a transitive') + results.violations[0].sourceLine.contains('fake.spring') && results.violations[0].sourceLine.contains('fake-spring-core') + !results.violations.any { it.sourceLine.contains('spring-core') && !it.sourceLine.contains('fake') } + } }