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

The trailing comma experimental rule does not working properly #1297

Closed
mohsenoid opened this issue Dec 1, 2021 · 4 comments · Fixed by #1301
Closed

The trailing comma experimental rule does not working properly #1297

mohsenoid opened this issue Dec 1, 2021 · 4 comments · Fixed by #1301

Comments

@mohsenoid
Copy link

mohsenoid commented Dec 1, 2021

The trailing comma experimental rule is not working properly for this file:

https://github.com/mohsenoid/Rick-and-Morty/blob/c9fb8dc093c7986055e8892b8e126f60bf0247cc/app/src/main/kotlin/com/mohsenoid/rickandmorty/data/db/DbRickAndMorty.kt#L51

Android Studio works correctly and adds a comma, but Ktlint doesn't and even removes the comma added by Android Studio!

Expected Behavior

Add a comma and do not remove the one from the Android Studio code styler

Observed Behavior

Do not add a comma and also remove the one from the Android Studio code styler

Steps to Reproduce

Take this file and enable both these rules in .editorconfig

ij_kotlin_allow_trailing_comma = true
ij_kotlin_allow_trailing_comma_on_call_site = true

execute this command:

ktlint -F --experimental "app/src/**/*.kt"

Your Environment

@paul-dingemans
Copy link
Collaborator

Can you be a little more explicit and copy the code example that causes the problem to this issue? Also try to minimise the code sample. Finally, provide the exact output that you expect.

@paul-dingemans
Copy link
Collaborator

Ok, I think I have reconstructed your problem.

The code below, should be accepted unchanged by ktlint:

            annotation class FooBar(
                val foo1: Array<String> = [],
                val foo2: Array<String> = [],
                val bar1: String = "",
                val bar2: String = "",
            )

            @FooBar(
                foo1 = [
                    "foo-1", // Keep the trailing comma as the argument value list foo1 is a multiline statement
                ],
                foo2 = ["foo-2"], // No trailing comma in the array as the argument value list foo2 is a single line statement
                bar1 = "bar-1", // Keep the trailing comma as the outer argument value list of the annotation is a multiline statement
            )
            val fooBar = null

paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Dec 5, 2021
The code previously contained special handling for case below:
    @annotation([
        "something",
    ])

This special case however was also triggered in case of:
    @annotation(
       foo = [
        "something"
       ],
       bar = "some-bar"
   )
In this case the normal handling of value argument lists should be
used once for the outer value argument list (containing elements foo
and bar) and once for the inner value argument list of parameter foo
(containing element "something"). So in example above, a trailing
comma has to be added to both the outer and the inner value argument
list.

Closes pinterest#1297
@mohsenoid
Copy link
Author

@paul-dingemans Thanks for taking care of that. 💪

romtsn added a commit that referenced this issue Dec 13, 2021
)

The code previously contained special handling for case below:
    @annotation([
        "something",
    ])

This special case however was also triggered in case of:
    @annotation(
       foo = [
        "something"
       ],
       bar = "some-bar"
   )
In this case the normal handling of value argument lists should be
used once for the outer value argument list (containing elements foo
and bar) and once for the inner value argument list of parameter foo
(containing element "something"). So in example above, a trailing
comma has to be added to both the outer and the inner value argument
list.

Closes #1297

Co-authored-by: Paul Dingemans <pdingemans@bol.com>
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
@krisgerhard
Copy link

Release it? 😊

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

Successfully merging a pull request may close this issue.

3 participants