proposal: lint to flag when an exported API uses types that are not exported

[Hixie]

It would be great to have a lint that flags when an exported API uses types that are not themselves exported.

For example:

// library a

class Foo { }

// library b
import 'package:a/a.dart';

abstract class Bar {
  Foo get foo;
}

// ...

Library b should lint that Foo is not exported, correction would be:

// library b
import 'package:a/a.dart';
export 'package:a/a.dart' show Foo; // <--

abstract class Bar {
  Foo get foo;
}

// ...

[a14n]

Attempting to make such a lint I face several questions/feedbacks:

I think this lint shouldn't apply to libraries that are not reusable library. To detect those API-libraies I look for a main function in the library, if the function exists the lint is disable on this lib. This allows to avoid lints on binaries, tests and everything executable.

Should the types used in parameter list be exported? On flutter codebase for instance there is Future<T> timeout(Duration timeLimit, { FutureOr<T> Function()? onTimeout }) : should FutureOr be exported? I tend to answer yes but I'd like to have your opinion.

On Flutter there is also cases with type alias where I'm not sure what to do. For instance Future<R> compute<Q, R>(isolates.ComputeCallback<Q, R> callback, Q message, { String? debugLabel }) where typedef ComputeCallback<Q, R> = FutureOr<R> Function(Q message); should we lint to export ComputeCallback, FutureOr or both?

Considering transitivity I'm not sure what the result should be. Finding what to export could be quite difficult and this could lead to a really large namespace at the end (with perhaps name clashes). Look at the following example:

// a.dart
class A {}

// b.dart
import 'a.dart';
export 'a.dart' show A;
class B {
  A a = A();
}

// c.dart
import 'c.dart';
export 'b.dart' show B;
// should we also export A ???
class C {
  B b = B();
}

[bwilkerson]

It isn't clear to me that we want this lint. There's a high cost to code completion when the namespace of a library is bigger than it needs to be. While I understand the convenience benefit that led to Flutter's design, I'm not convinced that the benefit justifies the cost, especially for other packages. As a result, I'm not sure that this is a style that we want to promote among the larger community.

I think this lint shouldn't apply to libraries that are not reusable library.

