Make dartdoc notion of "ambiguous reexport" a lint!

[kevmoo]

If a package has more than 1 public (top-level) library that export the same member, dartdoc is unclear about where the canonical home for that library is.

The warning we get from dartdoc is "ambiguous reexport".

It'd be great if we had a lint for this!

[pq]

Interesting. Don't we see this pattern in Flutter (e.g., cupertino vs. material that both export 'widgets.dart')? How does dart doc tackle that case?

(To be clear, I'm not arguing against this at all. Mainly just curious if there are exceptional cases to consider.)

[bwilkerson]

There has been some discussion of building a data structure that would record which private libraries are exported from which public libraries for the purpose of producing better imports in both code completion and fixes. This lint would presumably want to have the same data structure. (@scheglov perhaps this could make it onto the Q1 OKRs).

I assume then that this lint would flag

• every export of a private library that is exported from multiple public libraries, and
• every export of a public library from a public library.

Does that sound like what you'd expect?

[kevmoo]

@bwilkerson – yes!

On top of that, if we're doing the work anyway, it'd be AMAZING if we could figure out public members in src libraries that are not used and not exported – just so we could clean up! CC @natebosch

[bwilkerson]

... if we could figure out public members in src libraries that are not used and not exported ...

I'm not sure I understand what you mean by that, but in any case that would probably require a more global analysis than the original request.

Note that I was proposing that we track libraries, not members. Along those lines, would we need to be concerned with supporting a case such as having an internal library that defines both A and B and having A be exported by one public library and B be exported by a different public library? I kind of hope not. (If we do then we might well need to track this for individual members.)

[scheglov]

How will this be used? When doing code completion from outside, we don't suggest internal libraries at all. So, are talking about code completion from inside the package that both has an internal library and re-exports it? And do we want to prefer importing the internal library, or the public one?

[kevmoo]

Sorry, I got a bit off topic.

See

https://github.com/google/googleapis.dart/blob/master/googleapis_auth/lib/googleapis_auth.dart
https://github.com/google/googleapis.dart/blob/master/googleapis_auth/lib/auth_io.dart
https://github.com/google/googleapis.dart/blob/master/googleapis_auth/lib/auth_browser.dart
https://github.com/google/googleapis.dart/blob/master/googleapis_auth/lib/auth.dart

All of these libraries "contain" ServerRequestFailedException – so dartdoc gets confused. We add the

/// {@canonicalFor service_account_credentials.ServiceAccountCredentials}

comment to gooapis_auth.dart so dartdoc knows that's "home".

If you're not looking at dartdoc output, you'll never know this is an issue. It'd be nice if this was surfaced at analysis time so it could be fixed sooner!

[natebosch]

There are legitimate use cases for exposing the same symbols from multiple public libraries. If we do anything here I think we should ensure that the lint is suppressed by the same comment as DartDoc uses.

I think it could be helpful to guide users to define a canonical import for a symbol, but I don't think we need to disallow duplicative exports entirely.

[kevmoo]

What @natebosch said. This is about more alignment between dartdoc and analyzer (which I know @jacob314 and others have discussed)

[bwilkerson]

When doing code completion from outside, we don't suggest internal libraries at all.

Well, we used to, and we're still getting reports of this from users, but I can't reproduce it locally so maybe it was fixed and just hasn't been published yet.

In which case, the question is whether the information used to limit the suggestions could be used by this lint (if we write it).

[scheglov]

Oh, let me correct myself. With the new completion protocol and implementation, we will not suggest. And with the CL that was landed today for Import Library quick fix, and share this logic for allowing public or private libraries. We should have similar logic even with available declarations used today for completions. But we might miss it in some quick fixes.

As for lints, you mentioned two causes:

1. Every export of a public library from a public library - should be trivial to implement, does not require any analyzer support.
2. Every export of a private library that is exported from multiple public libraries. This is new. Currently we analyze library by library. So, after working with one library we know all its errors. But here we cannot do this anymore - we need to look at all libraries, and only then we can report errors for one of them. In this case "to look" means processing export directives. But in general, for finding unused declarations with global analysis, it is much more. From the analyze API point of view, it seems expensive to analyze all files on getResolvedUnit() call. But I can imagine additional global analysis result produced from AnalysisDriver after analysis is complete (right before switching to idle). It would include files which get more errors - both previously reported, and newly discovered during global analysis.