Should "self" importing trigger `unnecessary_import`

[FMorschel]

trafficstars

Edit: The old description talked about unused_import. Updated to reflect the comment from bwilkerson.

---

I noticed two things about a file importing itself:

1. We can shorten the number of imports if we want to export some of the imported files as well:

import '';

export 'other.dart';
export 'other2.dart';

void foo() {
  ClassFromOther();
  ClassFromOther2();
}

2. We can call things from inside the file with an alias

import '' as self;

void function() {}

class A {
  void method() {
    self.function();
  }
}

Apart from this, I could not find anything remotely useful in doing this. I did notice that it doesn't trigger unnecessary_import even if it is not importing anything new or aliased (not even by writing down the file name or package import).

Should unnecessary_import be triggered in such cases where it is doing nothing?

Or even when it is doing something? Maybe in those cases, we could add a new lint or something for avoid_self_importing to make sure people are clear on what they are importing.

P.S.: Not sure if this kind of discussion should be done here or inside linter. Feel free to move it there if that's better.

[dart-github-bot]

Summary: The user is questioning whether the unused_import lint should be triggered when a file imports itself, even if the import doesn't introduce new symbols or aliases. They suggest a new lint, avoid_self_importing, to discourage this practice.

[bwilkerson]

We make a distinction between two concepts:

• an unused import is one where none of the imported elements are referenced
• an unnecessary import is one where some of the imported elements are referenced, but the referenced elements would still be available even if the import were removed

The distinction is probably meaningless to users, but I'd put this under the second category.

I'm guessing, though, that this condition happens very seldom and hence isn't very high value to fix.

[FMorschel]

Edited the OP to address your comment. You're right about the distinction, it didn't occur to me. I rarely see the second lint.

I agree this probably doesn't happen very often.

I noted the second case when working in a project that uses build_runner and exporting the generated code from my manually written file. I accidentally imported itself when trying to use the values from the generated file. Later I noticed it and that got me confused, hence the discussion here.

[lrhn]

Doing import "";export "other.dart"; is an a-little-too-clever way of avoiding import "other.dart";export "other.dart";. If you have multiple libraries that are both imported and exported, then there is a saving in typing, but definitely not in readability. I'd warn unconditionally about importing your own library without a prefix, even if it's neither unnecessary or unused. Just write the real imports.

Importing your own library with a prefix is a way to get access to top-level (non-private) declarations that are shadowed by other declarations. It's also an ugly hack. I'd probably say nothing about it for now, but if we ever give people a better way to do scope-overriding, I'd start saying to use that instead. (Imagine library as foo; making foo a "prefix scope" for the current library's declarations.)

[FMorschel]

One thing we could already handle better would be something like this:

I have bug.dart exporting a.dart (names are not important since this is a dumb project for testing things like this) and the following:

We could already remove the line for the current file.

---

srawlins, has just found some code like this on the SDK and mentioned it on the #hackers-analyzer chat on Discord (https://dart-review.googlesource.com/c/sdk/+/390663).

[srawlins]

Yeah I think this would be complicated to fully implement.

• A self-import with a prefix has an effect that now you can prefix local elements, so don't report such a self-import.
• A self-import with no prefix should only have the effect of importing the exports. So you could: a. Only report self-imports in libraries with zero exports. Not bad; most libraries have zero exports. b. Dig in, and find which elements are referenced from the exports. Also check which elements are also available through imports. And either report the self-import regardless (if nothing from the exports are used, it is useless; if something from exports is used, it is too clever). The quick-fix becomes equally complicated.

[FMorschel]

I would lean on (first at least) simply showing the message if there is a self-import without an alias no matter what. Then the user can manually fix this so it doesn't get worse for readability (or maintenance later). Not really caring if the self-import is being used or not at this point.

Then if we see on the telemetry or some commotion from the community (or free time if we think it is important) we can go ahead and make a quick-fix or make the message trigger on different cases.

If the library has exports and is self-imported, even if it is not being used today, whenever the user tries to use a new class that is "only exported" it will be possible. I don't think it should wait for that to trigger the warning. By then it will already be weird, best if we warn everyone to fix it before anyway.

[FMorschel]

What about when we have a case like:

b.dart

export 'a.dart';

a.dart

import 'b.dart';  // Implicitly imports itself

I stumbled upon a similar case today because I accidentally imported a barrel file and moved some typedefs out of the file I was working on, but still wanted them to be exported from there.

This way, before exporting, I was missing the declarations for the typedefs, but after exporting, importing the declarations for the typedefs wasn't necessary because they were being imported by the barrel file containing the current file (had not updated the barrel file yet).

What should we do about these cases? I'd doubt that anyone really wants to do that either, so I'd say we should warn.

We could also lower the priority of the fixes that generate this warning if the processing isn't that hard, too.