I agree. But I don't think the criteria you listed is sufficient, and maybe not necessary. First, the package must be a published package, then the library needs to be inside the lib directory, and probably outside of lib/src. Given that, it isn't clear to me that the presence of a main function should disqualify the library (though I'm not opposed to including that condition).

Should the types used in parameter list be exported? ... On Flutter there is also cases with type alias where I'm not sure what to do.

I would count parameter types as being used by an API, and I would count the types references in a type alias as being used as well.

should we also export A ???

When you import a library L, modulo any combinators, you're making the export namespace of the library available to the importing library. The export namespace of L includes all of the public top-level names declared in L together with all of the names from any libraries exported by L. So in your example, A is already in the export namespace of c.dart and there's no need to have an explicit export. There might be value in exporting it, but no need.

[a14n]

Thanks for your feedback.

So in your example, A is already in the export namespace of c.dart and there's no need to have an explicit export. There might be value in exporting it, but no need.

I used export 'b.dart' show B; so unless I remove the show B part I don't expect A to be exported from c.dart. That's why I was wondering if a lint should be trigger to export also A.

[bwilkerson]

I used export 'b.dart' show B; ...

Yes, I missed that detail. Sorry.

I suspect then that the answer is 'yes', because a public API exported from c.dart references it. Although a fix would have a hard time currently knowing whether it should be exported from b.dart or whether to add a new export for a.dart.

[Hixie]

I think this lint shouldn't apply to libraries that are not reusable library. To detect those API-libraies I look for a main function in the library, if the function exists the lint is disable on this lib. This allows to avoid lints on binaries, tests and everything executable.

Makes sense.

Should the types used in parameter list be exported? On flutter codebase for instance there is Future<T> timeout(Duration timeLimit, { FutureOr<T> Function()? onTimeout }) : should FutureOr be exported? I tend to answer yes but I'd like to have your opinion.

Yes seems like the right answer to me.

On Flutter there is also cases with type alias where I'm not sure what to do. For instance Future<R> compute<Q, R>(isolates.ComputeCallback<Q, R> callback, Q message, { String? debugLabel }) where typedef ComputeCallback<Q, R> = FutureOr<R> Function(Q message); should we lint to export ComputeCallback, FutureOr or both?

Interesting question. I guess compute would need to export ComputeCallback and ComputeCallback would need to export FutureOr?

Considering transitivity I'm not sure what the result should be. Finding what to export could be quite difficult and this could lead to a really large namespace at the end (with perhaps name clashes). Look at the following example:

// a.dart
class A {}

// b.dart
import 'a.dart';
export 'a.dart' show A;
class B {
  A a = A();
}

// c.dart
import 'c.dart';
export 'b.dart' show B;
// should we also export A ???
class C {
  B b = B();
}

Yes, A is part of C's API.

It isn't clear to me that we want this lint. There's a high cost to code completion when the namespace of a library is bigger than it needs to be. While I understand the convenience benefit that led to Flutter's design, I'm not convinced that the benefit justifies the cost, especially for other packages. As a result, I'm not sure that this is a style that we want to promote among the larger community.

I don't think this is Flutter-specific. It's annoying any time I use an API and it turns out that just importing the API isn't enough to use the API.

I don't see why this should affect autocomplete, can you elaborate on that?

Personally I've found autocomplete to be largely unusable in general, first because it interferes with what I can see of the code, second because it intercepts arrow key movements unpredictably, and third because it rarely suggests what I need so using it is slower than just typing what I want, but I'm probably not the target audience.

[jakemac53]

I understand the usability concern driving this lint, but in general, packages should not export other packages. In fact, I would propose that we should instead have exactly that lint (do_not_export_other_packages).

The problem is that it does not work well with semantic versioning and the package ecosystem as a whole.

Basically, you are now tightly coupling your package with that package. Users of your package may have no idea the package you are exporting even exists (they don't import it), and they are unlikely to have an explicit dependency on it (for the same reason). But they do actually use it, and do depend on its API, so they should have a dependency on it.

You can attempt to resolve this, but ultimately you run into problems:

Option 1: You pin to exact versions of that dependency you are exporting

• Now you have to release any time your dependency releases. This causes cascading releases any time any package releases, which is a major burden for the ecosystem.
• You have to follow their version bumps with equivalent ones (feature releases for them are now feature releases for you, same for breaking changes).
• People are blocked from getting even patch releases from your exported dependency until you also publish.

Option 2: You pin to minor release ranges (>=1.1.0 <1.2.0) of the dependency you are exporting

• You still have to release whenever there is a feature release of the dependency you export, and you have to keep the range tight to keep people on the rails.
• You still have to do a major version bump any time they do
• People are blocked from getting minor releases from your exported dependency until you also publish.

Option 3: Keep a normal dependency range

• You still have to do a major version bump any time they do
• You run the risk of people getting new versions of an API they are relying on without actually realizing it. For instance they may add a constraint on the latest version of your package and assume all APIs exposed by it are available. But actually, for the re-exported apis they are unconstrained. You may be using an API that is available in version 1.1.x, but the package exporting that package allows a range with includes earlier versions. You had no idea this other package exists, and don't realize you need this extra constraint.
• You also can only support one major version of that package at a time, even if your package is actually compatible with multiple.

Option 4: Ignore the problem

This works until it doesn't. Ultimately it makes the ecosystem very fragile, especially if it becomes a widespread practice.

Other thoughts

If we did want to encourage this practice, we would need to supplement it with additional lints that ensure users add real dependencies when they end up using an API from another, exported package. That could possibly make this "safe", but only if that was a lint included in the core set of lints. It would be imperative that people follow it.

I think it also might be an expensive lint to run, at least more so than the current depend_on_referenced_packages lint which only looks at imports. Also, dynamic makes this an impossible lint to write perfectly, although that is probably OK.

Fwiw, technically this problem does exist already today in a restricted form, and such a lint would potentially also solve that problem. Basically, the existing issue is you can do foo.bar.baz to access APIs from another package, without importing it, as long as you don't actually reference the types. But this is a flaw in the existing system and not something we want to double down on.

[jakemac53]

Fwiw if lint is restricted to only suggest exporting libraries from within the same package then that doesn't have the above problems regarding versioning.

But we would want to make that an explicit part of the proposal then.

[jacob314]

package:flutter is pinning all its dependencies. I think it would be fine for the lint to suggest exporting both from within the package, pinned dependencies of the package, and the dart sdk.

Proposed lint name: batteries_included_library.

[natebosch]

The Dart SDK, including dart:core does not follow this rule today. For instance if you start with a method that returns a Future and you use return types, argument types, parameter types in function typed arguments, then you can see classes that you cannot reference: EventSink, FutureOr, StreamSubscription, StreamTransformer (dart:async), and Random (dart:math).

If we start making some libraries supersets of others we will need tools to help discover when a dependency is too high level. If I depend on package:foo which exports package:bar but I'm only using stuff from bar the build is less optimal.

[lrhn]

What @jakemac53 says! The lint I could get behind is that:

Your package's public API (which is probably the exported API of the libraries in the root of the lib/ dir) should be self contained. All types reachable in that API and defined in the same package, should also be exported. No types defined in other packages should be part of that API (it's a warning, it can be ignored if you are an exception and your package is really building on top of another package, and then you should be aware of all the issues @jakemac53 mentioned above). Using SDK types is always fine, user will have those available.

[pq]

Great conversation! FWIW, I'm very much w/ @jakemac53. Exploring something like what @lrhn proposed sounds valuable to me (though a little adjacent to what @Hixie was after in the OP).

[bwilkerson]

I don't see why this should affect autocomplete, can you elaborate on that?

In order to maximize the benefit of autocompletion, the tooling needs to make the text that the user is typing be as close to the top of the list of suggestions as often as possible.

If there were only one possible valid choice for what the user is trying to type then the analysis server could predict the text with 100% accuracy. If there were 10 possible valid choices, with nothing to indicate which is more likely, then the analysis server could predict the correct text 10% of the time (assuming a random order for the suggestions). Fortunately, there are usually indicators that can help it choose correctly more often than that, such as the required type of the expression at the completion location.

But when the target type is Widget, and there are hundreds of widget classes in the name scope, then there's little information (that we're currently aware of) for the analysis server to use to distinguish one class from another, and the probability that the server can correctly choose which class the user is trying to type is significantly low.

