From 24db208d65cd01216204b2e31573cd55ee59b13a Mon Sep 17 00:00:00 2001 From: Tony Robalik Date: Wed, 4 Dec 2024 11:29:27 -0800 Subject: [PATCH 1/2] fix: check size before calling single(). Declare IllegalArgumentException. --- .../grammar/kotlindsl/utils/DependencyExtractor.kt | 3 +++ .../recipes/dependencies/DependenciesSimplifier.kt | 9 ++++++--- .../dependencies/DependenciesSimplifierTest.kt | 13 +++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt b/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt index 2bc4720..a6abf2f 100644 --- a/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt +++ b/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt @@ -326,6 +326,9 @@ public class DependencyExtractor( } private fun PostfixUnaryExpressionContext.isDependencyDeclaration(): DeclarationDetectionResult { + // Something strange + if (postfixUnarySuffix().size != 1) return DeclarationDetectionResult.STATEMENT + // This is everything after the configuration, including optionally a trailing lambda val rawDependency = postfixUnarySuffix().single().callSuffix() val args = rawDependency.valueArguments().valueArgument() diff --git a/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt b/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt index c335e54..0be2bfa 100644 --- a/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt +++ b/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt @@ -119,8 +119,9 @@ public class DependenciesSimplifier private constructor( * Returns a [DependenciesSimplifier], which eagerly parses [buildScript]. * * @throws IllegalStateException if [DependencyExtractor] sees an expression it doesn't understand. + * @throws IllegalArgumentException if [DependencyExtractor] sees an expression it doesn't understand. */ - @Throws(IllegalStateException::class) + @Throws(IllegalStateException::class, IllegalArgumentException::class) public fun of(buildScript: Path): DependenciesSimplifier { return of(Parser.readOnlyInputStream(buildScript)) } @@ -129,8 +130,9 @@ public class DependenciesSimplifier private constructor( * Returns a [DependenciesSimplifier], which eagerly parses [buildScript]. * * @throws IllegalStateException if [DependencyExtractor] sees an expression it doesn't understand. + * @throws IllegalArgumentException if [DependencyExtractor] sees an expression it doesn't understand. */ - @Throws(IllegalStateException::class) + @Throws(IllegalStateException::class, IllegalArgumentException::class) public fun of(buildScript: String): DependenciesSimplifier { return of(buildScript.byteInputStream()) } @@ -139,8 +141,9 @@ public class DependenciesSimplifier private constructor( * Returns a [DependenciesSimplifier], which eagerly parses [buildScript]. * * @throws IllegalStateException if [DependencyExtractor] sees an expression it doesn't understand. + * @throws IllegalArgumentException if [DependencyExtractor] sees an expression it doesn't understand. */ - @Throws(IllegalStateException::class) + @Throws(IllegalStateException::class, IllegalArgumentException::class) private fun of(buildScript: InputStream): DependenciesSimplifier { val errorListener = CollectingErrorListener() diff --git a/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt b/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt index 39df181..b52051c 100644 --- a/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt +++ b/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt @@ -38,4 +38,17 @@ internal class DependenciesSimplifierTest { """.trimIndent() ) } + + @Test fun `can handle user weirdness`() { + // Expect parsing not to throw exception + DependenciesSimplifier.of( + """ + dependencies { + tasks.withType().configureEach { + doThing(listOf("--a", "b")) + } + } + """.trimIndent() + ) + } } From 6b4bb5276ea58b3d46b4129c307ebb60e37248b7 Mon Sep 17 00:00:00 2001 From: Tony Robalik Date: Wed, 4 Dec 2024 11:55:43 -0800 Subject: [PATCH 2/2] fix: be stricter about what is a dependencies block we can parse. --- .../kotlindsl/utils/DependencyExtractor.kt | 4 +++- .../dependencies/DependenciesSimplifier.kt | 21 +++++++++---------- .../DependenciesSimplifierTest.kt | 15 +++++++++++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt b/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt index a6abf2f..bb38133 100644 --- a/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt +++ b/core/src/main/kotlin/cash/grammar/kotlindsl/utils/DependencyExtractor.kt @@ -331,7 +331,9 @@ public class DependencyExtractor( // This is everything after the configuration, including optionally a trailing lambda val rawDependency = postfixUnarySuffix().single().callSuffix() - val args = rawDependency.valueArguments().valueArgument() + // Probably not actually a dependencies block + val valueArguments = rawDependency.valueArguments() ?: return DeclarationDetectionResult.STATEMENT + val args = valueArguments.valueArgument() // If there are more than one argument, it's a function call, not a dependency declaration return if (args.size <= 1) { diff --git a/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt b/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt index 0be2bfa..59edec4 100644 --- a/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt +++ b/recipes/dependencies/src/main/kotlin/cash/recipes/dependencies/DependenciesSimplifier.kt @@ -4,7 +4,6 @@ import cash.grammar.kotlindsl.model.DependencyDeclaration import cash.grammar.kotlindsl.parse.KotlinParseException import cash.grammar.kotlindsl.parse.Parser import cash.grammar.kotlindsl.parse.Rewriter -import cash.grammar.kotlindsl.utils.Blocks.isBuildscript import cash.grammar.kotlindsl.utils.Blocks.isDependencies import cash.grammar.kotlindsl.utils.CollectingErrorListener import cash.grammar.kotlindsl.utils.Context.leafRule @@ -14,6 +13,7 @@ import cash.grammar.kotlindsl.utils.Whitespace.trimGently import cash.grammar.utils.ifNotEmpty import com.squareup.cash.grammar.KotlinParser.NamedBlockContext import com.squareup.cash.grammar.KotlinParser.PostfixUnaryExpressionContext +import com.squareup.cash.grammar.KotlinParser.ScriptContext import com.squareup.cash.grammar.KotlinParser.StatementContext import com.squareup.cash.grammar.KotlinParserBaseListener import org.antlr.v4.runtime.CharStream @@ -34,7 +34,6 @@ public class DependenciesSimplifier private constructor( private val dependencyExtractor = DependencyExtractor(input, tokens, indent) private var changes = false - private var isInBuildscriptBlock = false public fun isChanged(): Boolean = changes @@ -49,24 +48,24 @@ public class DependenciesSimplifier private constructor( override fun enterNamedBlock(ctx: NamedBlockContext) { dependencyExtractor.onEnterBlock() - - if (ctx.isBuildscript) { - isInBuildscriptBlock = true - } } override fun exitNamedBlock(ctx: NamedBlockContext) { - if (ctx.isDependencies && !isInBuildscriptBlock) { + if (isRealDependenciesBlock(ctx)) { onExitDependenciesBlock(ctx) } - if (ctx.isBuildscript) { - isInBuildscriptBlock = false - } - dependencyExtractor.onExitBlock() } + private fun isRealDependenciesBlock(ctx: NamedBlockContext): Boolean { + // parent is StatementContext. Parent of that should be ScriptContext + // In contrast, with tasks.shadowJar { dependencies { ... } }, the parent.parent is StatementsContext + if (ctx.parent.parent !is ScriptContext) return false + + return ctx.isDependencies + } + private fun onExitDependenciesBlock(ctx: NamedBlockContext) { val container = dependencyExtractor.collectDependencies(ctx) container.getDependencyDeclarationsWithContext() diff --git a/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt b/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt index b52051c..e0b3bfc 100644 --- a/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt +++ b/recipes/dependencies/src/test/kotlin/cash/recipes/dependencies/DependenciesSimplifierTest.kt @@ -51,4 +51,19 @@ internal class DependenciesSimplifierTest { """.trimIndent() ) } + + @Test fun `handles dependencies blocks that aren't actually dependencies blocks`() { + // Expect parsing not to throw exception + DependenciesSimplifier.of( + """ + tasks.shadowJar { + dependencies { + exclude { dep -> + dep.name != "foo" + } + } + } + """.trimIndent() + ) + } }