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

Unused import rules does not remove all imports #1754

Closed
DavidCorrado opened this issue Dec 29, 2022 · 2 comments
Closed

Unused import rules does not remove all imports #1754

DavidCorrado opened this issue Dec 29, 2022 · 2 comments

Comments

@DavidCorrado
Copy link

Expected Behavior

I have found that the unused import rule does not remove all unused imports. I think this is related or the same issue as #1587.

Observed Behavior

The imports not removed are
import co.[ID].data.session.isLoggedIn
import kotlinx.coroutines.flow.collect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.setValue
They are removed via android studio optimize imports but not by ktlint. I ran ktlint 48 through the terminal to test

Posted in the kotlin slack https://kotlinlang.slack.com/archives/CKS3XG0LS/p1672347793114019
It appears that some imports are not going to be removed.

Summary

Does that mean its not worth the effort or its literally impossible? Is there something that can get exposed to you to make this possible? If the issue is that it would require a large refactor I think it would be valuable to know where in the code the limitation and how to resolve the limitation so someone can consider putting out a PR?

Also should we update the documentation to reference the current limitations? I am not sure I understand the rules of the limitations

@paul-dingemans
Copy link
Collaborator

Does that mean its not worth the effort or its literally impossible?

I think both of it. Of course it is technically possible to implement such a feature because the IntelliJ IDEA and Android Studio are both able to do it as well. But I expect that the cost of implementing it is unacceptable high in man hours and complexity.

Ktlint has one important restriction when linting/formatting the file. All information required should literally be available in the PSI file produced by the embedded kotlin provider. Lookin up information in other files in the project or in included dependencies does not fit in the scope of ktlint. From that perspective, I stronger and stronger believe that the no-unused-imports rule should be removed entirely from ktlint. It successfully detects some kind of unused imports. But for other imports it can not be detected within the scope of a single PSI file whether the import is used or not. Removing an import which seems unused but actually is required, results in code which no longer compiles. That would do more harm then not removing an unused import.

Of course my analysis above might be wrong. So if somebody contributes a Pull Request with actually guarantees that only unused imports are removed, then I am willing to consider to merge that into Ktlint (but only if the added complexity is reasonable with regard to the functionality).

@paul-dingemans paul-dingemans changed the title Unused import rules does not remove all imports the same as Intellij(Android Studio) Unused import rules does not remove all imports Apr 7, 2023
@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Apr 7, 2023
@paul-dingemans
Copy link
Collaborator

Also see #1529

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