Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ktlint.format does not invoke callback for auto corrected errors #1491

Closed
gabrielittner opened this issue Jun 11, 2022 · 6 comments · Fixed by #1499
Closed

Ktlint.format does not invoke callback for auto corrected errors #1491

gabrielittner opened this issue Jun 11, 2022 · 6 comments · Fixed by #1499

Comments

@gabrielittner
Copy link
Contributor

Expected Behavior

The cb parameter of ExperimentalParams is invoked for all errors with the correct value for correct.

Observed Behavior

cb is only invoked for errors that aren't auto correctable. From both the description of the parameter "callback invoked for each lint error" and the signature (e: LintError, corrected: Boolean) -> Unit this seems weird. In practice I'm never seeing corrected being true because those errors aren't reported at all.

Steps to Reproduce

Run the following snippet on a file that has both correctable and not correctable errors:

fun Path.format(root: Path) {
    val fileName = normalize().toString()
    val result = KtLint.lint(
        KtLint.ExperimentalParams(
            fileName = fileName,
            text = readText(),
            ruleSets = listOf(StandardRuleSetProvider().get()),
            script = !fileName.endsWith(".kt", ignoreCase = true),
            cb = { e, corrected -> println("$this: $e $corrected") },
        ),
    )
}

Your Environment

  • Version of ktlint used: 0.45.2
  • Relevant parts of the .editorconfig settings
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): Manually called with the code above
  • Operating System and version: macOS 12.4
@paul-dingemans
Copy link
Collaborator

The callback cb is called for each LintError including errors which can not be autocorrected. Let me explain.

The toString representation of the LintError which does not include the canBeAutoCorrected flag which leads to misleasing output in your example:

public data class LintError(
    val line: Int,
    val col: Int,
    val ruleId: String,
    val detail: String
) : Serializable {

    // fixme:
    // not included in equals/hashCode for backward-compatibility with ktlint < 0.25.0
    // subject to change in 1.0.0
    var canBeAutoCorrected: Boolean = false
        private set
...
}

I have included the flag canBeAutoCorrected as follows:

    @Test
    fun `lint`() {
        val code =
            """
                fun <T>
            /**
              * some comment
              */
            foo(t: T) = "some-result"
            """.trimIndent()
        KtLint.lint(
            KtLint.ExperimentalParams(
                text = code,
                ruleSets = listOf(
                    RuleSet("standard", DiscouragedCommentLocationRule(), IndentationRule())
                ),
                cb = { e, corrected -> println("$this: $e, ${e.canBeAutoCorrected} $corrected") }
            )
        )
    }

The output of test above is:

comLintError(line=1, col=1, ruleId=indent, detail=Unexpected indentation (4) (should be 0)), true false
comLintError(line=2, col=1, ruleId=experimental:discouraged-comment-location, detail=No comment expected at this location), false false
comLintError(line=3, col=1, ruleId=indent, detail=Unexpected indentation (2) (should be 1)), true false
comLintError(line=4, col=1, ruleId=indent, detail=Unexpected indentation (2) (should be 1)), true false

Note that for the second error, the flag canBeAutocorrected has value false. For all errors the flag corrected is false as the lint function does not correct the errors. For that the function format has to be called.

When the same is done for the format function:

    @Test
    fun `format`() {
        val code =
            """
                fun <T>
            /**
              * some comment
              */
            foo(t: T) = "some-result"
            """.trimIndent()
        KtLint.format(
            KtLint.ExperimentalParams(
                text = code,
                ruleSets = listOf(
                    RuleSet("standard", DiscouragedCommentLocationRule(), IndentationRule())
                ),
                cb = { e, corrected -> println("$this: $e, ${e.canBeAutoCorrected} $corrected") }
            )
        )
    }

the output is:

LintError(line=2, col=2, ruleId=experimental:discouraged-comment-location, detail=No comment expected at this location), false false

Note that the errors which are automatically corrected, are not included in the list.

@gabrielittner
Copy link
Contributor Author

Note that the errors which are automatically corrected, are not included in the list.

That's the issue, why are they not in the list? What's the point of the corrected boolean if anything where it would be true is omitted.

@gabrielittner
Copy link
Contributor Author

My example was supposed to use format, not lint, I copy and pasted the wrong snippet

@paul-dingemans
Copy link
Collaborator

Note that the errors which are automatically corrected, are not included in the list.

That's the issue, why are they not in the list? What's the point of the corrected boolean if anything where it would be true is omitted.

As far, as I can tell it was designed like this. But, I do admit that it does not seem logical.

I expect that the behavior can be changed without affecting the Ktlint CLI. But that would mean, that it would only have value for API users that are calling the format function directly. How do you intend to use this functionality if the callback was called?

@gabrielittner
Copy link
Contributor Author

Currently Gradle plugins for ktlint have a format and a check task that call format/lint on ktlint. My idea is to have a general task that runs format and collects the formatted output as well as all errors. The check and format tasks would use that as input instead of running ktlint themselves and then just print the errors or apply the formatting changes. The collection task can then also use the build cache regardless of whether you're linting or formatting (we lint on ci that would fill the build cache and a dev running format locally can benefit from it)

@paul-dingemans
Copy link
Collaborator

I can not envision whether this will work and/or when it becomes beneficial but that of course is up to you. How long does it take to run ktlint on your codebase? I use a couple of opensource projects with 400K lines to test ktlint which takes less than 30 seconds to complete.

paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Jun 16, 2022
paul-dingemans added a commit that referenced this issue Jul 14, 2022
paul-dingemans added a commit that referenced this issue Jul 24, 2022
* Call callback on all errors
* Add reporter for a format summary of one line per file.

Closes #1491
Closes #821
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants