Skip to content

Commit bcc5fe1

Browse files
VampireVampire
committed
fix: duplicate class detection for annotations
1 parent 04a70b0 commit bcc5fe1

File tree

3 files changed

+275
-1
lines changed

3 files changed

+275
-1
lines changed

‎src/functionalTest/groovy/com/autonomousapps/jvm/DuplicateClasspathSpec.groovy‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package com.autonomousapps.jvm
44

55
import com.autonomousapps.jvm.projects.BinaryIncompatibilityProject
6+
import com.autonomousapps.jvm.projects.DuplicateAnnotationClasspathProject
67
import com.autonomousapps.jvm.projects.DuplicateClasspathProject
78
import com.autonomousapps.utils.Colors
89

@@ -75,6 +76,40 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
7576
gradleVersion << [GRADLE_LATEST]
7677
}
7778
79+
def "buildHealth reports duplicates in annotations (#gradleVersion)"() {
80+
given:
81+
def project = new DuplicateAnnotationClasspathProject()
82+
gradleProject = project.gradleProject
83+
84+
when:
85+
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
86+
87+
then:
88+
assertThat(result.output).contains(
89+
'''\
90+
Warnings
91+
Some of your classpaths have duplicate classes, which means the compile and runtime behavior can be sensitive to the classpath order.
92+
93+
Source set: main
94+
\\--- compile classpath
95+
\\--- com/example/annotation/Annotate is provided by multiple dependencies: [:annotation-1, :annotation-2]
96+
\\--- runtime classpath
97+
\\--- com/example/annotation/Annotate is provided by multiple dependencies: [:annotation-1, :annotation-2]'''
98+
.stripIndent()
99+
)
100+
101+
and: 'Postscript is printed'
102+
assertThat(result.output).contains('ERRORS-ONLY POSTSCRIPT')
103+
104+
and:
105+
assertAbout(buildHealth())
106+
.that(project.actualProjectAdvice())
107+
.isEquivalentIgnoringModuleAdviceAndWarnings(project.expectedProjectAdvice())
108+
109+
where:
110+
gradleVersion << [GRADLE_LATEST]
111+
}
112+
78113
def "buildHealth reports filters duplicates (#gradleVersion)"() {
79114
given:
80115
def project = new DuplicateClasspathProject('fail', 'com/example/producer/Producer$Inner')

‎src/functionalTest/groovy/com/autonomousapps/jvm/projects/DuplicateAnnotationClasspathProject.groovy‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
package com.autonomousapps.jvm.projects
2+
3+
import com.autonomousapps.AbstractProject
4+
import com.autonomousapps.kit.GradleProject
5+
import com.autonomousapps.kit.Source
6+
import com.autonomousapps.model.Advice
7+
import com.autonomousapps.model.ProjectAdvice
8+
9+
import static com.autonomousapps.AdviceHelper.actualProjectAdvice
10+
import static com.autonomousapps.AdviceHelper.emptyProjectAdviceFor
11+
import static com.autonomousapps.AdviceHelper.projectAdviceForDependencies
12+
import static com.autonomousapps.AdviceHelper.projectCoordinates
13+
import static com.autonomousapps.kit.gradle.Dependency.project
14+
15+
final class DuplicateAnnotationClasspathProject extends AbstractProject {
16+
17+
private final boolean shouldFail
18+
final GradleProject gradleProject
19+
20+
DuplicateAnnotationClasspathProject(
21+
String onAnySeverity = 'fail',
22+
List<String> duplicateClassesFilter = null, String duplicateClassesSeverity = null
23+
) {
24+
this.shouldFail = onAnySeverity == 'fail'
25+
this.gradleProject = build(onAnySeverity, duplicateClassesFilter, duplicateClassesSeverity)
26+
}
27+
28+
private GradleProject build(String onAnySeverity, List<String> duplicateClassesFilter, String duplicateClassesSeverity) {
29+
def configuration = new DagpConfiguration(
30+
onAnySeverity,
31+
duplicateClassesFilter,
32+
duplicateClassesSeverity,
33+
).toString()
34+
35+
return newGradleProjectBuilder()
36+
.withRootProject { r ->
37+
r.withBuildScript { bs ->
38+
bs.withGroovy(configuration)
39+
}
40+
}
41+
// :consumer uses the Annotate class.
42+
// This class is provided by both
43+
// 1. :annotation-2, which is a direct dependency, and
44+
// 2. :annotation-1, which is a transitive dependency (of :unused)
45+
// These classes have incompatible definitions. :consumer _requires_ the version provided by :annotation-2.
46+
.withSubproject('consumer') { s ->
47+
s.sources = sourcesConsumer
48+
s.withBuildScript { bs ->
49+
bs.plugins = javaLibrary + plugins.gradleDependenciesSorter
50+
bs.dependencies(
51+
project('implementation', ':unused'),
52+
project('api', ':annotation-2'),
53+
)
54+
55+
// We need to sort the dependencies after rewriting so that the classpath loads the class with the
56+
// incompatible definition, causing `:consumer:compileJava` to fail.
57+
bs.withGroovy(
58+
'''\
59+
tasks.named { it == 'fixDependencies' }.configureEach {
60+
finalizedBy('sortDependencies')
61+
}
62+
'''
63+
)
64+
}
65+
}
66+
// Used
67+
.withSubproject('annotation-1') { s ->
68+
s.sources = sourcesAnnotation1
69+
s.withBuildScript { bs ->
70+
bs.plugins = javaLibrary
71+
}
72+
}
73+
// Unused, except its transitive is
74+
.withSubproject('unused') { s ->
75+
s.sources = sourcesUnused
76+
s.withBuildScript { bs ->
77+
bs.plugins = javaLibrary
78+
bs.dependencies(
79+
project('api', ':annotation-1'),
80+
)
81+
}
82+
}
83+
// Used?
84+
.withSubproject('annotation-2') { s ->
85+
s.sources = sourcesAnnotation2
86+
s.withBuildScript { bs ->
87+
bs.plugins = javaLibrary
88+
}
89+
}
90+
.write()
91+
}
92+
93+
private static final List<Source> sourcesConsumer = [
94+
Source.java(
95+
'''\
96+
package com.example.consumer;
97+
98+
import com.example.annotation.Annotate;
99+
100+
@Annotate(0)
101+
public class Consumer {
102+
}
103+
'''
104+
)
105+
.withPath('com.example.consumer', 'Consumer')
106+
.build(),
107+
]
108+
109+
private static final List<Source> sourcesAnnotation1 = [
110+
Source.java(
111+
'''\
112+
package com.example.annotation;
113+
114+
import java.lang.annotation.Target;
115+
import java.lang.annotation.Retention;
116+
117+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
118+
import static java.lang.annotation.ElementType.TYPE;
119+
120+
@Target(TYPE)
121+
@Retention(RUNTIME)
122+
public @interface Annotate {
123+
String value() default "";
124+
}
125+
'''
126+
)
127+
.withPath('com.example.annotation', 'Annotate')
128+
.build(),
129+
]
130+
131+
// Same class file as sourcesAnnotation1, incompatible definition
132+
private static final List<Source> sourcesAnnotation2 = [
133+
Source.java(
134+
'''\
135+
package com.example.annotation;
136+
137+
import java.lang.annotation.Target;
138+
import java.lang.annotation.Retention;
139+
140+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
141+
import static java.lang.annotation.ElementType.TYPE;
142+
143+
@Target(TYPE)
144+
@Retention(RUNTIME)
145+
public @interface Annotate {
146+
int value() default 0;
147+
}
148+
'''
149+
)
150+
.withPath('com.example.annotation', 'Annotate')
151+
.build(),
152+
]
153+
154+
private static final List<Source> sourcesUnused = [
155+
Source.java(
156+
'''\
157+
package com.example.unused;
158+
159+
import com.example.annotation.Annotate;
160+
161+
@Annotate("")
162+
public class Unused {
163+
}
164+
'''
165+
)
166+
.withPath('com.example.unused', 'Unused')
167+
.build(),
168+
]
169+
170+
Set<ProjectAdvice> actualProjectAdvice() {
171+
return actualProjectAdvice(gradleProject)
172+
}
173+
174+
private final Set<Advice> consumerAdvice = [
175+
Advice.ofRemove(projectCoordinates(':unused'), 'implementation'),
176+
// This is actually bad advice, but we can't detect it without re-reverting the change to detect binary
177+
// incompatibilities.
178+
Advice.ofAdd(projectCoordinates(':annotation-1'), 'api')
179+
]
180+
181+
Set<ProjectAdvice> expectedProjectAdvice() {
182+
return [
183+
projectAdviceForDependencies(':consumer', consumerAdvice, shouldFail),
184+
emptyProjectAdviceFor(':unused'),
185+
emptyProjectAdviceFor(':annotation-1'),
186+
emptyProjectAdviceFor(':annotation-2'),
187+
]
188+
}
189+
190+
static class DagpConfiguration {
191+
192+
private final String onAnySeverity
193+
private final List<String> duplicateClassesFilter
194+
private final String duplicateClassesSeverity
195+
196+
DagpConfiguration(String onAnySeverity, List<String> duplicateClassesFilter, String duplicateClassesSeverity) {
197+
this.onAnySeverity = onAnySeverity
198+
this.duplicateClassesFilter = duplicateClassesFilter
199+
this.duplicateClassesSeverity = duplicateClassesSeverity
200+
}
201+
202+
@Override
203+
String toString() {
204+
def builder = new StringBuilder()
205+
builder.append('dependencyAnalysis {\n')
206+
builder.append(' reporting {\n')
207+
builder.append(' onlyOnFailure(true)\n')
208+
// multiline for a "manual test" of colorized support for multiline strings
209+
builder.append(' postscript("""MULTILINE\nERRORS-ONLY POSTSCRIPT""")\n')
210+
builder.append(' }\n')
211+
212+
builder.append(' issues {\n')
213+
builder.append(' all {\n')
214+
builder.append(' onAny {\n')
215+
builder.append(" severity \'$onAnySeverity\'\n")
216+
builder.append(' }\n')
217+
218+
if (duplicateClassesFilter || duplicateClassesSeverity) {
219+
builder.append(' onDuplicateClassWarnings {\n')
220+
if (duplicateClassesSeverity) {
221+
builder.append(" severity \'$duplicateClassesSeverity\'\n")
222+
}
223+
duplicateClassesFilter.each {
224+
builder.append(" exclude \'$it\'\n")
225+
}
226+
builder.append(' }\n')
227+
}
228+
229+
builder.append(' }\n')
230+
builder.append(' }\n')
231+
builder.append('}')
232+
233+
return builder.toString()
234+
}
235+
}
236+
}

‎src/main/kotlin/com/autonomousapps/tasks/DiscoverClasspathDuplicationTask.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,10 @@ public abstract class DiscoverClasspathDuplicationTask : DefaultTask() {
7979
// Find all class files provided by more than one dependency
8080
.filterValues { it.size > 1 }
8181
// only warn about duplicates if it's about a class that's actually used.
82-
.filterKeys { it.replace('/', '.').removeSuffix(".class") in project.usedClasses }
82+
.filterKeys {
83+
val fqcn = it.replace('/', '.').removeSuffix(".class")
84+
fqcn in project.usedClasses || fqcn in project.usedAnnotationClassesBySrc
85+
}
8386
// filter out "duplicates" where the GAV is identical. This can be an issue with, say, Kotlin, which adds
8487
// variants of the same dependency with different capabilities. Not user-actionable.
8588
// E.g., "org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.1.20" is the GAV for both of:

0 commit comments

Comments
 (0)