pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.23k stars 512 forks source link

IndentationRule uses wrong continuation intendation #379

Closed vanniktech closed 5 years ago

vanniktech commented 5 years ago

When using the --experimental flag with ktlint 0.31.0 and the following configuration:

[*.{kt,kts}]
indent_size=2
max_line_length=200

the following code is flagged:

package com.vanniktech.color

import org.assertj.core.api.Java6Assertions.assertThat
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import java.awt.Color

class ColorExtensionsSpek : Spek({
  describe("color extensions") {
    mapOf(
        Color.RED to "#FF0000",
        Color.GREEN to "#00FF00",
        Color.BLUE to "#0000FF",
        Color.CYAN to "#00FFFF"
    ).forEach { color, string ->
      it("should convert rgb int color correctly to hex string") {
        assertThat(color.rgb.colorAsHex()).isEqualTo(string)
      }
    }
  }
})

This is the output:

ColorExtensionsSpek.kt:11:1: Unexpected indentation (8) (should be 6)
ColorExtensionsSpek.kt:12:1: Unexpected indentation (8) (should be 6)
ColorExtensionsSpek.kt:13:1: Unexpected indentation (8) (should be 6)
ColorExtensionsSpek.kt:14:1: Unexpected indentation (8) (should be 6)

The indentation should not be 6 though since it's on a new line and should be using continuation indentation which is 8.

shashachu commented 5 years ago

@vanniktech I believe since the Kotlin Android styledocs removed any mention of continuation indent, the indentation formatter does not use a separate continuation intent. It will simply do a +indent_size

vanniktech commented 5 years ago

It really should do a multiplication by two. Shouldn't it?

NightlyNexus commented 5 years ago

I haven't seen examples of Kotlin styles using separate rules for continuation indents over normal indents. if you see it in a Google or JetBrains style guide, can you post the link?

shashachu commented 5 years ago

Closing this for now because a continuation indent is not part of the style guide.

vanniktech commented 5 years ago

Then ktlint shouldn't flag this, should it?

NightlyNexus commented 5 years ago

i think it flags it because there should be only a single indent, not a special double indent.

vanniktech commented 5 years ago

Should not ktlint support this though? At least when specifying the continuation_indent_size.

NightlyNexus commented 5 years ago

Oh, I didn't realize there was a continuation indent size setting for ktlint. Seems like that should be removed, especially if it's not working.

shashachu commented 5 years ago

Ah yeah it got effectively deprecated here https://github.com/pinterest/ktlint/issues/171 (release 0.22) so it should just be removed.