From f51c1d744b6e5c767ef5c59e852bef7bd40b5538 Mon Sep 17 00:00:00 2001 From: Rick Busarow Date: Tue, 7 Jun 2022 11:21:14 -0500 Subject: [PATCH] fix suppressing findings within the AGP DSL fixes #710 --- .../core/DisableAndroidResourcesTest.kt | 38 +++ .../core/DisableViewBindingTest.kt | 38 +++ .../kotlin/modulecheck/finding/Problem.kt | 7 +- .../DisableViewBindingGenerationFinding.kt | 18 +- .../UnusedResourcesGenerationFinding.kt | 18 +- .../gradle/dsl/AndroidGradleSettings.kt | 6 +- .../parsing/gradle/dsl/Assignment.kt | 2 +- .../modulecheck/parsing/gradle/dsl/Block.kt | 3 + .../dsl/internal/AbstractDependenciesBlock.kt | 14 +- .../dsl/internal/AbstractPluginsBlock.kt | 10 +- .../parsing/groovy/antlr/EverythingPrinter.kt | 71 ++-- .../groovy/antlr/GroovyAndroidGradleParser.kt | 100 ++++-- .../groovy/antlr/GroovyDependenciesBlock.kt | 4 +- .../antlr/GroovyDependenciesBlockParser.kt | 2 +- .../groovy/antlr/GroovyPluginsBlock.kt | 4 +- .../groovy/antlr/GroovyPluginsBlockParser.kt | 2 +- .../parsing/groovy/antlr/extensions.kt | 96 +++++- .../antlr/GroovyAndroidGradleParserTest.kt | 111 +++--- .../antlr/GroovyDependencyBlockParserTest.kt | 4 +- .../parsing/psi/KotlinAndroidGradleParser.kt | 47 ++- .../parsing/psi/KotlinDependenciesBlock.kt | 4 +- .../psi/KotlinDependenciesBlockParser.kt | 7 +- .../parsing/psi/KotlinPluginsBlock.kt | 7 +- .../parsing/psi/KotlinPluginsBlockParser.kt | 2 +- .../psi/KotlinAndroidGradleParserTest.kt | 319 ++++++++++-------- .../reporting/graphviz/GraphvizFileWriter.kt | 4 +- .../rule/impl/DisableAndroidResourcesRule.kt | 4 +- .../rule/impl/DisableViewBindingRule.kt | 4 +- .../main/kotlin/modulecheck/utils/string.kt | 2 +- 29 files changed, 632 insertions(+), 316 deletions(-) diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/DisableAndroidResourcesTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/DisableAndroidResourcesTest.kt index 4faf3e46aa..3b30615f44 100644 --- a/modulecheck-core/src/test/kotlin/modulecheck/core/DisableAndroidResourcesTest.kt +++ b/modulecheck-core/src/test/kotlin/modulecheck/core/DisableAndroidResourcesTest.kt @@ -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`() { diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/DisableViewBindingTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/DisableViewBindingTest.kt index 2b0abc1cac..c5497006f8 100644 --- a/modulecheck-core/src/test/kotlin/modulecheck/core/DisableViewBindingTest.kt +++ b/modulecheck-core/src/test/kotlin/modulecheck/core/DisableViewBindingTest.kt @@ -531,6 +531,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`() { diff --git a/modulecheck-finding/api/src/main/kotlin/modulecheck/finding/Problem.kt b/modulecheck-finding/api/src/main/kotlin/modulecheck/finding/Problem.kt index 7b3602256d..ba3a7b0bd3 100644 --- a/modulecheck-finding/api/src/main/kotlin/modulecheck/finding/Problem.kt +++ b/modulecheck-finding/api/src/main/kotlin/modulecheck/finding/Problem.kt @@ -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 get() = lazyDeferred { - statementOrNull.await()?.suppressed + statementOrNull.await() + ?.suppressed ?.contains(findingName.id) ?: false } diff --git a/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/DisableViewBindingGenerationFinding.kt b/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/DisableViewBindingGenerationFinding.kt index 745d055e53..b4e61ef160 100644 --- a/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/DisableViewBindingGenerationFinding.kt +++ b/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/DisableViewBindingGenerationFinding.kt @@ -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 = lazyDeferred { null } - - override val statementTextOrNull: LazyDeferred = lazyDeferred { - + override val statementOrNull: LazyDeferred = lazyDeferred { dependentProject.buildFileParser.androidSettings() .assignments .firstOrNull { it.propertyFullName == "viewBinding" } - ?.declarationText + } + + override val statementTextOrNull: LazyDeferred = lazyDeferred { + statementOrNull.await()?.declarationText } override val positionOrNull: LazyDeferred = lazyDeferred { @@ -139,4 +140,9 @@ data class DisableViewBindingGenerationFinding( return buildFile.readText().replace(fullText, newBlockText) } + + companion object { + /** @suppress */ + val NAME = FindingName("disable-view-binding") + } } diff --git a/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/UnusedResourcesGenerationFinding.kt b/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/UnusedResourcesGenerationFinding.kt index f0de2b4305..40bfc5758f 100644 --- a/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/UnusedResourcesGenerationFinding.kt +++ b/modulecheck-finding/impl-android/src/main/kotlin/modulecheck/finding/android/UnusedResourcesGenerationFinding.kt @@ -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 = lazyDeferred { null } - - override val statementTextOrNull = lazyDeferred { - + override val statementOrNull: LazyDeferred = lazyDeferred { dependentProject.buildFileParser.androidSettings() .assignments .firstOrNull { it.propertyFullName == "androidResources" } - ?.declarationText + } + + override val statementTextOrNull = lazyDeferred { + statementOrNull.await()?.declarationText } override val positionOrNull: LazyDeferred = lazyDeferred { @@ -140,4 +141,9 @@ data class UnusedResourcesGenerationFinding( return buildFile.readText().replace(fullText, newBlockText) } + + companion object { + /** @suppress */ + val NAME = FindingName("disable-android-resources") + } } diff --git a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/AndroidGradleSettings.kt b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/AndroidGradleSettings.kt index 9b0a704d79..2d505e83f0 100644 --- a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/AndroidGradleSettings.kt +++ b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/AndroidGradleSettings.kt @@ -27,13 +27,15 @@ data class AndroidGradleSettings( data class AndroidBlock( override val fullText: String, override val lambdaContent: String, - override val settings: List + override val settings: List, + override val blockSuppressed: List ) : AgpBlock data class BuildFeaturesBlock( override val fullText: String, override val lambdaContent: String, - override val settings: List + override val settings: List, + override val blockSuppressed: List ) : AgpBlock } } diff --git a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Assignment.kt b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Assignment.kt index f6838bd8b3..caf7699e05 100644 --- a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Assignment.kt +++ b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Assignment.kt @@ -21,5 +21,5 @@ data class Assignment( val value: String, override val declarationText: String, override val statementWithSurroundingText: String = declarationText, - override val suppressed: List = emptyList() + override val suppressed: List ) : BuildFileStatement diff --git a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Block.kt b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Block.kt index fd035eaaa3..09f9fd8ee1 100644 --- a/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Block.kt +++ b/modulecheck-parsing/gradle/dsl/api/src/main/kotlin/modulecheck/parsing/gradle/dsl/Block.kt @@ -19,4 +19,7 @@ sealed interface Block { val fullText: String val lambdaContent: String val settings: List + + /** [FindingNames][modulecheck.finding.FindingName] which are suppressed at the block level */ + val blockSuppressed: List } diff --git a/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractDependenciesBlock.kt b/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractDependenciesBlock.kt index 1c000dde12..57c6cecc32 100644 --- a/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractDependenciesBlock.kt +++ b/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractDependenciesBlock.kt @@ -38,14 +38,14 @@ import modulecheck.utils.remove abstract class AbstractDependenciesBlock( private val logger: McLogger, - suppressedForEntireBlock: List, + blockSuppressed: List, 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> by resetManager.lazyResets { buildMap> { @@ -53,13 +53,13 @@ abstract class AbstractDependenciesBlock( 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() } } @@ -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) @@ -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) @@ -150,7 +150,7 @@ abstract class AbstractDependenciesBlock( configName = configName, declarationText = parsedString, statementWithSurroundingText = originalString, - suppressed = suppressed.updateOldSuppresses() + suppressedForEntireBlock, + suppressed = suppressed.updateOldSuppresses() + blockSuppressed, configurationNameTransform = configurationNameTransform ) diff --git a/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractPluginsBlock.kt b/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractPluginsBlock.kt index 92c164eb51..a2d085d236 100644 --- a/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractPluginsBlock.kt +++ b/modulecheck-parsing/gradle/dsl/internal/src/main/kotlin/modulecheck/parsing/gradle/dsl/internal/AbstractPluginsBlock.kt @@ -26,7 +26,7 @@ import java.io.File abstract class AbstractPluginsBlock( private val logger: McLogger, - suppressedForEntireBlock: List + blockSuppressed: List ) : PluginsBlock { private val resetManager = ResetManager() @@ -40,7 +40,7 @@ abstract class AbstractPluginsBlock( protected val allBlockStatements = mutableListOf() - private val suppressedForEntireBlock = suppressedForEntireBlock.updateOldSuppresses() + override val blockSuppressed = blockSuppressed.updateOldSuppresses() override val allSuppressions: Map> by resetManager.lazyResets { buildMap> { @@ -48,11 +48,11 @@ abstract class AbstractPluginsBlock( _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() } } @@ -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() diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/EverythingPrinter.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/EverythingPrinter.kt index 02d3b37dc4..6e1ff29a88 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/EverythingPrinter.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/EverythingPrinter.kt @@ -15,54 +15,53 @@ package modulecheck.parsing.groovy.antlr -import groovyjarjarantlr4.v4.runtime.tree.ParseTree -import groovyjarjarantlr4.v4.runtime.tree.RuleNode -import org.apache.groovy.parser.antlr4.GroovyParser -import org.apache.groovy.parser.antlr4.GroovyParserBaseVisitor +import groovyjarjarantlr4.v4.runtime.ParserRuleContext +import groovyjarjarantlr4.v4.runtime.RuleContext +import modulecheck.utils.requireNotNull -internal class EverythingPrinter : GroovyParserBaseVisitor() { +internal fun ParserRuleContext.printEverything() { - private val dashes = "------------------------------------------------------------" - - override fun visit(tree: ParseTree) { - - val parent = tree.parent?.let { it::class.java.simpleName } + val levels = mutableMapOf(this to 0) + val dashes = "------------------------------------------------------------" + fun printNode(nodeSimpleName: String, parentName: String, nodeText: String, level: Int) { println( - """ $dashes ${tree::class.java.simpleName} -- parent: $parent - | - |`${tree.text}` + """ + | $dashes $nodeSimpleName -- parent: $parentName | + | `$nodeText` """.trimMargin() + .lines() + .let { + it.dropLast(1) + it.last().replaceFirst(" ", "└─") + } + .joinToString("\n") + .prependIndent("│ ".repeat(level)) ) - super.visit(tree) } - override fun visitChildren(node: RuleNode) { + printNode( + nodeSimpleName = javaClass.simpleName, parentName = "null", nodeText = text, level = 0 + ) - val parent = node.parent?.let { it::class.java.simpleName } + childrenOfTypeRecursive() + .filterNot { it == this } + .forEach { node -> - println( - """ $dashes ${node::class.java.simpleName} -- parent: $parent - | - |`${node.text}` - | - """.trimMargin() - ) - super.visitChildren(node) - } + val parent = node.parent.requireNotNull { + "Parent is null for ${node.javaClass.simpleName}, but that's impossible?" + } - override fun visitExpression(ctx: GroovyParser.ExpressionContext) { + val parentLevel = levels.getValue(parent) + levels[node] = parentLevel + 1 - val parent = ctx.parent?.let { it::class.java.simpleName } + val parentName = parent.javaClass.simpleName - println( - """ $dashes ${ctx::class.java.simpleName} -- parent: $parent - | - |`${ctx.text}` - | - """.trimMargin() - ) - super.visitExpression(ctx) - } + printNode( + nodeSimpleName = node.javaClass.simpleName, + parentName = parentName, + nodeText = node.text, + level = parentLevel + 1 + ) + } } diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParser.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParser.kt index 623c75ce44..30a0403435 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParser.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParser.kt @@ -21,6 +21,7 @@ import modulecheck.parsing.gradle.dsl.AndroidGradleSettings import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.AndroidBlock import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.BuildFeaturesBlock import modulecheck.parsing.gradle.dsl.Assignment +import modulecheck.utils.prefixIfNot import modulecheck.utils.requireNotNull import org.apache.groovy.parser.antlr4.GroovyParser.AssignmentExprAltContext import org.apache.groovy.parser.antlr4.GroovyParser.ClosureOrLambdaExpressionContext @@ -28,6 +29,7 @@ import org.apache.groovy.parser.antlr4.GroovyParser.NamePartContext import org.apache.groovy.parser.antlr4.GroovyParser.PathElementContext import org.apache.groovy.parser.antlr4.GroovyParser.PathExpressionContext import org.apache.groovy.parser.antlr4.GroovyParser.PostfixExpressionContext +import org.apache.groovy.parser.antlr4.GroovyParserBaseVisitor import java.io.File import javax.inject.Inject @@ -41,8 +43,15 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { val allAssignments = mutableListOf() val buildFeaturesAssignments = mutableListOf() - fun List.mapAssignments(fullText: String) = - map { assignmentExpression -> + class AssignmentVisitor( + private val fullText: String, + private val blockSuppressed: List + ) : GroovyParserBaseVisitor() { + + val assignments = mutableListOf() + + override fun visitAssignmentExprAlt(assignmentExpression: AssignmentExprAltContext) { + super.visitAssignmentExprAlt(assignmentExpression) val assignmentText = assignmentExpression.originalText() @@ -59,27 +68,47 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { .last() .originalText() - Assignment( - fullText = fullText, - propertyFullName = propertyName, - value = valueText, - declarationText = assignmentText + val allSuppressed = blockSuppressed.plus( + assignmentExpression.precedingCommentNodeOrNull() + ?.originalText() + ?.suppressions() + .orEmpty() + ) + .distinct() + + assignments.add( + Assignment( + fullText = fullText, + propertyFullName = propertyName, + value = valueText, + declarationText = assignmentText, + suppressed = allSuppressed + ) ) } + } - val commandVisitor = commandExpressionVisitor(true) { ctx -> + val visitor = commandExpressionVisitor(true) { ctx -> val android = ctx.takeIf { it.isNamed("android") } ?: return@commandExpressionVisitor + val precedingCommentOrEmpty = android.precedingCommentNodeOrNull()?.originalText() ?: "" + + val androidSuppressed = precedingCommentOrEmpty.suppressions() + val androidLambda = android.lambdaBlock() val androidIsBlock = androidLambda != null - val androidAssignments = android.childrenOfTypeRecursive() - .mapAssignments(ctx.originalText()) + val androidBlockVisitor = AssignmentVisitor( + ctx.originalText().prefixIfNot(precedingCommentOrEmpty).trim(), + androidSuppressed + ) + + android.accept(androidBlockVisitor) - allAssignments += androidAssignments + allAssignments += androidBlockVisitor.assignments if (androidIsBlock) { @@ -87,16 +116,32 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { ?.originalText() .requireNotNull() - androidBlocks.add(AndroidBlock(android.originalText(), lambdaContent, androidAssignments)) + androidBlocks.add( + AndroidBlock( + fullText = android.originalText(), + lambdaContent = lambdaContent, + settings = androidBlockVisitor.assignments, + blockSuppressed = androidSuppressed + ) + ) android.accept( commandExpressionVisitor { buildFeatures -> if (buildFeatures.isNamed("buildFeatures")) { - val assignments = buildFeatures.childrenOfTypeRecursive() - .mapAssignments(ctx.originalText()) + val blockSuppressed = androidSuppressed.plus( + buildFeatures.precedingCommentNodeOrNull() + ?.originalText() + ?.suppressions() + .orEmpty() + ) + .distinct() + + val blockStatementVisitor = AssignmentVisitor(ctx.originalText(), blockSuppressed) - buildFeaturesAssignments += assignments + buildFeatures.accept(blockStatementVisitor) + + buildFeaturesAssignments += blockStatementVisitor.assignments buildFeatures.lambdaBlock() ?.closureContent() @@ -107,7 +152,8 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { BuildFeaturesBlock( fullText = android.originalText(), lambdaContent = buildFeaturesLambda, - settings = assignments + settings = blockStatementVisitor.assignments, + blockSuppressed = blockSuppressed ) ) } @@ -119,10 +165,20 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { pathExpressionVisitor { buildFeatures -> if (buildFeatures.isNamed("buildFeatures")) { - val assignments = buildFeatures.childrenOfTypeRecursive() - .mapAssignments(ctx.originalText()) - buildFeaturesAssignments += assignments + val blockSuppressed = androidSuppressed.plus( + buildFeatures.precedingCommentNodeOrNull() + ?.originalText() + ?.suppressions() + .orEmpty() + ) + .distinct() + + val blockStatementVisitor = AssignmentVisitor(ctx.originalText(), blockSuppressed) + + buildFeatures.accept(blockStatementVisitor) + + buildFeaturesAssignments += blockStatementVisitor.assignments val buildFeaturesLambdaContent = buildFeatures.pathElement() ?.lastOrNull() @@ -131,11 +187,13 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { ?.originalText() if (buildFeaturesLambdaContent != null) { + buildFeaturesBlocks.add( BuildFeaturesBlock( fullText = android.originalText(), lambdaContent = buildFeaturesLambdaContent, - settings = assignments + settings = blockStatementVisitor.assignments, + blockSuppressed = blockSuppressed ) ) } @@ -145,7 +203,7 @@ class GroovyAndroidGradleParser @Inject constructor() : AndroidGradleParser { } } - parser.accept(commandVisitor) + parser.accept(visitor) return AndroidGradleSettings( assignments = allAssignments, diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlock.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlock.kt index ce6c9397d9..0fc650c527 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlock.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlock.kt @@ -23,11 +23,11 @@ class GroovyDependenciesBlock internal constructor( logger: McLogger, override val fullText: String, override val lambdaContent: String, - suppressAll: List, + blockSuppressed: List, projectDependency: ProjectDependency.Factory ) : AbstractDependenciesBlock( logger = logger, - suppressedForEntireBlock = suppressAll, + blockSuppressed = blockSuppressed, configurationNameTransform = { it.value }, projectDependency = projectDependency ) { diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlockParser.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlockParser.kt index bc953e0fff..9777b74878 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlockParser.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependenciesBlockParser.kt @@ -173,7 +173,7 @@ class GroovyDependenciesBlockParser @Inject constructor( logger = logger, fullText = statement.originalText(), lambdaContent = blockBody, - suppressAll = blockSuppressed, + blockSuppressed = blockSuppressed, projectDependency = projectDependency ) diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlock.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlock.kt index 3d39ddfef5..a5ed3530b8 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlock.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlock.kt @@ -22,10 +22,10 @@ class GroovyPluginsBlock( logger: McLogger, override val fullText: String, override val lambdaContent: String, - suppressedForEntireBlock: List + blockSuppressed: List ) : AbstractPluginsBlock( logger = logger, - suppressedForEntireBlock = suppressedForEntireBlock + blockSuppressed = blockSuppressed ) { override fun findOriginalStringIndex(parsedString: String) = originalLines diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlockParser.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlockParser.kt index 7f4217f4ab..e4cc5d9f80 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlockParser.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/GroovyPluginsBlockParser.kt @@ -70,7 +70,7 @@ class GroovyPluginsBlockParser @Inject constructor( logger = logger, fullText = statement.originalText(), lambdaContent = blockBody, - suppressedForEntireBlock = blockSuppressed + blockSuppressed = blockSuppressed ) val blockStatementVisitor = object : GroovyParserBaseVisitor() { diff --git a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/extensions.kt b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/extensions.kt index 34bca15051..e349ae26f0 100644 --- a/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/extensions.kt +++ b/modulecheck-parsing/groovy-antlr/src/main/kotlin/modulecheck/parsing/groovy/antlr/extensions.kt @@ -19,19 +19,28 @@ import groovyjarjarantlr4.v4.runtime.CharStream import groovyjarjarantlr4.v4.runtime.ParserRuleContext import groovyjarjarantlr4.v4.runtime.RuleContext import groovyjarjarantlr4.v4.runtime.misc.Interval +import groovyjarjarantlr4.v4.runtime.tree.RuleNode import org.apache.groovy.parser.antlr4.GroovyParser.AssignmentExprAltContext +import org.apache.groovy.parser.antlr4.GroovyParser.BlockStatementContext import org.apache.groovy.parser.antlr4.GroovyParser.BlockStatementsContext import org.apache.groovy.parser.antlr4.GroovyParser.BlockStatementsOptContext import org.apache.groovy.parser.antlr4.GroovyParser.ClosureContext import org.apache.groovy.parser.antlr4.GroovyParser.ClosureOrLambdaExpressionContext import org.apache.groovy.parser.antlr4.GroovyParser.CommandExpressionContext +import org.apache.groovy.parser.antlr4.GroovyParser.GroovyParserRuleContext import org.apache.groovy.parser.antlr4.GroovyParser.IdentifierPrmrAltContext import org.apache.groovy.parser.antlr4.GroovyParser.NamePartContext +import org.apache.groovy.parser.antlr4.GroovyParser.NlsContext import org.apache.groovy.parser.antlr4.GroovyParser.PathElementContext import org.apache.groovy.parser.antlr4.GroovyParser.PathExpressionContext import org.apache.groovy.parser.antlr4.GroovyParser.PostfixExprAltContext import org.apache.groovy.parser.antlr4.GroovyParser.PostfixExpressionContext +import org.apache.groovy.parser.antlr4.GroovyParser.ScriptStatementContext +import org.apache.groovy.parser.antlr4.GroovyParser.ScriptStatementsContext +import org.apache.groovy.parser.antlr4.GroovyParser.SepContext import org.apache.groovy.parser.antlr4.GroovyParserBaseVisitor +import kotlin.reflect.KClass +import kotlin.reflect.safeCast internal inline fun RuleContext.parentOfType(): T? { return generateSequence(this as? RuleContext?) { it.parent } @@ -39,22 +48,71 @@ internal inline fun RuleContext.parentOfType(): T? { .firstOrNull() } -internal inline fun ParserRuleContext.childrenOfTypeRecursive(): List { - return generateSequence(sequenceOf(this)) { parserRuleContexts -> - parserRuleContexts.mapNotNull { it.children } - .flatten() - .filterIsInstance() - .takeIf { it.firstOrNull() != null } - } - .flatten() - .filterIsInstance() - .toList() +internal inline fun ParserRuleContext.childrenOfTypeRecursive(): Sequence { + return childrenOfTypeRecursive(T::class) +} + +internal fun ParserRuleContext.childrenOfTypeRecursive( + clazz: KClass +): Sequence { + + val childrenSequence = children + .orEmpty() + .asSequence() + .flatMap { child -> + when (child) { + is ParserRuleContext -> child.childrenOfTypeRecursive(clazz) + else -> sequenceOf(child) + } + } + + return sequenceOf(this) + .plus(childrenSequence) + .mapNotNull { clazz.safeCast(it) } } internal inline fun ParserRuleContext.childrenOfType(): List { return children.filterIsInstance() } +internal inline fun RuleNode.previousSibling(): T? { + + val siblings = (parent as? ParserRuleContext)?.children + ?.takeIf { it.count() > 1 } + ?: return null + + val indexInSiblings = siblings.indexOf(this).takeIf { it >= 1 } ?: return null + + return siblings[indexInSiblings - 1] as? T +} + +internal fun RuleNode.precedingCommentNodeOrNull(): GroovyParserRuleContext? { + + // First, try going all the way up to the parent scope -- top-level or the entire lambda content + // This works if this receiver is the first statement within that scope + val asBlockOrScriptStatement = this as? BlockStatementContext + ?: this as? ScriptStatementsContext + ?: (this as? RuleContext)?.parentOfType() + ?: (this as? RuleContext)?.parentOfType() + ?: return null + + val scopeAttempt = asBlockOrScriptStatement.previousSibling() + ?: asBlockOrScriptStatement.previousSibling() + + if (scopeAttempt != null) return scopeAttempt + + // If the first "scoped" attempt didn't work, then try whatever the hell this is. Multiple + // statements inside a lambda get weirdly stacked and grouped. Sometimes there are several + // statements in one node with a parent of `BlockStatementsOptContext`, and then a preceding + // comment is a sibling with that same parent. Even if there's another comment inside that + // grouped node. + val asBlockStatementsOpt = asBlockOrScriptStatement.parentOfType() + ?: return null + + return asBlockStatementsOpt.previousSibling() + ?: asBlockStatementsOpt.previousSibling() +} + internal inline fun ParserRuleContext.childOfType(): T? { return childrenOfType().let { ts -> @@ -73,7 +131,12 @@ internal inline fun ParserRuleContext.childOfType(): T } internal fun ParserRuleContext.originalText(stream: CharStream): String { - return stream.getText(Interval(start.startIndex, stop.stopIndex)) + return if (start == null || stop == null) { + // Weird edge cases around the very top and bottom of the parse tree + "" + } else { + stream.getText(Interval(start.startIndex, stop.stopIndex)) + } } internal fun CommandExpressionContext.pathExpression(): PathExpressionContext? { @@ -151,3 +214,14 @@ internal inline fun pathExpressionVisitor( } } } + +internal fun String.suppressions(): List { + return GroovyDependenciesBlockParser.NO_INSPECTION_REGEX + .findAll(this) + .map { it.destructured.component1() } + .toList() + .joinToString(",") + .split(",") + .map { it.trim() } + .filter { it.isNotEmpty() } +} diff --git a/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParserTest.kt b/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParserTest.kt index 7384db24a2..cb437adb18 100644 --- a/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParserTest.kt +++ b/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyAndroidGradleParserTest.kt @@ -15,30 +15,20 @@ package modulecheck.parsing.groovy.antlr -import hermit.test.junit.HermitJUnit5 -import io.kotest.matchers.shouldBe import modulecheck.parsing.gradle.dsl.AndroidGradleSettings import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.AndroidBlock import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.BuildFeaturesBlock import modulecheck.parsing.gradle.dsl.Assignment -import modulecheck.reporting.logging.PrintLogger -import modulecheck.testing.tempFile -import org.junit.jupiter.api.BeforeEach +import modulecheck.testing.BaseTest +import modulecheck.utils.child +import modulecheck.utils.createSafely import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.TestFactory -import org.junit.jupiter.api.TestInfo -internal class GroovyAndroidGradleParserTest : HermitJUnit5() { +internal class GroovyAndroidGradleParserTest : BaseTest() { - val logger by resets { PrintLogger() } - - val testFile by tempFile() - - private var testInfo: TestInfo? = null - - @BeforeEach - fun beforeEach(testInfo: TestInfo) { - this.testInfo = testInfo + val testFile by resets { + testProjectDir.child("build.gradle").createSafely() } @TestFactory @@ -73,7 +63,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf() ) val buildConfigAssignment = Assignment( fullText = "android {\n" + @@ -86,7 +77,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "buildConfig", value = "$enabled", - declarationText = "buildConfig = $enabled" + declarationText = "buildConfig = $enabled", + suppressed = listOf() ) val androidResourcesAssignment = Assignment( fullText = "android {\n" + @@ -96,8 +88,10 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "androidResources", value = "$enabled", - declarationText = "androidResources = $enabled" + declarationText = "androidResources = $enabled", + suppressed = listOf() ) + GroovyAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( assignments = listOf( viewBindingAssignment, @@ -123,7 +117,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { settings = listOf( viewBindingAssignment, buildConfigAssignment - ) + ), + blockSuppressed = listOf() ), AndroidBlock( fullText = "android {\n" + @@ -136,7 +131,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { " }\n", settings = listOf( androidResourcesAssignment - ) + ), + blockSuppressed = listOf() ) ), buildFeaturesBlocks = listOf( @@ -150,7 +146,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { " }\n" + "}", lambdaContent = "viewBinding = $enabled\n", - settings = listOf(viewBindingAssignment) + settings = listOf(viewBindingAssignment), + blockSuppressed = listOf() ), BuildFeaturesBlock( fullText = "android {\n" + @@ -162,7 +159,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { " }\n" + "}", lambdaContent = "buildConfig = $enabled\n", - settings = listOf(buildConfigAssignment) + settings = listOf(buildConfigAssignment), + blockSuppressed = listOf() ), BuildFeaturesBlock( fullText = "android {\n" + @@ -171,7 +169,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { " }\n" + "}", lambdaContent = "androidResources = $enabled\n", - settings = listOf(androidResourcesAssignment) + settings = listOf(androidResourcesAssignment), + blockSuppressed = listOf() ) ) ) @@ -202,7 +201,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf() ), Assignment( fullText = "android {\n" + @@ -213,7 +213,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "androidResources", value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" + declarationText = "androidResources = ${!enabled}", + suppressed = listOf() ) ), androidBlocks = listOf( @@ -238,7 +239,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf() ), Assignment( fullText = "android {\n" + @@ -249,9 +251,11 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "androidResources", value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" + declarationText = "androidResources = ${!enabled}", + suppressed = listOf() ) - ) + ), + blockSuppressed = listOf() ) ), buildFeaturesBlocks = listOf( @@ -274,7 +278,8 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf() ), Assignment( fullText = "android {\n" + @@ -285,9 +290,11 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { "}", propertyFullName = "androidResources", value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" + declarationText = "androidResources = ${!enabled}", + suppressed = listOf() ) - ) + ), + blockSuppressed = listOf() ) ) ) @@ -297,6 +304,10 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { fun `fully dot qualified boolean property`() = runTest { enabled -> val block = """ + plugins { + id 'my-plugin' + } + //noinspection android-level android.buildFeatures.androidResources = $enabled """.trimIndent() @@ -305,10 +316,11 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { GroovyAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( assignments = listOf( Assignment( - fullText = "android.buildFeatures.androidResources = $enabled", + fullText = "//noinspection android-level\nandroid.buildFeatures.androidResources = $enabled", propertyFullName = "androidResources", value = "$enabled", - declarationText = "android.buildFeatures.androidResources = $enabled" + declarationText = "android.buildFeatures.androidResources = $enabled", + suppressed = listOf("android-level") ) ), androidBlocks = listOf(), @@ -321,6 +333,7 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { val block = """ android.buildFeatures { + //noinspection assignment-level viewBinding = $enabled } """.trimIndent() @@ -331,30 +344,36 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { assignments = listOf( Assignment( fullText = "android.buildFeatures {\n" + + " //noinspection assignment-level\n" + " viewBinding = $enabled\n" + "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf("assignment-level") ) ), androidBlocks = listOf(), buildFeaturesBlocks = listOf( BuildFeaturesBlock( fullText = "android.buildFeatures {\n" + + " //noinspection assignment-level\n" + " viewBinding = $enabled\n" + "}", lambdaContent = "viewBinding = $enabled\n", settings = listOf( Assignment( fullText = "android.buildFeatures {\n" + + " //noinspection assignment-level\n" + " viewBinding = $enabled\n" + "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + suppressed = listOf("assignment-level") ) - ) + ), + blockSuppressed = listOf() ) ) ) @@ -365,6 +384,7 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { val block = """ android { + //noinspection buildFeatures-level buildFeatures.viewBinding = $enabled } """.trimIndent() @@ -375,29 +395,35 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { assignments = listOf( Assignment( fullText = "android {\n" + + " //noinspection buildFeatures-level\n" + " buildFeatures.viewBinding = $enabled\n" + "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "buildFeatures.viewBinding = $enabled" + declarationText = "buildFeatures.viewBinding = $enabled", + suppressed = listOf("buildFeatures-level") ) ), androidBlocks = listOf( AndroidBlock( fullText = "android {\n" + + " //noinspection buildFeatures-level\n" + " buildFeatures.viewBinding = $enabled\n" + "}", lambdaContent = "buildFeatures.viewBinding = $enabled\n", settings = listOf( Assignment( fullText = "android {\n" + + " //noinspection buildFeatures-level\n" + " buildFeatures.viewBinding = $enabled\n" + "}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "buildFeatures.viewBinding = $enabled" + declarationText = "buildFeatures.viewBinding = $enabled", + suppressed = listOf("buildFeatures-level") ) - ) + ), + blockSuppressed = listOf() ) ), buildFeaturesBlocks = listOf() @@ -409,10 +435,7 @@ internal class GroovyAndroidGradleParserTest : HermitJUnit5() { val paramsString = " -- enabled: $enabled" - val name = ( - testInfo?.displayName?.replace("()", "") - ?: "???" - ) + paramsString + val name = "${testDisplayName.replace("()", "")}$paramsString" DynamicTest.dynamicTest(name) { block(enabled) diff --git a/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependencyBlockParserTest.kt b/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependencyBlockParserTest.kt index e00bd662be..981f731128 100644 --- a/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependencyBlockParserTest.kt +++ b/modulecheck-parsing/groovy-antlr/src/test/kotlin/modulecheck/parsing/groovy/antlr/GroovyDependencyBlockParserTest.kt @@ -181,7 +181,7 @@ internal class GroovyDependenciesBlockParserTest : BaseTest() { """ ) { - suppressedForEntireBlock shouldBe listOf("must-be-api", "unused-dependency") + blockSuppressed shouldBe listOf("must-be-api", "unused-dependency") settings shouldBe listOf( ModuleDependencyDeclaration( @@ -217,7 +217,7 @@ internal class GroovyDependenciesBlockParserTest : BaseTest() { """ ) { - suppressedForEntireBlock shouldBe listOf("must-be-api", "unused-dependency") + blockSuppressed shouldBe listOf("must-be-api", "unused-dependency") settings shouldBe listOf( ModuleDependencyDeclaration( diff --git a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParser.kt b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParser.kt index 128413a940..89aba33c7b 100644 --- a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParser.kt +++ b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParser.kt @@ -23,6 +23,7 @@ import modulecheck.parsing.gradle.dsl.Assignment import modulecheck.parsing.psi.internal.asKtFile import modulecheck.parsing.psi.internal.getChildrenOfTypeRecursive import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.psi.KtAnnotatedExpression import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtConstantExpression @@ -32,6 +33,7 @@ import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType import org.jetbrains.kotlin.psi.psiUtil.getChildOfType import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType +import org.jetbrains.kotlin.psi.psiUtil.parents import java.io.File import javax.inject.Inject @@ -43,7 +45,7 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { val androidQualifiedSettings = file.getChildrenOfTypeRecursive() .filter { it.findDescendantOfType()?.text == "android" } - .mapNotNull { it.toAssignmentOrNull(it.text) } + .mapNotNull { it.toAssignmentOrNull(it.text, it.suppressedNames()) } val androidQualified = file.getChildrenOfTypeRecursive() .filter { it.getChildOfType()?.text == "android" } @@ -53,15 +55,18 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { val androidBlocks = androidBlocksPsi .mapNotNull { androidBlock -> + val blockSuppressed = androidBlock.suppressedNames() + val fullText = androidBlock.text - val content = androidBlock.getChildrenOfTypeRecursive() + val block = androidBlock.getChildrenOfTypeRecursive() .firstOrNull() - ?.text ?: return@mapNotNull null - val settings = androidBlock.mapAssignments(fullText) + val content = block?.text ?: return@mapNotNull null + + val settings = androidBlock.mapAssignments(fullText, blockSuppressed) - AndroidBlock(fullText, content, settings) + AndroidBlock(fullText, content, settings, blockSuppressed) } val buildFeaturesBlocks = (androidBlocksPsi + androidQualified) @@ -70,6 +75,8 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { android.buildFeaturesBlocks() .mapNotNull { buildFeaturesBlock -> + val blockSuppressed = buildFeaturesBlock.suppressedNames() + val fullText = android.text val contentBlock = buildFeaturesBlock.getChildrenOfTypeRecursive() @@ -78,9 +85,13 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { val content = contentBlock.text - val settings = contentBlock.mapAssignments(fullText) + val allSuppressed = buildFeaturesBlock.parents.filterIsInstance() + .flatMap { it.suppressedNames() } + .toList() - BuildFeaturesBlock(fullText, content, settings) + val settings = contentBlock.mapAssignments(fullText, allSuppressed) + + BuildFeaturesBlock(fullText, content, settings, blockSuppressed) } } @@ -96,12 +107,20 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { ) } - private fun PsiElement.mapAssignments(fullText: String): List { + private fun PsiElement.mapAssignments( + fullText: String, + blockSuppressed: List + ): List { return getChildrenOfTypeRecursive() - .mapNotNull { it.toAssignmentOrNull(fullText) } + .mapNotNull { it.toAssignmentOrNull(fullText, blockSuppressed) } } - private fun KtBinaryExpression.toAssignmentOrNull(fullText: String): Assignment? { + private fun KtBinaryExpression.toAssignmentOrNull( + fullText: String, + blockSuppressed: List + ): Assignment? { + + val suppressed = blockSuppressed.plus(suppressedNames()).distinct() val propertyFullName = getChildOfType()?.text ?: getChildOfType() @@ -109,16 +128,14 @@ class KotlinAndroidGradleParser @Inject constructor() : AndroidGradleParser { ?.lastOrNull() ?.text ?: return null - val value = getChildOfType() - ?.text ?: return null - - val assignmentText = text + val value = getChildOfType()?.text ?: return null return Assignment( fullText = fullText, propertyFullName = propertyFullName, value = value, - declarationText = assignmentText + declarationText = text, + suppressed = suppressed ) } } diff --git a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlock.kt b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlock.kt index d09720fa6d..b0f75d87ec 100644 --- a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlock.kt +++ b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlock.kt @@ -24,12 +24,12 @@ class KotlinDependenciesBlock internal constructor( logger: McLogger, override val fullText: String, override val lambdaContent: String, - suppressAll: List, + blockSuppressed: List, configurationNameTransform: ConfigurationNameTransform, projectDependency: ProjectDependency.Factory ) : AbstractDependenciesBlock( logger = logger, - suppressedForEntireBlock = suppressAll, + blockSuppressed = blockSuppressed, configurationNameTransform = configurationNameTransform, projectDependency = projectDependency ) { diff --git a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlockParser.kt b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlockParser.kt index 51b607586a..c9f893309c 100644 --- a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlockParser.kt +++ b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinDependenciesBlockParser.kt @@ -31,6 +31,7 @@ import org.jetbrains.kotlin.psi.KtAnnotatedExpression import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtLiteralStringTemplateEntry import org.jetbrains.kotlin.psi.KtStringTemplateExpression import org.jetbrains.kotlin.psi.KtTreeVisitorVoid @@ -74,7 +75,7 @@ class KotlinDependenciesBlockParser @Inject constructor( logger = logger, fullText = fullText, lambdaContent = blockWhiteSpace + contentString, - suppressAll = blockSuppressed, + blockSuppressed = blockSuppressed, configurationNameTransform = { it.buildFileInvocationText(invokesConfigurationNames) }, projectDependency = projectDependency ) @@ -283,6 +284,10 @@ inline fun literalStringTemplateRecursiveVisitor( } } +internal fun KtExpression.suppressedNames(): List = (parent as? KtAnnotatedExpression) + ?.suppressedNames() + .orEmpty() + internal fun KtAnnotatedExpression.suppressedNames(): List = annotationEntries .filter { it.typeReference?.text == "Suppress" || it.typeReference?.text == "SuppressWarnings" } .flatMap { it.valueArgumentList?.arguments.orEmpty() } diff --git a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlock.kt b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlock.kt index 9263fc492f..6f5564c2eb 100644 --- a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlock.kt +++ b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlock.kt @@ -22,8 +22,11 @@ class KotlinPluginsBlock( logger: McLogger, override val fullText: String, override val lambdaContent: String, - suppressedForEntireBlock: List -) : AbstractPluginsBlock(logger = logger, suppressedForEntireBlock = suppressedForEntireBlock) { + blockSuppressed: List +) : AbstractPluginsBlock( + logger = logger, + blockSuppressed = blockSuppressed +) { override fun findOriginalStringIndex(parsedString: String) = originalLines .indexOfFirst { originalLine -> diff --git a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlockParser.kt b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlockParser.kt index 624c64ad52..8892f6bc2a 100644 --- a/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlockParser.kt +++ b/modulecheck-parsing/psi/src/main/kotlin/modulecheck/parsing/psi/KotlinPluginsBlockParser.kt @@ -61,7 +61,7 @@ class KotlinPluginsBlockParser @Inject constructor( logger = logger, fullText = fullText, lambdaContent = blockWhiteSpace + contentString, - suppressedForEntireBlock = blockSuppressed + blockSuppressed = blockSuppressed ) contentBlock.children diff --git a/modulecheck-parsing/psi/src/test/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParserTest.kt b/modulecheck-parsing/psi/src/test/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParserTest.kt index 392084791e..ec07543518 100644 --- a/modulecheck-parsing/psi/src/test/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParserTest.kt +++ b/modulecheck-parsing/psi/src/test/kotlin/modulecheck/parsing/psi/KotlinAndroidGradleParserTest.kt @@ -15,37 +15,29 @@ package modulecheck.parsing.psi -import hermit.test.junit.HermitJUnit5 -import io.kotest.matchers.shouldBe +import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import modulecheck.parsing.gradle.dsl.AndroidGradleSettings import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.AndroidBlock import modulecheck.parsing.gradle.dsl.AndroidGradleSettings.AgpBlock.BuildFeaturesBlock import modulecheck.parsing.gradle.dsl.Assignment -import modulecheck.reporting.logging.PrintLogger -import modulecheck.testing.tempFile +import modulecheck.testing.BaseTest +import modulecheck.utils.child +import modulecheck.utils.createSafely import org.jetbrains.kotlin.cli.common.repl.replEscapeLineBreaks -import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.TestFactory -import org.junit.jupiter.api.TestInfo -internal class KotlinAndroidGradleParserTest : HermitJUnit5() { +internal class KotlinAndroidGradleParserTest : BaseTest() { - val logger by resets { PrintLogger() } - - val testFile by tempFile() - - private var testInfo: TestInfo? = null - - @BeforeEach - fun beforeEach(testInfo: TestInfo) { - this.testInfo = testInfo + val testFile by resets { + testProjectDir.child("build.gradle.kts").createSafely() } @TestFactory fun `lots of blocks`() = runTest { enabled -> val block = """ + @Suppress("disable-android-buildConfig") android { buildFeatures { viewBinding = $enabled @@ -67,59 +59,73 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "viewBinding = $enabled" + declarationText = "viewBinding = $enabled", + statementWithSurroundingText = "viewBinding = $enabled", + suppressed = listOf("disable-android-buildConfig") ) val buildConfigAssignment = Assignment( fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", propertyFullName = "buildConfig", value = "$enabled", - declarationText = "buildConfig = $enabled" + declarationText = "buildConfig = $enabled", + statementWithSurroundingText = "buildConfig = $enabled", + suppressed = listOf("disable-android-buildConfig") ) + val declarationText1 = "androidResources = $enabled" val androidResourcesAssignment = Assignment( fullText = "android {\n buildFeatures {\n androidResources = $enabled\n }\n}", propertyFullName = "androidResources", value = "$enabled", - declarationText = "androidResources = $enabled" + declarationText = declarationText1, + statementWithSurroundingText = declarationText1, + suppressed = listOf() ) - KotlinAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( - assignments = listOf( - viewBindingAssignment, - buildConfigAssignment, - androidResourcesAssignment - ), - androidBlocks = listOf( - AndroidBlock( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", - lambdaContent = "buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }", - settings = listOf( - viewBindingAssignment, - buildConfigAssignment - ) + val result = KotlinAndroidGradleParser().parse(testFile) + + result.assignments shouldContainExactlyInAnyOrder listOf( + viewBindingAssignment, + buildConfigAssignment, + androidResourcesAssignment + ) + + result.androidBlocks shouldContainExactlyInAnyOrder listOf( + AndroidBlock( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", + lambdaContent = "buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }", + settings = listOf( + viewBindingAssignment, + buildConfigAssignment ), - AndroidBlock( - fullText = "android {\n buildFeatures {\n androidResources = $enabled\n }\n}", - lambdaContent = "buildFeatures {\n androidResources = $enabled\n }", - settings = listOf( - androidResourcesAssignment - ) - ) + blockSuppressed = listOf("disable-android-buildConfig") ), - buildFeaturesBlocks = listOf( - BuildFeaturesBlock( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", - lambdaContent = "viewBinding = $enabled", - settings = listOf(viewBindingAssignment) - ), - BuildFeaturesBlock( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", - lambdaContent = "buildConfig = $enabled", - settings = listOf(buildConfigAssignment) + AndroidBlock( + fullText = "android {\n buildFeatures {\n androidResources = $enabled\n }\n}", + lambdaContent = "buildFeatures {\n androidResources = $enabled\n }", + settings = listOf( + androidResourcesAssignment ), - BuildFeaturesBlock( - fullText = "android {\n buildFeatures {\n androidResources = $enabled\n }\n}", - lambdaContent = "androidResources = $enabled", - settings = listOf(androidResourcesAssignment) - ) + blockSuppressed = listOf() + ) + ) + + result.buildFeaturesBlocks shouldContainExactlyInAnyOrder listOf( + BuildFeaturesBlock( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", + lambdaContent = "viewBinding = $enabled", + settings = listOf(viewBindingAssignment), + blockSuppressed = listOf() + ), + BuildFeaturesBlock( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n }\n buildFeatures {\n buildConfig = $enabled\n }\n}", + lambdaContent = "buildConfig = $enabled", + settings = listOf(buildConfigAssignment), + blockSuppressed = listOf() + ), + BuildFeaturesBlock( + fullText = "android {\n buildFeatures {\n androidResources = $enabled\n }\n}", + lambdaContent = "androidResources = $enabled", + settings = listOf(androidResourcesAssignment), + blockSuppressed = listOf() ) ) } @@ -138,68 +144,91 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { testFile.writeText(block) - KotlinAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( - assignments = listOf( - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "viewBinding", - value = "$enabled", - declarationText = "viewBinding = $enabled" + KotlinAndroidGradleParser().parse(testFile) shouldBe run { + val declarationText1 = "viewBinding = $enabled" + val declarationText2 = "androidResources = ${!enabled}" + val declarationText3 = "viewBinding = $enabled" + val declarationText4 = "androidResources = ${!enabled}" + val declarationText5 = "viewBinding = $enabled" + val declarationText6 = "androidResources = ${!enabled}" + AndroidGradleSettings( + assignments = listOf( + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "viewBinding", + value = "$enabled", + declarationText = declarationText1, + statementWithSurroundingText = declarationText1, + suppressed = listOf() + ), + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "androidResources", + value = "${!enabled}", + declarationText = declarationText2, + statementWithSurroundingText = declarationText2, + suppressed = listOf() + ) ), - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "androidResources", - value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" - ) - ), - androidBlocks = listOf( - AndroidBlock( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - lambdaContent = "buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }", - settings = listOf( - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "viewBinding", - value = "$enabled", - declarationText = "viewBinding = $enabled" + androidBlocks = listOf( + AndroidBlock( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + lambdaContent = "buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }", + settings = listOf( + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "viewBinding", + value = "$enabled", + declarationText = declarationText3, + statementWithSurroundingText = declarationText3, + suppressed = listOf() + ), + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "androidResources", + value = "${!enabled}", + declarationText = declarationText4, + statementWithSurroundingText = declarationText4, + suppressed = listOf() + ) ), - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "androidResources", - value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" - ) + blockSuppressed = listOf() ) - ) - ), - buildFeaturesBlocks = listOf( - BuildFeaturesBlock( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - lambdaContent = "viewBinding = $enabled\n androidResources = ${!enabled}", - settings = listOf( - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "viewBinding", - value = "$enabled", - declarationText = "viewBinding = $enabled" + ), + buildFeaturesBlocks = listOf( + BuildFeaturesBlock( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + lambdaContent = "viewBinding = $enabled\n androidResources = ${!enabled}", + settings = listOf( + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "viewBinding", + value = "$enabled", + declarationText = declarationText5, + statementWithSurroundingText = declarationText5, + suppressed = listOf() + ), + Assignment( + fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", + propertyFullName = "androidResources", + value = "${!enabled}", + declarationText = declarationText6, + statementWithSurroundingText = declarationText6, + suppressed = listOf() + ) ), - Assignment( - fullText = "android {\n buildFeatures {\n viewBinding = $enabled\n androidResources = ${!enabled}\n }\n}", - propertyFullName = "androidResources", - value = "${!enabled}", - declarationText = "androidResources = ${!enabled}" - ) + blockSuppressed = listOf() ) ) ) - ) + } } @TestFactory fun `fully dot qualified boolean property`() = runTest { enabled -> val block = """ + @Suppress("disable-android-resources") android.buildFeatures.androidResources = $enabled """.trimIndent() @@ -211,7 +240,9 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { fullText = "android.buildFeatures.androidResources = $enabled", propertyFullName = "androidResources", value = "$enabled", - declarationText = "android.buildFeatures.androidResources = $enabled" + declarationText = "android.buildFeatures.androidResources = $enabled", + statementWithSurroundingText = "android.buildFeatures.androidResources = $enabled", + suppressed = listOf("disable-android-resources") ) ), androidBlocks = listOf(), @@ -230,31 +261,40 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { testFile.writeText(block) - KotlinAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( - assignments = listOf( - Assignment( - fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", - propertyFullName = "viewBinding", - value = "$enabled", - declarationText = "viewBinding = $enabled" - ) - ), - androidBlocks = listOf(), - buildFeaturesBlocks = listOf( - BuildFeaturesBlock( - fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", - lambdaContent = "viewBinding = $enabled", - settings = listOf( - Assignment( - fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", - propertyFullName = "viewBinding", - value = "$enabled", - declarationText = "viewBinding = $enabled" - ) + KotlinAndroidGradleParser().parse(testFile) shouldBe run { + val declarationText1 = "viewBinding = $enabled" + val declarationText2 = "viewBinding = $enabled" + AndroidGradleSettings( + assignments = listOf( + Assignment( + fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", + propertyFullName = "viewBinding", + value = "$enabled", + declarationText = declarationText1, + statementWithSurroundingText = declarationText1, + suppressed = listOf() + ) + ), + androidBlocks = listOf(), + buildFeaturesBlocks = listOf( + BuildFeaturesBlock( + fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", + lambdaContent = "viewBinding = $enabled", + settings = listOf( + Assignment( + fullText = "android.buildFeatures {\n viewBinding = $enabled\n}", + propertyFullName = "viewBinding", + value = "$enabled", + declarationText = declarationText2, + statementWithSurroundingText = declarationText2, + suppressed = listOf() + ) + ), + blockSuppressed = listOf() ) ) ) - ) + } } @TestFactory @@ -262,6 +302,7 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { val block = """ android { + @Suppress("disable-view-binding") buildFeatures.viewBinding = $enabled } """.trimIndent() @@ -271,24 +312,29 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { KotlinAndroidGradleParser().parse(testFile) shouldBe AndroidGradleSettings( assignments = listOf( Assignment( - fullText = "android {\n buildFeatures.viewBinding = $enabled\n}", + fullText = "android {\n @Suppress(\"disable-view-binding\")\n buildFeatures.viewBinding = $enabled\n}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "buildFeatures.viewBinding = $enabled" + declarationText = "buildFeatures.viewBinding = $enabled", + statementWithSurroundingText = "buildFeatures.viewBinding = $enabled", + suppressed = listOf("disable-view-binding") ) ), androidBlocks = listOf( AndroidBlock( - fullText = "android {\n buildFeatures.viewBinding = $enabled\n}", - lambdaContent = "buildFeatures.viewBinding = $enabled", + fullText = "android {\n @Suppress(\"disable-view-binding\")\n buildFeatures.viewBinding = $enabled\n}", + lambdaContent = "@Suppress(\"disable-view-binding\")\n buildFeatures.viewBinding = $enabled", settings = listOf( Assignment( - fullText = "android {\n buildFeatures.viewBinding = $enabled\n}", + fullText = "android {\n @Suppress(\"disable-view-binding\")\n buildFeatures.viewBinding = $enabled\n}", propertyFullName = "viewBinding", value = "$enabled", - declarationText = "buildFeatures.viewBinding = $enabled" + declarationText = "buildFeatures.viewBinding = $enabled", + statementWithSurroundingText = "buildFeatures.viewBinding = $enabled", + suppressed = listOf("disable-view-binding") ) - ) + ), + blockSuppressed = listOf() ) ), buildFeaturesBlocks = listOf() @@ -300,10 +346,7 @@ internal class KotlinAndroidGradleParserTest : HermitJUnit5() { val paramsString = " -- enabled: $enabled" - val name = ( - testInfo?.displayName?.replace("()", "") - ?: "???" - ) + paramsString + val name = "${testDisplayName.replace("()", "")}$paramsString" DynamicTest.dynamicTest(name) { block(enabled) diff --git a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt index 6e86f1c612..f5f2114ab1 100644 --- a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt +++ b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt @@ -20,7 +20,7 @@ import modulecheck.api.context.ProjectDepth import modulecheck.config.ModuleCheckSettings import modulecheck.utils.child import modulecheck.utils.createSafely -import modulecheck.utils.indent +import modulecheck.utils.indentByBrackets import java.io.File import javax.inject.Inject @@ -60,7 +60,7 @@ class GraphvizFileWriter @Inject constructor( val dotString = graphviz .render(Format.DOT) .toString() - .indent() + .indentByBrackets() dotFile.createSafely(dotString) diff --git a/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableAndroidResourcesRule.kt b/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableAndroidResourcesRule.kt index c53be44d4e..28f7d8c7dd 100644 --- a/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableAndroidResourcesRule.kt +++ b/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableAndroidResourcesRule.kt @@ -22,7 +22,6 @@ import modulecheck.api.context.androidResourceReferencesForSourceSetName import modulecheck.api.context.dependents import modulecheck.api.context.referencesForSourceSetName import modulecheck.config.ModuleCheckSettings -import modulecheck.finding.FindingName import modulecheck.finding.android.UnusedResourcesGenerationFinding import modulecheck.parsing.gradle.model.AndroidPlatformPlugin.AndroidLibraryPlugin import modulecheck.project.McProject @@ -33,7 +32,7 @@ import javax.inject.Inject class DisableAndroidResourcesRule @Inject constructor() : DocumentedRule() { - override val name = FindingName("disable-android-resources") + override val name = UnusedResourcesGenerationFinding.NAME override val description = "Finds modules which have android resources R file generation enabled, " + "but don't actually use any resources from the module" @@ -48,7 +47,6 @@ class DisableAndroidResourcesRule @Inject constructor() : fun findingList() = listOf( UnusedResourcesGenerationFinding( - findingName = name, dependentProject = project, dependentPath = project.path, buildFile = project.buildFile ) ) diff --git a/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableViewBindingRule.kt b/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableViewBindingRule.kt index df3c5faf3f..83309fc42f 100644 --- a/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableViewBindingRule.kt +++ b/modulecheck-rule/impl/src/main/kotlin/modulecheck/rule/impl/DisableViewBindingRule.kt @@ -19,7 +19,6 @@ import modulecheck.api.context.androidDataBindingDeclarationsForSourceSetName import modulecheck.api.context.dependents import modulecheck.api.context.referencesForSourceSetName import modulecheck.config.ModuleCheckSettings -import modulecheck.finding.FindingName import modulecheck.finding.android.DisableViewBindingGenerationFinding import modulecheck.parsing.gradle.model.SourceSetName import modulecheck.project.McProject @@ -31,7 +30,7 @@ import javax.inject.Inject class DisableViewBindingRule @Inject constructor() : DocumentedRule() { - override val name = FindingName("disable-view-binding") + override val name = DisableViewBindingGenerationFinding.NAME override val description = "Finds modules which have ViewBinding enabled, " + "but don't actually use any generated ViewBinding objects from that module" @@ -91,7 +90,6 @@ class DisableViewBindingRule @Inject constructor() : DocumentedRule Unit) { } /** A naive auto-indent which just counts brackets. */ -fun String.indent(tab: String = " "): String { +fun String.indentByBrackets(tab: String = " "): String { var tabCount = 0