Skip to content

Commit

Permalink
feat: can set severity and filter duplicate class warnings. (#1340)
Browse files Browse the repository at this point in the history
* feat: can set severity and filter duplicate class warnings.

* test: add tests for duplicate class warning configuration.
  • Loading branch information
autonomousapps authored Dec 15, 2024
1 parent 67e1b87 commit af67f37
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
then:
assertThat(result.task(':consumer:compileJava').outcome).isEqualTo(TaskOutcome.FAILED)
// assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
// '''\
// dependencies {
// implementation project(':producer-2')
// }'''.stripIndent()
// )
// assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
// '''\
// dependencies {
// implementation project(':producer-2')
// }'''.stripIndent()
// )
where:
gradleVersion << [GRADLE_LATEST]
Expand Down Expand Up @@ -71,6 +71,57 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
gradleVersion << [GRADLE_LATEST]
}
def "buildHealth reports filters duplicates (#gradleVersion)"() {
given:
def project = new DuplicateClasspathProject('com/example/producer/Producer$Inner')
gradleProject = project.gradleProject
when:
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
then:
assertThat(result.output).contains(
'''\
Warnings
Some of your classpaths have duplicate classes, which means the compile and runtime behavior can be sensitive to the classpath order.
Source set: main
\\--- compile classpath
\\--- com/example/producer/Producer is provided by multiple dependencies: [:producer-1, :producer-2]
\\--- runtime classpath
\\--- com/example/producer/Producer is provided by multiple dependencies: [:producer-1, :producer-2]'''
.stripIndent()
)
and:
assertAbout(buildHealth())
.that(project.actualProjectAdvice())
.isEquivalentIgnoringModuleAdviceAndWarnings(project.expectedProjectAdvice)
where:
gradleVersion << [GRADLE_LATEST]
}
def "buildHealth reports ignores duplicates (#gradleVersion)"() {
given:
def project = new DuplicateClasspathProject(null, 'ignore')
gradleProject = project.gradleProject
when:
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
then:
assertThat(result.output).doesNotContain('Warnings')
and:
assertAbout(buildHealth())
.that(project.actualProjectAdvice())
.isEquivalentIgnoringModuleAdviceAndWarnings(project.expectedProjectAdvice)
where:
gradleVersion << [GRADLE_LATEST]
}
@PendingFeature(reason = "This feature was reverted")
def "can report on which of the duplicates is needed for binary compatibility (#gradleVersion)"() {
given:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,17 @@ final class DuplicateClasspathProject extends AbstractProject {

final GradleProject gradleProject

DuplicateClasspathProject() {
this.gradleProject = build()
DuplicateClasspathProject(String filter = null, String severity = null) {
this.gradleProject = build(filter, severity)
}

private GradleProject build() {
private GradleProject build(String filter, String severity) {
def configuration = new DagpConfiguration(filter, severity).toString()

return newGradleProjectBuilder()
.withRootProject { r ->
r.withBuildScript { bs ->
bs.withGroovy(
'''\
dependencyAnalysis {
issues {
all {
onAny {
severity 'fail'
}
}
}
}'''.stripIndent()
)
bs.withGroovy(configuration)
}
}
// :consumer uses the Producer class.
Expand Down Expand Up @@ -187,4 +178,43 @@ final class DuplicateClasspathProject extends AbstractProject {
emptyProjectAdviceFor(':producer-1'),
emptyProjectAdviceFor(':producer-2'),
]

static class DagpConfiguration {

private final String filter
private final String severity

DagpConfiguration(String filter, String severity) {
this.filter = filter
this.severity = severity
}

@Override
String toString() {
def builder = new StringBuilder()
builder.append('dependencyAnalysis {\n')
builder.append(' issues {\n')
builder.append(' all {\n')
builder.append(' onAny {\n')
builder.append(' severity \'fail\'\n')
builder.append(' }\n')

if (filter || severity) {
builder.append(' onDuplicateClassWarnings {\n')
if (severity) {
builder.append(" severity \'$severity\'\n")
}
if (filter) {
builder.append(" exclude \'$filter\'\n")
}
builder.append(' }\n')
}

builder.append(' }\n')
builder.append(' }\n')
builder.append('}')

return builder.toString()
}
}
}
4 changes: 4 additions & 0 deletions src/main/kotlin/com/autonomousapps/extension/IssueHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ abstract class IssueHandler @Inject constructor(
return globalDslService.unusedAnnotationProcessorsIssueFor(projectPath)
}

internal fun onDuplicateClassWarnings(projectPath: String): List<Provider<Behavior>> {
return globalDslService.onDuplicateClassWarnings(projectPath)
}

internal fun redundantPluginsIssueFor(projectPath: String): Provider<Behavior> {
return globalDslService.redundantPluginsIssueFor(projectPath)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ import javax.inject.Inject
* ignoreSourceSet(...)
*
* // Specify severity and exclude rules for all types of dependency violations.
* onAny { ... }
* onAny {
* severity(<"fail"|"warn"|"ignore">)
*
* // using version catalog accessors
* exclude(libs.guava, ...)
*
* // using basic string coordinates
* exclude("com.google.guava:guava", ...)
* }
*
* // Specify severity and exclude rules for unused dependencies.
* onUnusedDependencies { ... }
Expand All @@ -47,8 +55,15 @@ import javax.inject.Inject
*
* // Specify severity and exclude rules for module structure advice.
* onModuleStructure {
* severity(<'fail'|'warn'|'ignore'>)
* exclude('android')
* severity(<"fail"|"warn"|"ignore">)
* exclude("android")
* }
*
* onDuplicateClassWarnings {
* severity(<"fail"|"warn"|"ignore">)
*
* // Fully-qualified class reference to exclude, slash- or dot-delimited
* exclude("org/jetbrains/annotations/NotNull", "org.jetbrains.annotations.Nullable")
* }
* }
* }
Expand Down Expand Up @@ -76,6 +91,7 @@ abstract class ProjectIssueHandler @Inject constructor(
internal val runtimeOnlyIssue = objects.newInstance<Issue>()
internal val redundantPluginsIssue = objects.newInstance<Issue>()
internal val moduleStructureIssue = objects.newInstance<Issue>()
internal val duplicateClassWarningsIssue = objects.newInstance<Issue>()

internal val ignoreSourceSets = objects.setProperty<String>()

Expand Down Expand Up @@ -125,4 +141,8 @@ abstract class ProjectIssueHandler @Inject constructor(
fun onModuleStructure(action: Action<Issue>) {
action.execute(moduleStructureIssue)
}

fun onDuplicateClassWarnings(action: Action<Issue>) {
action.execute(duplicateClassWarningsIssue)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
// SPDX-License-Identifier: Apache-2.0
package com.autonomousapps.internal.advice

import com.autonomousapps.model.PluginAdvice
import com.autonomousapps.extension.Behavior
import com.autonomousapps.extension.Fail
import com.autonomousapps.internal.DependencyScope
import com.autonomousapps.internal.utils.filterToSet
import com.autonomousapps.internal.utils.lowercase
import com.autonomousapps.model.Advice
import com.autonomousapps.model.DuplicateClass
import com.autonomousapps.model.ModuleAdvice
import com.autonomousapps.model.PluginAdvice
import com.autonomousapps.model.Advice as DependencyAdvice

/** Given the set of all behaviors, determine whether the analysis should fail the build. */
Expand All @@ -20,20 +21,33 @@ internal class SeverityHandler(
private val incorrectConfigurationBehavior: Pair<Behavior, List<Behavior>>,
private val compileOnlyBehavior: Pair<Behavior, List<Behavior>>,
private val unusedProcsBehavior: Pair<Behavior, List<Behavior>>,
private val duplicateClassWarningsBehavior: Pair<Behavior, List<Behavior>>,
private val redundantPluginsBehavior: Behavior,
private val moduleStructureBehavior: Behavior,
) {

fun shouldFailDeps(advice: Set<DependencyAdvice>): Boolean {
return shouldFailFor(anyBehavior, advice) ||
shouldFailFor(unusedDependenciesBehavior, advice.filterToSet { it.isRemove() }) ||
shouldFailFor(usedTransitiveDependenciesBehavior, advice.filterToSet { it.isAdd() }) ||
shouldFailFor(incorrectConfigurationBehavior, advice.filterToSet { it.isChange() }) ||
shouldFailFor(compileOnlyBehavior, advice.filterToSet { it.isCompileOnly() }) ||
shouldFailFor(unusedProcsBehavior, advice.filterToSet { it.isProcessor() })
return shouldFailForAdvice(anyBehavior, advice) ||
shouldFailForAdvice(unusedDependenciesBehavior, advice.filterToSet { it.isRemove() }) ||
shouldFailForAdvice(usedTransitiveDependenciesBehavior, advice.filterToSet { it.isAdd() }) ||
shouldFailForAdvice(incorrectConfigurationBehavior, advice.filterToSet { it.isChange() }) ||
shouldFailForAdvice(compileOnlyBehavior, advice.filterToSet { it.isCompileOnly() }) ||
shouldFailForAdvice(unusedProcsBehavior, advice.filterToSet { it.isProcessor() })
}

fun shouldFailPlugins(pluginAdvice: Set<PluginAdvice>): Boolean {
return (redundantPluginsBehavior.isFail() || anyBehavior.first.isFail()) && pluginAdvice.isNotEmpty()
}

fun shouldFailModuleStructure(moduleAdvice: Set<ModuleAdvice>): Boolean {
return (moduleStructureBehavior.isFail() || anyBehavior.first.isFail()) && ModuleAdvice.isNotEmpty(moduleAdvice)
}

fun shouldFailDuplicateClasses(duplicateClasses: Set<DuplicateClass>): Boolean {
return shouldFailForDuplicateClasses(duplicateClassWarningsBehavior, duplicateClasses)
}

private fun shouldFailFor(
private fun shouldFailForAdvice(
spec: Pair<Behavior, List<Behavior>>,
advice: Set<DependencyAdvice>,
): Boolean {
Expand All @@ -50,7 +64,6 @@ internal class SeverityHandler(
b.sourceSetName == from || b.sourceSetName == to
}


// Looking for a match between sourceSet-specific behavior and advice.
var shouldFail = false
behaviors.forEach { b ->
Expand All @@ -71,12 +84,37 @@ internal class SeverityHandler(
return advice.any(bySourceSets) || (spec.first.isFail() && globalAdvice.isNotEmpty())
}

fun shouldFailPlugins(pluginAdvice: Set<PluginAdvice>): Boolean {
return (redundantPluginsBehavior.isFail() || anyBehavior.first.isFail()) && pluginAdvice.isNotEmpty()
}
private fun shouldFailForDuplicateClasses(
spec: Pair<Behavior, List<Behavior>>,
duplicateClasses: Set<DuplicateClass>,
): Boolean {
// Seed the "global" warnings with the set of all possible warnings. Later on we'll drain this set as elements of it
// are "consumed" by sourceSet-specific behaviors.
val globalAdvice = duplicateClasses.toMutableSet()

fun shouldFailModuleStructure(moduleAdvice: Set<ModuleAdvice>): Boolean {
return (moduleStructureBehavior.isFail() || anyBehavior.first.isFail()) && ModuleAdvice.isNotEmpty(moduleAdvice)
val bySourceSets: (DuplicateClass) -> Boolean = { d ->
// These are the custom behaviors, if any, associated with the source sets represented by this warning.
val behaviors = spec.second.filter { b ->
b.sourceSetName == DependencyScope.sourceSetName(d.classpathName)
}

// Looking for a match between sourceSet-specific behavior and warning.
var shouldFail = false
behaviors.forEach { b ->
val s = b.sourceSetName.lowercase()
val from = d.classpathName.lowercase().startsWith(s)

if (from) {
shouldFail = shouldFail || b.isFail()
globalAdvice.remove(d)
}
}

shouldFail
}

// If all advice is sourceSet-specific, then globalAdvice will be empty.
return duplicateClasses.any(bySourceSets) || (spec.first.isFail() && globalAdvice.isNotEmpty())
}

private fun Behavior.isFail(): Boolean = this is Fail
Expand Down
7 changes: 7 additions & 0 deletions src/main/kotlin/com/autonomousapps/model/DuplicateClass.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.autonomousapps.model

import com.autonomousapps.extension.Behavior
import com.autonomousapps.internal.utils.LexicographicIterableComparator
import com.autonomousapps.model.declaration.Variant
import com.squareup.moshi.JsonClass
Expand All @@ -25,6 +26,12 @@ data class DuplicateClass(
const val RUNTIME_CLASSPATH_NAME = "runtime"
}

private val dotty = className.replace('/', '.')

internal fun containsMatchIn(behavior: Behavior): Boolean {
return behavior.filter.contains(className) || behavior.filter.contains(dotty)
}

override fun compareTo(other: DuplicateClass): Int {
return compareBy(DuplicateClass::variant)
.thenBy(DuplicateClass::classpathName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ abstract class GlobalDslService @Inject constructor(
return issuesFor(projectPath) { it.unusedAnnotationProcessorsIssue }
}

internal fun onDuplicateClassWarnings(projectPath: String): List<Provider<Behavior>> {
return issuesFor(projectPath) { it.duplicateClassWarningsIssue }
}

internal fun redundantPluginsIssueFor(projectPath: String): Provider<Behavior> {
return overlay(all.redundantPluginsIssue, projects.findByName(projectPath)?.redundantPluginsIssue)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ internal class ProjectPlugin(private val project: Project) {
compileOnlyBehavior.addAll(compileOnlyIssueFor(theProjectPath))
runtimeOnlyBehavior.addAll(runtimeOnlyIssueFor(theProjectPath))
unusedProcsBehavior.addAll(unusedAnnotationProcessorsIssueFor(theProjectPath))
duplicateClassWarningsBehavior.addAll(onDuplicateClassWarnings(theProjectPath))

// These don't have sourceSet-specific behaviors
redundantPluginsBehavior.set(redundantPluginsIssueFor(theProjectPath))
Expand Down
Loading

0 comments on commit af67f37

Please sign in to comment.