pinterest / ktlint

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

`multiline-if-else` breaks if / else statement + Elvis operator #2501

Closed FloEdelmann closed 8 months ago

FloEdelmann commented 8 months ago

Expected Behavior

return if (foo) bar else null
    ?: baz

This is equivalent to:

return (if (foo) bar else null)
    ?: baz

Observed Behavior

return if (foo) {
    bar
} else {
    null
    ?: baz
}

Note that the Elvis operator no longer acts on the whole if/else statement, but only on the else branch.

Steps to Reproduce

return if (foo) bar else null
    ?: baz

Real codebase example: https://github.com/streetcomplete/StreetComplete/blob/v56.0/app/src/main/java/de/westnordost/streetcomplete/util/NameAndLocationLabel.kt#L116-L123

Your Environment

Version of ktlint used: v1.1.1

Relevant parts of the .editorconfig settings:

ktlint_standard_annotation = disabled
ktlint_standard_argument-list-wrapping = disabled
ktlint_standard_blank-line-before-declaration = disabled
ktlint_standard_block-comment-initial-star-alignment = disabled
ktlint_standard_chain-wrapping = disabled
ktlint_standard_comment-wrapping = disabled
ktlint_standard_filename = disabled
ktlint_standard_function-naming = disabled
ktlint_standard_function-signature = disabled
ktlint_standard_if-else-wrapping = disabled
ktlint_standard_indent = disabled
ktlint_standard_max-line-length = disabled
ktlint_standard_multiline-expression-wrapping = disabled
ktlint_standard_no-blank-line-in-list = disabled
ktlint_standard_no-consecutive-comments = disabled
ktlint_standard_no-empty-first-line-in-class-body = disabled
ktlint_standard_no-multi-spaces = disabled
ktlint_standard_no-single-line-block-comment = disabled
ktlint_standard_no-wildcard-imports = disabled
ktlint_standard_package-name = disabled
ktlint_standard_parameter-list-wrapping = disabled
ktlint_standard_paren-spacing = disabled
ktlint_standard_property-naming = disabled
ktlint_standard_property-wrapping = disabled
ktlint_standard_spacing-between-declarations-with-annotations = disabled
ktlint_standard_spacing-between-declarations-with-comments = disabled
ktlint_standard_statement-wrapping = disabled
ktlint_standard_string-template-indent = disabled
ktlint_standard_trailing-comma-on-call-site = disabled
ktlint_standard_trailing-comma-on-declaration-site = disabled
ktlint_standard_value-argument-comment = disabled
ktlint_standard_value-parameter-comment = disabled
ktlint_standard_wrapping = disabled

Operating System and version: Ubuntu 22.04

paul-dingemans commented 8 months ago

This is equivalent to:

I think you're wrong. Test below succeeds:

    @Test
    fun `Issue 2501`() {
        assertThat(doSomething(true, "bar", "baz")).isEqualTo("bar")
        assertThat(doSomething(true, null, "baz")).isNull()
        assertThat(doSomething(false, "bar", "baz")).isEqualTo("baz")
    }

    fun doSomething(
        foo: Boolean,
        bar: String?,
        baz: String?,
    ): String? {
        return if (foo) bar else null
            ?: baz
    }

Also, the PSI parser of kotlint shows that the elvis operator is part of the else branch only: Screenshot 2024-01-11 at 19 03 58

When using parenthesis around the if-else, the PSI structure changes to: Screenshot 2024-01-11 at 19 04 40

FloEdelmann commented 8 months ago

Oh, you're right, so it only worked in the code I linked above because the if branch's value was never null and it then makes no difference whether to put the Elvis operator inside the else branch or outside the whole if/else statement. Thanks for checking!