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

Unnecessary import error for used imports with * at the end #1277

Closed
illuzor opened this issue Nov 6, 2021 · 18 comments · Fixed by #1402
Closed

Unnecessary import error for used imports with * at the end #1277

illuzor opened this issue Nov 6, 2021 · 18 comments · Fixed by #1402
Labels
Milestone

Comments

@illuzor
Copy link

illuzor commented Nov 6, 2021

Expected Behavior

No error

Observed Behavior

Error

Steps to Reproduce

Disable no-wildcard-imports rule and create a file with wildcard import

Your Environment

  • Version of ktlint used: 0.43.0
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): gradlew:ktlintCheck
  • Version of Gradle used (if applicable): 7.2
  • Operating System and version: Windows 10
  • Link to your project (if it's a public repository): ...

.editorconfig:

[*.{kt,kts}]
disabled_rules=no-wildcard-imports

Code:

import test.*

fun main() {
    Test1()
    Test2()
    Test3()
}

Output:

> Task :ktlintMainSourceSetCheck FAILED
D:\_sources\IDEA\tests\ktlint_error\src\main\kotlin\demo.kt:1:1 Unnecessary import
3 actionable tasks: 1 executed, 2 up-to-date

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':ktlintMainSourceSetCheck'.
> A failure occurred while executing org.jlleitschuh.gradle.ktlint.worker.ConsoleReportWorkAction
   > KtLint found code style violations. Please see the following reports:
     - D:\_sources\IDEA\tests\ktlint_error\build\reports\ktlint\ktlintMainSourceSetCheck\ktlintMainSourceSetCheck.txt

Fails only if run by the plugin task gradlew ktlintCheck.
Does not fail if run ktlint from command line

Sample project: ktlint_error.zip

@romtsn
Copy link
Collaborator

romtsn commented Nov 6, 2021

you're disabling the wrong rule, it should actually be no-unused-imports

@romtsn romtsn closed this as completed Nov 6, 2021
@illuzor
Copy link
Author

illuzor commented Nov 6, 2021

@romtsn no, I`m disabling correct rule.
no-unused-imports should fail when wildcard import is unused, but in my case it is used and should not fail

@romtsn romtsn reopened this Nov 7, 2021
@romtsn
Copy link
Collaborator

romtsn commented Nov 7, 2021

Ah, alright, then it's indeed a bug

@romtsn romtsn added the bug label Nov 7, 2021
@paul-dingemans
Copy link
Collaborator

@illuzor Please copy paste the code samples and output in the issue instead of an image. Also you should provide the expected code.

@paul-dingemans
Copy link
Collaborator

With file Foo.kt and content:

import some.path.*

interface Foo : Bar

and .editorconfig:

[*.{kt,kts}]
disabled_rules=no-wildcard-imports

ktlint does not fail for me as expected. However when I remove the disabled_rules from the .editorconfig it does fail.

@illuzor
Copy link
Author

illuzor commented Nov 11, 2021

@paul-dingemans done

@paul-dingemans
Copy link
Collaborator

@romtsn This problem has been introduced by fixing #1256. I am stuck with trying to resolve this issue without reverting #1256.

The basic problem is that due to #1256 the import in code below is removed as no reference is found to package test.

import test.*

fun main() {
    Test() // defined in package test
}

So when visiting the CALL_EXPRESSION Test() or one level deeper at the IDENTIFIER node Test , I need to extract information from the psi node about the import that is required or the fully qualified name of the class. But in those nodes, I can not find any helpful information. Do you have any idea how to approach this?

@GabrielLasso
Copy link

@romtsn I had the same problem, then I created an example to reproduce it and I was going to report it, but I found this issue reporting the same problem.

Here is the example I created, I hope it is useful somehow

https://github.com/GabrielLasso/ktlint-bug-example

@henrik242
Copy link

@paul-dingemans Any progress on this? This bug removes in-use star imports when I use the formatter through kotlinter.

@paul-dingemans
Copy link
Collaborator

Sorry no progress. Waiting for response of @romtsn

@romtsn
Copy link
Collaborator

romtsn commented Dec 14, 2021

I'd need to look into that, but from the top of my head we probably can't extract this info easily. the proper way would be to depend on the compiler analysis, but that requires quite some setup

@henrik242
Copy link

My suggestion would be to revert the change that introduced this regression, and work from there. This way projects like kotlinter can continue to track the latest ktlint.

@Mehly
Copy link

Mehly commented Feb 16, 2022

Any updates on this issue yet?

We're unable to upgrade to the latest version of ktlint due to this issue because Android Studio by default enforces wildcard-imports after a certain threshold of imports from the same package has been reached.

@jakoss
Copy link

jakoss commented Feb 16, 2022

@Mehly You can turn that off in IntelliJ settings:
image

But I do agree, we're still on 0.41.0 since this is the last version that does work for us just fine

@romtsn
Copy link
Collaborator

romtsn commented Feb 16, 2022

@Mehly you can also disable it through .editorconfig ij_kotlin_name_count_to_use_star_import = 2147483647

@InTheCloudDan
Copy link

I just ran into this issue also, but it looks like if I disable that line with no-used-imports it keeps it there:
https://github.com/launchdarkly/ld-intellij/pull/13/files#diff-6f0b4a794e32dea16c8b2b28f38d8c2a6d7b1969a523045139b96ec474b753acR14

I'm not sure why wildcards are working in other files and it's just this one. But if I remove that disable line the import will be deleted.

@paul-dingemans
Copy link
Collaborator

@InTheCloudDan can you please try to minimize your example and just paste it here? Also, it would be better to use an EOL comment to prevent removal of the import on that specific line. Now you have disabled it for the remainder of the file.

@paul-dingemans paul-dingemans added this to the 0.45.0 milestone Mar 9, 2022
paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Mar 12, 2022
…lead to removal of required imports

There is no reliable way to determined whether a wildcard import is actually used
in the file. The AST does not seem to contain information about the actuall class
that an identifier refers to. It is preferred to not remove unused imports than
that needed imports are removed.

Revert pinterest#1256
Closes pinterest#1277
@paul-dingemans
Copy link
Collaborator

@romtsn This problem has been introduced by fixing #1256. I am stuck with trying to resolve this issue without reverting #1256.

The basic problem is that due to #1256 the import in code below is removed as no reference is found to package test.

import test.*

fun main() {
    Test() // defined in package test
}

So when visiting the CALL_EXPRESSION Test() or one level deeper at the IDENTIFIER node Test , I need to extract information from the psi node about the import that is required or the fully qualified name of the class. But in those nodes, I can not find any helpful information. Do you have any idea how to approach this?

Solution of #1256 has been reverted as it leads to compilation failures when required wildcard important are removed. Downside is that some unused wilcard imports may not be detected. But that is not considered as harmfull.

paul-dingemans added a commit that referenced this issue Mar 12, 2022
…emoval of required imports (#1402)

There is no reliable way to determined whether a wildcard import is actually used
in the file. The AST does not seem to contain information about the actuall class
that an identifier refers to. It is preferred to not remove unused imports than
that needed imports are removed.

Revert #1256
Closes #1277
Closes #1393
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants