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

Formatting Regression in KtLint 1.4.1: Violates max_line_length and Reduces Readability #2881

Closed
leinardi opened this issue Nov 28, 2024 · 10 comments

Comments

@leinardi
Copy link
Contributor

Expected Behavior

When formatting Kotlin code using KtLint 1.4.1, the formatter should produce output that respects the max_line_length setting defined in the .editorconfig file. Additionally, the formatted code should follow Kotlin Coding Conventions for readability, ensuring appropriate line breaks for long function calls.

For example, formatting this code:

@GskVersion4_14
public fun foreach(flags: PathForeachFlags, func: PathForeachFunc): Boolean = gsk_path_foreach(gskPathPointer.reinterpret(), flags.mask, PathForeachFuncFunc.reinterpret(), StableRef.create(func).asCPointer()).asBoolean()

with max_line_length=120 should produce this result, as in KtLint 1.3.1:

public fun foreach(
    flags: PathForeachFlags,
    func: PathForeachFunc,
): Boolean =
    gsk_path_foreach(
        gskPathPointer.reinterpret(),
        flags.mask,
        PathForeachFuncFunc.reinterpret(),
        StableRef.create(func).asCPointer()
    ).asBoolean()

This output is both more readable and adheres to the max_line_length constraint.

Observed Behavior

Using KtLint 1.4.1, the same input code is formatted as:

public fun foreach(
    flags: PathForeachFlags,
    func: PathForeachFunc,
): Boolean = gsk_path_foreach(gskPathPointer.reinterpret(), flags.mask, PathForeachFuncFunc.reinterpret(), StableRef.create(func).asCPointer()).asBoolean()

This output violates the max_line_length constraint and reduces readability.

Steps to Reproduce

  1. Prepare a .editorconfig file with the following configuration: .editorconfig
  2. Use KtLint 1.4.1 to format the following code snippet:
    @GskVersion4_14
    public fun foreach(flags: PathForeachFlags, func: PathForeachFunc): Boolean = gsk_path_foreach(gskPathPointer.reinterpret(), flags.mask, PathForeachFuncFunc.reinterpret(), StableRef.create(func).asCPointer()).asBoolean()
  3. Compare the output of KtLint 1.4.1 to the output of KtLint 1.3.1 or the expected result above.

Command executed:

import com.pinterest.ktlint.cli.ruleset.core.api.RuleSetProviderV3
import com.pinterest.ktlint.rule.engine.api.Code
import com.pinterest.ktlint.rule.engine.api.EditorConfigDefaults
import com.pinterest.ktlint.rule.engine.api.EditorConfigDefaults.Companion.load
import com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine
import com.pinterest.ktlint.rule.engine.core.api.AutocorrectDecision
import com.pinterest.ktlint.rule.engine.core.api.propertyTypes
import org.gtkkn.gir.log.logger
import java.io.File
import java.nio.file.Path
import java.util.ServiceLoader

class KtLintFormatter(outputDir: File) {
    private val ruleProviders = buildSet {
        ServiceLoader.load(RuleSetProviderV3::class.java).flatMapTo(this) { it.getRuleProviders() }
    }
    private val ktLintRuleEngine: KtLintRuleEngine

    init {
        val editorConfigPath = findEditorConfigPath(outputDir)
        if (editorConfigPath == null) {
            logger.error { "Unable to find .editorconfig file in or above directory ${outputDir.absolutePath}" }
        }
        ktLintRuleEngine = KtLintRuleEngine(
            ruleProviders = ruleProviders,
            editorConfigDefaults = editorConfigPath?.let { load(it, ruleProviders.propertyTypes()) }
                ?: EditorConfigDefaults.EMPTY_EDITOR_CONFIG_DEFAULTS,
        )
    }

    fun formatAndWriteKotlinFile(
        outputDirectory: File,
        packageName: String,
        simpleName: String,
        content: String
    ) {
        // Create the package directory structure
        val packageDir = packageName.replace(".", "/")
        val dir = File(outputDirectory, packageDir).apply { mkdirs() }

        // Create the Kotlin file
        val kotlinFile = File(dir, "$simpleName.kt")
        kotlinFile.createNewFile()

        // Format and write the file
        kotlinFile.writeText(
            ktLintRuleEngine.format(Code.fromSnippet(content)) { _ ->
                AutocorrectDecision.ALLOW_AUTOCORRECT
            },
        )
    }

    private fun findEditorConfigPath(startDir: File): Path? {
        var currentDir: File? = startDir

        while (currentDir != null) {
            val editorConfig = File(currentDir, ".editorconfig")
            if (editorConfig.exists()) {
                return editorConfig.toPath()
            }

            currentDir = currentDir.parentFile
        }

        return null
    }
}

