Skip to content

Commit 76d9b5a

Browse files
fix: handle dependencies blocks that have a single line.
1 parent 9a6c1a7 commit 76d9b5a

2 files changed

Lines changed: 119 additions & 14 deletions

File tree

src/main/kotlin/com/autonomousapps/internal/parse/KotlinBuildScriptDependenciesRewriter.kt

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ package com.autonomousapps.internal.parse
44

55
import com.autonomousapps.exception.BuildScriptParseException
66
import com.autonomousapps.internal.advice.AdvicePrinter
7-
import com.autonomousapps.internal.antlr.v4.runtime.*
7+
import com.autonomousapps.internal.antlr.v4.runtime.CharStream
8+
import com.autonomousapps.internal.antlr.v4.runtime.CommonTokenStream
9+
import com.autonomousapps.internal.antlr.v4.runtime.Token
810
import com.autonomousapps.internal.cash.grammar.kotlindsl.parse.Parser
911
import com.autonomousapps.internal.cash.grammar.kotlindsl.parse.Rewriter
1012
import com.autonomousapps.internal.cash.grammar.kotlindsl.utils.Blocks.isBuildscript
@@ -82,7 +84,8 @@ internal class KotlinBuildScriptDependenciesRewriter(
8284
override fun exitNamedBlock(ctx: NamedBlockContext) {
8385
if (ctx.isDependencies && !inBuildscriptBlock) {
8486
hasDependenciesBlock = true
85-
insertAdvice(advice, ctx.stop, withDependenciesBlock = false)
87+
val ktfmt = ctx.start.line == ctx.stop.line
88+
insertAdvice(advice, ctx.stop, withDependenciesBlock = false, ktfmt = ktfmt)
8689
}
8790

8891
// Must be last
@@ -97,21 +100,36 @@ internal class KotlinBuildScriptDependenciesRewriter(
97100
// Exit early if this build script has a dependencies block. If it doesn't, we may need to add missing dependencies.
98101
if (hasDependenciesBlock) return
99102

100-
insertAdvice(advice, ctx.stop, withDependenciesBlock = true)
103+
insertAdvice(advice, ctx.stop, withDependenciesBlock = true, ktfmt = false)
101104
}
102105

103-
private fun insertAdvice(advice: Set<Advice>, beforeToken: Token, withDependenciesBlock: Boolean) {
104-
val prefix = if (withDependenciesBlock) "\ndependencies {\n" else ""
106+
/**
107+
* ktfmt has this extremely dumb behavior where it collapses blocks to be on one line whenever possible. It will do
108+
* this:
109+
* ```
110+
* dependencies { implementation(libs.foo.bar) }
111+
* ```
112+
* This probably happens with other tools but I'm feeling petty. If we detect that the start and stop of a
113+
* `dependencies` block is on the same line, we insert a single newline before adding new dependencies, else the build
114+
* script wouldn't compile because it was looking like this:
115+
* ```
116+
* dependencies { implementation(libs.foo.bar) implementation(libs.baz) }
117+
* ```
118+
*/
119+
private fun insertAdvice(advice: Set<Advice>, beforeToken: Token, withDependenciesBlock: Boolean, ktfmt: Boolean) {
120+
val prefix = if (withDependenciesBlock) "\ndependencies {\n" else if (ktfmt) "\n" else ""
105121
val postfix = if (withDependenciesBlock) "\n}\n" else "\n"
106122

107-
advice.filterToOrderedSet { it.isAnyAdd() }.ifNotEmpty { addAdvice ->
108-
rewriter.insertBefore(
109-
beforeToken,
110-
addAdvice.joinToString(prefix = prefix, postfix = postfix, separator = "\n") { a ->
111-
printer.toDeclaration(a)
112-
}
113-
)
114-
}
123+
advice
124+
.filterToOrderedSet { it.isAnyAdd() }
125+
.ifNotEmpty { addAdvice ->
126+
rewriter.insertBefore(
127+
beforeToken,
128+
addAdvice.joinToString(prefix = prefix, postfix = postfix, separator = "\n") { a ->
129+
printer.toDeclaration(a)
130+
}
131+
)
132+
}
115133
}
116134

117135
private fun handleDependencies(ctx: NamedBlockContext) {

src/test/kotlin/com/autonomousapps/internal/parse/KotlinBuildScriptDependenciesRewriterTest.kt

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.autonomousapps.internal.parse
44

5-
import com.autonomousapps.model.internal.ProjectType
65
import com.autonomousapps.internal.advice.AdvicePrinter
76
import com.autonomousapps.internal.advice.DslKind
87
import com.autonomousapps.internal.utils.intoSet
98
import com.autonomousapps.model.Advice
109
import com.autonomousapps.model.Coordinates
1110
import com.autonomousapps.model.GradleVariantIdentification
1211
import com.autonomousapps.model.ProjectCoordinates
12+
import com.autonomousapps.model.internal.ProjectType
1313
import com.google.common.truth.Truth.assertThat
1414
import org.junit.jupiter.api.Nested
1515
import org.junit.jupiter.api.Test
@@ -687,6 +687,93 @@ internal class KotlinBuildScriptDependenciesRewriterTest {
687687
).inOrder()
688688
}
689689

690+
@Nested
691+
inner class Ktfmt {
692+
@Test fun `can add a single dependency`() {
693+
// Given
694+
val sourceFile = dir.resolve("build.gradle.kts")
695+
sourceFile.writeText(
696+
"""
697+
plugins {
698+
id("foo")
699+
}
700+
701+
dependencies { implementation(libs.foo.bar) }
702+
""".trimIndent()
703+
)
704+
val advice = setOf(Advice.ofAdd(Coordinates.of(":sad-robot"), "runtimeOnly"))
705+
706+
// When
707+
val parser = KotlinBuildScriptDependenciesRewriter.of(
708+
sourceFile,
709+
advice,
710+
AdvicePrinter(
711+
dslKind = DslKind.KOTLIN,
712+
projectType = projectType,
713+
useTypesafeProjectAccessors = false,
714+
)
715+
)
716+
717+
// Then
718+
// nb: there's an extra whitespace at the end of the `dependencies` line and this is necessary for the test to pass
719+
assertThat(parser.rewritten()).isEqualTo(
720+
"""
721+
plugins {
722+
id("foo")
723+
}
724+
725+
dependencies { implementation(libs.foo.bar)
726+
runtimeOnly(project(":sad-robot"))
727+
}
728+
""".trimIndent()
729+
)
730+
}
731+
732+
@Test fun `can add two dependencies`() {
733+
// Given
734+
val sourceFile = dir.resolve("build.gradle.kts")
735+
sourceFile.writeText(
736+
"""
737+
plugins {
738+
id("foo")
739+
}
740+
741+
dependencies { implementation(libs.foo.bar) }
742+
""".trimIndent()
743+
)
744+
val advice = setOf(
745+
Advice.ofAdd(Coordinates.of(":sad-robot"), "runtimeOnly"),
746+
Advice.ofAdd(Coordinates.of(":marvin"), "runtimeOnly"),
747+
)
748+
749+
// When
750+
val parser = KotlinBuildScriptDependenciesRewriter.of(
751+
sourceFile,
752+
advice,
753+
AdvicePrinter(
754+
dslKind = DslKind.KOTLIN,
755+
projectType = projectType,
756+
useTypesafeProjectAccessors = false,
757+
)
758+
)
759+
760+
// Then
761+
// nb: there's an extra whitespace at the end of the `dependencies` line and this is necessary for the test to pass
762+
assertThat(parser.rewritten()).isEqualTo(
763+
"""
764+
plugins {
765+
id("foo")
766+
}
767+
768+
dependencies { implementation(libs.foo.bar)
769+
runtimeOnly(project(":marvin"))
770+
runtimeOnly(project(":sad-robot"))
771+
}
772+
""".trimIndent()
773+
)
774+
}
775+
}
776+
690777
@Nested
691778
inner class TestFixtures {
692779
@Test fun `test fixtures of different dependency`() {

0 commit comments

Comments
 (0)