From ea415d23ca7ecb0991c5572976fe833f94f1a22c Mon Sep 17 00:00:00 2001 From: nickreid Date: Thu, 24 Aug 2023 08:42:44 -0700 Subject: [PATCH 01/20] Keep imports from the same package if the name is overloaded (#414) Summary: Pull Request resolved: https://github.com/facebook/ktfmt/pull/414 Reviewed By: cgrushko Differential Revision: D48604136 Pulled By: hick209 fbshipit-source-id: b25854479bb7dfe1ccced55bb6e420a894fca505 --- .../ktfmt/format/RedundantImportDetector.kt | 27 +- .../facebook/ktfmt/format/FormatterTest.kt | 446 +++++++++++------- 2 files changed, 281 insertions(+), 192 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt b/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt index 7efa893e..ba285f6d 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt @@ -112,11 +112,10 @@ internal class RedundantImportDetector(val enabled: Boolean) { importCleanUpCandidates = importList.imports .filter { import -> + val identifier = import.identifier ?: return@filter false import.isValidImport && - !import.isAllUnder && - import.identifier != null && - requireNotNull(import.identifier) !in OPERATORS && - !COMPONENT_OPERATOR_REGEX.matches(import.identifier.orEmpty()) + identifier !in OPERATORS && + !COMPONENT_OPERATOR_REGEX.matches(identifier) } .toSet() @@ -160,20 +159,20 @@ internal class RedundantImportDetector(val enabled: Boolean) { fun getRedundantImportElements(): List { if (!enabled) return emptyList() - val redundantImports = mutableListOf() + val identifierCounts = + importCleanUpCandidates.groupBy { it.identifier }.mapValues { it.value.size } - // Collect unused imports - for (import in importCleanUpCandidates) { - val isUnused = import.aliasName !in usedReferences && import.identifier !in usedReferences - val isFromSamePackage = import.importedFqName?.parent() == thisPackage && import.alias == null - if (isUnused || isFromSamePackage) { - redundantImports += import - } + return importCleanUpCandidates.filter { + val isUsed = it.identifier in usedReferences + val isFromThisPackage = it.importedFqName?.parent() == thisPackage + val hasAlias = it.alias != null + val isOverload = requireNotNull(identifierCounts[it.identifier]) > 1 + // Remove if... + !isUsed || (isFromThisPackage && !hasAlias && !isOverload) } - - return redundantImports } + /** The imported short name, possibly an alias name, if any. */ private inline val KtImportDirective.identifier: String? get() = importPath?.importedName?.identifier?.trim('`') } diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index af193bb4..02394608 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -1179,31 +1179,121 @@ class FormatterTest { } @Test - fun `imports from the same package are removed`() { + fun `used imports from this package are removed`() { val code = """ - |package com.example - | - |import com.example.Sample - |import com.example.Sample.CONSTANT - |import com.example.a.foo - | - |fun test() { - | foo(CONSTANT, Sample()) - |} - |""" + |package com.example + | + |import com.example.Sample + |import com.example.Sample.CONSTANT + |import com.example.a.foo + | + |fun test() { + | foo(CONSTANT, Sample()) + |} + |""" .trimMargin() val expected = """ - |package com.example - | - |import com.example.Sample.CONSTANT - |import com.example.a.foo - | - |fun test() { - | foo(CONSTANT, Sample()) - |} - |""" + |package com.example + | + |import com.example.Sample.CONSTANT + |import com.example.a.foo + | + |fun test() { + | foo(CONSTANT, Sample()) + |} + |""" + .trimMargin() + assertThatFormatting(code).isEqualTo(expected) + } + + @Test + fun `potentially unused imports from this package are kept if they are overloaded`() { + val code = + """ + |package com.example + | + |import com.example.a + |import com.example.b + |import com.example.c + |import com.notexample.a + |import com.notexample.b + |import com.notexample.notC as c + | + |fun test() { + | a("hello") + | c("hello") + |} + |""" + .trimMargin() + val expected = + """ + |package com.example + | + |import com.example.a + |import com.example.c + |import com.notexample.a + |import com.notexample.notC as c + | + |fun test() { + | a("hello") + | c("hello") + |} + |""" + .trimMargin() + assertThatFormatting(code).isEqualTo(expected) + } + + @Test + fun `used imports from this package are kept if they are aliased`() { + val code = + """ + |package com.example + | + |import com.example.b as a + |import com.example.c + | + |fun test() { + | a("hello") + |} + |""" + .trimMargin() + val expected = + """ + |package com.example + | + |import com.example.b as a + | + |fun test() { + | a("hello") + |} + |""" + .trimMargin() + assertThatFormatting(code).isEqualTo(expected) + } + + @Test + fun `unused imports are computed using only the alias name if present`() { + val code = + """ + |package com.example + | + |import com.notexample.a as b + | + |fun test() { + | a("hello") + |} + |""" + .trimMargin() + val expected = + """ + |package com.example + | + |fun test() { + | a("hello") + |} + |""" .trimMargin() assertThatFormatting(code).isEqualTo(expected) } @@ -1212,66 +1302,66 @@ class FormatterTest { fun `keep import elements only mentioned in kdoc`() { val code = """ - |package com.example.kdoc - | - |import com.example.Bar - |import com.example.Example - |import com.example.Foo - |import com.example.JavaDocLink - |import com.example.Param - |import com.example.R - |import com.example.ReturnedValue - |import com.example.Sample - |import com.example.unused - |import com.example.exception.AnException - |import com.example.kdoc.Doc - | - |/** - | * [Foo] is something only mentioned here, just like [R.layout.test] and [Doc]. - | * - | * Old {@link JavaDocLink} that gets removed. - | * - | * @throws AnException - | * @exception Sample.SampleException - | * @param unused [Param] - | * @property JavaDocLink [Param] - | * @return [Unit] as [ReturnedValue] - | * @sample Example - | * @see Bar for more info - | * @throws AnException - | */ - |class Dummy - |""" + |package com.example.kdoc + | + |import com.example.Bar + |import com.example.Example + |import com.example.Foo + |import com.example.JavaDocLink + |import com.example.Param + |import com.example.R + |import com.example.ReturnedValue + |import com.example.Sample + |import com.example.unused + |import com.example.exception.AnException + |import com.example.kdoc.Doc + | + |/** + | * [Foo] is something only mentioned here, just like [R.layout.test] and [Doc]. + | * + | * Old {@link JavaDocLink} that gets removed. + | * + | * @throws AnException + | * @exception Sample.SampleException + | * @param unused [Param] + | * @property JavaDocLink [Param] + | * @return [Unit] as [ReturnedValue] + | * @sample Example + | * @see Bar for more info + | * @throws AnException + | */ + |class Dummy + |""" .trimMargin() val expected = """ - |package com.example.kdoc - | - |import com.example.Bar - |import com.example.Example - |import com.example.Foo - |import com.example.Param - |import com.example.R - |import com.example.ReturnedValue - |import com.example.Sample - |import com.example.exception.AnException - | - |/** - | * [Foo] is something only mentioned here, just like [R.layout.test] and [Doc]. - | * - | * Old {@link JavaDocLink} that gets removed. - | * - | * @param unused [Param] - | * @return [Unit] as [ReturnedValue] - | * @property JavaDocLink [Param] - | * @throws AnException - | * @throws AnException - | * @exception Sample.SampleException - | * @sample Example - | * @see Bar for more info - | */ - |class Dummy - |""" + |package com.example.kdoc + | + |import com.example.Bar + |import com.example.Example + |import com.example.Foo + |import com.example.Param + |import com.example.R + |import com.example.ReturnedValue + |import com.example.Sample + |import com.example.exception.AnException + | + |/** + | * [Foo] is something only mentioned here, just like [R.layout.test] and [Doc]. + | * + | * Old {@link JavaDocLink} that gets removed. + | * + | * @param unused [Param] + | * @return [Unit] as [ReturnedValue] + | * @property JavaDocLink [Param] + | * @throws AnException + | * @throws AnException + | * @exception Sample.SampleException + | * @sample Example + | * @see Bar for more info + | */ + |class Dummy + |""" .trimMargin() assertThatFormatting(code).isEqualTo(expected) } @@ -1280,15 +1370,15 @@ class FormatterTest { fun `keep import elements only mentioned in kdoc, single line`() { assertFormatted( """ - |import com.shopping.Bag - | - |/** - | * Some summary. - | * - | * @param count you can fit this many in a [Bag] - | */ - |fun fetchBananas(count: Int) - |""" + |import com.shopping.Bag + | + |/** + | * Some summary. + | * + | * @param count you can fit this many in a [Bag] + | */ + |fun fetchBananas(count: Int) + |""" .trimMargin()) } @@ -1296,16 +1386,16 @@ class FormatterTest { fun `keep import elements only mentioned in kdoc, multiline`() { assertFormatted( """ - |import com.shopping.Bag - | - |/** - | * Some summary. - | * - | * @param count this is how many of these wonderful fruit you can fit into the useful object that - | * you may refer to as a [Bag] - | */ - |fun fetchBananas(count: Int) - |""" + |import com.shopping.Bag + | + |/** + | * Some summary. + | * + | * @param count this is how many of these wonderful fruit you can fit into the useful object that + | * you may refer to as a [Bag] + | */ + |fun fetchBananas(count: Int) + |""" .trimMargin()) } @@ -1313,69 +1403,69 @@ class FormatterTest { fun `keep component imports`() = assertFormatted( """ - |import com.example.component1 - |import com.example.component10 - |import com.example.component120 - |import com.example.component2 - |import com.example.component3 - |import com.example.component4 - |import com.example.component5 - |""" + |import com.example.component1 + |import com.example.component10 + |import com.example.component120 + |import com.example.component2 + |import com.example.component3 + |import com.example.component4 + |import com.example.component5 + |""" .trimMargin()) @Test fun `keep operator imports`() = assertFormatted( """ - |import com.example.and - |import com.example.compareTo - |import com.example.contains - |import com.example.dec - |import com.example.div - |import com.example.divAssign - |import com.example.equals - |import com.example.get - |import com.example.getValue - |import com.example.hasNext - |import com.example.inc - |import com.example.invoke - |import com.example.iterator - |import com.example.minus - |import com.example.minusAssign - |import com.example.mod - |import com.example.modAssign - |import com.example.next - |import com.example.not - |import com.example.or - |import com.example.plus - |import com.example.plusAssign - |import com.example.provideDelegate - |import com.example.rangeTo - |import com.example.rem - |import com.example.remAssign - |import com.example.set - |import com.example.setValue - |import com.example.times - |import com.example.timesAssign - |import com.example.unaryMinus - |import com.example.unaryPlus - |""" + |import com.example.and + |import com.example.compareTo + |import com.example.contains + |import com.example.dec + |import com.example.div + |import com.example.divAssign + |import com.example.equals + |import com.example.get + |import com.example.getValue + |import com.example.hasNext + |import com.example.inc + |import com.example.invoke + |import com.example.iterator + |import com.example.minus + |import com.example.minusAssign + |import com.example.mod + |import com.example.modAssign + |import com.example.next + |import com.example.not + |import com.example.or + |import com.example.plus + |import com.example.plusAssign + |import com.example.provideDelegate + |import com.example.rangeTo + |import com.example.rem + |import com.example.remAssign + |import com.example.set + |import com.example.setValue + |import com.example.times + |import com.example.timesAssign + |import com.example.unaryMinus + |import com.example.unaryPlus + |""" .trimMargin()) @Test fun `keep unused imports when formatting options has feature turned off`() { val code = """ - |import com.unused.FooBarBaz as Baz - |import com.unused.Sample - |import com.unused.a as `when` - |import com.unused.a as wow - |import com.unused.a.* - |import com.unused.b as `if` - |import com.unused.b as we - |import com.unused.bar // test - |import com.unused.`class` - |""" + |import com.unused.FooBarBaz as Baz + |import com.unused.Sample + |import com.unused.a as `when` + |import com.unused.a as wow + |import com.unused.a.* + |import com.unused.b as `if` + |import com.unused.b as we + |import com.unused.bar // test + |import com.unused.`class` + |""" .trimMargin() assertThatFormatting(code) @@ -1387,34 +1477,34 @@ class FormatterTest { fun `comments between imports are moved above import list`() { val code = """ - |package com.facebook.ktfmt - | - |/* leading comment */ - |import com.example.abc - |/* internal comment 1 */ - |import com.example.bcd - |// internal comment 2 - |import com.example.Sample - |// trailing comment - | - |val x = Sample(abc, bcd) - |""" + |package com.facebook.ktfmt + | + |/* leading comment */ + |import com.example.abc + |/* internal comment 1 */ + |import com.example.bcd + |// internal comment 2 + |import com.example.Sample + |// trailing comment + | + |val x = Sample(abc, bcd) + |""" .trimMargin() val expected = """ - |package com.facebook.ktfmt - | - |/* leading comment */ - |/* internal comment 1 */ - |// internal comment 2 - |import com.example.Sample - |import com.example.abc - |import com.example.bcd - | - |// trailing comment - | - |val x = Sample(abc, bcd) - |""" + |package com.facebook.ktfmt + | + |/* leading comment */ + |/* internal comment 1 */ + |// internal comment 2 + |import com.example.Sample + |import com.example.abc + |import com.example.bcd + | + |// trailing comment + | + |val x = Sample(abc, bcd) + |""" .trimMargin() assertThatFormatting(code).isEqualTo(expected) } @@ -1423,12 +1513,12 @@ class FormatterTest { fun `no redundant newlines when there are no imports`() = assertFormatted( """ - |package foo123 - | - |/* - |bar - |*/ - |""" + |package foo123 + | + |/* + |bar + |*/ + |""" .trimMargin()) @Test From 46ac818a283aad03dc7e15bc635668f9b8c9b35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Thu, 24 Aug 2023 08:48:39 -0700 Subject: [PATCH 02/20] Added link to live playground directly into README file Summary: Created from CodeHub with https://fburl.com/edit-in-codehub Reviewed By: cgrushko Differential Revision: D48647840 fbshipit-source-id: 1ddc9d78f0d5c41e7cb36ebf2c5a8f29f0b27779 --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 71b7c8be..b4863c36 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,11 @@ For comparison, the same code formatted by [`ktlint`](https://github.com/pintere | ------ | --------| | ![ktlint](docs/images/ktlint.png) | ![IntelliJ](docs/images/intellij.png) | +## Playground + +We have a [live playground](https://facebook.github.io/ktfmt/) where you can easily see how ktfmt would format your code. +Give it a try! https://facebook.github.io/ktfmt/ + ## Using the formatter ### IntelliJ, Android Studio, and other JetBrains IDEs From 17d80bf596708380fd73a5beb2bbe4cb02c31a16 Mon Sep 17 00:00:00 2001 From: Christopher-Marcel Esser Date: Wed, 13 Sep 2023 14:32:48 -0700 Subject: [PATCH 03/20] Support context receivers (#400) Summary: Resolves https://github.com/facebook/ktfmt/issues/397, https://github.com/facebook/ktfmt/issues/314 and https://github.com/facebook/ktfmt/issues/374. Pull Request resolved: https://github.com/facebook/ktfmt/pull/400 Reviewed By: davidtorosyan Differential Revision: D48169986 Pulled By: hick209 fbshipit-source-id: df4ffe4d939635b3db481ba478d52bffa4c78ec1 --- core/pom.xml | 2 +- .../ktfmt/format/KotlinInputAstVisitor.kt | 32 ++++++++++++++ .../facebook/ktfmt/format/FormatterTest.kt | 42 +++++++++++++++++++ .../facebook/ktfmt/format/TokenizerTest.kt | 24 +++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) diff --git a/core/pom.xml b/core/pom.xml index 62e841ef..369d6d16 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -16,7 +16,7 @@ 0.10.1 - 1.6.10 + 1.6.20-M1 true 1.8 com.facebook.ktfmt.cli.Main diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index 814c3db8..22fe7bcc 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -52,6 +52,7 @@ import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression import org.jetbrains.kotlin.psi.KtConstantExpression import org.jetbrains.kotlin.psi.KtConstructorDelegationCall import org.jetbrains.kotlin.psi.KtContainerNode +import org.jetbrains.kotlin.psi.KtContextReceiverList import org.jetbrains.kotlin.psi.KtContinueExpression import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry import org.jetbrains.kotlin.psi.KtDestructuringDeclaration @@ -94,6 +95,7 @@ import org.jetbrains.kotlin.psi.KtQualifiedExpression import org.jetbrains.kotlin.psi.KtReferenceExpression import org.jetbrains.kotlin.psi.KtReturnExpression import org.jetbrains.kotlin.psi.KtScript +import org.jetbrains.kotlin.psi.KtScriptInitializer import org.jetbrains.kotlin.psi.KtSecondaryConstructor import org.jetbrains.kotlin.psi.KtSimpleNameExpression import org.jetbrains.kotlin.psi.KtStringTemplateExpression @@ -124,6 +126,7 @@ import org.jetbrains.kotlin.psi.psiUtil.children import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespace import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.psi.psiUtil.startsWithComment +import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes /** An AST visitor that builds a stream of {@link Op}s to format. */ class KotlinInputAstVisitor( @@ -162,6 +165,7 @@ class KotlinInputAstVisitor( builder.sync(function) builder.block(ZERO) { visitFunctionLikeExpression( + function.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), function.modifierList, "fun", function.typeParameterList, @@ -282,6 +286,7 @@ class KotlinInputAstVisitor( * list of supertypes. */ private fun visitFunctionLikeExpression( + contextReceiverList: KtContextReceiverList?, modifierList: KtModifierList?, keyword: String, typeParameters: KtTypeParameterList?, @@ -294,6 +299,9 @@ class KotlinInputAstVisitor( typeOrDelegationCall: KtElement?, ) { builder.block(ZERO) { + if (contextReceiverList != null) { + visitContextReceiverList(contextReceiverList) + } if (modifierList != null) { visitModifierList(modifierList) } @@ -1372,6 +1380,7 @@ class KotlinInputAstVisitor( builder.block(ZERO) { visitFunctionLikeExpression( + null, accessor.modifierList, accessor.namePlaceholder.text, null, @@ -1461,8 +1470,12 @@ class KotlinInputAstVisitor( override fun visitClassOrObject(classOrObject: KtClassOrObject) { builder.sync(classOrObject) + val contextReceiverList = classOrObject.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST) val modifierList = classOrObject.modifierList builder.block(ZERO) { + if (contextReceiverList != null) { + visitContextReceiverList(contextReceiverList) + } if (modifierList != null) { visitModifierList(modifierList) } @@ -1533,6 +1546,7 @@ class KotlinInputAstVisitor( val delegationCall = constructor.getDelegationCall() visitFunctionLikeExpression( + constructor.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), constructor.modifierList, "constructor", null, @@ -1638,6 +1652,20 @@ class KotlinInputAstVisitor( builder.forcedBreak() } + /** Example `context(Logger, Raise)` */ + override fun visitContextReceiverList(contextReceiverList: KtContextReceiverList) { + builder.sync(contextReceiverList) + builder.token("context") + visitEachCommaSeparated( + contextReceiverList.contextReceivers(), + prefix = "(", + postfix = ")", + breakAfterPrefix = false, + breakBeforePostfix = false + ) + builder.forcedBreak() + } + /** For example `@Magic private final` */ override fun visitModifierList(list: KtModifierList) { builder.sync(list) @@ -2427,6 +2455,7 @@ class KotlinInputAstVisitor( override fun visitScript(script: KtScript) { markForPartialFormat() var lastChildHadBlankLineBefore = false + var lastChildIsContextReceiver = false var first = true for (child in script.blockExpression.children) { if (child.text.isBlank()) { @@ -2436,6 +2465,8 @@ class KotlinInputAstVisitor( val childGetsBlankLineBefore = child !is KtProperty if (first) { builder.blankLineWanted(OpsBuilder.BlankLineWanted.PRESERVE) + } else if (lastChildIsContextReceiver) { + builder.blankLineWanted(OpsBuilder.BlankLineWanted.NO) } else if (child !is PsiComment && (childGetsBlankLineBefore || lastChildHadBlankLineBefore)) { builder.blankLineWanted(OpsBuilder.BlankLineWanted.YES) @@ -2443,6 +2474,7 @@ class KotlinInputAstVisitor( visit(child) builder.guessToken(";") lastChildHadBlankLineBefore = childGetsBlankLineBefore + lastChildIsContextReceiver = child is KtScriptInitializer && child.firstChild?.firstChild?.firstChild?.text == "context" first = false } markForPartialFormat() diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index 02394608..42acdf67 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -6764,6 +6764,48 @@ class FormatterTest { assertThatFormatting(code).isEqualTo(expected) } + @Test + fun `context receivers`() { + val code = + """ + |context(Something) + | + |class A { + | context( + | // Test comment. + | Logger, Raise) + | + | @SomeAnnotation + | + | fun doNothing() {} + | + | context(SomethingElse) + | + | private class NestedClass {} + |} + |""" + .trimMargin() + + val expected = + """ + |context(Something) + |class A { + | context( + | // Test comment. + | Logger, + | Raise) + | @SomeAnnotation + | fun doNothing() {} + | + | context(SomethingElse) + | private class NestedClass {} + |} + |""" + .trimMargin() + + assertThatFormatting(code).isEqualTo(expected) + } + companion object { /** Triple quotes, useful to use within triple-quoted strings. */ private const val TQ = "\"\"\"" diff --git a/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt b/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt index 3f8fe71e..e5741d25 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt @@ -110,4 +110,28 @@ class TokenizerTest { .containsExactly(0, -1, 1, 2, 3, -1, 4, -1, 5, 6, 7) .inOrder() } + + @Test + fun `Context receivers are parsed correctly`() { + val code = """ + |context(Something) + |class A { + | context( + | // Test comment. + | Logger, Raise) + | fun test() {} + |} + |""".trimMargin().trimMargin() + + val file = Parser.parse(code) + val tokenizer = Tokenizer(code, file) + file.accept(tokenizer) + + assertThat(tokenizer.toks.map { it.originalText }) + .containsExactly("context", "(", "Something", ")", "\n", "class", " ", "A", " ", "{", "\n", " ", "context", "(", "\n", " ", "// Test comment.", "\n", " ", "Logger", ",", " ", "Raise", "<", "Error", ">", ")", "\n", " ", "fun", " ", "test", "(", ")", " ", "{", "}", "\n", "}") + .inOrder() + assertThat(tokenizer.toks.map { it.index }) + .containsExactly(0, 1, 2, 3, -1, 4, -1, 5, -1, 6, -1, -1, 7, 8, -1, -1, 9, -1, -1, 10, 11, -1, 12, 13, 14, 15, 16, -1, -1, 17, -1, 18, 19, 20, -1, 21, 22, -1, 23) + .inOrder() + } } From 2e52a655c46cd3a664cc0f2d288356a7c9409083 Mon Sep 17 00:00:00 2001 From: Nate Stedman Date: Thu, 14 Sep 2023 00:31:18 -0700 Subject: [PATCH 04/20] Remove TypeNameClassifier Summary: This code is not used. Reviewed By: hick209 Differential Revision: D49261409 fbshipit-source-id: 22f54d4b56035e600c97d910a84d399576b6d2bd --- .../ktfmt/format/TypeNameClassifier.kt | 147 ------------------ 1 file changed, 147 deletions(-) delete mode 100644 core/src/main/java/com/facebook/ktfmt/format/TypeNameClassifier.kt diff --git a/core/src/main/java/com/facebook/ktfmt/format/TypeNameClassifier.kt b/core/src/main/java/com/facebook/ktfmt/format/TypeNameClassifier.kt deleted file mode 100644 index 76fac3e2..00000000 --- a/core/src/main/java/com/facebook/ktfmt/format/TypeNameClassifier.kt +++ /dev/null @@ -1,147 +0,0 @@ -/* - * Copyright 2015 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -// This was copied from https://github.com/google/google-java-format and converted to Kotlin, -// because the original is package-private. - -package com.facebook.ktfmt.format - -import com.google.common.base.Verify -import java.util.Optional - -/** Heuristics for classifying qualified names as types. */ -object TypeNameClassifier { - - /** A state machine for classifying qualified names. */ - private enum class TyParseState(val isSingleUnit: Boolean) { - - /** The start state. */ - START(false) { - override fun next(n: JavaCaseFormat): TyParseState { - return when (n) { - JavaCaseFormat.UPPERCASE -> - // if we see an UpperCamel later, assume this was a class - // e.g. com.google.FOO.Bar - AMBIGUOUS - JavaCaseFormat.LOWER_CAMEL -> REJECT - JavaCaseFormat.LOWERCASE -> - // could be a package - START - JavaCaseFormat.UPPER_CAMEL -> TYPE - } - } - }, - - /** The current prefix is a type. */ - TYPE(true) { - override fun next(n: JavaCaseFormat): TyParseState { - return when (n) { - JavaCaseFormat.UPPERCASE, - JavaCaseFormat.LOWER_CAMEL, - JavaCaseFormat.LOWERCASE -> FIRST_STATIC_MEMBER - JavaCaseFormat.UPPER_CAMEL -> TYPE - } - } - }, - - /** The current prefix is a type, followed by a single static member access. */ - FIRST_STATIC_MEMBER(true) { - override fun next(n: JavaCaseFormat): TyParseState { - return REJECT - } - }, - - /** Anything not represented by one of the other states. */ - REJECT(false) { - override fun next(n: JavaCaseFormat): TyParseState { - return REJECT - } - }, - - /** An ambiguous type prefix. */ - AMBIGUOUS(false) { - override fun next(n: JavaCaseFormat): TyParseState { - return when (n) { - JavaCaseFormat.UPPERCASE -> AMBIGUOUS - JavaCaseFormat.LOWER_CAMEL, - JavaCaseFormat.LOWERCASE -> REJECT - JavaCaseFormat.UPPER_CAMEL -> TYPE - } - } - }; - - /** Transition function. */ - abstract fun next(n: JavaCaseFormat): TyParseState - } - - /** - * Returns the end index (inclusive) of the longest prefix that matches the naming conventions of - * a type or static field access, or -1 if no such prefix was found. - * - * Examples: - * * ClassName - * * ClassName.staticMemberName - * * com.google.ClassName.InnerClass.staticMemberName - */ - internal fun typePrefixLength(nameParts: List): Optional { - var state = TyParseState.START - var typeLength = Optional.empty() - for (i in nameParts.indices) { - state = state.next(JavaCaseFormat.from(nameParts[i])) - if (state === TyParseState.REJECT) { - break - } - if (state.isSingleUnit) { - typeLength = Optional.of(i) - } - } - return typeLength - } - - /** Case formats used in Java identifiers. */ - enum class JavaCaseFormat { - UPPERCASE, - LOWERCASE, - UPPER_CAMEL, - LOWER_CAMEL; - - companion object { - - /** Classifies an identifier's case format. */ - internal fun from(name: String): JavaCaseFormat { - Verify.verify(name.isNotEmpty()) - var firstUppercase = false - var hasUppercase = false - var hasLowercase = false - var first = true - for (char in name) { - if (!Character.isAlphabetic(char.code)) { - continue - } - if (first) { - firstUppercase = Character.isUpperCase(char) - first = false - } - hasUppercase = hasUppercase or Character.isUpperCase(char) - hasLowercase = hasLowercase or Character.isLowerCase(char) - } - return if (firstUppercase) { - if (hasLowercase) UPPER_CAMEL else UPPERCASE - } else { - if (hasUppercase) LOWER_CAMEL else LOWERCASE - } - } - } - } -} From a06e35330e75cf39c895ae12961b250fa2ba251f Mon Sep 17 00:00:00 2001 From: generatedunixname89002005325672 Date: Thu, 14 Sep 2023 07:46:14 -0700 Subject: [PATCH 05/20] Daily `arc lint --take KTFMT` Reviewed By: ivanmurashko Differential Revision: D49274915 fbshipit-source-id: 405a79b9019dae90cd3f2d9e62220df3d9dd942f --- .../ktfmt/format/KotlinInputAstVisitor.kt | 18 ++-- .../facebook/ktfmt/format/TokenizerTest.kt | 93 +++++++++++++++++-- 2 files changed, 97 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index 22fe7bcc..eabe6658 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -1470,7 +1470,8 @@ class KotlinInputAstVisitor( override fun visitClassOrObject(classOrObject: KtClassOrObject) { builder.sync(classOrObject) - val contextReceiverList = classOrObject.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST) + val contextReceiverList = + classOrObject.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST) val modifierList = classOrObject.modifierList builder.block(ZERO) { if (contextReceiverList != null) { @@ -1657,12 +1658,11 @@ class KotlinInputAstVisitor( builder.sync(contextReceiverList) builder.token("context") visitEachCommaSeparated( - contextReceiverList.contextReceivers(), - prefix = "(", - postfix = ")", - breakAfterPrefix = false, - breakBeforePostfix = false - ) + contextReceiverList.contextReceivers(), + prefix = "(", + postfix = ")", + breakAfterPrefix = false, + breakBeforePostfix = false) builder.forcedBreak() } @@ -2474,7 +2474,9 @@ class KotlinInputAstVisitor( visit(child) builder.guessToken(";") lastChildHadBlankLineBefore = childGetsBlankLineBefore - lastChildIsContextReceiver = child is KtScriptInitializer && child.firstChild?.firstChild?.firstChild?.text == "context" + lastChildIsContextReceiver = + child is KtScriptInitializer && + child.firstChild?.firstChild?.firstChild?.text == "context" first = false } markForPartialFormat() diff --git a/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt b/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt index e5741d25..27b18377 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/TokenizerTest.kt @@ -113,7 +113,8 @@ class TokenizerTest { @Test fun `Context receivers are parsed correctly`() { - val code = """ + val code = + """ |context(Something) |class A { | context( @@ -121,17 +122,97 @@ class TokenizerTest { | Logger, Raise) | fun test() {} |} - |""".trimMargin().trimMargin() + |""" + .trimMargin() + .trimMargin() val file = Parser.parse(code) val tokenizer = Tokenizer(code, file) file.accept(tokenizer) assertThat(tokenizer.toks.map { it.originalText }) - .containsExactly("context", "(", "Something", ")", "\n", "class", " ", "A", " ", "{", "\n", " ", "context", "(", "\n", " ", "// Test comment.", "\n", " ", "Logger", ",", " ", "Raise", "<", "Error", ">", ")", "\n", " ", "fun", " ", "test", "(", ")", " ", "{", "}", "\n", "}") - .inOrder() + .containsExactly( + "context", + "(", + "Something", + ")", + "\n", + "class", + " ", + "A", + " ", + "{", + "\n", + " ", + "context", + "(", + "\n", + " ", + "// Test comment.", + "\n", + " ", + "Logger", + ",", + " ", + "Raise", + "<", + "Error", + ">", + ")", + "\n", + " ", + "fun", + " ", + "test", + "(", + ")", + " ", + "{", + "}", + "\n", + "}") + .inOrder() assertThat(tokenizer.toks.map { it.index }) - .containsExactly(0, 1, 2, 3, -1, 4, -1, 5, -1, 6, -1, -1, 7, 8, -1, -1, 9, -1, -1, 10, 11, -1, 12, 13, 14, 15, 16, -1, -1, 17, -1, 18, 19, 20, -1, 21, 22, -1, 23) - .inOrder() + .containsExactly( + 0, + 1, + 2, + 3, + -1, + 4, + -1, + 5, + -1, + 6, + -1, + -1, + 7, + 8, + -1, + -1, + 9, + -1, + -1, + 10, + 11, + -1, + 12, + 13, + 14, + 15, + 16, + -1, + -1, + 17, + -1, + 18, + 19, + 20, + -1, + 21, + 22, + -1, + 23) + .inOrder() } } From ccc2b74d3cc99e348988c36856149bb0aa1957b6 Mon Sep 17 00:00:00 2001 From: nickreid Date: Thu, 14 Sep 2023 09:27:35 -0700 Subject: [PATCH 06/20] Fix double indentation in Elvis chains (#416) Summary: This is consistent with other binary operators, and prevents a strange second indent in Elvis chains. Pull Request resolved: https://github.com/facebook/ktfmt/pull/416 Reviewed By: strulovich Differential Revision: D49260663 Pulled By: hick209 fbshipit-source-id: 21feb4fb57700bb93ba01aec43a2e7da7d2afa57 --- .../ktfmt/format/KotlinInputAstVisitor.kt | 35 +++++++++----- .../facebook/ktfmt/format/FormatterTest.kt | 46 +++++++++++++++++++ 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index eabe6658..941eb2e5 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -1207,20 +1207,31 @@ class KotlinInputAstVisitor( val leftMostExpression = parts.first() visit(leftMostExpression.left) for (leftExpression in parts) { - when (leftExpression.operationToken) { - KtTokens.RANGE -> {} - KtTokens.ELVIS -> builder.breakOp(Doc.FillMode.INDEPENDENT, " ", expressionBreakIndent) - else -> builder.space() - } - builder.token(leftExpression.operationReference.text) val isFirst = leftExpression === leftMostExpression - if (isFirst) { - builder.open(expressionBreakIndent) - } + when (leftExpression.operationToken) { - KtTokens.RANGE -> {} - KtTokens.ELVIS -> builder.space() - else -> builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO) + KtTokens.RANGE -> { + if (isFirst) { + builder.open(expressionBreakIndent) + } + builder.token(leftExpression.operationReference.text) + } + KtTokens.ELVIS -> { + if (isFirst) { + builder.open(expressionBreakIndent) + } + builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO) + builder.token(leftExpression.operationReference.text) + builder.space() + } + else -> { + builder.space() + if (isFirst) { + builder.open(expressionBreakIndent) + } + builder.token(leftExpression.operationReference.text) + builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO) + } } visit(leftExpression.right) } diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index 42acdf67..4358c34d 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -4379,6 +4379,52 @@ class FormatterTest { .trimMargin(), deduceMaxWidth = true) + @Test + fun `chain of Elvis operator`() = + assertFormatted( + """ + |--------------------------- + |fun f() { + | return option1() + | ?: option2() + | ?: option3() + | ?: option4() + | ?: option5() + |} + |""" + .trimMargin(), + deduceMaxWidth = true) + + @Test + fun `Elvis operator mixed with plus operator breaking on plus`() = + assertFormatted( + """ + |------------------------ + |fun f() { + | return option1() + | ?: option2() + + | option3() + | ?: option4() + + | option5() + |} + |""" + .trimMargin(), + deduceMaxWidth = true) + + @Test + fun `Elvis operator mixed with plus operator breaking on elvis`() = + assertFormatted( + """ + |--------------------------------- + |fun f() { + | return option1() + | ?: option2() + option3() + | ?: option4() + option5() + |} + |""" + .trimMargin(), + deduceMaxWidth = true) + @Test fun `handle comments in the middle of calling chain`() = assertFormatted( From b1d2c1bb4a4209fa824d85f9b04586ad84c7121a Mon Sep 17 00:00:00 2001 From: Nate Stedman Date: Thu, 14 Sep 2023 15:56:58 -0700 Subject: [PATCH 07/20] Improve argsfile support Summary: Currently `ktfmt` only supports argsfiles when a single one is provided, and no other flags are provided. However, you might want to use the argsfile only for the file list arguments (of unknown length), but use normal CLI arguments for passing options. Improve `ktfmt` to allow argsfiles anywhere in the argument list by parsing arguments recursively if an argsfile (`@` prefix) is found. Reviewed By: hick209 Differential Revision: D49261046 fbshipit-source-id: 02c1e3dbad58fa9c5a74f2f635c842a13adfc9f6 --- .../java/com/facebook/ktfmt/cli/ParsedArgs.kt | 42 ++++++------ .../com/facebook/ktfmt/cli/ParsedArgsTest.kt | 65 ++++++++++++------- 2 files changed, 63 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt index 2c849722..9fee6d0a 100644 --- a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt +++ b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt @@ -38,16 +38,8 @@ data class ParsedArgs( ) { companion object { + /** processArgs parses command-line arguments passed to ktfmt. */ fun processArgs(err: PrintStream, args: Array): ParsedArgs { - if (args.size == 1 && args[0].startsWith("@")) { - return parseOptions(err, File(args[0].substring(1)).readLines().toTypedArray()) - } else { - return parseOptions(err, args) - } - } - - /** parseOptions parses command-line arguments passed to ktfmt. */ - fun parseOptions(err: PrintStream, args: Array): ParsedArgs { val fileNames = mutableListOf() var formattingOptions = FormattingOptions() var dryRun = false @@ -55,21 +47,25 @@ data class ParsedArgs( var removeUnusedImports = true var stdinName: String? = null - for (arg in args) { - when { - arg == "--dropbox-style" -> formattingOptions = Formatter.DROPBOX_FORMAT - arg == "--google-style" -> formattingOptions = Formatter.GOOGLE_FORMAT - arg == "--kotlinlang-style" -> formattingOptions = Formatter.KOTLINLANG_FORMAT - arg == "--dry-run" || arg == "-n" -> dryRun = true - arg == "--set-exit-if-changed" -> setExitIfChanged = true - arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false - arg.startsWith("--stdin-name") -> stdinName = parseKeyValueArg(err, "--stdin-name", arg) - arg.startsWith("--") -> err.println("Unexpected option: $arg") - arg.startsWith("@") -> err.println("Unexpected option: $arg") - else -> fileNames.add(arg) + fun parseArgs(args: Array) { + for (arg in args) { + when { + arg == "--dropbox-style" -> formattingOptions = Formatter.DROPBOX_FORMAT + arg == "--google-style" -> formattingOptions = Formatter.GOOGLE_FORMAT + arg == "--kotlinlang-style" -> formattingOptions = Formatter.KOTLINLANG_FORMAT + arg == "--dry-run" || arg == "-n" -> dryRun = true + arg == "--set-exit-if-changed" -> setExitIfChanged = true + arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false + arg.startsWith("--stdin-name") -> stdinName = parseKeyValueArg(err, "--stdin-name", arg) + arg.startsWith("--") -> err.println("Unexpected option: $arg") + arg.startsWith("@") -> parseArgs(readArgsFile(arg)) + else -> fileNames.add(arg) + } } } + parseArgs(args) + return ParsedArgs( fileNames, formattingOptions.copy(removeUnusedImports = removeUnusedImports), @@ -87,5 +83,9 @@ data class ParsedArgs( } return parts[1] } + + /** Reads an argument file prefixed with "@" */ + private fun readArgsFile(arg: String): Array = + File(arg.substring(1)).readLines().toTypedArray() } } diff --git a/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt b/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt index 9f0919bf..08930f3c 100644 --- a/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt @@ -49,15 +49,7 @@ class ParsedArgsTest { } @Test - fun `files to format are returned and flags starting with @ are reported`() { - val (parsed, out) = parseTestOptions("foo.kt", "@unknown") - - assertThat(parsed.fileNames).containsExactly("foo.kt") - assertThat(out.toString()).isEqualTo("Unexpected option: @unknown\n") - } - - @Test - fun `parseOptions uses default values when args are empty`() { + fun `processArgs uses default values when args are empty`() { val (parsed, _) = parseTestOptions("foo.kt") val formattingOptions = parsed.formattingOptions @@ -74,7 +66,7 @@ class ParsedArgsTest { } @Test - fun `parseOptions recognizes --dropbox-style and rejects unknown flags`() { + fun `processArgs recognizes --dropbox-style and rejects unknown flags`() { val (parsed, out) = parseTestOptions("--dropbox-style", "foo.kt", "--unknown") assertThat(parsed.fileNames).containsExactly("foo.kt") @@ -84,43 +76,43 @@ class ParsedArgsTest { } @Test - fun `parseOptions recognizes --google-style`() { + fun `processArgs recognizes --google-style`() { val (parsed, _) = parseTestOptions("--google-style", "foo.kt") assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT) } @Test - fun `parseOptions recognizes --dry-run`() { + fun `processArgs recognizes --dry-run`() { val (parsed, _) = parseTestOptions("--dry-run", "foo.kt") assertThat(parsed.dryRun).isTrue() } @Test - fun `parseOptions recognizes -n as --dry-run`() { + fun `processArgs recognizes -n as --dry-run`() { val (parsed, _) = parseTestOptions("-n", "foo.kt") assertThat(parsed.dryRun).isTrue() } @Test - fun `parseOptions recognizes --set-exit-if-changed`() { + fun `processArgs recognizes --set-exit-if-changed`() { val (parsed, _) = parseTestOptions("--set-exit-if-changed", "foo.kt") assertThat(parsed.setExitIfChanged).isTrue() } @Test - fun `parseOptions defaults to removing imports`() { + fun `processArgs defaults to removing imports`() { val (parsed, _) = parseTestOptions("foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isTrue() } @Test - fun `parseOptions recognizes --do-not-remove-unused-imports to removing imports`() { + fun `processArgs recognizes --do-not-remove-unused-imports to removing imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() } @Test - fun `parseOptions handles dropbox style and --do-not-remove-unused-imports`() { + fun `processArgs handles dropbox style and --do-not-remove-unused-imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "--dropbox-style", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() @@ -128,33 +120,33 @@ class ParsedArgsTest { } @Test - fun `parseOptions handles google style and --do-not-remove-unused-imports`() { + fun `processArgs handles google style and --do-not-remove-unused-imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "--google-style", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() assertThat(parsed.formattingOptions.style).isEqualTo(FormattingOptions.Style.GOOGLE) } @Test - fun `parseOptions --stdin-name`() { + fun `processArgs --stdin-name`() { val (parsed, _) = parseTestOptions("--stdin-name=my/foo.kt") assertThat(parsed.stdinName).isEqualTo("my/foo.kt") } @Test - fun `parseOptions --stdin-name with empty value`() { + fun `processArgs --stdin-name with empty value`() { val (parsed, _) = parseTestOptions("--stdin-name=") assertThat(parsed.stdinName).isEqualTo("") } @Test - fun `parseOptions --stdin-name without value`() { + fun `processArgs --stdin-name without value`() { val (parsed, out) = parseTestOptions("--stdin-name") assertThat(out).isEqualTo("Found option '--stdin-name', expected '--stdin-name='\n") assertThat(parsed.stdinName).isNull() } @Test - fun `parseOptions --stdin-name prefix`() { + fun `processArgs --stdin-name prefix`() { val (parsed, out) = parseTestOptions("--stdin-namea") assertThat(out).isEqualTo("Found option '--stdin-namea', expected '--stdin-name='\n") assertThat(parsed.stdinName).isNull() @@ -185,8 +177,35 @@ class ParsedArgsTest { assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) } + @Test + fun `processArgs use normal arguments and the @file option with file containing arguments`() { + val out = ByteArrayOutputStream() + val file = root.resolve("existing-file") + file.writeText("--set-exit-if-changed\nFile1.kt\nFile2.kt\n") + + val parsed = + ParsedArgs.processArgs( + PrintStream(out), arrayOf("--google-style", "--dry-run", "@" + file.absolutePath)) + + assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT) + assertThat(parsed.dryRun).isTrue() + assertThat(parsed.setExitIfChanged).isTrue() + assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) + } + + @Test + fun `processArgs use the @file option recursively`() { + val out = ByteArrayOutputStream() + val inner = root.resolve("inner").apply { writeText("File1.kt\nFile2.kt\n") } + val outer = root.resolve("outer").apply { writeText("@${inner.absolutePath}") } + + val parsed = ParsedArgs.processArgs(PrintStream(out), arrayOf("@" + outer.absolutePath)) + + assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) + } + private fun parseTestOptions(vararg args: String): Pair { val out = ByteArrayOutputStream() - return Pair(ParsedArgs.parseOptions(PrintStream(out), arrayOf(*args)), out.toString()) + return Pair(ParsedArgs.processArgs(PrintStream(out), arrayOf(*args)), out.toString()) } } From ce7997e8b0184f79505a0c606c69387b83e04840 Mon Sep 17 00:00:00 2001 From: Nate Stedman Date: Fri, 15 Sep 2023 07:06:29 -0700 Subject: [PATCH 08/20] Back out "Improve argsfile support" Reviewed By: hick209 Differential Revision: D49316904 fbshipit-source-id: fa0edb8b13a42c708ac847e5edcca470f62d4eaa --- .../java/com/facebook/ktfmt/cli/ParsedArgs.kt | 42 ++++++------ .../com/facebook/ktfmt/cli/ParsedArgsTest.kt | 65 +++++++------------ 2 files changed, 44 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt index 9fee6d0a..2c849722 100644 --- a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt +++ b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt @@ -38,8 +38,16 @@ data class ParsedArgs( ) { companion object { - /** processArgs parses command-line arguments passed to ktfmt. */ fun processArgs(err: PrintStream, args: Array): ParsedArgs { + if (args.size == 1 && args[0].startsWith("@")) { + return parseOptions(err, File(args[0].substring(1)).readLines().toTypedArray()) + } else { + return parseOptions(err, args) + } + } + + /** parseOptions parses command-line arguments passed to ktfmt. */ + fun parseOptions(err: PrintStream, args: Array): ParsedArgs { val fileNames = mutableListOf() var formattingOptions = FormattingOptions() var dryRun = false @@ -47,25 +55,21 @@ data class ParsedArgs( var removeUnusedImports = true var stdinName: String? = null - fun parseArgs(args: Array) { - for (arg in args) { - when { - arg == "--dropbox-style" -> formattingOptions = Formatter.DROPBOX_FORMAT - arg == "--google-style" -> formattingOptions = Formatter.GOOGLE_FORMAT - arg == "--kotlinlang-style" -> formattingOptions = Formatter.KOTLINLANG_FORMAT - arg == "--dry-run" || arg == "-n" -> dryRun = true - arg == "--set-exit-if-changed" -> setExitIfChanged = true - arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false - arg.startsWith("--stdin-name") -> stdinName = parseKeyValueArg(err, "--stdin-name", arg) - arg.startsWith("--") -> err.println("Unexpected option: $arg") - arg.startsWith("@") -> parseArgs(readArgsFile(arg)) - else -> fileNames.add(arg) - } + for (arg in args) { + when { + arg == "--dropbox-style" -> formattingOptions = Formatter.DROPBOX_FORMAT + arg == "--google-style" -> formattingOptions = Formatter.GOOGLE_FORMAT + arg == "--kotlinlang-style" -> formattingOptions = Formatter.KOTLINLANG_FORMAT + arg == "--dry-run" || arg == "-n" -> dryRun = true + arg == "--set-exit-if-changed" -> setExitIfChanged = true + arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false + arg.startsWith("--stdin-name") -> stdinName = parseKeyValueArg(err, "--stdin-name", arg) + arg.startsWith("--") -> err.println("Unexpected option: $arg") + arg.startsWith("@") -> err.println("Unexpected option: $arg") + else -> fileNames.add(arg) } } - parseArgs(args) - return ParsedArgs( fileNames, formattingOptions.copy(removeUnusedImports = removeUnusedImports), @@ -83,9 +87,5 @@ data class ParsedArgs( } return parts[1] } - - /** Reads an argument file prefixed with "@" */ - private fun readArgsFile(arg: String): Array = - File(arg.substring(1)).readLines().toTypedArray() } } diff --git a/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt b/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt index 08930f3c..9f0919bf 100644 --- a/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt @@ -49,7 +49,15 @@ class ParsedArgsTest { } @Test - fun `processArgs uses default values when args are empty`() { + fun `files to format are returned and flags starting with @ are reported`() { + val (parsed, out) = parseTestOptions("foo.kt", "@unknown") + + assertThat(parsed.fileNames).containsExactly("foo.kt") + assertThat(out.toString()).isEqualTo("Unexpected option: @unknown\n") + } + + @Test + fun `parseOptions uses default values when args are empty`() { val (parsed, _) = parseTestOptions("foo.kt") val formattingOptions = parsed.formattingOptions @@ -66,7 +74,7 @@ class ParsedArgsTest { } @Test - fun `processArgs recognizes --dropbox-style and rejects unknown flags`() { + fun `parseOptions recognizes --dropbox-style and rejects unknown flags`() { val (parsed, out) = parseTestOptions("--dropbox-style", "foo.kt", "--unknown") assertThat(parsed.fileNames).containsExactly("foo.kt") @@ -76,43 +84,43 @@ class ParsedArgsTest { } @Test - fun `processArgs recognizes --google-style`() { + fun `parseOptions recognizes --google-style`() { val (parsed, _) = parseTestOptions("--google-style", "foo.kt") assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT) } @Test - fun `processArgs recognizes --dry-run`() { + fun `parseOptions recognizes --dry-run`() { val (parsed, _) = parseTestOptions("--dry-run", "foo.kt") assertThat(parsed.dryRun).isTrue() } @Test - fun `processArgs recognizes -n as --dry-run`() { + fun `parseOptions recognizes -n as --dry-run`() { val (parsed, _) = parseTestOptions("-n", "foo.kt") assertThat(parsed.dryRun).isTrue() } @Test - fun `processArgs recognizes --set-exit-if-changed`() { + fun `parseOptions recognizes --set-exit-if-changed`() { val (parsed, _) = parseTestOptions("--set-exit-if-changed", "foo.kt") assertThat(parsed.setExitIfChanged).isTrue() } @Test - fun `processArgs defaults to removing imports`() { + fun `parseOptions defaults to removing imports`() { val (parsed, _) = parseTestOptions("foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isTrue() } @Test - fun `processArgs recognizes --do-not-remove-unused-imports to removing imports`() { + fun `parseOptions recognizes --do-not-remove-unused-imports to removing imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() } @Test - fun `processArgs handles dropbox style and --do-not-remove-unused-imports`() { + fun `parseOptions handles dropbox style and --do-not-remove-unused-imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "--dropbox-style", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() @@ -120,33 +128,33 @@ class ParsedArgsTest { } @Test - fun `processArgs handles google style and --do-not-remove-unused-imports`() { + fun `parseOptions handles google style and --do-not-remove-unused-imports`() { val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "--google-style", "foo.kt") assertThat(parsed.formattingOptions.removeUnusedImports).isFalse() assertThat(parsed.formattingOptions.style).isEqualTo(FormattingOptions.Style.GOOGLE) } @Test - fun `processArgs --stdin-name`() { + fun `parseOptions --stdin-name`() { val (parsed, _) = parseTestOptions("--stdin-name=my/foo.kt") assertThat(parsed.stdinName).isEqualTo("my/foo.kt") } @Test - fun `processArgs --stdin-name with empty value`() { + fun `parseOptions --stdin-name with empty value`() { val (parsed, _) = parseTestOptions("--stdin-name=") assertThat(parsed.stdinName).isEqualTo("") } @Test - fun `processArgs --stdin-name without value`() { + fun `parseOptions --stdin-name without value`() { val (parsed, out) = parseTestOptions("--stdin-name") assertThat(out).isEqualTo("Found option '--stdin-name', expected '--stdin-name='\n") assertThat(parsed.stdinName).isNull() } @Test - fun `processArgs --stdin-name prefix`() { + fun `parseOptions --stdin-name prefix`() { val (parsed, out) = parseTestOptions("--stdin-namea") assertThat(out).isEqualTo("Found option '--stdin-namea', expected '--stdin-name='\n") assertThat(parsed.stdinName).isNull() @@ -177,35 +185,8 @@ class ParsedArgsTest { assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) } - @Test - fun `processArgs use normal arguments and the @file option with file containing arguments`() { - val out = ByteArrayOutputStream() - val file = root.resolve("existing-file") - file.writeText("--set-exit-if-changed\nFile1.kt\nFile2.kt\n") - - val parsed = - ParsedArgs.processArgs( - PrintStream(out), arrayOf("--google-style", "--dry-run", "@" + file.absolutePath)) - - assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT) - assertThat(parsed.dryRun).isTrue() - assertThat(parsed.setExitIfChanged).isTrue() - assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) - } - - @Test - fun `processArgs use the @file option recursively`() { - val out = ByteArrayOutputStream() - val inner = root.resolve("inner").apply { writeText("File1.kt\nFile2.kt\n") } - val outer = root.resolve("outer").apply { writeText("@${inner.absolutePath}") } - - val parsed = ParsedArgs.processArgs(PrintStream(out), arrayOf("@" + outer.absolutePath)) - - assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt")) - } - private fun parseTestOptions(vararg args: String): Pair { val out = ByteArrayOutputStream() - return Pair(ParsedArgs.processArgs(PrintStream(out), arrayOf(*args)), out.toString()) + return Pair(ParsedArgs.parseOptions(PrintStream(out), arrayOf(*args)), out.toString()) } } From 2be7c7e7d3a8fa466a0d9c08616980727eafb87f Mon Sep 17 00:00:00 2001 From: Nate Stedman Date: Fri, 15 Sep 2023 07:19:43 -0700 Subject: [PATCH 09/20] Update ktfmt component on FBS:master Reviewed By: hick209 Differential Revision: D49300079 fbshipit-source-id: 7e704e2366feee3f6f2c25ac25d3daea407e71f9 --- .../main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt b/core/src/main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt index 928a7868..3d26e8d4 100644 --- a/core/src/main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt +++ b/core/src/main/java/com/facebook/ktfmt/kdoc/ParagraphListBuilder.kt @@ -232,7 +232,7 @@ class ParagraphListBuilder( } if (lineWithIndentation.startsWith(" ") && // markdown preformatted text - (i == 1 || lineContent(lines[i - 2]).isBlank()) && // we've already ++'ed i above + (i == 1 || lineContent(lines[i - 2]).isBlank()) && // we've already ++'ed i above // Make sure it's not just deeply indented inside a different block (paragraph.prev == null || lineWithIndentation.length - lineWithoutIndentation.length >= From 9c06ac168c415cf09fa49e800255fd7a441ebdea Mon Sep 17 00:00:00 2001 From: nickreid Date: Fri, 15 Sep 2023 08:00:09 -0700 Subject: [PATCH 10/20] Use inExpression in a nullsafe way (#417) Summary: peekLast is a JDK method using platform types for the return value. Pull Request resolved: https://github.com/facebook/ktfmt/pull/417 Reviewed By: strulovich Differential Revision: D49260503 Pulled By: hick209 fbshipit-source-id: 3a113bebbe32b180ebaf30e1718bcaf7e69ca52b --- .../facebook/ktfmt/format/KotlinInputAstVisitor.kt | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index 941eb2e5..80093d0c 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -1673,7 +1673,8 @@ class KotlinInputAstVisitor( prefix = "(", postfix = ")", breakAfterPrefix = false, - breakBeforePostfix = false) + breakBeforePostfix = false, + ) builder.forcedBreak() } @@ -2425,7 +2426,7 @@ class KotlinInputAstVisitor( * @throws FormattingError */ override fun visitElement(element: PsiElement) { - inExpression.addLast(element is KtExpression || inExpression.peekLast()) + inExpression.addLast(element is KtExpression || inExpression.last()) val previous = builder.depth() try { super.visitElement(element) @@ -2493,10 +2494,6 @@ class KotlinInputAstVisitor( markForPartialFormat() } - private fun inExpression(): Boolean { - return inExpression.peekLast() - } - /** * markForPartialFormat is used to delineate the smallest areas of code that must be formatted * together. @@ -2505,7 +2502,7 @@ class KotlinInputAstVisitor( * covered by an area marked by this method. */ private fun markForPartialFormat() { - if (!inExpression()) { + if (!inExpression.last()) { builder.markForPartialFormat() } } From 1f18a40a04247576fe8b04aa971053d132932660 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 15 Sep 2023 11:25:27 -0700 Subject: [PATCH 11/20] Bump word-wrap from 1.2.3 to 1.2.4 in /website (#410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4.
Release notes

Sourced from word-wrap's releases.

1.2.4

What's Changed

New Contributors

Full Changelog: https://github.com/jonschlinkert/word-wrap/compare/1.2.3...1.2.4

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=word-wrap&package-manager=npm_and_yarn&previous-version=1.2.3&new-version=1.2.4)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `dependabot rebase` will rebase this PR - `dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `dependabot merge` will merge this PR after your CI passes on it - `dependabot squash and merge` will squash and merge this PR after your CI passes on it - `dependabot cancel merge` will cancel a previously requested merge and block automerging - `dependabot reopen` will reopen this PR if it is closed - `dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/facebook/ktfmt/network/alerts).
Pull Request resolved: https://github.com/facebook/ktfmt/pull/410 Reviewed By: davidtorosyan Differential Revision: D49321531 Pulled By: hick209 fbshipit-source-id: 060921a61bb3fc5dffc27f17c1b86c85d51a6ed8 --- website/package-lock.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/website/package-lock.json b/website/package-lock.json index 43d1698b..90b7647f 100644 --- a/website/package-lock.json +++ b/website/package-lock.json @@ -5000,9 +5000,9 @@ "dev": true }, "node_modules/word-wrap": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.3.tgz", - "integrity": "sha512-Hz/mrNwitNRh/HUAtM/VT/5VH+ygD6DV7mYKZAtHOrbs8U7lvPS6xf7EJKMF0uW1KJCl0H701g3ZGus+muE5vQ==", + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.4.tgz", + "integrity": "sha512-2V81OA4ugVo5pRo46hAoD2ivUJx8jXmWXfUkY4KFNw0hEptvN0QfH3K4nHiwzGeKl5rFKedV48QVoqYavy4YpA==", "dev": true, "engines": { "node": ">=0.10.0" @@ -9174,9 +9174,9 @@ "dev": true }, "word-wrap": { - "version": "1.2.3", - "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.3.tgz", - "integrity": "sha512-Hz/mrNwitNRh/HUAtM/VT/5VH+ygD6DV7mYKZAtHOrbs8U7lvPS6xf7EJKMF0uW1KJCl0H701g3ZGus+muE5vQ==", + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/word-wrap/-/word-wrap-1.2.4.tgz", + "integrity": "sha512-2V81OA4ugVo5pRo46hAoD2ivUJx8jXmWXfUkY4KFNw0hEptvN0QfH3K4nHiwzGeKl5rFKedV48QVoqYavy4YpA==", "dev": true }, "wrap-ansi": { From 2758ba277cad4f247b2e93e4d72ff06a9c1eebe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Mon, 18 Sep 2023 07:41:56 -0700 Subject: [PATCH 12/20] Bump version to 0.45 Reviewed By: strulovich Differential Revision: D49324491 fbshipit-source-id: 74a309446631e0aa2d7ec8bb5e4ea7e96bd2636f --- core/pom.xml | 2 +- pom.xml | 2 +- version.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 369d6d16..6ef4b4e7 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -11,7 +11,7 @@ com.facebook ktfmt-parent - 0.45-SNAPSHOT + 0.45 diff --git a/pom.xml b/pom.xml index 4fe51f1b..23aafbcf 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.facebook ktfmt-parent - 0.45-SNAPSHOT + 0.45 pom Ktfmt Parent diff --git a/version.txt b/version.txt index b443d321..4911aab3 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.45-SNAPSHOT +0.45 From 699a2ec3e49202bf337e3752aeeb93912d2edf57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Mon, 18 Sep 2023 07:41:56 -0700 Subject: [PATCH 13/20] Bump version to 0.46-SNAPSHOT Reviewed By: natestedman Differential Revision: D49324490 fbshipit-source-id: 9fe3dc663f21e82610d607b5e11445bae9d5cf99 --- core/pom.xml | 2 +- pom.xml | 4 ++-- version.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 6ef4b4e7..8ef784c6 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -11,7 +11,7 @@ com.facebook ktfmt-parent - 0.45 + 0.46-SNAPSHOT diff --git a/pom.xml b/pom.xml index 23aafbcf..36fe0f2d 100644 --- a/pom.xml +++ b/pom.xml @@ -1,11 +1,11 @@ - 4.0.0 com.facebook ktfmt-parent - 0.45 + 0.46-SNAPSHOT pom Ktfmt Parent diff --git a/version.txt b/version.txt index 4911aab3..9d75cedc 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.45 +0.46-SNAPSHOT From 289f4bbc9384327cb9671838817d88dacdae548f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Mon, 18 Sep 2023 07:41:56 -0700 Subject: [PATCH 14/20] Bump Kotlin version to 1.8.22 Summary: This also required a bump on the Gradle version to 6.8.3, instead I just took the latest. Reviewed By: natestedman Differential Revision: D49326207 fbshipit-source-id: a4dcd301b7c5aac43cb75422c542b770928cd8fd --- core/pom.xml | 2 +- online_formatter/build.gradle.kts | 8 +++----- online_formatter/gradle/wrapper/gradle-wrapper.properties | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 8ef784c6..1511db23 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -16,7 +16,7 @@ 0.10.1 - 1.6.20-M1 + 1.8.22 true 1.8 com.facebook.ktfmt.cli.Main diff --git a/online_formatter/build.gradle.kts b/online_formatter/build.gradle.kts index 140c3384..60d07a40 100644 --- a/online_formatter/build.gradle.kts +++ b/online_formatter/build.gradle.kts @@ -14,9 +14,7 @@ * limitations under the License. */ -import org.jetbrains.kotlin.gradle.tasks.KotlinCompile - -plugins { kotlin("jvm") version "1.5.0" } +plugins { kotlin("jvm") version "1.8.22" } repositories { mavenLocal() @@ -35,11 +33,11 @@ dependencies { testImplementation(kotlin("test-junit")) } +kotlin { jvmToolchain(11) } + tasks { test { useJUnit() } - withType() { kotlinOptions.jvmTarget = "11" } - val packageFat by creating(Zip::class) { from(compileKotlin) diff --git a/online_formatter/gradle/wrapper/gradle-wrapper.properties b/online_formatter/gradle/wrapper/gradle-wrapper.properties index be52383e..db9a6b82 100644 --- a/online_formatter/gradle/wrapper/gradle-wrapper.properties +++ b/online_formatter/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.7-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists From 45e78e23eff0697666ca2f9a3535118ff1b13dba Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Mon, 18 Sep 2023 11:29:41 -0700 Subject: [PATCH 15/20] Adjust .editorconfig for kotlinlang style for IntelliJ to better align with ktfmt (#412) Summary: Two changes: 1. Field annotations are set to wrap `off`, to align with ktfmt that places them on the same line as the field. e.g. ```kotlin get:Input internal abstract val versionPropertyName: Property ``` ...will now match what ktfmt does: ```kotlin get:Input internal abstract val versionPropertyName: Property ``` 2. Import order; IntelliJ places java.*, javax.*, kotlin.* at the end, after other imports, whereas ktfmt places all imports in alphabetical order: e.g. ```kotlin import org.gradle.api.file.FileSystemOperations import org.gradle.api.file.RegularFileProperty import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.ListProperty import org.gradle.api.provider.Property import java.util.* import javax.inject.Inject ``` ... will now be consistent with ktfmt, all sorted alphabetically. ```kotlin import java.util.* import javax.inject.Inject import org.gradle.api.file.FileSystemOperations import org.gradle.api.file.RegularFileProperty import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.ListProperty import org.gradle.api.provider.Property ``` Pull Request resolved: https://github.com/facebook/ktfmt/pull/412 Reviewed By: strulovich Differential Revision: D49319451 Pulled By: hick209 fbshipit-source-id: f2b6a163cd18ae3ec2f82227b6dd2f18eea11098 --- docs/editorconfig/.editorconfig-kotlinlang | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/editorconfig/.editorconfig-kotlinlang b/docs/editorconfig/.editorconfig-kotlinlang index 2f558675..85d6e057 100644 --- a/docs/editorconfig/.editorconfig-kotlinlang +++ b/docs/editorconfig/.editorconfig-kotlinlang @@ -42,10 +42,11 @@ ij_kotlin_continuation_indent_in_supertype_lists = false ij_kotlin_else_on_new_line = false ij_kotlin_enum_constants_wrap = off ij_kotlin_extends_list_wrap = normal -ij_kotlin_field_annotation_wrap = split_into_lines +ij_kotlin_field_annotation_wrap = off ij_kotlin_finally_on_new_line = false ij_kotlin_if_rparen_on_new_line = false ij_kotlin_import_nested_classes = false +ij_kotlin_imports_layout = * ij_kotlin_insert_whitespaces_in_simple_one_line_method = true ij_kotlin_keep_blank_lines_before_right_brace = 2 ij_kotlin_keep_blank_lines_in_code = 2 From 0af717280b05d622e55ed9d43284b7aaf3f314e6 Mon Sep 17 00:00:00 2001 From: nickreid Date: Mon, 18 Sep 2023 11:36:43 -0700 Subject: [PATCH 16/20] Fix indentation of trailing comments in a bunch of cases (#418) Summary: Pull Request resolved: https://github.com/facebook/ktfmt/pull/418 Reviewed By: strulovich Differential Revision: D49318446 Pulled By: hick209 fbshipit-source-id: 7ea64f7055f57adf665e6ad3912b3465f5386c95 --- .../ktfmt/format/KotlinInputAstVisitor.kt | 259 ++++++++++-------- .../facebook/ktfmt/format/FormatterTest.kt | 162 ++++++++++- .../format/GoogleStyleFormatterKtTest.kt | 156 ++++++++++- 3 files changed, 461 insertions(+), 116 deletions(-) diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index 80093d0c..4e0debd4 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -123,10 +123,12 @@ import org.jetbrains.kotlin.psi.KtWhenConditionWithExpression import org.jetbrains.kotlin.psi.KtWhenExpression import org.jetbrains.kotlin.psi.KtWhileExpression import org.jetbrains.kotlin.psi.psiUtil.children +import org.jetbrains.kotlin.psi.psiUtil.getNextSiblingIgnoringWhitespace import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespace import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.psi.psiUtil.startsWithComment import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes +import org.jetbrains.kotlin.psi.stubs.impl.KotlinPlaceHolderStubImpl /** An AST visitor that builds a stream of {@link Op}s to format. */ class KotlinInputAstVisitor( @@ -165,17 +167,17 @@ class KotlinInputAstVisitor( builder.sync(function) builder.block(ZERO) { visitFunctionLikeExpression( - function.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), - function.modifierList, - "fun", - function.typeParameterList, - function.receiverTypeReference, - function.nameIdentifier?.text, - true, - function.valueParameterList, - function.typeConstraintList, - function.bodyBlockExpression ?: function.bodyExpression, - function.typeReference, + contextReceiverList = + function.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), + modifierList = function.modifierList, + keyword = "fun", + typeParameters = function.typeParameterList, + receiverTypeReference = function.receiverTypeReference, + name = function.nameIdentifier?.text, + parameterList = function.valueParameterList, + typeConstraintList = function.typeConstraintList, + bodyExpression = function.bodyBlockExpression ?: function.bodyExpression, + typeOrDelegationCall = function.typeReference, ) } } @@ -288,24 +290,38 @@ class KotlinInputAstVisitor( private fun visitFunctionLikeExpression( contextReceiverList: KtContextReceiverList?, modifierList: KtModifierList?, - keyword: String, + keyword: String?, typeParameters: KtTypeParameterList?, receiverTypeReference: KtTypeReference?, name: String?, - emitParenthesis: Boolean, parameterList: KtParameterList?, typeConstraintList: KtTypeConstraintList?, bodyExpression: KtExpression?, typeOrDelegationCall: KtElement?, ) { - builder.block(ZERO) { + fun emitTypeOrDelegationCall(block: () -> Unit) { + if (typeOrDelegationCall != null) { + builder.block(ZERO) { + if (typeOrDelegationCall is KtConstructorDelegationCall) { + builder.space() + } + builder.token(":") + block() + } + } + } + + val forceTrailingBreak = name != null + builder.block(ZERO, isEnabled = forceTrailingBreak) { if (contextReceiverList != null) { visitContextReceiverList(contextReceiverList) } if (modifierList != null) { visitModifierList(modifierList) } - builder.token(keyword) + if (keyword != null) { + builder.token(keyword) + } if (typeParameters != null) { builder.space() builder.block(ZERO) { visit(typeParameters) } @@ -324,46 +340,35 @@ class KotlinInputAstVisitor( builder.token(name) } } - if (emitParenthesis) { - builder.token("(") - } - var paramBlockNeedsClosing = false - builder.block(ZERO) { - if (parameterList != null && parameterList.parameters.isNotEmpty()) { - paramBlockNeedsClosing = true - builder.open(expressionBreakIndent) - builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakIndent) - visit(parameterList) - } - if (emitParenthesis) { - if (parameterList != null && parameterList.parameters.isNotEmpty()) { - builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakNegativeIndent) - } + + if (parameterList != null && parameterList.hasEmptyParens()) { + builder.block(ZERO) { + builder.token("(") builder.token(")") - } else { - if (paramBlockNeedsClosing) { - builder.close() + emitTypeOrDelegationCall { + builder.breakOp(Doc.FillMode.INDEPENDENT, " ", expressionBreakIndent) + builder.block(expressionBreakIndent) { visit(typeOrDelegationCall) } } } - if (typeOrDelegationCall != null) { - builder.block(ZERO) { - if (typeOrDelegationCall is KtConstructorDelegationCall) { - builder.space() - } - builder.token(":") - if (parameterList?.parameters.isNullOrEmpty()) { - builder.breakOp(Doc.FillMode.INDEPENDENT, " ", expressionBreakIndent) - builder.block(expressionBreakIndent) { visit(typeOrDelegationCall) } - } else { - builder.space() - builder.block(expressionBreakNegativeIndent) { visit(typeOrDelegationCall) } - } + } else { + builder.block(expressionBreakIndent) { + if (parameterList != null) { + visitEachCommaSeparated( + list = parameterList.parameters, + hasTrailingComma = parameterList.trailingComma != null, + prefix = "(", + postfix = ")", + wrapInBlock = false, + breakBeforePostfix = true, + ) + } + emitTypeOrDelegationCall { + builder.space() + builder.block(expressionBreakNegativeIndent) { visit(typeOrDelegationCall) } } } } - if (paramBlockNeedsClosing) { - builder.close() - } + if (typeConstraintList != null) { builder.space() visit(typeConstraintList) @@ -387,7 +392,7 @@ class KotlinInputAstVisitor( } builder.guessToken(";") } - if (name != null) { + if (forceTrailingBreak) { builder.forcedBreak() } } @@ -726,6 +731,20 @@ class KotlinInputAstVisitor( return extractCallExpression(this)?.lambdaArguments?.isNotEmpty() ?: false } + /** Does this list have parens with only whitespace between them? */ + private fun KtParameterList.hasEmptyParens(): Boolean { + val left = this.leftParenthesis ?: return false + val right = this.rightParenthesis ?: return false + return left.getNextSiblingIgnoringWhitespace() == right + } + + /** Does this list have parens with only whitespace between them? */ + private fun KtValueArgumentList.hasEmptyParens(): Boolean { + val left = this.leftParenthesis ?: return false + val right = this.rightParenthesis ?: return false + return left.getNextSiblingIgnoringWhitespace() == right + } + /** * emitQualifiedExpression formats call expressions that are either part of a qualified * expression, or standing alone. This method makes it easier to handle both cases uniformly. @@ -811,26 +830,26 @@ class KotlinInputAstVisitor( arguments.first().getArgumentExpression() is KtLambdaExpression && arguments.first().getArgumentName() == null val hasTrailingComma = list.trailingComma != null + val hasEmptyParens = list.hasEmptyParens() val wrapInBlock: Boolean val breakBeforePostfix: Boolean val leadingBreak: Boolean val breakAfterPrefix: Boolean - if (isSingleUnnamedLambda) { wrapInBlock = true breakBeforePostfix = false - leadingBreak = arguments.isNotEmpty() && hasTrailingComma + leadingBreak = !hasEmptyParens && hasTrailingComma breakAfterPrefix = false } else { wrapInBlock = !isGoogleStyle - breakBeforePostfix = isGoogleStyle && arguments.isNotEmpty() - leadingBreak = arguments.isNotEmpty() - breakAfterPrefix = arguments.isNotEmpty() + breakBeforePostfix = isGoogleStyle && !hasEmptyParens + leadingBreak = !hasEmptyParens + breakAfterPrefix = !hasEmptyParens } return visitEachCommaSeparated( - list.arguments, + arguments, hasTrailingComma, wrapInBlock = wrapInBlock, breakBeforePostfix = breakBeforePostfix, @@ -1391,17 +1410,16 @@ class KotlinInputAstVisitor( builder.block(ZERO) { visitFunctionLikeExpression( - null, - accessor.modifierList, - accessor.namePlaceholder.text, - null, - null, - null, - accessor.bodyExpression != null || accessor.bodyBlockExpression != null, - accessor.parameterList, - null, - accessor.bodyBlockExpression ?: accessor.bodyExpression, - accessor.returnTypeReference, + contextReceiverList = null, + modifierList = accessor.modifierList, + keyword = accessor.namePlaceholder.text, + typeParameters = null, + receiverTypeReference = null, + name = null, + parameterList = getParameterListWithBugFixes(accessor), + typeConstraintList = null, + bodyExpression = accessor.bodyBlockExpression ?: accessor.bodyExpression, + typeOrDelegationCall = accessor.returnTypeReference, ) } } @@ -1417,6 +1435,33 @@ class KotlinInputAstVisitor( return 0 } + // Bug in Kotlin 1.9.10: KtProperyAccessor is the direct parent of the left and right paren + // elements. Also parameterList is always null for getters. As a workaround, we create our own + // fake KtParameterList. + private fun getParameterListWithBugFixes(accessor: KtPropertyAccessor): KtParameterList? { + if (accessor.bodyExpression == null && accessor.bodyBlockExpression == null) return null + + return object : + KtParameterList( + KotlinPlaceHolderStubImpl(accessor.stub, KtStubElementTypes.VALUE_PARAMETER_LIST)) { + override fun getParameters(): List { + return accessor.valueParameters + } + + override fun getTrailingComma(): PsiElement? { + return accessor.parameterList?.trailingComma + } + + override fun getLeftParenthesis(): PsiElement? { + return accessor.leftParenthesis + } + + override fun getRightParenthesis(): PsiElement? { + return accessor.rightParenthesis + } + } + } + /** * Returns whether an expression is a lambda or initializer expression in which case we will want * to avoid indenting the lambda block @@ -1531,45 +1576,42 @@ class KotlinInputAstVisitor( builder.sync(constructor) builder.block(ZERO) { if (constructor.hasConstructorKeyword()) { - builder.open(ZERO) builder.breakOp(Doc.FillMode.UNIFIED, " ", ZERO) - visit(constructor.modifierList) - builder.token("constructor") - } - - builder.block(ZERO) { - builder.token("(") - builder.block(expressionBreakIndent) { - builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakIndent) - visit(constructor.valueParameterList) - builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakNegativeIndent) - if (constructor.hasConstructorKeyword()) { - builder.close() - } - } - builder.token(")") } + visitFunctionLikeExpression( + contextReceiverList = null, + modifierList = constructor.modifierList, + keyword = if (constructor.hasConstructorKeyword()) "constructor" else null, + typeParameters = null, + receiverTypeReference = null, + name = null, + parameterList = constructor.valueParameterList, + typeConstraintList = null, + bodyExpression = constructor.bodyExpression, + typeOrDelegationCall = null, + ) } } /** Example `private constructor(n: Int) : this(4, 5) { ... }` inside a class's body */ override fun visitSecondaryConstructor(constructor: KtSecondaryConstructor) { builder.sync(constructor) - - val delegationCall = constructor.getDelegationCall() - visitFunctionLikeExpression( - constructor.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), - constructor.modifierList, - "constructor", - null, - null, - null, - true, - constructor.valueParameterList, - null, - constructor.bodyExpression, - if (!delegationCall.isImplicit) delegationCall else null, - ) + builder.block(ZERO) { + val delegationCall = constructor.getDelegationCall() + visitFunctionLikeExpression( + contextReceiverList = + constructor.getStubOrPsiChild(KtStubElementTypes.CONTEXT_RECEIVER_LIST), + modifierList = constructor.modifierList, + keyword = "constructor", + typeParameters = null, + receiverTypeReference = null, + name = null, + parameterList = constructor.valueParameterList, + typeConstraintList = null, + bodyExpression = constructor.bodyExpression, + typeOrDelegationCall = if (!delegationCall.isImplicit) delegationCall else null, + ) + } } override fun visitConstructorDelegationCall(call: KtConstructorDelegationCall) { @@ -2097,17 +2139,14 @@ class KotlinInputAstVisitor( /** Example `` */ override fun visitTypeParameterList(list: KtTypeParameterList) { builder.sync(list) - builder.block(ZERO) { - builder.token("<") - val parameters = list.parameters - if (parameters.isNotEmpty()) { - // Break before args. - builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakIndent) - builder.block(expressionBreakIndent) { - visitEachCommaSeparated(list.parameters, list.trailingComma != null, wrapInBlock = true) - } - } - builder.token(">") + builder.block(expressionBreakIndent) { + visitEachCommaSeparated( + list = list.parameters, + hasTrailingComma = list.trailingComma != null, + prefix = "<", + postfix = ">", + wrapInBlock = !isGoogleStyle, + ) } } @@ -2453,7 +2492,7 @@ class KotlinInputAstVisitor( builder.blankLineWanted( when { isFirst -> OpsBuilder.BlankLineWanted.NO - child is PsiComment -> OpsBuilder.BlankLineWanted.NO + child is PsiComment -> continue child is KtScript && importListEmpty -> OpsBuilder.BlankLineWanted.PRESERVE else -> OpsBuilder.BlankLineWanted.YES }) diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index 4358c34d..be8823f9 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -2251,6 +2251,7 @@ class FormatterTest { fun `a few variations of constructors`() = assertFormatted( """ + |------------------------------------------------------ |class Foo constructor(number: Int) {} | |class Foo2 private constructor(number: Int) {} @@ -2269,8 +2270,19 @@ class FormatterTest { | number5: Int, | number6: Int |) {} + | + |class Foo6 + |@Inject + |private constructor(hasSpaceForAnnos: Innnt) { + | // @Inject + |} + | + |class FooTooLongForCtorAndSupertypes + |@Inject + |private constructor(x: Int) : NoooooooSpaceForAnnos {} |""" - .trimMargin()) + .trimMargin(), + deduceMaxWidth = true) @Test fun `a primary constructor without a class body `() = @@ -3610,9 +3622,9 @@ class FormatterTest { .trimMargin()) @Test - fun `handle file annotations`() = - assertFormatted( - """ + fun `handle file annotations`() { + assertFormatted( + """ |@file:JvmName("DifferentName") | |package com.somecompany.example @@ -3623,7 +3635,53 @@ class FormatterTest { | val a = example2("and 1") |} |""" - .trimMargin()) + .trimMargin()) + + assertFormatted( + """ + |@file:JvmName("DifferentName") // Comment + | + |package com.somecompany.example + | + |import com.somecompany.example2 + | + |class Foo { + | val a = example2("and 1") + |} + |""" + .trimMargin()) + + assertFormatted( + """ + |@file:JvmName("DifferentName") + | + |// Comment + | + |package com.somecompany.example + | + |import com.somecompany.example2 + | + |class Foo { + | val a = example2("and 1") + |} + |""" + .trimMargin()) + + assertFormatted( + """ + |@file:JvmName("DifferentName") + | + |// Comment + |package com.somecompany.example + | + |import com.somecompany.example2 + | + |class Foo { + | val a = example2("and 1") + |} + |""" + .trimMargin()) + } @Test fun `handle init block`() = @@ -6852,6 +6910,100 @@ class FormatterTest { assertThatFormatting(code).isEqualTo(expected) } + @Test + fun `trailing comment after function in class`() = + assertFormatted( + """ + |class Host { + | fun fooBlock() { + | return + | } // Trailing after fn + | // Hanging after fn + | + | // End of class + |} + | + |class Host { + | fun fooExpr() = 0 // Trailing after fn + | // Hanging after fn + | + | // End of class + |} + | + |class Host { + | constructor() {} // Trailing after fn + | // Hanging after fn + | + | // End of class + |} + | + |class Host + |// Primary constructor + |constructor() // Trailing after fn + | // Hanging after fn + |{ + | // End of class + |} + | + |class Host { + | fun fooBlock() { + | return + | } + | + | // Between elements + | + | fun fooExpr() = 0 + | + | // Between elements + | + | fun fooBlock() { + | return + | } + |} + |""" + .trimMargin()) + + @Test + fun `trailing comment after function top-level`() { + assertFormatted( + """ + |fun fooBlock() { + | return + |} // Trailing after fn + |// Hanging after fn + | + |// End of file + |""" + .trimMargin()) + + assertFormatted( + """ + |fun fooExpr() = 0 // Trailing after fn + |// Hanging after fn + | + |// End of file + |""" + .trimMargin()) + + assertFormatted( + """ + |fun fooBlock() { + | return + |} + | + |// Between elements + | + |fun fooExpr() = 0 + | + |// Between elements + | + |fun fooBlock() { + | return + |} + |""" + .trimMargin()) + } + companion object { /** Triple quotes, useful to use within triple-quoted strings. */ private const val TQ = "\"\"\"" diff --git a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt index 3d1c3ce6..c87e3718 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt @@ -105,7 +105,7 @@ class GoogleStyleFormatterKtTest { } @Test - fun `class params are placed each in their own line`() = + fun `class value params are placed each in their own line`() = assertFormatted( """ |----------------------------------------- @@ -147,6 +147,58 @@ class GoogleStyleFormatterKtTest { formattingOptions = Formatter.GOOGLE_FORMAT, deduceMaxWidth = true) + @Test + fun `class type params are placed each in their own line`() = + assertFormatted( + """ + |------------------------------------ + |class Foo< + | TypeA : Int, + | TypeC : String + |> { + | // Class name + type params too long for one line + | // Type params could fit on one line but break + |} + | + |class Foo< + | TypeA : Int, + | TypeB : Double, + | TypeC : String + |> { + | // Type params can't fit on one line + |} + | + |class Foo< + | TypeA : Int, + | TypeB : Double, + | TypeC : String + |> + | + |class Foo< + | TypeA : Int, + | TypeB : Double, + | TypeC : String + |>() { + | // + |} + | + |class Bi< + | TypeA : Int, + | TypeB : Double, + | TypeC : String + |>(a: Int, var b: Int, val c: Int) { + | // TODO: Breaking the type param list + | // should propagate to the value param list + |} + | + |class C { + | // Class name + type params fit on one line + |} + |""" + .trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + @Test fun `function params are placed each in their own line`() = assertFormatted( @@ -1309,6 +1361,108 @@ class GoogleStyleFormatterKtTest { formattingOptions = Formatter.GOOGLE_FORMAT, deduceMaxWidth = true) + @Test + fun `leading and trailing comments in block-like lists`() = + assertFormatted( + """ + |-------------------------------- + |@Anno( + | array = + | [ + | // Comment + | someItem + | // Comment + | ] + |) + |class Host( + | // Comment + | val someItem: Int + | // Comment + |) { + | constructor( + | // Comment + | someItem: Int + | // Comment + | ) : this( + | // Comment + | someItem + | // Comment + | ) + | + | fun foo( + | // Comment + | someItem: Int + | // Comment + | ): Int { + | foo( + | // Comment + | someItem + | // Comment + | ) + | } + | + | var x: Int = 0 + | set( + | // Comment + | someItem: Int + | // Comment + | ) = Unit + | + | fun < + | // Comment + | someItem : Int + | // Comment + | > bar(): Int { + | bar< + | // Comment + | someItem + | // Comment + | >() + | } + |} + |""" + .trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + + @Test + fun `comments in empty block-like lists`() = + assertFormatted( + """ + |-------------------------------- + |@Anno( + | array = + | [ + | // Comment + | ] + |) + |class Host( + | // Comment + |) { + | constructor( + | // Comment + | ) : this( + | // Comment + | ) + | + | val x: Int + | get( + | // Comment + | ) = 0 + | + | fun foo( + | // Comment + | ): Int { + | foo( + | // Comment + | ) + | } + |} + |""" + .trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + companion object { /** Triple quotes, useful to use within triple-quoted strings. */ private const val TQ = "\"\"\"" From 30ae6f969dd570755570259305aa9c678f61ccab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Mon, 18 Sep 2023 13:14:51 -0700 Subject: [PATCH 17/20] Bump version to 0.46 Reviewed By: strulovich Differential Revision: D49381562 fbshipit-source-id: 6f2188db281add318bf800ffd9e3e804f86caa29 --- core/pom.xml | 2 +- pom.xml | 2 +- version.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 1511db23..d0962d48 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -11,7 +11,7 @@ com.facebook ktfmt-parent - 0.46-SNAPSHOT + 0.46 diff --git a/pom.xml b/pom.xml index 36fe0f2d..415355b2 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.facebook ktfmt-parent - 0.46-SNAPSHOT + 0.46 pom Ktfmt Parent diff --git a/version.txt b/version.txt index 9d75cedc..2c5f97ec 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.46-SNAPSHOT +0.46 From e4f41908adebfd138de8852e4c4c3e864cdcd051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nivaldo=20Bondan=C3=A7a?= Date: Mon, 18 Sep 2023 13:14:51 -0700 Subject: [PATCH 18/20] Bump version to 0.47-SNAPSHOT Reviewed By: strulovich Differential Revision: D49381561 fbshipit-source-id: de0bf6758b4fbf2ffc5857e2d0687fdc0281f2a2 --- core/pom.xml | 2 +- pom.xml | 2 +- version.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index d0962d48..267689e1 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -11,7 +11,7 @@ com.facebook ktfmt-parent - 0.46 + 0.47-SNAPSHOT diff --git a/pom.xml b/pom.xml index 415355b2..f1c1df7d 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.facebook ktfmt-parent - 0.46 + 0.47-SNAPSHOT pom Ktfmt Parent diff --git a/version.txt b/version.txt index 2c5f97ec..d7bf00b2 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.46 +0.47-SNAPSHOT From 8ad087272a537a168ae2bb0897b2ff8a790344c2 Mon Sep 17 00:00:00 2001 From: haruka Date: Mon, 18 Sep 2023 13:35:43 -0700 Subject: [PATCH 19/20] Plugin doesn't work with if "Only VCS changed text" is selected from code-reformat settings (#386) Summary: Fixes facebook/ktfmt#228 The code reformat with "Only VCS changed text" falls back to the IDE Kotlin settings due to a missing override in KtfmtCodeStyleManager.java. This is also true of the popular "Save Actions" plugin which reformats with the same command. This change adds the missing override exactly as it appears in the reference project google/google-java-format at https://github.com/google/google-java-format/blob/d86e930de93f123994fba151a8d289b8035db87b/idea_plugin/src/com/google/googlejavaformat/intellij/GoogleJavaFormatCodeStyleManager.java#L76 Confirmed to fix the issue on Android Studio 2021.1.1 with the manual code-reformat and in the Save Actions plugin. Pull Request resolved: https://github.com/facebook/ktfmt/pull/386 Reviewed By: strulovich Differential Revision: D49322592 Pulled By: hick209 fbshipit-source-id: 99d56d428603292ffb6efb6551e7fa619367c0bd --- .../ktfmt/intellij/KtfmtCodeStyleManager.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/KtfmtCodeStyleManager.java b/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/KtfmtCodeStyleManager.java index d2184ea1..7594050a 100644 --- a/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/KtfmtCodeStyleManager.java +++ b/ktfmt_idea_plugin/src/main/java/com/facebook/ktfmt/intellij/KtfmtCodeStyleManager.java @@ -26,10 +26,13 @@ import com.intellij.psi.PsiDocumentManager; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; +import com.intellij.psi.codeStyle.ChangedRangesInfo; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.impl.CheckUtil; import com.intellij.util.IncorrectOperationException; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.TreeMap; @@ -66,6 +69,17 @@ public void reformatText(PsiFile file, Collection ranges) } } + @Override + public void reformatTextWithContext(@NotNull PsiFile file, @NotNull ChangedRangesInfo info) + throws IncorrectOperationException { + List ranges = new ArrayList<>(); + if (info.insertedRanges != null) { + ranges.addAll(info.insertedRanges); + } + ranges.addAll(info.allChangedRanges); + reformatTextWithContext(file, ranges); + } + @Override public void reformatTextWithContext(PsiFile file, Collection ranges) { if (overrideFormatterForFile(file)) { From 102c89dd896fe6c87275843ffce011a673b82885 Mon Sep 17 00:00:00 2001 From: David Torosyan Date: Thu, 21 Sep 2023 13:13:36 -0700 Subject: [PATCH 20/20] Add unit tests to capture line break behavior on type specifiers Summary: It came up recently that this formatting (for the `where` case) looks weird and should probably be indented. To make sure we track any fixes (or just are aware that we've always had this issue) add a couple of unit tests. One to demonstrate the issue, the other to show a better case. Note that a previous change (D34347730) added a similar (but different) unit test. Reviewed By: hick209 Differential Revision: D49507677 fbshipit-source-id: 777eb3becf51d0061fea19953fd950a71d8fe36b --- .../facebook/ktfmt/format/FormatterTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index be8823f9..082fdddf 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -7004,6 +7004,32 @@ class FormatterTest { .trimMargin()) } + @Test + fun `line break on base class`() = + assertFormatted( + """ + |--------------------------- + |class Basket() : + | WovenObject { + | // some body + |} + |""" + .trimMargin(), + deduceMaxWidth = true) + + @Test + fun `line break on type specifier`() = + assertFormatted( + """ + |--------------------------- + |class Basket() where + |T : Fruit { + | // some body + |} + |""" + .trimMargin(), + deduceMaxWidth = true) + companion object { /** Triple quotes, useful to use within triple-quoted strings. */ private const val TQ = "\"\"\""