Skip to content

Commit 2f23f6f

Browse files
committed
fix suppressing findings within the AGP DSL
fixes #710
1 parent d10137d commit 2f23f6f

File tree

27 files changed

+586
-312
lines changed

27 files changed

+586
-312
lines changed

modulecheck-core/src/test/kotlin/modulecheck/core/DisableViewBindingTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,44 @@ class DisableViewBindingTest : RunnerTest() {
452452
logger.parsedReport() shouldBe listOf()
453453
}
454454

455+
@Test
456+
fun `unused ViewBinding should pass if suppressed`() {
457+
458+
val lib1 = androidLibrary(":lib1", "com.modulecheck.lib1") {
459+
buildFile {
460+
"""
461+
plugins {
462+
id("com.android.library")
463+
kotlin("android")
464+
}
465+
466+
android {
467+
@Suppress("disable-view-binding")
468+
buildFeatures.viewBinding = true
469+
}
470+
"""
471+
}
472+
}
473+
474+
run(
475+
autoCorrect = false
476+
).isSuccess shouldBe true
477+
478+
lib1.buildFile shouldHaveText """
479+
plugins {
480+
id("com.android.library")
481+
kotlin("android")
482+
}
483+
484+
android {
485+
@Suppress("disable-view-binding")
486+
buildFeatures.viewBinding = true
487+
}
488+
"""
489+
490+
logger.parsedReport() shouldBe listOf()
491+
}
492+
455493
@Test
456494
fun `unused ViewBinding without auto-correct should fail`() {
457495

modulecheck-finding/api/src/main/kotlin/modulecheck/finding/Problem.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@ interface Problem :
2424
Finding,
2525
DependencyFinding {
2626

27+
/**
28+
* Whether this Problem should be ignored. True if the associated statement is annotated with
29+
* `@Suppress` and the corresponding finding ID.
30+
*/
2731
val isSuppressed: LazyDeferred<Boolean>
2832
get() = lazyDeferred {
29-
statementOrNull.await()?.suppressed
33+
statementOrNull.await()
34+
?.suppressed
3035
?.contains(findingName.id)
3136
?: false
3237
}

modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/DisableViewBindingGenerationFinding.kt

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,26 @@ import org.jetbrains.kotlin.util.suffixIfNot
3333
import java.io.File
3434

3535
data class DisableViewBindingGenerationFinding(
36-
override val findingName: FindingName,
3736
override val dependentProject: McProject,
3837
override val dependentPath: ProjectPath.StringProjectPath,
3938
override val buildFile: File
4039
) : Finding, Fixable {
4140

41+
override val findingName = NAME
42+
4243
override val message: String
4344
get() = "Android viewBinding generation is enabled, but no generated code is being used."
4445

4546
override val dependencyIdentifier = ""
4647

47-
override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred { null }
48-
49-
override val statementTextOrNull: LazyDeferred<String?> = lazyDeferred {
50-
48+
override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred {
5149
dependentProject.buildFileParser.androidSettings()
5250
.assignments
5351
.firstOrNull { it.propertyFullName == "viewBinding" }
54-
?.declarationText
52+
}
53+
54+
override val statementTextOrNull: LazyDeferred<String?> = lazyDeferred {
55+
statementOrNull.await()?.declarationText
5556
}
5657

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

140141
return buildFile.readText().replace(fullText, newBlockText)
141142
}
143+
144+
companion object {
145+
/** @suppress */
146+
val NAME = FindingName("disable-view-binding")
147+
}
142148
}

modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/UnusedResourcesGenerationFinding.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ data class UnusedResourcesGenerationFinding(
4444

4545
override val dependencyIdentifier = ""
4646

47-
override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred { null }
48-
49-
override val statementTextOrNull = lazyDeferred {
50-
47+
override val statementOrNull: LazyDeferred<BuildFileStatement?> = lazyDeferred {
5148
dependentProject.buildFileParser.androidSettings()
5249
.assignments
5350
.firstOrNull { it.propertyFullName == "androidResources" }
54-
?.declarationText
51+
}
52+
53+
override val statementTextOrNull = lazyDeferred {
54+
statementOrNull.await()?.declarationText
5555
}
5656

5757
override val positionOrNull: LazyDeferred<Position?> = lazyDeferred {

modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/AndroidGradleSettings.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@ data class AndroidGradleSettings(
2727
data class AndroidBlock(
2828
override val fullText: String,
2929
override val lambdaContent: String,
30-
override val settings: List<Assignment>
30+
override val settings: List<Assignment>,
31+
override val blockSuppressed: List<String>
3132
) : AgpBlock
3233

3334
data class BuildFeaturesBlock(
3435
override val fullText: String,
3536
override val lambdaContent: String,
36-
override val settings: List<Assignment>
37+
override val settings: List<Assignment>,
38+
override val blockSuppressed: List<String>
3739
) : AgpBlock
3840
}
3941
}

modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Assignment.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,5 @@ data class Assignment(
2121
val value: String,
2222
override val declarationText: String,
2323
override val statementWithSurroundingText: String = declarationText,
24-
override val suppressed: List<String> = emptyList()
24+
override val suppressed: List<String>
2525
) : BuildFileStatement

modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Block.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,7 @@ sealed interface Block<T> {
1919
val fullText: String
2020
val lambdaContent: String
2121
val settings: List<T>
22+
23+
/** [FindingNames][modulecheck.finding.FindingName] which are suppressed at the block level */
24+
val blockSuppressed: List<String>
2225
}

modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractDependenciesBlock.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,28 @@ import modulecheck.utils.remove
3838

3939
abstract class AbstractDependenciesBlock(
4040
private val logger: McLogger,
41-
suppressedForEntireBlock: List<String>,
41+
blockSuppressed: List<String>,
4242
private val configurationNameTransform: ConfigurationNameTransform,
4343
private val projectDependency: ProjectDependency.Factory
4444
) : DependenciesBlock {
4545

4646
private val resetManager = ResetManager()
4747

48-
val suppressedForEntireBlock = suppressedForEntireBlock.updateOldSuppresses()
48+
final override val blockSuppressed = blockSuppressed.updateOldSuppresses()
4949

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

5353
allModuleDeclarations.forEach { (configuredModule, declarations) ->
5454

5555
val cached = getOrPut(configuredModule) {
56-
suppressedForEntireBlock.mapTo(mutableSetOf()) { FindingName(it) }
56+
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
5757
}
5858

5959
declarations.forEach { moduleDependencyDeclaration ->
6060

6161
cached += moduleDependencyDeclaration.suppressed.updateOldSuppresses()
62-
.plus(suppressedForEntireBlock)
62+
.plus(blockSuppressed)
6363
.asFindingNames()
6464
}
6565
}
@@ -95,7 +95,7 @@ abstract class AbstractDependenciesBlock(
9595
moduleName = coordinates.moduleName,
9696
version = coordinates.version,
9797
coordinates = coordinates,
98-
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
98+
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
9999
configurationNameTransform = configurationNameTransform
100100
)
101101
_allDeclarations.add(declaration)
@@ -116,7 +116,7 @@ abstract class AbstractDependenciesBlock(
116116
configName = configName,
117117
declarationText = parsedString,
118118
statementWithSurroundingText = originalString,
119-
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
119+
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
120120
configurationNameTransform = configurationNameTransform
121121
)
122122
_allDeclarations.add(declaration)
@@ -150,7 +150,7 @@ abstract class AbstractDependenciesBlock(
150150
configName = configName,
151151
declarationText = parsedString,
152152
statementWithSurroundingText = originalString,
153-
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock,
153+
suppressed = suppressed.updateOldSuppresses() + blockSuppressed,
154154
configurationNameTransform = configurationNameTransform
155155
)
156156

modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractPluginsBlock.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import java.io.File
2626

2727
abstract class AbstractPluginsBlock(
2828
private val logger: McLogger,
29-
suppressedForEntireBlock: List<String>
29+
blockSuppressed: List<String>
3030
) : PluginsBlock {
3131

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

4141
protected val allBlockStatements = mutableListOf<String>()
4242

43-
private val suppressedForEntireBlock = suppressedForEntireBlock.updateOldSuppresses()
43+
override val blockSuppressed = blockSuppressed.updateOldSuppresses()
4444

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

4848
_allDeclarations.forEach { pluginDeclaration ->
4949

5050
val cached = getOrPut(pluginDeclaration) {
51-
suppressedForEntireBlock.mapTo(mutableSetOf()) { FindingName(it) }
51+
blockSuppressed.mapTo(mutableSetOf()) { FindingName(it) }
5252
}
5353

5454
cached += pluginDeclaration.suppressed.updateOldSuppresses()
55-
.plus(suppressedForEntireBlock)
55+
.plus(blockSuppressed)
5656
.asFindingNames()
5757
}
5858
}
@@ -67,7 +67,7 @@ abstract class AbstractPluginsBlock(
6767
val declaration = PluginDeclaration(
6868
statementWithSurroundingText = originalString,
6969
declarationText = parsedString,
70-
suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock
70+
suppressed = suppressed.updateOldSuppresses() + blockSuppressed
7171
)
7272
_allDeclarations.add(declaration)
7373
resetManager.resetAll()

modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/EverythingPrinter.kt

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,54 +15,53 @@
1515

1616
package modulecheck.parsing.groovy.antlr
1717

18-
import groovyjarjarantlr4.v4.runtime.tree.ParseTree
19-
import groovyjarjarantlr4.v4.runtime.tree.RuleNode
20-
import org.apache.groovy.parser.antlr4.GroovyParser
21-
import org.apache.groovy.parser.antlr4.GroovyParserBaseVisitor
18+
import groovyjarjarantlr4.v4.runtime.ParserRuleContext
19+
import groovyjarjarantlr4.v4.runtime.RuleContext
20+
import modulecheck.utils.requireNotNull
2221

23-
internal class EverythingPrinter : GroovyParserBaseVisitor<Unit>() {
22+
internal fun ParserRuleContext.printEverything() {
2423

25-
private val dashes = "------------------------------------------------------------"
26-
27-
override fun visit(tree: ParseTree) {
28-
29-
val parent = tree.parent?.let { it::class.java.simpleName }
24+
val levels = mutableMapOf<RuleContext, Int>(this to 0)
25+
val dashes = "------------------------------------------------------------"
3026

27+
fun printMe(nodeSimpleName: String, parentName: String, nodeText: String, level: Int) {
3128
println(
32-
""" $dashes ${tree::class.java.simpleName} -- parent: $parent
33-
|
34-
|`${tree.text}`
29+
"""
30+
| $dashes $nodeSimpleName -- parent: $parentName
3531
|
32+
| `$nodeText`
3633
""".trimMargin()
34+
.lines()
35+
.let {
36+
it.dropLast(1) + it.last().replaceFirst(" ", "└─")
37+
}
38+
.joinToString("\n")
39+
.prependIndent("".repeat(level))
3740
)
38-
super.visit(tree)
3941
}
4042

41-
override fun visitChildren(node: RuleNode) {
43+
printMe(
44+
nodeSimpleName = javaClass.simpleName, parentName = "null", nodeText = text, level = 0
45+
)
4246

43-
val parent = node.parent?.let { it::class.java.simpleName }
47+
childrenOfTypeRecursive<ParserRuleContext>()
48+
.filterNot { it == this }
49+
.forEach { node ->
4450

45-
println(
46-
""" $dashes ${node::class.java.simpleName} -- parent: $parent
47-
|
48-
|`${node.text}`
49-
|
50-
""".trimMargin()
51-
)
52-
super.visitChildren(node)
53-
}
51+
val parent = node.parent.requireNotNull {
52+
"Parent is null for ${node.javaClass.simpleName}, but that's impossible?"
53+
}
5454

55-
override fun visitExpression(ctx: GroovyParser.ExpressionContext) {
55+
val parentLevel = levels.getValue(parent)
56+
levels[node] = parentLevel + 1
5657

57-
val parent = ctx.parent?.let { it::class.java.simpleName }
58+
val parentName = parent.javaClass.simpleName
5859

59-
println(
60-
""" $dashes ${ctx::class.java.simpleName} -- parent: $parent
61-
|
62-
|`${ctx.text}`
63-
|
64-
""".trimMargin()
65-
)
66-
super.visitExpression(ctx)
67-
}
60+
printMe(
61+
nodeSimpleName = node.javaClass.simpleName,
62+
parentName = parentName,
63+
nodeText = node.text,
64+
level = parentLevel + 1
65+
)
66+
}
6867
}

0 commit comments

Comments
 (0)