Skip to content

Commit

Permalink
fix suppressing findings within the AGP DSL
Browse files Browse the repository at this point in the history
fixes #710
  • Loading branch information
RBusarow committed Jun 7, 2022
1 parent d10137d commit b39dc87
Show file tree
Hide file tree
Showing 29 changed files with 632 additions and 316 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,44 @@ class DisableAndroidResourcesTest : RunnerTest() {
logger.parsedReport() shouldBe listOf()
}

@Test
fun `unused resource generation should pass if suppressed`() {

val lib1 = androidLibrary(":lib1", "com.modulecheck.lib1") {
buildFile {
"""
plugins {
id("com.android.library")
kotlin("android")
}
android {
@Suppress("disable-android-resources")
buildFeatures.androidResources = true
}
"""
}
}

run(
autoCorrect = false
).isSuccess shouldBe true

lib1.buildFile shouldHaveText """
plugins {
id("com.android.library")
kotlin("android")
}
android {
@Suppress("disable-android-resources")
buildFeatures.androidResources = true
}
"""

logger.parsedReport() shouldBe listOf()
}

@Test
fun `unused resource generation should be ignored in dynamic-feature module`() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,44 @@ class DisableViewBindingTest : RunnerTest() {
logger.parsedReport() shouldBe listOf()
}

@Test
fun `unused ViewBinding should pass if suppressed`() {

val lib1 = androidLibrary(":lib1", "com.modulecheck.lib1") {
buildFile {
"""
plugins {
id("com.android.library")
kotlin("android")
}
android {
@Suppress("disable-view-binding")
buildFeatures.viewBinding = true
}
"""
}
}

run(
autoCorrect = false
).isSuccess shouldBe true

lib1.buildFile shouldHaveText """
plugins {
id("com.android.library")
kotlin("android")
}
android {
@Suppress("disable-view-binding")
buildFeatures.viewBinding = true
}
"""

logger.parsedReport() shouldBe listOf()
}

@Test
fun `unused ViewBinding without auto-correct should fail`() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ interface Problem :
Finding,
DependencyFinding {

/**
* Whether this Problem should be ignored. True if the associated statement is annotated with
* `@Suppress` and the corresponding finding ID.
*/
val isSuppressed: LazyDeferred<Boolean>
get() = lazyDeferred {
statementOrNull.await()?.suppressed
statementOrNull.await()
?.suppressed
?.contains(findingName.id)
?: false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,26 @@ import org.jetbrains.kotlin.util.suffixIfNot
import java.io.File

data class DisableViewBindingGenerationFinding(
override val findingName: FindingName,
override val dependentProject: McProject,
override val dependentPath: ProjectPath.StringProjectPath,
override val buildFile: File
) : Finding, Fixable {

override val findingName = NAME

override val message: String
get() = "Android viewBinding generation is enabled, but no generated code is being used."

override val dependencyIdentifier = ""

override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred { null }

override val statementTextOrNull: LazyDeferred<String?> = lazyDeferred {

override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred {
dependentProject.buildFileParser.androidSettings()
.assignments
.firstOrNull { it.propertyFullName == "viewBinding" }
?.declarationText
}

override val statementTextOrNull: LazyDeferred<String?> = lazyDeferred {
statementOrNull.await()?.declarationText
}

override val positionOrNull: LazyDeferred<Position?> = lazyDeferred {
Expand Down Expand Up @@ -139,4 +140,9 @@ data class DisableViewBindingGenerationFinding(

return buildFile.readText().replace(fullText, newBlockText)
}

companion object {
/** @suppress */
val NAME = FindingName("disable-view-binding")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,26 @@ import org.jetbrains.kotlin.util.suffixIfNot
import java.io.File

data class UnusedResourcesGenerationFinding(
override val findingName: FindingName,
override val dependentProject: McProject,
override val dependentPath: ProjectPath.StringProjectPath,
override val buildFile: File
) : Finding, Fixable {

override val findingName = NAME

override val message: String
get() = "`androidResources` generation is enabled, but no resources are defined in this module."

override val dependencyIdentifier = ""

override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred { null }

override val statementTextOrNull = lazyDeferred {

override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred {
dependentProject.buildFileParser.androidSettings()
.assignments
.firstOrNull { it.propertyFullName == "androidResources" }
?.declarationText
}

override val statementTextOrNull = lazyDeferred {
statementOrNull.await()?.declarationText
}

override val positionOrNull: LazyDeferred<Position?> = lazyDeferred {
Expand Down Expand Up @@ -140,4 +141,9 @@ data class UnusedResourcesGenerationFinding(

return buildFile.readText().replace(fullText, newBlockText)
}

companion object {
/** @suppress */
val NAME = FindingName("disable-android-resources")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ data class AndroidGradleSettings(
data class AndroidBlock(
override val fullText: String,
override val lambdaContent: String,
override val settings: List<Assignment>
override val settings: List<Assignment>,
override val blockSuppressed: List<String>
) : AgpBlock

data class BuildFeaturesBlock(
override val fullText: String,
override val lambdaContent: String,
override val settings: List<Assignment>
override val settings: List<Assignment>,
override val blockSuppressed: List<String>
) : AgpBlock
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ data class Assignment(
val value: String,
override val declarationText: String,
override val statementWithSurroundingText: String = declarationText,
override val suppressed: List<String> = emptyList()
override val suppressed: List<String>
) : BuildFileStatement
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ sealed interface Block<T> {
val fullText: String
val lambdaContent: String
val settings: List<T>

/** [FindingNames][modulecheck.finding.FindingName] which are suppressed at the block level */
val blockSuppressed: List<String>
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,28 @@ import modulecheck.utils.remove

abstract class AbstractDependenciesBlock(
private val logger: McLogger,
suppressedForEntireBlock: List<String>,
blockSuppressed: List<String>,
private val configurationNameTransform: ConfigurationNameTransform,
private val projectDependency: ProjectDependency.Factory
) : DependenciesBlock {

private val resetManager = ResetManager()

val suppressedForEntireBlock = suppressedForEntireBlock.updateOldSuppresses()
final override val blockSuppressed = blockSuppressed.updateOldSuppresses()

override val allSuppressions: Map<ProjectDependency, Set<FindingName>> by resetManager.lazyResets {
buildMap<ProjectDependency, MutableSet<FindingName>> {

allModuleDeclarations.forEach { (configuredModule, declarations) ->

val cached = getOrPut(configuredModule) {
suppressedForEntireBlock.mapTo(mutableSetOf()) { FindingName(it) }
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
}

declarations.forEach { moduleDependencyDeclaration ->

cached += moduleDependencyDeclaration.suppressed.updateOldSuppresses()
.plus(suppressedForEntireBlock)
.plus(blockSuppressed)
.asFindingNames()
}
}
Expand Down Expand Up @@ -95,7 +95,7 @@ abstract class AbstractDependenciesBlock(
moduleName = coordinates.moduleName,
version = coordinates.version,
coordinates = coordinates,
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
configurationNameTransform = configurationNameTransform
)
_allDeclarations.add(declaration)
Expand All @@ -116,7 +116,7 @@ abstract class AbstractDependenciesBlock(
configName = configName,
declarationText = parsedString,
statementWithSurroundingText = originalString,
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
configurationNameTransform = configurationNameTransform
)
_allDeclarations.add(declaration)
Expand Down Expand Up @@ -150,7 +150,7 @@ abstract class AbstractDependenciesBlock(
configName = configName,
declarationText = parsedString,
statementWithSurroundingText = originalString,
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
configurationNameTransform = configurationNameTransform
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import java.io.File

abstract class AbstractPluginsBlock(
private val logger: McLogger,
suppressedForEntireBlock: List<String>
blockSuppressed: List<String>
) : PluginsBlock {

private val resetManager = ResetManager()
Expand All @@ -40,19 +40,19 @@ abstract class AbstractPluginsBlock(

protected val allBlockStatements = mutableListOf<String>()

private val suppressedForEntireBlock = suppressedForEntireBlock.updateOldSuppresses()
override val blockSuppressed = blockSuppressed.updateOldSuppresses()

override val allSuppressions: Map<PluginDeclaration, Set<FindingName>> by resetManager.lazyResets {
buildMap<PluginDeclaration, MutableSet<FindingName>> {

_allDeclarations.forEach { pluginDeclaration ->

val cached = getOrPut(pluginDeclaration) {
suppressedForEntireBlock.mapTo(mutableSetOf()) { FindingName(it) }
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
}

cached += pluginDeclaration.suppressed.updateOldSuppresses()
.plus(suppressedForEntireBlock)
.plus(blockSuppressed)
.asFindingNames()
}
}
Expand All @@ -67,7 +67,7 @@ abstract class AbstractPluginsBlock(
val declaration = PluginDeclaration(
statementWithSurroundingText = originalString,
declarationText = parsedString,
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock
suppressed = suppressed.updateOldSuppresses() + blockSuppressed
)
_allDeclarations.add(declaration)
resetManager.resetAll()
Expand Down
Loading

0 comments on commit b39dc87

Please sign in to comment.