We face the same challenge when suggesting static fields on classes like Icons and Colors.

[Hixie]

Wouldn't making sure that we export the relevant types make it more likely that autocomplete could find the relevant types, if it's limited to things that are currently in scope?

In my experience, we currently show lots of stuff in the autocomplete dropdown that are not relevant [1]. It's not clear to me that changing what's in scope slightly would have any notable impact on autocomplete. If it really is that sensitive to what's in scope, then we should be thinking much more carefully about what we export (for example, maybe material shouldn't reexport all of widgets and widgets shouldn't reexport all of painting, for example, and we should instead require people to import widgets or painting when material isn't enough).

[1] For example, in my brand new main.dart from the Flutter skeleton template in VSCode if I type runApp( the autocomplete options are settingsController (not a valid thing to ever put as the first argument to runApp), true, false, null (none of those are ever ok), const (plausible, though if I select it it doesn't suggest anything to follow up with and leaves me with invalid code), await (extremely unlikely, and again if I select it it doesn't suggest anything to follow up with and leaves me with invalid code), every constructor that's in scope, even though many of them are not widgets and therefore aren't appropriate, every constant and variable that's in scope (including all the non-widgets that would be useless here), lots of duplicates (e.g. Checkbox is listed twice), and a whole bunch of things that aren't in scope (for example Platform is listed twice, once to autoimport dart:html and once to autoimport dart:io). The list is literally hundreds if not thousands of items long. I don't think exporting a few more types will make the slightest difference here.

[Hixie]

Incidentally I remembered the other day that we have another very similar lint already, that flags privates in public APIs. Making this consistent with that lint makes sense to me.

[a14n]

Some additional feedbacks from my experiment with this lint on Flutter (personal thought: I'm now rather against this lint) :

Transitivity issues

Here's a example of diff after implementing the transitivity feature (if A uses B in its public API and B uses C in its public API, then A lib needs to export B and C even if C is not directly used)

+++ b/packages/flutter/lib/src/foundation/consolidate_response.dart
@@ -7,8 +7,60 @@ import 'dart:convert';
 import 'dart:io';
 import 'dart:typed_data';

-export 'dart:io' show HttpClientResponse;
-export 'dart:typed_data' show Uint8List;
+export 'dart:async'
+    show
+        EventSink,
+        StreamConsumer,
+        StreamSink,
+        StreamSubscription,
+        StreamTransformer,
+        StreamTransformerBase;
+export 'dart:convert'
+    show
+        Codec,
+        Converter,
+        Encoding;
+export 'dart:io'
+    show
+        ConnectionTask,
+        ContentType,
+        Cookie,
+        HeaderValue,
+        HttpClientResponse,
+        HttpClientResponseCompressionState,
+        HttpConnectionInfo,
+        HttpHeaders,
+        IOSink,
+        InternetAddress,
+        InternetAddressType,
+        RawSocketOption,
+        RedirectInfo,
+        Socket,
+        SocketOption,
+        X509Certificate;
+export 'dart:typed_data'
+    show
+        ByteBuffer,
+        ByteData,
+        Endian,
+        Float32List,
+        Float32x4,
+        Float32x4List,
+        Float64List,
+        Float64x2,
+        Float64x2List,
+        Int16List,
+        Int32List,
+        Int32x4,
+        Int32x4List,
+        Int64List,
+        Int8List,
+        TypedData,
+        Uint16List,
+        Uint32List,
+        Uint64List,
+        Uint8ClampedList,
+        Uint8List;

This leads to several issues.

namespace groth

With this change a lib using consolidate_response.dart will now see 44 additional names even if it never uses any of them.

name clashes

After this change the Flutter codebase contains several errors about class name conflicts (eg. there are 2 Codec classes: one from dart:ui and one from dart:convert).

If a lib has those 2 Codec classes in its public API it will be impossible to export its whole public API (only one Codec can be exported).

Cleanup

With a lot of export, it's easy to make a mistake by exporting a name that is not in the public API. How to detect that? We could make a lint for that (unnecessary_export) but there are a lot of place where exports are not necessary related to this rule and are wanted (eg. lib/a.dart exporting lib/src/b.dart) so this lint is quite impossible to do IMHO.

[Hixie]

Do you have concrete examples of the transitivity thing? e.g. what API does Flutter expose which exposes dart:convert's Codec? Float32x4List may be an even better example; what API chain leads to that one?

[a14n]

From my previous example on consolidate_response.dart that exports HttpClientResponse and Uint8List (in its direct public API):

Uint8List has a constructor Uint8List.view(ByteBuffer buffer, [int offsetInBytes = 0, int? length]) and ByteBuffer has almost every dart:typed_data types in its public API.

dart:convert's Codec comes from the path : HttpClientResponse has a method Future<Socket> detachSocket() returning Socket that has a encoding property of type Encoding that is a Codec<String, List<int>> that has a Codec in its public API.

[Hixie]

Hmm. Interesting conundrum. For the first one, I would argue that since it returns a Uint8List we don't care about Uint8List's constructor (though I have no idea how the lint is supposed to know that).

For the second though, ugh, that's pretty bad. I can't come up with a plausible reason why Codec is clearly not part of the HttpClientResponse API, but you're right that we clearly don't really want to export that whole API...

It's kind of like you want to draw a line around the API, like, "export HttpClientResponse but ignore detach, that's not really an API you'll need to use this one", or "export Uint8List but ignore the constructor, you don't need to build it to use me".

What are the kinds of transitive walk that you're doing? The earlier discussion was about superclasses, and specifically the types of inherited public members, which seem to me to obviously be part of the API. But maybe we don't go any deeper than just that?

[a14n]

...I would argue that since it returns a Uint8List we don't care about Uint8List's constructor...

Interesting! I'm now wondering what was your initial problem that makes you request such a lint. Is it because of always_specify_types that forces you to write the type every time you declare a variable (and add the corresponding import)? Given that always_specify_types is rarely used I'm not sure it is worth it to export public API because most of the time users relay on type inference, never write types, so they will not have any benefits.

What are the kinds of transitive walk that you're doing?

You can take a look at my current implementation ( far from perfect :) ). Before adding transitivity I was collecting types on constructors, fields, methods, top level variables/functions. After adding transitivity I collect recursivelly also types from classes already collected by exploring their superclass, interfaces, constructors, fields, methods.

[lrhn]

When you look at APIs, you look at the types it exposes. Class types represent interfaces, not the entire class declaration (that's why subclasses are subtypes, even if statics are not inherited). That means its only instance members, not static members or constructors, which are relevant to the transitive type closure of an API.

If you then start exporting the declaration of those transitive type dependencies, then their class APIs become part of your library API too, and then you do need to look at the exposed constructors and static members. So, the transitive type closure of a library is not the same as the transitive type closure you get by exporting your transitively used types. The transitive type closure is not closed under export.

Which is why exporting your dependencies might not be the solution you're looking for. It introduces more problems that needs solutions.

[eernstg]

@a14n asked about the motivation for proposing the lint which is the topic of this issue, and suggested that it could be the simultaneous use of always_specify_types.

Following that line of thinking, we could perhaps describe the purpose of the lint as follows: We want each exported namespace N to be closed, such that every member access based on a declaration imported from N will have a type which is also imported from N. This could improve the level of explicit dependency documentation, and it might improve on the service offered by IDE completion and such. It could also reduce the need for further imports for developers of code that imports such a namespace, because all reachable APIs are already available in the imported namespace.

In that sense it's a nice usability property that a widely used library could have, a new kind of 'batteries included' property that we might call 'all reachable imports included'.

It is possible that I took too many steps at once here, so maybe the intention is just that we get 'all imports reachable in one step', or something like that—that is, if we import a function f returning a Foo that has a bar method that returns a Bar, we'd insist that the imported namespace includes Foo, but not that it includes Bar. I find it somewhat difficult to justify why any specific level of transitivity would be optimal, though, because we might well have expressions like f().foo().bar().someMemberOfBar(some, args), and why wouldn't we expect Bar to be imported if we insist that Foo is imported, and similarly for the types in the signature of someMemberOfBar?

I can understand why the 'closedness' property looks good, but I'd also like to mention that the situation has a structure which is quite similar to the situation that we encountered with reflection several years ago: Anything that is based on automatically following the transitive closure of a software dependency could give rise to a space explosion. With reflection we'd get reflection support for all kinds of things even in the case where only a few things are used, and with 'all reachable imports included' (and maybe even 'all imports reachable in k steps' for some k) we'd get access to a large set of reachable declarations, even in the case where only a few of them are used.

I don't know how costly it is to import a far larger set of libraries than what is actually required in order to be able to compile the library, so we'd need more input on that from the relevant tool teams.

In any case, this makes me think that it could be better to improve on the support for adding further imports to the current library based on the fact that a given member access has a type that involves a declaration which is not imported. For example, we're writing someExpression.foo().bar(), and the return type of foo() has not been imported; we could then have a lint and a quick fix that adds the given import to the current library, such that the type of the object that has a bar method is imported.

This would make it possible for developers to maintain a certain discipline with imports: If a member access r.m (or r.m<...>(...) or r.m = e etc.) is performed, the types in the signature of m have also been imported, in particular if it can be maintained easily, by performing a quick fix at the expression. However, some developers/organizations might prefer to follow a slightly less demanding discipline: If a member access r.m (or r.m<...>(...) or r.m = e etc.) is performed, the type whose interface contains the signature of m has also been imported. The former ensures that everything which could be done in the next member access is already using an imported type, and the latter just ensures that member accesses that have already been performed are based on a type which has been imported. I guess the choice would depend on the intended workflow, and the actual consequences for completion, etc, so I don't know which one is more promising (but, obviously, the latter one is less costly).

[Hixie]

The usual motivation for me is things like calling paintZigZag and needing to pass an Offset and Paint and Canvas but finding I don't have any of those APIs in my namespace so now I need to import each of them to use them and then finding that now I have Paint but don't have StrokeCap so I can't set the paint's stroke cap. If I want to use paintZigZag then I definitely also want to use Offset and Paint and Canvas and not exporting those is just a bad developer experience.

[Hixie]

(Originally I would maintain this property of the APIs myself, making sure we exported what mattered every time we added anything to each library's APIs, but as the team has grown this has not scaled and new contributors often don't know about this facet of API design and so don't enforce it and so quickly nobody is enforcing it because the precedent is you don't export the types you use and so on.)

[a14n]

The usual motivation for me is things like calling paintZigZag and needing to pass an Offset and Paint and Canvas but finding I don't have any of those APIs in my namespace so now I need to import each of them to use them and then finding that now I have Paint but don't have StrokeCap so I can't set the paint's stroke cap. If I want to use paintZigZag then I definitely also want to use Offset and Paint and Canvas and not exporting those is just a bad developer experience.

I'd say it's something that should be addressed by auto-complete. The static anaylisis is aware that after typing paintZigZag an argument of type Offset is needed and so the autocomplete list should suggest a list where first items return Offset (eg. first from the current file then from the public API fo the lib where paintZigZag is defined and finally from other lib accessible from the transitive graph of dependencies).

Given my current implementation of this lint, I'm not sure it goes somewhere. As you confirmed, too much elements are exported and it's quite impossible to have a logic to automatically cut some branches.

[Hixie]

I'd say it's something that should be addressed by auto-complete.

I've never experienced autocomplete that doesn't frustrate me, so that wouldn't be my first choice. :-)

[Hixie]

I think there's value in this lint even if it's just one layer deep (i.e. not transitive). The PRs you landed in the Flutter framework were all improvements, IMHO.

[a14n]

I think there's value in this lint even if it's just one layer deep (i.e. not transitive). The PRs you landed in the Flutter framework were all improvements, IMHO.

If we go in this direction we will face cleanup issue in the future if some APIs change. For instance if we widden a parameter type of a constructor from A(Child p) to A(ParentOfChild p) the previous export ... show Child will not be necessary anymore.

[Hixie]

Sure.

[n7trd]

Created the issue https://github.com/flutter/flutter/issues/100444 a while ago to highlight the problem of re-exports in public apis . In short, it is very difficult to get an overview of an api with so many re-exports. What types are exported by cupertino? What types are exported by fluent_ui (follows same re-export pattern)?

[bwilkerson]

Does the lint library_private_types_in_public_api satisfy this request?

[jakemac53]

Does the lint library_private_types_in_public_api satisfy this request?

I don't believe this is the same thing no - the request is to have a lint telling you to re-export public types that appear in your API (but to be clear I don't think that is a good idea because you can't reasonably generalize when you should do that or not, I think it should be a human choice).

[incendial]

For anyone looking for such functionality - it's now available as DCM command https://dcm.dev/docs/cli/check-exports-completeness.