dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

Scope of imported extensions #4056

Open lrhn opened 3 weeks ago

lrhn commented 3 weeks ago

Extensions are accessible if they are imported into the current library. That used to be the same as being imported into the library's import scope.

With enhanced parts (parts with imports!) an import in a part file is scoped to only that part and its sub-parts. That should apply to extension availability too! This needs to be added to the enhanced parts specification, and said explicitly. Otherwise "imported into the current library" can be interpreted in a number of different ways, none of them what's intended.

So:

An extension is accessible if it's declared in the current library, or if it has been imported into an import scope or a prefixed import scope in the current Dart file's combined import scope chain. The combined import scope chain is alternating prefix import scopes mapping names to prefixed import scopes, and import scopes with directly imported declarations, each pair of such introduced by one file on the path to the current file in a part-file tree structure. The extension is available in a Dart file if it was imported in to any of these import scopes of the Dart file's combined import scope chain.

An extension is imported into a scope if there exists an non-deferred import directive which:

  • imports declarations into that scope (an import with no prefix imports into the import scope of the current file, an import with a prefix imports into the prefixed import scope for that prefix name in the current file's prefix import scope),
  • imports a library which has the extension declaration in its export scope,
  • and does not hide the name of the extension.
    • An import hides a name if it has a hide modifier with the name in its identifier list, or if it has a show modifier that does not have the name in its identifier list, or if the name is loadLibrary and the import is deferred (which doesn't apply here).

This is deliberately different from saying that the name is accessible in the scope. The name can be shadowed by later imports, and then name can even be conflicted where it's imported. The only thing that matters is that it's not hidden.

This does break one of the rules I want for part files: That they can ignore parent file imports completely. That's not true if an extension imported by a parent can conflict with an extension imported in a part, even if the name of the extension can be ignored.

@dart-lang/analyzer-team Should extensions imported in a part file take precedence over extensions inherited from a parent file?

That would be the consistent solution. If a part file imports an extension, and it uses a member from that extension, then it keeps working even if a parent file includes another extension with the same name on the same type.

It does mean that this overrules specificity. If a part file imports an extension foo on List<Object> and a parent file had imported an extension foo on List<int>, then <int>[1].foo would resolve to the one on List<Object>.

There are no existing libraries with imports in part files, so nothing will change at launch of the enhanced parts feature. Parts can still see parent (library) imports, and won't have any imports that interferes.

Here is a PR which does the change for both accessibility and specificity, giving closer imports precedence: https://github.com/dart-lang/language/pull/4058

@dart-lang/language-team

eernstg commented 1 week ago

Should extensions imported in a part file take precedence over extensions inherited from a parent file?

I'd prefer to not do that.

As usual, I prefer to give a high priority to software correctness, even in some cases where a situation must be disambiguated by means of a small amount of extra syntax. Extension disambiguation is already a subtle set of rules, and I think it's better to use these rules everywhere, rather than introducing a special (and completely new) kind of disambiguation criterion here.

Note the subtlety of the situation (I've used exports in order to reduce the number of declarations, but we could also just write separate declarations with the same names and signatures directly in 'imported_lib1' and 'imported_lib2'):

graph BT;
  main["main_lib.dart"]
  part1["part1.dart"] -->|part of| main
  part2["part2.dart"] -->|part of| part1
  part1 -->|import| lib1["imported_lib1.dart\nexport 'ext1.dart';\nexport 'ext2.dart';"]
  part2 -->|import| lib2["imported_lib2.dart\nexport 'ext1.dart';"]
// --- Library 'ext1.dart'.

extension E1<X> on List<X> {
  void foo() {}  
}

extension E2 on List<num> {
  void foo() {}
}

// --- Library 'ext2.dart'.

extension E2<X extends num> on List<X> {
  void foo() {}
}

// --- Part 'part1.dart'.

void bar() {
  <int>[].foo(); // ext2.E2.
  <num>[].foo(); // Error, ambiguous.
  E1(<num>[]).foo(); // ext1.E1.
  E2(<num>[]).foo(); // Error, ambiguous.
}

// --- Part 'part2.dart'.

void baz() {
  <int>[].foo(); // ??? ext1.E1 has priority by import, ext2.E2 has priority by tiebreaker.
  <num>[].foo(); // ext1.E2.
  E1(<num>[]).foo(); // ext1.E1.
  E2(<num>[]).foo(); // ext2.E2.
}

With <int>[].foo() in 'part2.dart', the instantiated on-type of ext1.E1 and ext2.E2 is List<int> (and for ext1.E2 it is List<num>, so ext1.E2 is out). The instantiation to bound of the on-type is List<dynamic> and List<num>, respectively, so ext2.E2 wins in terms of the tiebreaker rule that relies on instantiation to bound. However, now we also have a rule which says that ext1.E1 has a higher priority because it is imported by 'part2.dart' itself. I don't know how to compute result, so I marked the case with '???'.

If we do as I'd prefer, and do not introduce a prioritization which is based on the import inheritance structure, then the outcome in 'part1.dart' and in 'part2.dart' would be the same: 'part2.dart' has access to all three extensions on an equal footing because they have been imported (here, or in a parent), and they haven't been hidden in said import.

lrhn commented 1 week ago

The linked proposal unambiguously makes a closer import be more specific than a further-away import, and type-based and "platform-library-deprioritization" specificity is only used for extensions imported in the same file.

You either import what you need, and then that takes precedence over anything imported, or you inherit what your parent had.

If you have two extensions both applying to the same type, then they should be related and imported together, or it's very likely they will conflict. (Or come to conflict in the future if one author makes their extension more general, or adds a more specific variant, both of which are compatible with existing uses of their own extension.)

I like that a part file has the ability to separate itself entirely from its inherited parent-file imports, by simply importing everything it needs. I think that's a good ability to have for people writing macros, and it avoids users asking for ways to specify that they don't inherit.

I don't think the example of importing ext1.dart and wanting to still have ext2.darts same-named extension available is as realistic as importing ext1.dart and only wanting that.

eernstg commented 1 week ago

The linked proposal unambiguously makes a closer import be more specific than a further-away import

Right, I just saw that by checking the PR again. (I'll adjust my comment above to acknowledge this.)

and type-based and "platform-library-deprioritization" specificity is only used for extensions imported in the same file.

This is still a completely new rule that is added to the process of resolving extension member invocations. For example, in the program I included here, <Object>[].foo() in 'part2.dart' will be resolved using extensions that are imported by 'part1.dart', and <num>[].foo() will be resolved using extensions that are imported by 'part2.dart', ignoring that some extensions imported by 'part1.dart' would be applicable, except that they have lower priority according to the import structure.

I like that a part file has the ability to separate itself entirely from its inherited parent-file imports, by simply importing everything it needs.

This is not a property that our design supports so far. For example, if a parent adds an imported name 'n' (this could occur because a new import is added, or because an existing imported library is updated to a new version), that new available declaration named 'n' could be the result of name resolution for a particular occurrence of the identifier 'n', and this occurrence could previously have been resolved as this.n, denoting an inherited instance member.

Also, if a part file like 'part2.dart' is modified such that an extension is now accessible (could be a new import, could be an import of an updated library), and it happens to be applicable to some member invocations in 'part2.dart' (or one of its descendants), this addition will turn off all the extension that we previously had access to, because they are imported by a parent. I don't think that's a particularly user-friendly failure mode. How would you say "but you shouldn't turn those extensions off"?