Skip to content

Don't call a dependency overshot if it's already declared in that source set #521

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

Merged
merged 2 commits into from
Apr 9, 2022
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 @@ -21,7 +21,10 @@ import modulecheck.parsing.gradle.ConfigurationName
import modulecheck.project.ConfiguredProjectDependency
import modulecheck.project.McProject
import modulecheck.project.ProjectContext
import modulecheck.project.toSourceSetDependency
import modulecheck.utils.SafeCache
import modulecheck.utils.mapToSet
import modulecheck.utils.unsafeLazy

data class OverShotDependencies(
private val delegate: SafeCache<ConfigurationName, List<OverShotDependencyFinding>>,
Expand All @@ -39,12 +42,19 @@ data class OverShotDependencies(
.flatMap { unused ->

val unusedCpd = unused.cpd()
val unusedSsd =
unusedCpd.toSourceSetDependency(unused.configurationName.toSourceSetName())
val unusedSourceSetName = unused.configurationName.toSourceSetName()

val allUsedByConfigName = unusedSourceSetName
.withDownStream(project)
.mapNotNull { sourceSetName ->

val existingDependencies by unsafeLazy {
project.projectDependencies[sourceSetName]
.mapToSet { it.toSourceSetDependency(sourceSetName) }
}

val configName = sourceSetName.implementationConfig()

val seed = when {
Expand All @@ -69,10 +79,10 @@ data class OverShotDependencies(
)
}
.filterNot {
it.isTestFixture == unused.cpd().isTestFixture &&
it.configurationName.toSourceSetName() == unused.cpd()
.configurationName
.toSourceSetName()

val asSsd = it.toSourceSetDependency(sourceSetName)

asSsd == unusedSsd || existingDependencies.contains(asSsd)
}
.firstNotNullOfOrNull { cpd ->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import modulecheck.api.settings.ChecksSettings
import modulecheck.core.InheritedDependencyFinding
import modulecheck.core.context.asApiOrImplementation
import modulecheck.core.internal.uses
import modulecheck.parsing.gradle.ProjectPath.StringProjectPath
import modulecheck.parsing.gradle.SourceSetName
import modulecheck.parsing.gradle.sortedByInheritance
import modulecheck.project.ConfiguredProjectDependency
import modulecheck.project.McProject
import modulecheck.project.SourceSetDependency
import modulecheck.project.TransitiveProjectDependency
import modulecheck.project.toSourceSetDependency
import modulecheck.utils.flatMapToSet
import modulecheck.utils.mapAsync

Expand All @@ -39,22 +40,6 @@ class InheritedDependencyRule : ModuleCheckRule<InheritedDependencyFinding> {

override suspend fun check(project: McProject): List<InheritedDependencyFinding> {

data class SourceSetDependency(
val sourceSetName: SourceSetName,
val path: StringProjectPath,
val isTestFixture: Boolean
)

fun ConfiguredProjectDependency.toSourceSetDependency(
sourceSetName: SourceSetName = configurationName.toSourceSetName(),
path: StringProjectPath = this@toSourceSetDependency.path,
isTestFixture: Boolean = this@toSourceSetDependency.isTestFixture
) = SourceSetDependency(
sourceSetName = sourceSetName,
path = path,
isTestFixture = isTestFixture
)

// For each source set, the set of all module paths and whether they're test fixtures
val dependencyPathCache = mutableMapOf<SourceSetName, Set<SourceSetDependency>>()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,107 @@ class OverShotDependenciesTest : RunnerTest() {
)
}

// https://github.com/RBusarow/ModuleCheck/issues/520
@Test
fun `overshot as debugApi from invisible dependency with a visible unrelated debugApi project dependency`() {

val lib1 = kotlinProject(":lib1") {
addKotlinSource(
"""
package com.modulecheck.lib1

open class Lib1Class
""".trimIndent()
)
}

val lib2 = androidLibrary(":lib2", "com.modulecheck.lib2") {
// lib1 is added as a dependency, but it's not in the build file.
// This is intentional, because it mimics the behavior of a convention plugin
// which adds a dependency without any visible declaration in the build file
addDependency(ConfigurationName.api, lib1)

buildFile {
"""
plugins {
kotlin("jvm")
}

dependencies {
debugApi(project(path = ":lib4"))
}
"""
}
addKotlinSource(
"""
package com.modulecheck.lib2

import com.modulecheck.lib1.Lib1Class

val clazz = Lib1Class()
""".trimIndent(),
SourceSetName.DEBUG
)
}

run().isSuccess shouldBe false

lib2.buildFile shouldHaveText """
plugins {
kotlin("jvm")
}

dependencies {
debugApi(project(path = ":lib1"))
debugApi(project(path = ":lib4"))
}
"""

logger.parsedReport() shouldBe listOf(
":lib2" to listOf(
overshot(
fixed = true,
configuration = "debugApi",
dependency = ":lib1",
position = null
),
unusedDependency(
fixed = false,
configuration = "api",
dependency = ":lib1",
position = null
)
)
)

logger.clear()

// this second run should not have an overshot finding, and shouldn't modify the build file
run().isSuccess shouldBe false

lib2.buildFile shouldHaveText """
plugins {
kotlin("jvm")
}

dependencies {
debugApi(project(path = ":lib1"))
debugApi(project(path = ":lib4"))
}
"""

logger.parsedReport() shouldBe listOf(
":lib2" to listOf(
unusedDependency(
fixed = false,
configuration = "api",
dependency = ":lib1",
position = null
)
)
)
}

@Test
fun `overshot as testImplementation from invisible dependency with a visible unrelated implementation project dependency`() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package modulecheck.project

import modulecheck.parsing.gradle.ConfigurationName
import modulecheck.parsing.gradle.ProjectPath.StringProjectPath
import modulecheck.parsing.gradle.SourceSetName

data class ConfiguredProjectDependency(
Expand Down Expand Up @@ -77,3 +78,19 @@ data class DownstreamDependency(
val dependentProject: McProject,
val configuredProjectDependency: ConfiguredProjectDependency
)

data class SourceSetDependency(
val sourceSetName: SourceSetName,
val path: StringProjectPath,
val isTestFixture: Boolean
)

fun ConfiguredProjectDependency.toSourceSetDependency(
sourceSetName: SourceSetName = configurationName.toSourceSetName(),
path: StringProjectPath = this@toSourceSetDependency.path,
isTestFixture: Boolean = this@toSourceSetDependency.isTestFixture
) = SourceSetDependency(
sourceSetName = sourceSetName,
path = path,
isTestFixture = isTestFixture
)