consider a bulk fix for removing unused imports

[pq]

It seems like this is a pretty common case and a fix trivial.

There is a potential interaction w/ data-driven fixes. Brian pointed out in chat:

we could have a data-driven fix that adds a reference to a name imported by a previously unused import. Remove the import because it used to be unused would leave the user with an undefined identifier that they might not know how to import.

My inclination is to cross that bridge when we get to it?

[bwilkerson]

I don't think it's a question of "if"; I think we already have the interaction. Whether users will experience the interaction depends on whether we apply any data-driven fixes in a library with an unused import.

I think we would need to have a way to handle this gracefully before we added this support.

[bwilkerson]

In looking at #43887, I had an idea for a possible solution.

We could re-run analysis after applying the fixes and clean up any unused imports only at that point. I believe this would remove the possibility of interactions between other fixes and the unused import fix, but it does carry a performance penalty.

If we do at some point decide to iterate until there's nothing left to fix (other than unused_imports) then I think we'd already be incurring the cost, so I think this final cleanup would be fairly inexpensive. We should measure the performance of a second analysis (on multiple sizes of packages) to determine how big the performance impact is likely to be. Then we can decide whether to apply this solution (now or later) and whether it's reasonable to iterate at some point in the future.

[bwilkerson]

This has now landed.

[JamesMcIntosh]

@bwilkerson I'm running Dart SDK version: 2.15.1 (stable) but it doesn't fix unused imports, is this not released yet?

[bwilkerson]

I'm sorry, but I really don't know how to figure out which release a change was first part of. I believe the CL that fixed the problem was https://dart-review.googlesource.com/c/sdk/+/221140, but I don't know whether that change is included in 2.15.1. But if you can put together a small reproduction case I can test it against the tip of tree to see whether there's a bug or whether you need to wait for a later release.

[JamesMcIntosh]

Thanks @bwilkerson!
Something as simple this demonstrates the behaviour:

import 'dart:math';

class UnusedImport {

}

The import dart:math is never used so should be clean up but dart fix does not have a fix for it

% dart analyze
   info - lib/UnusedImport.dart:1:8 - Unused import: 'dart:math'. Try removing the import directive. - unused_import

% dart fix --dry-run
Computing fixes in api (dry run)... 1.9s
Nothing to fix!

[bwilkerson]

I created a directory named fix and added a file named fix_test.dart with the content you sent. I then ran the tools from an SDK built from the tip of main. They produced the following output:

$ dart analyze
   info • fix_test.dart:1:8 • Unused import: 'dart:math'. Try removing the import directive. • unused_import

$ dart fix --dry-run
Computing fixes in fix (dry run)... 11.4s

1 proposed fix in 1 file.

fix_test.dart
  unused_import • 1 fix

That suggests to me that the change is not in 2.15.1, but will be in a later release. Unfortunately I don't know which release it will ship in.

[JamesMcIntosh]

@bwilkerson Thanks for your help!

I switched to the Flutter beta branch which has 2.16.0-134.1.beta and it worked as expected, guess it'll ship with the next flutter release.

It must be part of this issue listed in the change log #46100
https://github.com/dart-lang/sdk/blob/main/CHANGELOG.md#2160

[bwilkerson]

You're very welcome. Glad it's working for you on the beta branch.