pinterest / ktlint

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

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

Open leinardi opened 3 days ago

leinardi commented 3 days ago

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

paul-dingemans commented 3 days ago

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 commented 2 days ago

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 commented 2 days ago

Suggestions:

leinardi commented 2 days ago

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...