Skip to content

Fix trailing lambda with chained call indentation (#589)#626

Closed
ptitjes wants to merge 2 commits into
facebook:mainfrom
ptitjes:p/589-correct-trailing-lambda-indentation-with-chained-call
Closed

Fix trailing lambda with chained call indentation (#589)#626
ptitjes wants to merge 2 commits into
facebook:mainfrom
ptitjes:p/589-correct-trailing-lambda-indentation-with-chained-call

Conversation

@ptitjes

@ptitjes ptitjes commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the indentation of trailing lambda with chained calls.
See #589.

I used an AI agent to understand how ktfmt and google-java-format work. This PR has been implemented with the help of an AI agent. I don't know much about the PSI AST shapes, so I wouldn't be surprised if the AST tests and visits are not general enough, or simply wrong.

@meta-cla 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 Jun 8, 2026
@meta-codesync

meta-codesync Bot commented Jun 15, 2026

Copy link
Copy Markdown

@hick209 has imported this pull request. If you are a Meta employee, you can view this in D108649008.

@hick209

hick209 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

This said no internal changes here. Which is odd, I was expecting a bunch of them.
Tomorrow I'll do some extra validation of it, but just wanted to let you know this much already

@ptitjes

ptitjes commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@hick209 tell me if you need me to add more tests or fix something.

@hick209

hick209 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Oops, look at me not keeping my promises. Sorry about that, will look at it again right now!

@hick209 hick209 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is incomplete as I suspected.

Here's the test I made, which I thought should happen the other way around

  @Test
  fun nivaldo_test() {
    // this passed
    assertFormatted(
        """
        |fun quux() {
        |  runnnnn {
        |        foo()
        |        bar()
        |      }
        |      .baz()
        |}
        |"""
            .trimMargin()
    )

    // this failed
    assertFormatted(
        """
        |fun quux() {
        |  runnnnn {
        |    foo()
        |    bar()
        |  }
        |      .baz()
        |}
        |"""
            .trimMargin()
    )
  }

Please LMK if I'm missing something here.

Also don't forget to update the CHANGELOG.md so I don't forget to give you proper attribution

Comment thread core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt Outdated
@ptitjes ptitjes force-pushed the p/589-correct-trailing-lambda-indentation-with-chained-call branch from 6aeba2f to c94e0d5 Compare June 18, 2026 09:19
@ptitjes

ptitjes commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Yes, this is incomplete as I suspected.

Hmmm, sorry, it seems I forgot half of the issue... I fixed it and added a bunch more tests.
Also fixed it to use the continuation indent for chained calls (and not the block indent as it was before).

Also don't forget to update the CHANGELOG.md so I don't forget to give you proper attribution

Done.

I applied the new formatting (1 change in KotlinInputAstVisitor.kt) in a separate commit, so that you can see the changes. Now I expect you will have more changes on your internal projects...

@ptitjes ptitjes requested a review from hick209 June 18, 2026 09:25
@ptitjes ptitjes force-pushed the p/589-correct-trailing-lambda-indentation-with-chained-call branch from c94e0d5 to d69e01b Compare June 18, 2026 13:19
@hick209

hick209 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

It still says no changes in our mega repo 🤔

Maybe we are missing something here. Let me test it out further to see if we are missing any use case

Edit: This might actually be a bug on our end here. I'm investigating
Edit 2: My script was too narrow and this pattern is not all that wide spread. That's why it was not caught. Seems like we are good now!

Thanks for the awesome work!

@cortinico cortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@meta-codesync meta-codesync Bot closed this in b8c9910 Jun 22, 2026
@meta-codesync meta-codesync Bot added the Merged label Jun 22, 2026
@meta-codesync

meta-codesync Bot commented Jun 22, 2026

Copy link
Copy Markdown

@hick209 merged this pull request in b8c9910.

@ptitjes ptitjes deleted the p/589-correct-trailing-lambda-indentation-with-chained-call branch June 23, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants