Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indentation level is removed when a function call has lambda expression in one parameter #1221

Closed
dimsuz opened this issue Aug 26, 2021 · 5 comments

Comments

@dimsuz
Copy link

dimsuz commented Aug 26, 2021

Observed Behavior

Formatting this (correctly formatted) code with ktlint

class MoneyValidatorsTest : ShouldSpec({
  context("isMoneyValue") {
    should("not error on strings with integers") {
      checkAll( Arb.int().map { it.toString() }, Arb.currency()) { value, currency ->
        val sut = isMoneyValue(currency)

        val result = sut.validate(value)

        result shouldBe Ok(Money(value, currency))
      }
    }
  }
}

results in

class MoneyValidatorsTest : ShouldSpec({
  context("isMoneyValue") {
    should("not error on strings with integers") {
      checkAll(Arb.int().map { it.toString() }, Arb.currency()) { value, currency ->
      val sut = isMoneyValue(currency)

      val result = sut.validate(value)

      result shouldBe Ok(Money(value, currency))
    }
    }
  }
}

Notice how all the lines under checkAll have wrong level of indentation. Notice two curly braces one above another.

Expected Behavior

Above code should be formatted like this:

(Using IDE formatter also produces the correct results)

class MoneyValidatorsTest : ShouldSpec({
  context("isMoneyValue") {
    should("not error on strings with integers") {
      checkAll(Arb.int().map { it.toString() }, Arb.currency()) { value, currency ->
        val sut = isMoneyValue(currency)

        val result = sut.validate(value)

        result shouldBe Ok(Money(value, currency))
      }
    }
  }
}

Note that if I move Arb.int().map { it.toString() } expression out of the function call, ktlint formats this correclty:

class MoneyValidatorsTest : ShouldSpec({
  context("isMoneyValue") {
    should("not error on strings with integers") {
      val valueGen = Arb.int().map { it.toString() }
      checkAll(valueGen, Arb.currency()) { value, currency ->
        val sut = isMoneyValue(currency)

        val result = sut.validate(value)

        result shouldBe Ok(Money(value, currency))
      }
    }
  }
}

Your Environment

  • Version of ktlint used: 0.42.1
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task):
  • Version of Gradle used (if applicable): 7.0.2
  • Operating System and version: Arch Linux
@JolandaVerhoef
Copy link

We're seeing the same behavior after upgrading from 0.41.0 to 0.42.1. You can replicate by copying the following:

private object Test2 {
    fun test() {
        doSomething({}, 1) {
        // Wrong indentation by Spotless
    }
    }

    fun doSomething(x: () -> Unit, y: Int, z: () -> Unit) {}
}

private object Test3 {
    fun test() {
        doSomething(1) {
            // Indented correctly by Spotless
        }
    }

    fun doSomething(y: Int, z: () -> Unit) {}
}

private object Test4 {
    fun test() {
        doSomething({}) {
            // Indented correctly by Spotless
        }
    }

    fun doSomething(x: () -> Unit, z: () -> Unit) {}
}

@liutikas
Copy link
Contributor

This is also blocking androidx project from upgrading as it causes a large diff making code harder to read.

@paul-dingemans
Copy link
Collaborator

Possibly related to #1210

copybara-service bot pushed a commit to androidx/androidx that referenced this issue Sep 25, 2021
also disabling the indent rule until pinterest/ktlint#1221 is resolved

Bug: 197692691
Test: ./gradlew ktlint
Test: treehugger runs busytown/android.sh

Change-Id: I1362ad19afaa6e5b5d49d075b9e5dad1127ebeeb
@paul-dingemans
Copy link
Collaborator

This was fixed by #1202 and no longer occurs in ktlint 0.43.x

@romtsn
Copy link
Collaborator

romtsn commented Dec 11, 2021

thanks for checking 👍

@liutikas just casually checked your config - you can re-enable import-ordering as it supports IJ config now

@romtsn romtsn closed this as completed Dec 11, 2021
copybara-service bot pushed a commit to androidx/androidx that referenced this issue May 10, 2023
It's supposed to accept the order of our imports now - see pinterest/ktlint#1221 (comment)

Also formatting the compose desktop samples project which I missed previously

Bug: 276953541

Test: Run `./studiow g`, open GlanceAppWidgetPreviews.kt, press Ctrl-Alt-L to reformat the file, and observe that the imports don't get moved
Test: Run `./studiow g`, open GlanceAppWidgetViewAdapterTest.kt, press Ctrl-Alt-L to reformat the file, and observe that the imports don't get moved
Change-Id: I53502c7fecb7e0d367ccee2b98d5508bc1e1ef0f
MatkovIvan pushed a commit to MatkovIvan/compose-multiplatform that referenced this issue May 10, 2023
It's supposed to accept the order of our imports now - see pinterest/ktlint#1221 (comment)

Also formatting the compose desktop samples project which I missed previously

Bug: 276953541

Test: Run `./studiow g`, open GlanceAppWidgetPreviews.kt, press Ctrl-Alt-L to reformat the file, and observe that the imports don't get moved
Test: Run `./studiow g`, open GlanceAppWidgetViewAdapterTest.kt, press Ctrl-Alt-L to reformat the file, and observe that the imports don't get moved
Change-Id: I53502c7fecb7e0d367ccee2b98d5508bc1e1ef0f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants