Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: can set severity and filter duplicate class warnings. #1340

Merged
merged 2 commits into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading