Skip to content

Commit be797ed

Browse files
authored
Fix IllegalResolutionException in UnusedDependencyExcludeRule for Gradle 9.x (#437)
1 parent a7a091b commit be797ed

File tree

2 files changed

+221
-26
lines changed

2 files changed

+221
-26
lines changed

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import com.netflix.nebula.lint.rule.GradleAstUtil
2020
import com.netflix.nebula.lint.rule.GradleDependency
2121
import com.netflix.nebula.lint.rule.ModelAwareGradleLintRule
2222
import org.codehaus.groovy.ast.expr.MethodCallExpression
23-
import org.gradle.api.artifacts.Configuration
24-
import org.gradle.api.specs.Specs
2523

2624
class UnusedDependencyExcludeRule extends ModelAwareGradleLintRule {
2725
String description = 'excludes that have no effect on the classpath should be removed for clarity'
@@ -51,28 +49,29 @@ class UnusedDependencyExcludeRule extends ModelAwareGradleLintRule {
5149
}
5250

5351
private boolean isExcludeUnnecessary(String group, String name) {
54-
// Since Gradle does not expose any information about which excludes were effective, we will create a new configuration
55-
// lintExcludeConf, add the dependency and resolve it.
56-
Configuration lintExcludeConf = project.configurations.create("lintExcludes")
57-
project.dependencies.add(lintExcludeConf.name, "$dependency.group:$dependency.name:$dependency.version")
52+
// Use detached configurations instead of project configurations to avoid threading issues
53+
// in Gradle 9.x which requires exclusive locks for configuration resolution
54+
def detachedConf = project.configurations.detachedConfiguration(
55+
project.dependencies.create("$dependency.group:$dependency.name:$dependency.version")
56+
)
5857

59-
// If we find a dependency in the transitive closure of this special conf, then we can infer that the exclusion is
60-
// doing something. Note that all*.exclude style exclusions are applied to all of the configurations at the time
61-
// of project evaluation, but not lintExcludeConf.
58+
// This is thread-safe and doesn't require exclusive locks
59+
def resolutionResult = detachedConf.incoming.resolutionResult
60+
6261
def excludeIsInTransitiveClosure = false
63-
def deps = lintExcludeConf.resolvedConfiguration.lenientConfiguration.getFirstLevelModuleDependencies()
64-
while(!deps.isEmpty() && !excludeIsInTransitiveClosure) {
65-
deps = deps.collect { d ->
66-
if((!group || d.moduleGroup == group) && (!name || d.moduleName == name)) {
67-
excludeIsInTransitiveClosure = true
68-
}
69-
d.children
62+
63+
def allComponents = resolutionResult.allComponents
64+
65+
for (component in allComponents) {
66+
def moduleVersion = component.moduleVersion
67+
if (moduleVersion &&
68+
(!group || moduleVersion.group == group) &&
69+
(!name || moduleVersion.name == name)) {
70+
excludeIsInTransitiveClosure = true
71+
break
7072
}
71-
.flatten()
7273
}
73-
74-
project.configurations.remove(lintExcludeConf)
75-
76-
!excludeIsInTransitiveClosure
74+
75+
return !excludeIsInTransitiveClosure
7776
}
7877
}

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

Lines changed: 201 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015-2019 Netflix, Inc.
2+
* Copyright 2015-2025 Netflix, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,7 +28,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec {
2828

2929
def 'unused exclude violates'() {
3030
when:
31-
// trivial case: no dependencies
3231
project.buildFile << """
3332
apply plugin: 'java'
3433
dependencies {
@@ -58,7 +57,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec {
5857

5958
def 'unused exclude violates - api configuration'() {
6059
when:
61-
// trivial case: no dependencies
6260
project.buildFile << """
6361
apply plugin: 'java-library'
6462
dependencies {
@@ -89,7 +87,7 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec {
8987

9088
def 'exclude matching a transitive dependency does not violate'() {
9189
when:
92-
// trivial case: no dependencies
90+
9391
project.buildFile << """
9492
apply plugin: 'java'
9593
dependencies {
@@ -120,7 +118,6 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec {
120118
@Issue('#57')
121119
def 'exclude on a dependency that is unresolvable is considered unapplicable'() {
122120
when:
123-
// trivial case: no dependencies
124121
project.buildFile << """
125122
apply plugin: 'java'
126123
dependencies {
@@ -147,4 +144,203 @@ class UnusedDependencyExcludeRuleSpec extends AbstractRuleSpec {
147144
then:
148145
results.violates()
149146
}
147+
148+
def 'detects multiple unused excludes on same dependency'() {
149+
when:
150+
project.buildFile << """
151+
apply plugin: 'java'
152+
dependencies {
153+
implementation('commons-configuration:commons-configuration:1.10') {
154+
exclude group: 'com.google.guava', module: 'guava'
155+
exclude group: 'com.fasterxml.jackson.core', module: 'jackson-core'
156+
exclude group: 'commons-lang', module: 'commons-lang'
157+
}
158+
}
159+
"""
160+
161+
project.with {
162+
apply plugin: 'java'
163+
repositories {
164+
mavenCentral()
165+
}
166+
dependencies {
167+
implementation('commons-configuration:commons-configuration:1.10') {
168+
exclude group: 'com.google.guava', module: 'guava'
169+
exclude group: 'com.fasterxml.jackson.core', module: 'jackson-core'
170+
exclude group: 'commons-lang', module: 'commons-lang'
171+
}
172+
}
173+
}
174+
175+
def results = runRulesAgainst(rule)
176+
177+
then:
178+
results.violates()
179+
180+
results.violations.size() == 2
181+
results.violations.any { it.message.contains('the excluded dependency is not a transitive') && it.sourceLine.contains('com.google.guava') }
182+
results.violations.any { it.message.contains('the excluded dependency is not a transitive') && it.sourceLine.contains('com.fasterxml.jackson.core') }
183+
!results.violations.any { it.sourceLine.contains('commons-lang') && it.sourceLine.contains('commons-lang') }
184+
}
185+
186+
def 'works with testImplementation configuration'() {
187+
when:
188+
project.buildFile << """
189+
apply plugin: 'java'
190+
dependencies {
191+
testImplementation('junit:junit:4.13.2') {
192+
exclude group: 'fake.group', module: 'fake-module'
193+
}
194+
}
195+
"""
196+
197+
project.with {
198+
apply plugin: 'java'
199+
repositories {
200+
mavenCentral()
201+
}
202+
dependencies {
203+
testImplementation('junit:junit:4.13.2') {
204+
exclude group: 'fake.group', module: 'fake-module'
205+
}
206+
}
207+
}
208+
209+
def results = runRulesAgainst(rule)
210+
211+
then:
212+
results.violates()
213+
results.violations[0].message.contains('the excluded dependency is not a transitive')
214+
results.violations[0].sourceLine.contains('fake.group') && results.violations[0].sourceLine.contains('fake-module')
215+
}
216+
217+
def 'handles exclude with only group specified'() {
218+
when:
219+
project.buildFile << """
220+
apply plugin: 'java'
221+
dependencies {
222+
implementation('commons-configuration:commons-configuration:1.10') {
223+
exclude group: 'com.google.guava'
224+
}
225+
}
226+
"""
227+
228+
project.with {
229+
apply plugin: 'java'
230+
repositories {
231+
mavenCentral()
232+
}
233+
dependencies {
234+
implementation('commons-configuration:commons-configuration:1.10') {
235+
exclude group: 'com.google.guava'
236+
}
237+
}
238+
}
239+
240+
def results = runRulesAgainst(rule)
241+
242+
then:
243+
results.violates()
244+
results.violations[0].message.contains('the excluded dependency is not a transitive')
245+
}
246+
247+
def 'handles exclude with only module specified'() {
248+
when:
249+
project.buildFile << """
250+
apply plugin: 'java'
251+
dependencies {
252+
implementation('commons-configuration:commons-configuration:1.10') {
253+
exclude module: 'guava'
254+
}
255+
}
256+
"""
257+
258+
project.with {
259+
apply plugin: 'java'
260+
repositories {
261+
mavenCentral()
262+
}
263+
dependencies {
264+
implementation('commons-configuration:commons-configuration:1.10') {
265+
exclude module: 'guava'
266+
}
267+
}
268+
}
269+
270+
def results = runRulesAgainst(rule)
271+
272+
then:
273+
results.violates()
274+
results.violations[0].message.contains('the excluded dependency is not a transitive')
275+
}
276+
277+
@Issue('Gradle 9.x compatibility - detached configurations')
278+
def 'rule creates and cleans up configurations properly'() {
279+
when:
280+
project.buildFile << """
281+
apply plugin: 'java'
282+
dependencies {
283+
implementation('commons-configuration:commons-configuration:1.10') {
284+
exclude group: 'com.google.guava', module: 'guava'
285+
}
286+
}
287+
"""
288+
289+
project.with {
290+
apply plugin: 'java'
291+
repositories {
292+
mavenCentral()
293+
}
294+
dependencies {
295+
implementation('commons-configuration:commons-configuration:1.10') {
296+
exclude group: 'com.google.guava', module: 'guava'
297+
}
298+
}
299+
}
300+
301+
def configurationsBefore = project.configurations.collect { it.name }
302+
def results = runRulesAgainst(rule)
303+
def configurationsAfter = project.configurations.collect { it.name }
304+
305+
then:
306+
results.violates()
307+
configurationsBefore == configurationsAfter
308+
!configurationsAfter.any { it.contains('lintExcludes') }
309+
}
310+
311+
def 'handles complex dependency with multiple transitives'() {
312+
when:
313+
project.buildFile << """
314+
apply plugin: 'java'
315+
dependencies {
316+
implementation('org.springframework:spring-web:5.3.0') {
317+
exclude group: 'org.springframework', module: 'spring-core' // valid
318+
exclude group: 'fake.spring', module: 'fake-spring-core' // invalid
319+
}
320+
}
321+
"""
322+
323+
project.with {
324+
apply plugin: 'java'
325+
repositories {
326+
mavenCentral()
327+
}
328+
dependencies {
329+
implementation('org.springframework:spring-web:5.3.0') {
330+
exclude group: 'org.springframework', module: 'spring-core'
331+
exclude group: 'fake.spring', module: 'fake-spring-core'
332+
}
333+
}
334+
}
335+
336+
def results = runRulesAgainst(rule)
337+
338+
then:
339+
results.violates()
340+
341+
results.violations.size() == 1
342+
results.violations[0].message.contains('the excluded dependency is not a transitive')
343+
results.violations[0].sourceLine.contains('fake.spring') && results.violations[0].sourceLine.contains('fake-spring-core')
344+
!results.violations.any { it.sourceLine.contains('spring-core') && !it.sourceLine.contains('fake') }
345+
}
150346
}

0 commit comments

Comments
 (0)