Your Environment

  • Version of ktlint used: 1.4.1
  • Relevant .editorconfig settings:
    [*.{kt,kts}]
    max_line_length = 120
  • Integration method: Custom task using KtLintRuleEngine directly.
  • Operating System and version: Ubuntu 24.04
@paul-dingemans
Copy link
Collaborator

You call the KtLintRuleEngine incorrectly:

ktLintRuleEngine.format(Code.fromSnippet(content))

You have provide a code snippet without providing its virtual path. As of that ktlint can not locate the proper .editorconfig file. Instead of fromSnippet use fromSnippetWithPath. The provide virtualPath to this snippet should be in the same directory, or a subdirectory of the directory where your .editorconfig is located.

@leinardi
Copy link
Contributor Author

leinardi commented Nov 29, 2024

Thank you for your suggestion. I updated my code to use Code.fromSnippetWithPath, ensuring the virtualPath points to the generated file's location. Here are the changes I made:

// Provide a virtual path for the code snippet as a Path
val virtualPath = kotlinFile.toPath()

// Format and write the file
kotlinFile.writeText(
    ktLintRuleEngine.format(Code.fromSnippetWithPath(content, virtualPath)) { _ ->
        AutocorrectDecision.ALLOW_AUTOCORRECT
    },
)

The .editorconfig file is located at the root of the project, and the virtualPath points to a subdirectory of the root, as seen in this file: AboutDialog.kt.

However, the formatting behavior remains inconsistent with version 1.3.1.
Screenshot from 2024-11-29 09-26-48

Do you have any other suggestions that I can try?

@paul-dingemans
Copy link
Collaborator

Suggestions:

  • Double check the virtualPath. It must be an absolute path.
  • Add the root = true property in .editorconfig (https://editorconfig.org/)
  • Use ktlint-gradle instead of your custom plugin, or look at how they call KtlintRuleEngine

@leinardi
Copy link
Contributor Author

Unfortunate virtualPath is absolute, the root = true is there. The Gradle plugin is not an option. I just don't get why this works fine with 1.3.1 but not 1.4.1...

@paul-dingemans
Copy link
Collaborator

I just don't get why this works fine with 1.3.1 but not 1.4.1...

I have no clue why it breaks. When I use your .editorconfig and provided code sample, then Ktlint CLI produces the expected results.

It would be helpful if you can enable debug logging for ktlint, or set a breakpoint at this line and inspect the result.

@leinardi
Copy link
Contributor Author

leinardi commented Dec 2, 2024

Sure, I'm happy to help debugging this issue, just let me know what else can I do to help (I'm also available if you want to have a remote debug session in a call).

This is how it looks on 1.3.1:
image
as you can see, filePath is null, but editorConfigDefaults contains the correct rules, I guess because of this:

    init {
        val editorConfigPath = findEditorConfigPath(outputDir)
        if (editorConfigPath == null) {
            logger.error { "Unable to find .editorconfig file in or above directory ${outputDir.absolutePath}" }
        }
        ktLintRuleEngine = KtLintRuleEngine(
            ruleProviders = ruleProviders,
            editorConfigDefaults = editorConfigPath?.let { load(it, ruleProviders.propertyTypes()) }
                ?: EditorConfigDefaults.EMPTY_EDITOR_CONFIG_DEFAULTS,
        )
    }

Version 1.4.1 looks more or less the same:
image

The .editorconfig path is /home/rleinardi/Workspace/My/gtk-kn/.editorconfig, not sure if it's normal that has been detected as /home/rleinardi/Workspace/My/gtk-kn/gir/./.kt, could this be the issue? gir is the module that is running KtLint, but the .editorconfig is in the root of the project. But in any case, since editorConfigDefaults contains all the rules, shouldn't this be enough for 1.4.1? I still don't get why the difference in formatting from 1.3.1, since they seem to have the same configuration.

Please let me know if I can provide other details that can help to solve this issue.

@paul-dingemans
Copy link
Collaborator

I have identified the problem. You have disabled ktlint_standard_max-line-length rule. As of that rules no longer respect the max_line_length property. This was an intended change which should have been more clearly documented in the changelog.

@leinardi
Copy link
Contributor Author

leinardi commented Dec 2, 2024

@paul-dingemans thank you! I can confirm that after removing ktlint_standard_max-line-length now 1.4.1 respects the max-line-length! I don't even remember why I had it disabled.

But there is still 1 final difference between 1.3.1 and 1.4.1: the latter uses a different spacing for the callback parameters than 1.3.1 and the formatter used by Intellij IDEA:
image

Is there a setting that I can change to restore the same spacing that 1.3.1 and IDEA have?

@paul-dingemans
Copy link
Collaborator

No, this behavior is determined by the ktlint_code_style setting. See #2816 for more information.

@leinardi
Copy link
Contributor Author

leinardi commented Dec 3, 2024

Thank you! Using ktlint_code_style = intellij_idea fixed the issue!

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

No branches or pull requests

2 participants