Sort modifiers according to Kotlin conventions by user1223644 · Pull Request #641 · Kotlin/ktfmt

user1223644 · 2026-07-14T16:29:14Z

Sorts declaration modifiers according to the order defined by the Kotlin coding conventions.

For example:

final @Magic public class Foo

suspend override protected fun foo() {}

is formatted as:

@Magic public final class Foo

protected override suspend fun foo() {}

Annotations and comments keep their relative association. Unsupported modifiers such as noinline, crossinline, in, out, and reified act as ordering barriers.

The cleanup applies to both full and partial formatting.

Tests:

./gradlew ktfmtFormat check --no-daemon

Fixes #293

cgrushko · 2026-07-14T20:31:42Z

Thanks for the pull request!

For transparency, we'd appreciate if you can note whether / how much you've used the help of AI coding tools for creating this PR. Thanks!

user1223644 · 2026-07-14T21:55:47Z

I used codex extensively to help implement this change. I personally reviewed and tested all changes before submitting

hick209

LGTM, but now it's up to Kotlin Foundations on whether they want to take this in or not.

There were also plenty of changes recently, so you will need a rebase.

zarechenskiy

I haven't really looked into the code, only at the expect/actual results. For the code part, @AbdullinAM will review it

zarechenskiy · 2026-08-24T07:30:47Z

+      setOf("override"),
+      setOf("lateinit"),
+      setOf("tailrec"),
+      setOf("vararg"),


vararg is a parameter modifier. It seems we don't need it, do we?

Or, if we also want to tackle all modifiers, then it makes sense to include noinline and other parameter modifiers as well

Yes, I think current implementation requires having all modifiers. Currently implementation fails with a parsing error on this example:

inline fun f(@Magic // c noinline b: () -> Unit) {}

Modifier sorter does not consider noinline as modifier and removes the line break after comment, transforming code into invalid

inline fun f(@Magic // c noinline b: () -> Unit) {}

zarechenskiy · 2026-08-24T07:41:59Z

+  @Test
+  fun `modifier sorting preserves line separators and context receivers`() {
+    val code = "context(Something)\r\noverride public fun f() {}\r\n"
+    val expected = "context(Something)\r\npublic override fun f() {}\r\n"


Context receivers were replaced with context parameters, so let's use them in tests

zarechenskiy · 2026-08-24T07:45:09Z

+  }
+
+  @Test
+  fun `modifier sorting preserves line separators and context receivers`() {


Speaking of context parameters: I believe we should put context to the first place among the modifiers. So while this test makes sense, we also have to check that something like:

internal context(other: Int) fun something(s: String) {}

is formatted into:

context(other: Int) internal fun something(s: String) {}

otherwise, there might be some confusion whether some modifier is applied to context or to a function (it always applies to a function)

zarechenskiy · 2026-08-24T07:55:01Z

+import org.jetbrains.kotlin.psi.psiUtil.endOffset
+import org.jetbrains.kotlin.psi.psiUtil.startOffset
+
+/** Sorts declaration modifiers according to the Kotlin coding conventions. */


This comment doesn't add much value, let's remove it

zarechenskiy · 2026-08-24T08:14:29Z

+@Magic public final class Interleaved

-public @Magic(1, "argument") final class InterleavedWithArgs
+@Magic(1, "argument") public final class InterleavedWithArgs


Let's also check that we don't touch annotations in case of use-site targets, for instance:

@Target(AnnotationTarget.VALUE_PARAMETER) annotation class Anno class F { final internal val @receiver:Anno Int.prop get() = 4 }

Should be formatted as internal final val @receiver:Anno Int.prop get() = 4

AbdullinAM

Thanks for the implementation! There are some technical issues, nothing too major though. But I would suggest you fix @zarechenskiy's comments first

P.s. If at any point you need help or want to pass over the PR --- I can take over and finalise it myself.

AbdullinAM · 2026-08-24T09:50:29Z

+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Test
+
+class ModifierSorterTest {


I think we should stick to a new file-based tests format, see FormatTest for an example.

AbdullinAM · 2026-08-24T10:07:57Z

+      setOf("override"),
+      setOf("lateinit"),
+      setOf("tailrec"),
+      setOf("vararg"),


Yes, I think current implementation requires having all modifiers. Currently implementation fails with a parsing error on this example:

inline fun f(@Magic // c noinline b: () -> Unit) {}

Modifier sorter does not consider noinline as modifier and removes the line break after comment, transforming code into invalid

inline fun f(@Magic // c noinline b: () -> Unit) {}

AbdullinAM · 2026-08-24T10:17:46Z

+      val followingComments: MutableList<PsiComment> = mutableListOf(),
+  )
+
+  private sealed interface Part {


It feels like this abstraction is not necessary. Both subclasses are only created in one place in code and no distinction is made between them after:

val parts = mutableListOf<Part>() // line 93 parts.add(...) // lines 98, 107 val sorted = parts.map { it.text }.joinWithSpaces() // line 112

Lets remove this abstraction. Or add documentation explaining why its necessary

AbdullinAM · 2026-08-24T10:25:27Z

+    flushSortableSegment()
+
+    val sorted = parts.map { it.text }.joinWithSpaces()
+    return sorted.takeIf { it != list.text }


One potential problem of this implementation is that we don't preserve the line breaks between modifiers. So we will trigger replacement/parsing even if the original code just contained line breaks like

public override fun outsideSelection() {}

I would suggest modifying the code to detect if the modifiers were actually sorted

AbdullinAM · 2026-08-24T10:37:44Z

+    }
+
+    if (leadingComments.isNotEmpty()) {
+      units.lastOrNull()?.followingComments?.addAll(leadingComments)


To be honest I was not able to come up with a test case that covers this condition (except for test cases with currently unsupported modifiers). If we handle all modifiers properly --- would we even need that?

AbdullinAM · 2026-08-24T10:42:07Z

+    }
+    val sortedCode = result.toString()
+    return if (innermostReplacements.size == replacements.size) sortedCode
+    else sort(Parser.parse(sortedCode))


I suspect that this sorter can be implemented in one pass (i.e. requiring only one re-parsing). I'm not 100% sure if we need it though, because it can complicate the code. So we can merge this version and improve it later

AbdullinAM · 2026-08-24T10:58:37Z

+    return sortedUnits.map { it.render() }.joinWithSpaces()
+  }
+
+  private fun PsiElement.isAnnotation(): Boolean = this is KtAnnotation || this is KtAnnotationEntry


We can give annotations rank -1, so we don't have to always handle them separately

AbdullinAM · 2026-08-24T10:59:44Z

+    }
+
+    val sortedUnits =
+        units.filter { it.isAnnotation } + units.filterNot { it.isAnnotation }.sortedBy { it.rank }


Having a rank for annotations will simplify this code

AbdullinAM · 2026-08-24T11:01:17Z

+    }
+  }
+
+  private fun Iterable<String>.joinWithSpaces(): String = buildString {


There are several cases in code where you use list.map { ... }.joinWithSpaces(). I would suggest you modify this fun to take a lambda that transforms the list element into string and then you can write:

list.joinWithSpaces { ... }

This will be similar to joinToString from stdlib

meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 14, 2026

user1223644 marked this pull request as ready for review July 14, 2026 16:30

hick209 requested changes Aug 14, 2026

View reviewed changes

AbdullinAM assigned zarechenskiy Aug 21, 2026

Sort modifiers by Kotlin conventions

d8741f2

user1223644 force-pushed the Codex/issue-293-modifier-order branch from e379d46 to d8741f2 Compare August 23, 2026 20:31

zarechenskiy requested changes Aug 24, 2026

View reviewed changes

zarechenskiy requested a review from AbdullinAM August 24, 2026 09:48

AbdullinAM requested changes Aug 24, 2026

View reviewed changes

Uh oh!

Conversation

user1223644 commented Jul 14, 2026

Uh oh!

cgrushko commented Jul 14, 2026

Uh oh!

user1223644 commented Jul 14, 2026

Uh oh!

hick209 left a comment

Choose a reason for hiding this comment

Uh oh!

zarechenskiy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AbdullinAM left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AbdullinAM Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AbdullinAM left a comment •

edited

Loading

AbdullinAM Aug 24, 2026 •

edited

Loading