unused_import false positive for loadLibrary

[davidmorgan]

If an import is just used to call loadLibrary on a deferred library, the linter reports it as unused.

This is a false positive, it's used :)

import 'foo.dart' deferred as foo; // LINT (false positive)

void main() {
  foo.loadLibrary();
}

[srawlins]

The analyzer did not used to report such an import as unused, until I fixed the bug 😄 .

Can you explain how the import is used? It looks pretty unused.

[davidmorgan]

This use pattern is a performance optimization: we know the deferred library will be needed soon after startup, so we trigger loading in 'main' even though the actual usage is elsewhere.

[srawlins]

I think that is an exceptional use case, and that the value of the unused_import hint would be much reduced if we did not report such imports as unused.

Another exceptional use case that has been requested is when an import exists just so that code-generators can find various annotations in imported libraries. Again, the value would be greatly reduced if unused_import never reported an import of a library that contained an annotation.

I think the most pragmatic practice for this optimization would be to //ignore the import.

[jcollins-g]

One way to look at this is, "is the act of triggering a deferred library load, using the library?" IMO since the person is referencing the name foo, that should count as using it.

[jcollins-g]

marking as P3 as there is not yet agreement as to whether this is a bug.

[Levi-Lesches]

so we trigger loading in 'main' even though the actual usage is elsewhere.

is the act of triggering a deferred library load, using the library?

Shouldn't the analyzer just notice when the library is used "elsewhere"? If you just load it in and don't use it yet, the analyzer should report it as unused, but once you use it, doesn't the error go away? It worked when I tested it.

[davidmorgan]

Ah, yes, of course it's a hint and not a lint, thanks Phil+Sam :)

The fact that I was confused stems partly from us recommending the 'ignore'--this is the first time I've seen a recommendation to ignore a hint as there is no other workaround for a reasonable thing to do--seems to me to fall clearly under the policy of 'hints must have no false positives'.

[bwilkerson]

The loadLibrary function is a synthetic top-level function that exists because of the deferred import. If the import is removed then the function would no longer be defined. It seems to me that any use of loadLibrary therefore makes the import necessary. I think a similar line of reasoning also applied to the prefix. Because the import is deferred the prefix can't be declared by any other import, hence any use of the prefix makes the import necessary.

I believe that it's a bug that the analyzer isn't recognizing this kind of use.

[Levi-Lesches]

Isn't the point of this hint to detect when a library is imported/loaded but not actually used? The docs say:

The analyzer produces this diagnostic when an import isn’t needed because none of the names that are imported are referenced within the importing library.

Loading the library doesn't seem to fit the criteria here. But after you do actually use the imported members elsewhere, the hint goes away.

[srawlins]

If the import is removed then the function would no longer be defined. It seems to me that any use of loadLibrary therefore makes the import necessary.

I don't see how one follows from the other.

I really think this is an exceptional side effect case. A user could say, "if I remove this import, then there would be no way to know if the imported file becomes deleted." There are many side effects which the analyzer could never account for.

If we revert the change to mark an otherwise-unused import as unused, from which loadLibrary is called, it will be a net negative for our users (my strong opinion); certain otherwise-unused imports will not be reported as unused, causing code bloat and the other negative effects of unused code, and rare cases of deferred-loading-for-performance (which can easily be //ignored) will benefit.

If we want to re-categorize unused_import as a Lint because it has numerous false positives, I see very few downsides there.

If we want to revert the change, and add an additional Hint/Lint which specifically warns about otherwise-unused deferred imports from which loadLibrary is referenced, I see fewer downsides there.