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

If-else brackets compliance #1573

Closed
pinguinjkeke opened this issue Aug 4, 2022 · 5 comments · Fixed by #1767
Closed

If-else brackets compliance #1573

pinguinjkeke opened this issue Aug 4, 2022 · 5 comments · Fixed by #1767
Labels
custom (3rd party) rule Could be implemented as rule but does not fit within ktlint's standard and experimental rulesets. enhancement ktlint-official-codestyle
Milestone

Comments

@pinguinjkeke
Copy link

pinguinjkeke commented Aug 4, 2022

Expected Rule behavior

If one of if/else/else if use braces then all parts must use it
Correct:

if (true) 1 else 2

if (true) 1 else if (false) 2 else 3

if (true) {
    1
} else {
    2
}

if (true) {
    1
} else if (false) {
    2
} else {
    3
}

Incorrect:

if (true) 1 else {
    2
}

if (true) {
    1
} else 2

if (true) {
    1
} else if (false) 2 else {
    3
}
@paul-dingemans
Copy link
Collaborator

From a perspective of a consistent code style, I do prefer this as well. It however is not documented explicitly as such in the Kotlin Coding Conventions. As of that it does not fit in the standard and experimental ruleset of KtLint. It could be implemented in an additional ruleset.

@paul-dingemans paul-dingemans added custom (3rd party) rule Could be implemented as rule but does not fit within ktlint's standard and experimental rulesets. and removed multiline-if-else rule labels Aug 19, 2022
@obecker
Copy link

obecker commented Nov 27, 2022

Has this been implemented in the last release? I'm currently upgrading detekt in our project to the latest version (1.22.0 which uses ktlint 0.47.1). Now the multiline-if-else rule reports for code snippets like this

if (condition) {
    // ...
} else null

that it is missing braces in the else null line.

@paul-dingemans
Copy link
Collaborator

Has this been implemented in the last release? I'm currently upgrading detekt in our project to the latest version (1.22.0 which uses ktlint 0.47.1). Now the multiline-if-else rule reports for code snippets like this

if (condition) {
    // ...
} else null

that it is missing braces in the else null line.

The behavior of this rules is indeed changed as part of PR #1565 (https://github.com/pinterest/ktlint/pull/1565/files#diff-23102555750e484a7df496237f01c33f59ceca2b0ab415b05de3b05c79c4cd2eR500-R520)

paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Jan 5, 2023
…e style. This rule enforces consistent usage of braces in all branches of a singe if or if-else-if statement.

Closes pinterest#1573
@paul-dingemans paul-dingemans added this to the 0.49.0 milestone Jan 16, 2023
paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Jan 18, 2023
…e style. This rule enforces consistent usage of braces in all branches of a singe if or if-else-if statement.

Closes pinterest#1573
@pinguinjkeke
Copy link
Author

pinguinjkeke commented Mar 30, 2023

@paul-dingemans for me it looks like not solved. if (true) 1 else 2 seems incorrect with this ktlint rule

@paul-dingemans
Copy link
Collaborator

for me it looks like not solved. if (true) 1 else 2 seems incorrect with this ktlint rule

Please be more specific. What exactly seems incorrect? What are your .editorconfig settings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
custom (3rd party) rule Could be implemented as rule but does not fit within ktlint's standard and experimental rulesets. enhancement ktlint-official-codestyle
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants