Skip to content

Commit bb1d589

Browse files
committed
Introduce FormattingOptions.useExperimentalEngine
* Private, internal-only mode for ongoing development * A separate KotlinInputAstVisitor to evolve independently so no actual users are affected * Add a single test-case to showcase the idea
1 parent 88ddd04 commit bb1d589

10 files changed

Lines changed: 96 additions & 25 deletions

File tree

core/api/ktfmt.api

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ public final class com/facebook/ktfmt/format/FormattingOptions {
129129
public static final field DEFAULT_CONTINUATION_INDENT I
130130
public static final field DEFAULT_MAX_WIDTH I
131131
public fun <init> (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZ)V
132-
public fun <init> (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZ)V
133-
public synthetic fun <init> (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZILkotlin/jvm/internal/DefaultConstructorMarker;)V
132+
public fun <init> (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZZ)V
133+
public synthetic fun <init> (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZZILkotlin/jvm/internal/DefaultConstructorMarker;)V
134134
public fun <init> (IIIZZZ)V
135135
public synthetic fun <init> (IIIZZZILkotlin/jvm/internal/DefaultConstructorMarker;)V
136136
public final fun component1 ()I
@@ -140,8 +140,8 @@ public final class com/facebook/ktfmt/format/FormattingOptions {
140140
public final fun component5 ()Z
141141
public final fun component6 ()Z
142142
public final fun component7 ()Z
143-
public final fun copy (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZ)Lcom/facebook/ktfmt/format/FormattingOptions;
144-
public static synthetic fun copy$default (Lcom/facebook/ktfmt/format/FormattingOptions;IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZILjava/lang/Object;)Lcom/facebook/ktfmt/format/FormattingOptions;
143+
public final fun copy (IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZZ)Lcom/facebook/ktfmt/format/FormattingOptions;
144+
public static synthetic fun copy$default (Lcom/facebook/ktfmt/format/FormattingOptions;IIILcom/facebook/ktfmt/format/TrailingCommaManagementStrategy;ZZZZILjava/lang/Object;)Lcom/facebook/ktfmt/format/FormattingOptions;
145145
public fun equals (Ljava/lang/Object;)Z
146146
public final fun getBlockIndent ()I
147147
public final fun getContinuationIndent ()I
@@ -183,7 +183,7 @@ public final class com/facebook/ktfmt/format/KotlinInput : com/google/googlejava
183183
public fun getkN ()I
184184
}
185185

186-
public final class com/facebook/ktfmt/format/KotlinInputAstVisitor : org/jetbrains/kotlin/psi/KtTreeVisitorVoid {
186+
public class com/facebook/ktfmt/format/KotlinInputAstVisitor : org/jetbrains/kotlin/psi/KtTreeVisitorVoid {
187187
public fun <init> (Lcom/facebook/ktfmt/format/FormattingOptions;Lcom/google/googlejavaformat/OpsBuilder;)V
188188
public fun visitAnnotatedExpression (Lorg/jetbrains/kotlin/psi/KtAnnotatedExpression;)V
189189
public fun visitAnnotation (Lorg/jetbrains/kotlin/psi/KtAnnotation;)V

core/src/main/java/com/facebook/ktfmt/format/Formatter.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,11 @@ object Formatter {
284284
if (KotlinVersion.CURRENT < MINIMUM_KOTLIN_VERSION) {
285285
throw RuntimeException("Unsupported runtime Kotlin version: " + KotlinVersion.CURRENT)
286286
}
287-
return KotlinInputAstVisitor(options, builder)
287+
return if (options.useExperimentalEngine) {
288+
KotlinLangInputAstVisitor(options, builder)
289+
} else {
290+
KotlinInputAstVisitor(options, builder)
291+
}
288292
}
289293

290294
private fun checkEscapeSequences(code: String) {

core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ data class FormattingOptions(
8585
* newline) decisions
8686
*/
8787
val debuggingPrintOpsAfterFormatting: Boolean = false,
88+
89+
// Development only, kotlinlang codestyle experiments
90+
private val experimentalEngine: Boolean = false,
8891
) {
8992
companion object {
9093
const val DEFAULT_MAX_WIDTH: Int = 100
@@ -135,6 +138,9 @@ data class FormattingOptions(
135138
internal val manageTrailingCommas: Boolean
136139
get() = trailingCommaManagementStrategy != NONE
137140

141+
internal val useExperimentalEngine: Boolean
142+
get() = experimentalEngine
143+
138144
/**
139145
* Returns a [Builder] pre-populated with this instance's values.
140146
*

core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,13 @@ import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes
139139
import org.jetbrains.kotlin.psi.stubs.impl.KotlinPlaceHolderStubImpl
140140

141141
/** An AST visitor that builds a stream of {@link Op}s to format. */
142-
class KotlinInputAstVisitor(
142+
open class KotlinInputAstVisitor(
143143
private val options: FormattingOptions,
144144
private val builder: OpsBuilder,
145145
) : KtTreeVisitorVoid() {
146146

147+
internal open val forceAnnotationBreaks: Boolean = false
148+
147149
/** Standard indentation for a block */
148150
private val blockIndent: Indent.Const = Indent.Const.make(options.blockIndent, 1)
149151

@@ -2144,7 +2146,9 @@ class KotlinInputAstVisitor(
21442146
visit(psi)
21452147
}
21462148

2147-
if (onlyAnnotationsSoFar) {
2149+
if (onlyAnnotationsSoFar && forceAnnotationBreaks && psi is KtAnnotationEntry) {
2150+
builder.forcedBreak()
2151+
} else if (onlyAnnotationsSoFar) {
21482152
builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO)
21492153
} else {
21502154
builder.space()
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package com.facebook.ktfmt.format
2+
3+
import com.google.googlejavaformat.OpsBuilder
4+
5+
internal class KotlinLangInputAstVisitor(
6+
options: FormattingOptions,
7+
builder: OpsBuilder,
8+
) : KotlinInputAstVisitor(options, builder) {
9+
override val forceAnnotationBreaks: Boolean = true
10+
}

core/src/test/java/org/jetbrains/ktfmt/FormatterTestFactory.kt

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import kotlin.io.path.name
1212
import kotlin.io.path.nameWithoutExtension
1313
import kotlin.io.path.readText
1414
import kotlin.io.path.writeText
15-
import kotlin.io.resolve
1615
import org.junit.jupiter.api.Assertions.assertEquals
1716
import org.junit.jupiter.api.DynamicContainer
1817
import org.junit.jupiter.api.DynamicNode
@@ -26,7 +25,7 @@ import org.junit.jupiter.api.TestFactory
2625
* 1) Create a folder with the test group in resources, e.g. "cases/enums/"
2726
* 2) Populate it with tests: `Foo.input` is an input for the formatter, `Foo.output` is an expected
2827
* output. If an .output is not present, it is assumed that formatting `.input` is an idempotent
29-
* op
28+
* op. A `Foo.new.output` file additionally runs the case with [NEW_FORMAT]
3029
* 3) Create a test class in Tests.kt:
3130
* ```
3231
* class EnumsTest() : FormatterTestFactory()
@@ -59,8 +58,16 @@ abstract class FormatterTestFactory(
5958
trailingCommaManagementStrategy = TrailingCommaManagementStrategy.NONE,
6059
)
6160

61+
val NEW_FORMAT =
62+
Formatter.KOTLINLANG_FORMAT.copy(
63+
experimentalEngine = true,
64+
)
65+
66+
// Add other formats if needed for extensibility
67+
val FORMAT_VARIANTS = mapOf("new" to NEW_FORMAT)
68+
6269
// Without this, neither 'overwrite' nor navigation in IJ will work
63-
val root = run {
70+
val ROOT = run {
6471
val location = javaClass.protectionDomain?.codeSource?.location!!
6572
val root = location.toURI().path.substringBefore("build/classes/kotlin/test")
6673
Path.of(root).resolve("src/test/resources/cases")
@@ -72,7 +79,7 @@ abstract class FormatterTestFactory(
7279
// Loads e.g. cases/enums/, all cases from the folder at once
7380
val cases = load(group)
7481
check(cases.isNotEmpty()) {
75-
"No '.input' files in ${root.resolve(group)}"
82+
"No '.input' files in ${ROOT.resolve(group)}"
7683
}
7784

7885
return cases.map { case ->
@@ -85,7 +92,7 @@ abstract class FormatterTestFactory(
8592
}
8693

8794
private fun load(group: String): List<TestDescription> {
88-
val directory = root.resolve(group)
95+
val directory = ROOT.resolve(group)
8996
require(directory.isDirectory()) { "No such directory: $directory" }
9097

9198
return directory
@@ -106,17 +113,20 @@ abstract class FormatterTestFactory(
106113
}
107114

108115
private fun tests(expectation: TestCase): List<DynamicTest> {
109-
val uri = expectation.description.inputPath.toUri()
116+
val uri = (expectation.output ?: expectation.description.inputPath).toUri()
110117
// format(expected) == actual
111118
val checks = mutableListOf(
112-
DynamicTest.dynamicTest("Formats as expected${expectation.label}", uri) {
119+
DynamicTest.dynamicTest(
120+
"${expectation.label} Formats as expected",
121+
uri,
122+
) {
113123
formatsAsExpected(expectation)
114124
},
115125
)
116126
// format(format(expected)) == format(expected)
117127
if (expectation.output != null) {
118128
checks +=
119-
DynamicTest.dynamicTest("Format is idempotent${expectation.label}", uri) {
129+
DynamicTest.dynamicTest("${expectation.label} Format is idempotent", uri) {
120130
outputIsIdempotent(expectation)
121131
}
122132
}
@@ -157,8 +167,8 @@ abstract class FormatterTestFactory(
157167

158168
private fun failureMessage(expectation: TestCase, actual: String): String = buildString {
159169
append(
160-
expectation.description.displayName,
161170
expectation.label,
171+
expectation.description.displayName,
162172
" is not formatted as expected.\n",
163173
)
164174
append(
@@ -191,14 +201,27 @@ abstract class FormatterTestFactory(
191201
val displayName: String
192202
get() = "$group/$name"
193203

194-
fun expectations(groupOptions: FormattingOptions): List<TestCase> = listOf(
195-
TestCase(
196-
description = this,
197-
variant = null,
198-
output = output,
199-
expected = output?.readText(Charsets.UTF_8) ?: input, // No .output, idempotency
200-
options = groupOptions,
201-
),
204+
fun expectations(groupOptions: FormattingOptions): List<TestCase> = buildList {
205+
add(testCase(variant = null, output = output, options = groupOptions))
206+
207+
FORMAT_VARIANTS.forEach { (variant, options) ->
208+
val variantOutput = expectation(variant).takeIf { it.isRegularFile() }
209+
if (variantOutput != null) {
210+
add(testCase(variant = variant, output = variantOutput, options = options))
211+
}
212+
}
213+
}
214+
215+
private fun testCase(
216+
variant: String?,
217+
output: Path?,
218+
options: FormattingOptions,
219+
): TestCase = TestCase(
220+
description = this,
221+
variant = variant,
222+
output = output,
223+
expected = output?.readText(Charsets.UTF_8) ?: input, // No .output, idempotency
224+
options = options,
202225
)
203226

204227
fun expectation(variant: String?): Path {
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package org.jetbrains.ktfmt
2+
3+
import org.jetbrains.ktfmt.testutil.FormatterTestFactory
4+
5+
class NewCodeStyleFormatterTest : FormatterTestFactory("new_codestyle")
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class Holder private constructor(val conf: Configuration) {
2+
3+
var someProperty: Boolean = conf.someProperty
4+
@Deprecated("Deprecateion message")
5+
@TestOnly
6+
set
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class Holder private constructor(val conf: Configuration) {
2+
3+
var someProperty: Boolean = conf.someProperty
4+
@Deprecated("Deprecateion message")
5+
@TestOnly
6+
set
7+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class Holder private constructor(val conf: Configuration) {
2+
3+
var someProperty: Boolean = conf.someProperty
4+
@Deprecated("Deprecateion message") @TestOnly set
5+
}

0 commit comments

Comments
 (0)