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

take toggleOffOn into account for linters #719

Open
MarijanGazica opened this issue Oct 16, 2020 · 3 comments
Open

take toggleOffOn into account for linters #719

MarijanGazica opened this issue Oct 16, 2020 · 3 comments

Comments

@MarijanGazica
Copy link

First of all, I'm expecting a solution to this to be something extremely obvious but here it is.
Spotless setup in the project looks like this:

apply plugin: "com.diffplug.spotless"

spotless {

    ext.targetBranch = "develop"
    if (project.hasProperty('target_branch') && project.getProperty('target_branch')?.trim()) {
        targetBranch = project.getProperty('target_branch')
    }

    ratchetFrom "origin/$targetBranch"

    kotlin {
        target '**/*.kt'
        trimTrailingWhitespace()
        indentWithSpaces()
        toggleOffOn('format:off', 'format:on')

        ktlint().userData([
                'max_line_length': '120',
                'disabled_rules' : 'import-ordering' //https://github.com/pinterest/ktlint/issues/527, https://youtrack.jetbrains.com/issue/KT-10974
        ])
    }
}

The problem occurs when trying to exclude a part of the code using spotless:off and spotless:on
I also tried (as seen in the snippet above) with different tags but running ./gradlew spotlessCheck still fails.

The part where it fails looks something like this:


    // format:off
    private fun functionName(): ReturnType = when (this) {
        // A bunch of stuff, one line is >120 chars, our line limit
    }
    // format:on

Reason for failure:

java.lang.AssertionError: Error on line: 70, column: 1
Exceeded max line length (120)

What is it that I'm missing here?

@nedtwigg nedtwigg changed the title toggleOffOn not taken in account toggleOffOn not taken in account (for errors) Oct 16, 2020
@nedtwigg
Copy link
Member

nedtwigg commented Oct 16, 2020

If ktlint responded to exceeding max line length by autowrapping, then the off/on tags would work. But ktlint apparently doesn't respond by autowrapping, it just yells.

For everything that ktlint "just fixes", the off/on tags will work. For things that ktlint doesn't "just fix", and instead yells about, these tags will not work.

The fix would be to make ktlint respond to "exceed max line lingth" by autowrapping, rather than throwing an exception.

Details

Unfortunately, this is behaving correctly. You can split code quality tools into two categories:

  • "problem solver" f(String content) -> String fixedContent
  • "problem yeller" f(String content) -> Optional<ErrorMsgAndLocation>

The great thing about "problem solvers" is that you can compose them. Your configuration block has several (trimTrailingWhitespace, indentWithSpaces, ktlint). The way that toggleOffOn works is by storing what is between the off/on tags before the steps run, and then putting it back at the end. Spotless is all about composing problem solvers.

The terrible thing about "problem yellers" is that you can't compose them. They don't fix problems, they just yell about them. In the case of ktlint, it just yells about the first thing that it finds. If you fix the max line length on line 70, it will next yell about the problem on line 77. That's a big limitation of "yellers" as opposed to "fixers".

Most tools are either problem yellers or solvers. Ktlint is the rare exception, where it seems to be 50/50. Which means that half the time it works great with Spotless, but the other half of the time it just shouts "everybody stop" and people are disappointed that it doesn't play nice with the rest of the Spotless infrastructure.

One option is to look at ktfmt, a recent tool from facebook which is all "solver", and no "yeller". 1

[1] All "solvers" are at least a little "yeller". For example, if you pass C++ to ktfmt, it will throw a parsing exception and yell. But most autoformatting tools use yelling as a last resort. ktlint is the rare case that uses yelling for many fixable issues like "line is too long".

@MarijanGazica
Copy link
Author

Sorry for the late reply, just wanted to thank you for a great response.
I'll look into the direction you pointed

@nedtwigg
Copy link
Member

nedtwigg commented Jan 14, 2022

We can make this work in the future. Probably the spotless:off / spotless:on mechanism isn't the right one, but maybe instead spotless:lintoff:CODE_FOR_LINT. For each lint, search for that string on the affected lines, and if it's there suppress it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants