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

Remove unused imports is not "across files" #57142

Closed
FMorschel opened this issue Nov 18, 2024 · 5 comments
Closed

Remove unused imports is not "across files" #57142

FMorschel opened this issue Nov 18, 2024 · 5 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

I see that RemoveUnusedImport from analysis_server\lib\src\services\correction\dart\remove_unused_import.dart has the CorrectionApplicability.acrossSingleFile and this is the comment:

Bulk application is supported by a distinct import cleanup fix phase.

I had a project with multiple unused imports because I added an export. And now multiple files had this warning - a single unused import each.

VS Code problems tab had no option to fix them all at once so I ran the CLI dart fix and almost everything got fixed but my tests (I'm unsure why I could not fix anything there even though my analysis_option says nothing about ignoring them (working on a package).

What can we do about this? The above comment tells me nothing about what other phase is fixing that as well.

@dart-github-bot
Copy link
Collaborator

Summary: The RemoveUnusedImport analyzer only handles unused imports within a single file. A user requests a solution to address unused imports across multiple files simultaneously.

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 18, 2024
@lrhn lrhn added type-enhancement A request for a change that isn't a bug and removed type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 18, 2024
@bwilkerson
Copy link
Member

... I ran the CLI dart fix and almost everything got fixed but my tests ...

Which directory were you in when you ran dart fix. By default it will apply fixes in the current working directory.

If that wasn't the problem, then I don't know what the issue is, but it's a bug for it to have not fixed code in the test directory.

The above comment tells me nothing about what other phase is fixing that as well.

I'm not sure what you mean by "fixing that as well." If you mean imports in test files, then the answer is that they should be fixed in the same phase as imports in any other file. Otherwise, please clarify.

@FMorschel
Copy link
Contributor Author

Which directory were you in when you ran dart fix. By default it will apply fixes in the current working directory.

Nice question. I simulated it and it was fixed on the tests.

I may have changed the directory to lib or something and forgot about it. I'll keep that in mind next time. Maybe I was under the impression it would look at the full project because it needs to look at analysis_options.yaml to define what should it test for with the analysis server.


I'm not sure what you mean by "fixing that as well." If you mean imports in test files, then the answer is that they should be fixed in the same phase as imports in any other file.

Yes, thanks. But I guess my real question here is:

Bulk application is supported by a distinct import cleanup fix phase.

What is this "distinct import cleanup fix phase"?

I don't think it was clear enough for me to understand.

@bwilkerson
Copy link
Member

In case it's useful, you can pass the target directory on the command-line. That would allow you to write a script to run dart fix on your project's root directory that would work no matter which directory you happen to be in at the moment.

What is this "distinct import cleanup fix phase"?

The dart fix tool works by analyzing the files in a directory, looking for fixes that have a CorrectionApplicability that allows them to be bulk-applied, and applying those fixes. These steps are repeated until either there are no remaining diagnostics with fixes or until the maximum number of iterations have been performed (to prevent infinite loops). That's the first phase.

After this it performs one more analysis looking for unused imports and applies those fixes all in the final phase. The reason it's a separate phase is because a fix in one iteration can make an import be unused while a fix in a subsequent phase can cause it to be used again. To reduce the total amount of work being done and the chance of conflicting edits from one iteration to the next, we moved that work to happen once at the end.

@FMorschel
Copy link
Contributor Author

I see. Thanks for answering everything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants