Ktlint: [27][28] - ktformat not formating indentation correctly

[jaredsburrows]

Before:

class TestA {
    companion object {
            private const val TAG = "TestA" // 8 spaces indent - should be 4
    }
}

After:

class TestA {
    companion object {
            private const val TAG = "TestA" // 8 spaces indent - should be 4
    }
}

Should be:

class TestA {
    companion object {
        private const val TAG = "TestA" // 4 spaces indent
    }
}

[shyiko]

Hi Jared.
Yeah, IndentationRule is pretty basic at the moment. All it does is check that indentation is a multiple of indent_size. A proper implementation shouldn't be too hard (a single scan over AST to calc expected indentation should do it) but just haven't found the time to do it (yet).
PR is (more than) welcome.

[iboss-ptk]

I long for this feature every day. I would love to help @shyiko :D

[thuytrinh]

@shyiko If I'm correct, that's why the following 8-space intents weren't formatted into 4-space intents in my case:

Before and after formatting (code hasn't changed at all):

class AwesomeGoogleFit constructor(private val context: Context) :
        GoogleApiClient.ConnectionCallbacks,
        GoogleApiClient.OnConnectionFailedListener {

    private val client: GoogleApiClient = GoogleApiClient.Builder(context, this, this)
            .addApi(Fitness.SESSIONS_API)
            .addScope(Scope(Scopes.FITNESS_ACTIVITY_READ_WRITE))
            .build()
}

Expected:

class AwesomeGoogleFit constructor(private val context: Context) :
    GoogleApiClient.ConnectionCallbacks,
    GoogleApiClient.OnConnectionFailedListener {

    private val client: GoogleApiClient = GoogleApiClient.Builder(context, this, this)
        .addApi(Fitness.SESSIONS_API)
        .addScope(Scope(Scopes.FITNESS_ACTIVITY_READ_WRITE))
        .build()
}

However if I formatted by using Android Studio, it just worked as expected. My Android Studio has been using the code style configs which were changed by ktlint --apply-to-idea-project.

[thuytrinh]

@shyiko But there's one thing I can't understand is that, ktlint still manages to format following 8-space intents into 4 spaces:

 data class AwesomeState(
-        val a: A? = null,
-        val b: Boolean = false,
-        val c: Boolean = false,
-        val d: D? = null
+    val a: A? = null,
+    val b: Boolean = false,
+    val c: Boolean = false,
+    val d: D? = null
 )

Why?

[shyiko]

@iboss-ptk that would be great! give it try. https://github.com/shyiko/ktlint#ast & https://plugins.jetbrains.com/plugin/227-psiviewer might be useful (don't be afraid to ask if you hit the wall).

@thuytrinh parameter wrapping is handled by ParameterListWrappingRule.

[iboss-ptk]

@shyiko any idea how to determine if two indentation triggers happens to be on the same line eg.

val a = listOf(1,2,3)
    .map { it
        .toString()
    }

At .toString(), the indentation should be 8 spaces (default config) right? But it's in the {} block and it's in a dot chain. How can we determine that those triggers are on the same line?

[shyiko]

@iboss-ptk not sure I understand the question.

\n\s*.toString() inside block {} requires +8 indent. The fact that it is misaligned should not change that.

If there is anything after { (aside from parameters) in a multi-line block (it in this example) it should be moved to the next line.

In other words,

val a = listOf(1,2,3)
    .map { it
        .toString()
    }

piped through IndentRule should produce

val a = listOf(1,2,3)
    .map { 
        it
            .toString()
    }

Other rules can decide whether to rewrite that to

val a = listOf(1,2,3).map { it.toString() }

or

val a = listOf(1,2,3)
    .map { it.toString() }

[iboss-ptk]

@shyiko shouldn't it produce this?

val a = listOf(1,2,3)
    .map {
        it
            .toString()
    }

And isn't that not about indentation anymore? I can't think of error message about indentation in this case since it looks more like something else.

[iboss-ptk]

And similar case, if ( and { appears to be on the same line, should we indent stuffs in the block once or twice?

addOnLayoutChangeListener(object : View.OnLayoutChangeListener {
    override fun onLayoutChange(
    )
})

[shyiko]

@iboss-ptk sorry, I must have had tabs in there (weren't visible on my machine). https://github.com/shyiko/ktlint/issues/285#issuecomment-429596071 updated.

And isn't that not about indentation anymore?

You can leave "If there is anything after { (aside from parameters) in a multi-line block (it in this example) it should be moved to the next line." out of IndentRule if you want (a different rule can handle that). But I expect this can complicate things.

I can't think of error message about indentation in this case since it looks more like something else.

Something like "Newline missing after {" should do it.

Regarding

addOnLayoutChangeListener(object : View.OnLayoutChangeListener {
    override fun onLayoutChange(
    )
})

It's indented as expected (+4).

[iboss-ptk]

@shyiko So you would suggest to handle the it case here right? I can do that :)

That aside, I have another question. If there are binary expressions, how should we handle it? Since I see intellij handle it differently.

If it's in the if condition block it will align without continuation indentation.

if (current is PsiWhiteSpace &&
    current.treeNext !is PsiComment &&
    current.treeNext.firstChildNode !is PsiComment &&
    current.treeNext.elementType != KtTokens.WHERE_KEYWORD &&
    !current.isPartOf(PsiComment::class) &&
    !current.isPartOf(KtTypeConstraintList::class)
) {
    val continuationCount = scopeStack.count { it.isContinuation }
    val normalIndentationCount = scopeStack.count { !it.isContinuation }
    handleNewline(current, emit, autoCorrect, normalIndentationCount, continuationCount)

    if (current.textContains('\t')) {
        handleTab(current, emit, autoCorrect)
    }
}

But if not, it will behave like chaining dot

1 +
    2 +
    3

Should we follow that behaviour?

[shyiko]

@iboss-ptk Yep, IndentRule should match IntelliJ as closely as possible (when in doubt - just check what IntelliJ does).

[shyiko]

Moved to #338.