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

Comments above annotations are separated by the annotation with a blank line #2416

Closed
Vampire opened this issue Dec 5, 2023 · 8 comments · Fixed by #2429
Closed

Comments above annotations are separated by the annotation with a blank line #2416

Vampire opened this issue Dec 5, 2023 · 8 comments · Fixed by #2429
Milestone

Comments

@Vampire
Copy link

Vampire commented Dec 5, 2023

Given code like

val gradle = gradle as ExtensionAware
// work-around for https://youtrack.jetbrains.com/issue/KT-38871
// Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

or

val gradle = gradle as ExtensionAware
/*
 * work-around for https://youtrack.jetbrains.com/issue/KT-38871
 * Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
 */
@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

If you reformat those snippets with default settings / empty editorconfig, in both cases there is a blank line added between the comment and the annotation, while I do not find any rule in the coding guidelines that suggests an annotation has to be preceeded by a blank line and it seems natural that those comments refer to the following line.

The rule triggering this is

Declarations and declarations with annotations should have an empty space between. (standard:spacing-between-declarations-with-annotations)

Your Environment

  • Version of ktlint used: 1.0.1
  • Relevant parts of the .editorconfig settings: nothing
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): CLI
  • Operating System and version: Windows 10
@paul-dingemans
Copy link
Collaborator

I can not reproduce the problem with any of the code styles. Can you please run command like ktlint --log-level=debug **/Bar.kts (replace Bar.kts with your file) and post the output here?

@Vampire
Copy link
Author

Vampire commented Dec 7, 2023

Ah sorry, the example was too minimal, I updated the original description

@paul-dingemans
Copy link
Collaborator

It looks like this behavior was added due to https://youtrack.jetbrains.com/issue/KT-35106. For backwards compatibilty this rule should be kept around.

When you run ktlint with formatting, you see output below:

src/main/kotlin/Foo.kt:2:1: Declarations and declarations with annotations should have an empty space between. (standard:spacing-between-declarations-with-annotations)
src/main/kotlin/Foo.kt:2:1: Declarations and declarations with comments should have an empty space between. (standard:spacing-between-declarations-with-comments)

Based on this output you can decide to disable the rule in the .editorconfig via property ktlint_standard_spacing-between-declarations-with-comments.

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2023
@Vampire
Copy link
Author

Vampire commented Dec 7, 2023

But shouldn't the comment be above the comment, so like

val gradle = gradle as ExtensionAware

// work-around for https://youtrack.jetbrains.com/issue/KT-38871
// Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

?

Having it like that upfront also makes ktlint happy and not change it.

@paul-dingemans
Copy link
Collaborator

Given that I provide code below:

val gradle = gradle as ExtensionAware
// work-around for https://youtrack.jetbrains.com/issue/KT-38871
// Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

to ktlint command ktlint -F --stdin results in:

val gradle = gradle as ExtensionAware

// work-around for https://youtrack.jetbrains.com/issue/KT-38871
// Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

So I am confused what you mean. Isn't this exactly what you expect?

@Vampire
Copy link
Author

Vampire commented Dec 7, 2023

Yes, but not what I get.
I get

val gradle = gradle as ExtensionAware
// work-around for https://youtrack.jetbrains.com/issue/KT-38871
// Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore

@Suppress("UNUSED_VARIABLE")
val foo by gradle.extra {
    true
}

@Vampire
Copy link
Author

Vampire commented Dec 7, 2023

Argh, I'm really bad at knitting MCVEs today.
Don't have it top-level in a class but inside a method, sorry again.

@paul-dingemans
Copy link
Collaborator

Ok, now the bug can be reproduced with code:

fun foo() {
    val gradle = gradle as ExtensionAware
    // work-around for https://youtrack.jetbrains.com/issue/KT-38871
    // Kotlin 1.9+ and thus Gradle 8.3+ fixed it and no suppression is necessary anymore
    @Suppress("UNUSED_VARIABLE")
    val foo by gradle.extra {
        true
    }
}

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