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

False positive from standard:no-unused-imports when using org.gradle.kotlin.dsl.assign #2343

Closed
erdi opened this issue Nov 6, 2023 · 6 comments · Fixed by #2382
Closed

Comments

@erdi
Copy link

erdi commented Nov 6, 2023

Expected Behavior

When importing org.gradle.kotlin.dsl.assign and using that extension method in the file, standard:no-unused-imports should not raise a violation.

Observed Behavior

standard:no-unused-imports raises a violation if org.gradle.kotlin.dsl.assign is imported even though the import is used in a file.

Context

Gradle 8.1 introduced simple property assignment in Kotlin DSL scripts as an experimental feature. That feature became stable in Gradle 8.4. It looks like the implementation involves a compiler plugin.

FWIW, a similar issue (in that the import was reported as unused) seems to have been fixed in IntelliJ.

As a side note, I tried disabling that rule using EOL comment on the line importing org.gradle.kotlin.dsl.assign, // ktlint-disable standard:no-unused-imports, but that had no effect, the only way I was able to suppress it was for the whole file, using @file:Suppress("ktlint:standard:no-unused-imports"). Should I open an issue for that?

Your Environment

  • Version of ktlint used: 0.49.1
  • Name and version of integration used: org.jlleitschuh.gradle.ktlint:11.6.1 Gradle plugin
  • Version of Gradle used (if applicable): 8.4
  • Operating System and version: macOS 14.1 (23B74)
@paul-dingemans
Copy link
Collaborator

When importing org.gradle.kotlin.dsl.assign and using that extension method in the file, standard:no-unused-imports should not raise a violation.

Please include a code sample that reproduces the issue.

As a side note, I tried disabling that rule using EOL comment on the line importing org.gradle.kotlin.dsl.assign, // ktlint-disable standard:no-unused-imports, but that had no effect, the only way I was able to suppress it was for the whole file, using @file:Suppress("ktlint:standard:no-unused-imports"). Should I open an issue for that?

No, you don't need to open an issue for that. The // ktlint-disable directives are deprecated in version 0.50 and removed in 1.0 in favor of the @Suppress annotation. So adding the @file:Suppress(...) was the proper way to achieve this. As the the directive is phased out, and no bugfix releases are published once a new minor or major release is published, this will not be investigated or fixed.

@erdi
Copy link
Author

erdi commented Nov 7, 2023

Very odd, I'm unable to reproduce this in a standalone project. What's more, in a standalone project importing org.gradle.kotlin.dsl.assign seems to be completely ignored by ktlint so even if I import it but then not use it then no standard:no-unused-imports violations are being raised... 🤔

@erdi
Copy link
Author

erdi commented Nov 7, 2023

I'm going to close this because this is clearly something specific to the setup of my project and I unfortunately cannot isolate it into a standalone sample even after trying various theories for over an hour. Sorry about the noise.

@erdi erdi closed this as completed Nov 7, 2023
@erdi erdi reopened this Nov 7, 2023
@erdi erdi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2023
@erdi
Copy link
Author

erdi commented Nov 20, 2023

Ok, so now that I need to add more and more suppressions this started bugging me enough to attach the debugger and see why I was unable to reproduce. It turns out it was because in my reproducer sample I used <...>.assign as the package name of the class containing the false positive, so ktlint thought that assign symbol was being referenced. 🤦 After I moved it out of that package the problem is reproducible.

Reproducer sample is available at: https://github.com/erdi/ktlint-assign-bug. There are instructions for how to reproduce in the readme file.

I believe the fix will be to add# assign to NoUnusedImportsRule#OPERATOR_SET. Feels like this issue is similar to #40 and #54.

@erdi erdi reopened this Nov 20, 2023
@paul-dingemans
Copy link
Collaborator

Reproducer sample is available at: https://github.com/erdi/ktlint-assign-bug. There are instructions for how to reproduce in the readme file.

Can you check whether this a public repository? The link does not work. Neither do I see another public repository in your profile that could match.

@erdi
Copy link
Author

erdi commented Nov 20, 2023

Apologies, I unintentionally made the repository private, my settings had to change and rather than my repositories being public on creation by default they are now private.

I've now changed that repository to be public.

Screenshot 2023-11-20 at 19 46 40

paul-dingemans added a commit that referenced this issue Nov 26, 2023
Removing this import results in compilation error when shorthand "=" is used instead of function `assign(..)`.

Closes #2343
@paul-dingemans paul-dingemans added this to the 1.1 milestone Nov 26, 2023
paul-dingemans added a commit that referenced this issue Nov 27, 2023
Removing this import results in compilation error when shorthand "=" is used instead of function `assign(..)`.

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

Successfully merging a pull request may close this issue.

2 participants