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

Reconsider allowing standard:no-consecutive-comments if there are empty lines, and allow automatic fix #2772

Closed
scscgit opened this issue Aug 17, 2024 · 2 comments

Comments

@scscgit
Copy link

scscgit commented Aug 17, 2024

Observed Behavior

#1454 introduces rule standard:no-consecutive-comments, which doesn't allow mixing comment types even if there is empty line between them. For example, you may want to create a large comment common to multiple dependencies, and then follow the structure of having one comment describing one individual dependency, which of course is friendlier on version control diffs:

dependencies {
    /**
     * Common libraries
     * ... some longer trivia...
     */

    // LazyInitializer if you don't want to use @Getter(lazy = true)
    implementation("org.apache.commons:commons-lang3:3.16.0")

    // Support .jar files in a `/libraries` directory without making these deps transitive
    implementation(fileTree("libraries"))
}

I haven't found this convention in Kotlin Coding Conventions, and it's not linked in any place accessible when we stumble upon this error.

Additionally, when we run ./gradlew spotlessApply to fix all code style issues, this problem cannot be fixed automatically, completely halting our workflow as if some fatal error occurred. What's worse, the name of the file where the issue happened isn't even displayed in the error message; we can be glad that at least the error line is shown.

Furthermore, what's worse is that there is no user-friendly way to disable this rule. I haven't found anything in the documentation, except maybe disabling all experimental rules altogether. And I stumbled upon another bug within the Spotless Gradle Plugin diffplug/spotless#2226 where even if I want to override EditorConfig (which I don't have in the project), suddenly the "default" behavior of this plugin completely changes, and this (experimental?) rule becomes disabled altogether whenever any override parameter is specified - it can be one that doesn't exist at all. So, the issue accidentally gets resolved even if you mistakenly add a parameter like "standard:no-consecutive-comments" to "false", which shouldn't work (but would be intuitive, because there should be exactly one ID from the error message that we can directly add to our Gradle configuration to exclude it). I'm also asking you to add a documentation of how this rule can be disabled to the content of the error message if possible.

Expected Behavior

There should be at least a parameter we can use to allow mixing comment types when they are separated by an empty line if it can't be allowed by default. Error message should allow us to resolve this problem automatically, or even better, if you actually have any real conventions about how this should be handled, you can apply it as an automatic fix.

Your Environment

  • Version of ktlint used: 1.3.1
  • Relevant parts of the .editorconfig settings: none
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): https://github.com/diffplug/spotless/tree/main/plugin-gradle
  • Version of Gradle used (if applicable): 8.9
  • Operating System and version: Windows 10
@paul-dingemans
Copy link
Collaborator

I haven't found this convention in Kotlin Coding Conventions

That is correct. It is an attempt to prevent unclear documentation/code. For example a KDoc, followed by a commented-out function.

, and it's not linked in any place accessible when we stumble upon this error.

I am not sure what you mean with this. Do you mean any other reference to such a covention outside the ktlint documentation?

Additionally, when we run ./gradlew spotlessApply to fix all code style issues, this problem cannot be fixed automatically, completely halting our workflow as if some fatal error occurred. What's worse, the name of the file where the issue happened isn't even displayed in the error message; we can be glad that at least the error line is shown.

Furthermore, what's worse is that there is no user-friendly way to disable this rule. I haven't found anything in the documentation, except maybe disabling all experimental rules altogether. And I stumbled upon another bug within the Spotless Gradle Plugin diffplug/spotless#2226 where even if I want to override EditorConfig (which I don't have in the project), suddenly the "default" behavior of this plugin completely changes, and this (experimental?) rule becomes disabled altogether whenever any override parameter is specified - it can be one that doesn't exist at all. So, the issue accidentally gets resolved even if you mistakenly add a parameter like "standard:no-consecutive-comments" to "false", which shouldn't work (but would be intuitive, because there should be exactly one ID from the error message that we can directly add to our Gradle configuration to exclude it). I'm also asking you to add a documentation of how this rule can be disabled to the content of the error message if possible.

For questions regarding any integration with ktlint (Spotless or Gradle), please contact the maintainers of those projects. Ktlint offers functionality to disable individual rules (https://pinterest.github.io/ktlint/latest/faq/#how-do-i-enable-or-disable-a-rule), or to suppress errors at certain locations (https://pinterest.github.io/ktlint/latest/faq/#how-do-i-suppress-errors-for-a-lineblockfile). Also, you can use the Ktlint CLI (https://pinterest.github.io/ktlint/latest/install/cli/) which gives clear feedback for your example:

 $ ktlint-1.3.1 --relative -F  **/Foo3.kts
src/main/kotlin/Foo3.kts:2:5: A KDoc is not allowed inside 'block' (cannot be auto-corrected) (standard:kdoc)
src/main/kotlin/Foo3.kts:7:5: an EOL comment may not be preceded by a KDoc. Reversed order is allowed though when separated by a newline. (cannot be auto-corrected) (standard:no-consecutive-comments)
src/main/kotlin/Foo3.kts:2:5: A KDoc is not allowed inside 'block' (cannot be auto-corrected) (standard:kdoc)
src/main/kotlin/Foo3.kts:7:5: an EOL comment may not be preceded by a KDoc. Reversed order is allowed though when separated by a newline. (cannot be auto-corrected) (standard:no-consecutive-comments)

Summary error count (descending) by rule:
  standard:kdoc: 2
  standard:no-consecutive-comments: 2

As ktlint offers functionality to disable this specific rule, I see no reason for changing ktlint.

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2024
@scscgit
Copy link
Author

scscgit commented Aug 20, 2024

There surely could be a rule against it being followed by a commented-out function; especially if there is no empty space in between.

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