nbadal / ktlint-intellij-plugin

Ktlint plugin for IntelliJ IDEA + Android Studio
MIT License
157 stars 23 forks source link

Manual reformat (plugin version 0.24.0, Ruleset 1.3.0) #551

Closed ievgen-kapinos closed 1 month ago

ievgen-kapinos commented 1 month ago

Hi, Paul

My team has 20+ repositories. We try to keep our Code Style the same. As a follow-up on https://github.com/nbadal/ktlint-intellij-plugin/issues/540#issuecomment-2181260336 I've commented .idea/ktlint-plugin.xml file with the same content (fixed Ruleset=1.3.0) into all repositories.

All was Okay on a Plugin version 0.23.0. Generated code was compliant. We check it additionally on CI/CD via https://github.com/JLLeitschuh/ktlint-gradle also linked to the same Ruleset version (1.3.0)

Issue But, after upgrading Plugin to version 0.24.0 manual reformatting periodically generates code not compliant with Ruleset 1.3.0. To reproduce an issue you need to open file, click on any place in Editor and execute Menu > Code > Reformat Code command few times.

Important to mention, latest version of IDE was used (on 2024-07-15):

After downgrading plugin back to 0.23.0 issue has gone.

https://github.com/user-attachments/assets/cd26885b-3976-4982-9863-20aefba48ebf

Our use case:

Additional question So far it looks Ktlint plugin does not support full functionality for non-latest Ruleset (manual fixing individual violations). Am I right?

paul-dingemans commented 1 month ago

Are you sure that that you have set the ktlint versions in the plugin preferences? Your .idea/ktlint-plugin.xml file should contain the ktlintRulesetVersion setting:

<?xml version="1.0" encoding="UTF-8"?>
<project version="4">
  <component name="KtLint plugin">
    ... 
    <ktlintRulesetVersion>V1_3_0</ktlintRulesetVersion>
    ...
  </component>
</project>

If this element is not present, it will use the latest ktlint version by default. With plugin version 0.24.0 this has been updated to ktlint 1.3.1.

Issue But, after upgrading Plugin to version 0.24.0 manual reformatting periodically generates code not compliant with Ruleset 1.3.0. To reproduce an issue you need to open file, click on any place in Editor and execute Menu > Code > Reformat Code command few times.

Please share a code sample which reproduces above.

ievgen-kapinos commented 1 month ago

Yes, .idea/ktlint-plugin.xml looks like in your snippet. Here is a printscreen of what I have. You can also find in a video (from description) how Settings > Tools > Ktlint looks like in IDE. Version 1.3.0 is applied

Screenshot 2024-07-16 at 09 47 18

I cannot share that particular snippet, but created a new one with same behaviour. This version was formatted with plugin 0.23.0

package my.test

import io.kotest.core.spec.style.StringSpec

class MyTest :
    StringSpec({
        "test" {
            listOf(1, 2, 3).map { outV ->

                val x =
                    listOf(1, 2, 3)
                        .toMutableList()
                        .map { inV ->
                            outV * 10 + inV
                        }.toList()

                x.forEach { println(it) }
            }
        }
    })

After I've updated to 0.24.0, restarted IDE. Ruleset stays the same (1.3.0). I've reformatted code two times. And .toMutableList() part was moved.

package my.test

import io.kotest.core.spec.style.StringSpec

class MyTest : StringSpec({
    "test" {
        listOf(1, 2, 3).map { outV ->

            val x =
                listOf(1, 2, 3).toMutableList().map { inV ->
                    outV * 10 + inV
                }.toList()

            x.forEach { println(it) }
        }
    }
})

My college reproduced same issue with same IDE and Ktlint plugin version

paul-dingemans commented 1 month ago

I didn't notice that you included a video in your original post. I do not think this problem is related to ktlint though. I can reproduce the above behavior with both 2024.1.3 and 2024.1.4.

Intellij IDEA has some special behavior that with two consecutive reformats, you get the ability to remove custom line breaks. When you invoke the reformat with a shortcut instead of using the menu, you will see a popup about this second invocation: Screenshot 2024-07-16 at 15 09 05

After second invocation, you get another popup where you can choose to remove or keep the custom line breaks: Screenshot 2024-07-16 at 15 10 43

Please check your settings, wether it has been set: Screenshot 2024-07-16 at 15 12 24

ievgen-kapinos commented 1 month ago

Hi @paul-dingemans

Amazing finding. Workaround works.

But I need to admit this issue does not exist with checked Reformat again..., if in Ktlint settings I select Ruleset version: Default (recommended)

Here is the video (note at 00:27 I've clicked Ctrl+Z to revert code) https://github.com/user-attachments/assets/54689601-93dd-4103-b7d2-1e4a15bea19a

paul-dingemans commented 1 month ago

Most likely I have found the problem. The 1.3.0 ruleset actually referred to 1.2.1 instead of 1.3.0. I can not fully explain why it resulted in the reported behavior, but my gut feeling indicates this as a potential root cause.

paul-dingemans commented 1 month ago

The problem is not solved, but I have more confidence that the problem is not caused by ktlint. Can you try to reproduce following.

I have refactored you code sample so that I do not have to import kotest, but still have a compilable file:

// Required to get compilable file without importing KoTest
private operator fun String.invoke(value: Any) {
    TODO("Not yet implemented")
}

// Required to get compilable file without importing KoTest
open class StringSpec(value: Any)

class MyTest :
    StringSpec({
        "test" {
            listOf(1, 2, 3).map { outV ->
                val x = listOf(1, 2, 3)
                    .toMutableList()
                    .map { inV ->
                        outV * 10 + inV
                    }.toList()

                x.forEach { println(it) }
            }
        }
    })

[OPTIONAL] In Intellij IDEA you can enable debug logging in case you want to follow along what is happening. Go to menu Help -> Diagnostic Tools -> Debug log settings. Add following:

com.pinterest:trace
org.nbadal:all

First line enables all logging of ktlint. Second line enables logging of the ktlint plugin.

Via menu Help -> Open log in editor, you can see the logging of the idea. For convenience, I just delete everything inside the file. Note that ktlint produces lots of tracing logging for indentation. Also, it does not work to set debug level to "debug" instead of "trace".

If you format code above, you should see ktlint log lines in the log. Now disable the ktlint plugin, and check that no ktlint log lines appear any more. The code still gets reformatted sometimes as you initially reported.

If you enable the plugin again, it evens gets more interesting that the unwanted behavior disappears whenever I add fragment below to the same file (regardless of which ktlint version I use):

fun x(a: Any, b: Any, c: Any) =
    a.toString() + b.toString() + a.toString() + b.toString() + a.toString() + b.toString() +
        a.toString() +
        b.toString()

Removing the fragment, and the unwanted behavior is back. It seems to be important that fragment above is a multiline expression which exceeds the maximum line length of 100 which is used for android_studio code style.

ievgen-kapinos commented 1 month ago

Will definitely do next week. Sorry, to much work.

ievgen-kapinos commented 1 month ago

Most likely I have found the problem. The 1.3.0 ruleset actually referred to 1.2.1 instead of 1.3.0. I can not fully explain why it resulted in the reported behavior, but my gut feeling indicates this as a potential root cause.

I've installed plugin ktlint-0.24.1-dev.2024-07-18_16-46-40.zip from

https://plugins.jetbrains.com/plugin/15057-ktlint/versions

and it works in my setup. Skimmed your next message. Even if you not 100% happy with result. Plugin state is better than before and it fixes issue on some setups. Can we have stable release with this bug fix?

Checking your next message...

ievgen-kapinos commented 1 month ago

TL;DR

Cannot reproduce.

Have you restored original.idea/ktlint-plugin.xml file? It is auto-deleted after disabling plugin. In result plugin configuration erased: Mode is unset and Ruleset is default.

Details

Need to admit, my setup is:

My .editorconfig file looks like this

root = true

[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
max_line_length = 120
trim_trailing_whitespace = true
insert_final_newline = true

[*.{kt,kts}]
indent_size = 4
ktlint_code_style = ktlint_official
ij_kotlin_code_style_defaults = KOTLIN_OFFICIAL
ij_kotlin_name_count_to_use_star_import = 2147483647
ij_kotlin_name_count_to_use_star_import_for_members = 2147483647
ij_kotlin_packages_to_use_import_on_demand = unset

The problem is not solved, but I have more confidence that the problem is not caused by ktlint. Can you try to reproduce following.

I have refactored you code sample so that I do not have to import kotest, but still have a compilable file:

I've copied your code and trying to reproduce an issue with it. This code is not compatible with ruleset 1.3.0 which I have. So I reformatted it (max with is 60 characters) and got changed these lines

open class StringSpec(
    value: Any,
)

and

                val x =
                    listOf(1, 2, 3)

resulting file I use a a reference.

[OPTIONAL] In Intellij IDEA you can enable debug logging in case you want to follow along what is happening.

Enabled

If you format code above, you should see ktlint log lines in the log.

I see them

Now disable the ktlint plugin, and check that no ktlint log lines appear any more.

True. I've also restarted IDE for sure

❗ On restart.idea/ktlint-plugin.xml was deleted. So I've restored it

The code still gets reformatted sometimes as you initially reported.

True... I suppose standard IDE formatting is applied. So it is expected to me.

If you enable the plugin again,

Done, I've also restarted IDE for sure

it evens gets more interesting that the unwanted behavior disappears whenever I add fragment below ...

In my case all following attempts to reformat code do not change code. If I add or remove this long function fun x(a: Any, b: Any, c: Any) (btw it is also not complaint with 1.3.0 ruleset), it does not affect formatting for other part of the same file in my setup.

paul-dingemans commented 1 month ago

Even if you not 100% happy with result. Plugin state is better than before and it fixes issue on some setups. Can we have stable release with this bug fix?

Bugfix release is created. It is now pending, and waiting for approval of Jetbrains (max 2 business days).