Body scope for augmentations

[johnniwinther]

The scope for class-like augmentations in the proposal for augmentations is inconsistent with the library scope for enhanced parts.

For enhanced parts, all files, parts or library, within the same library have simple name access to the entities declared in the library. This is motivated with the ability to seemlessly split a library into several parts without changing the semantics. For instance splitting

// foo.dart
method1() { method2(); }
method2() { method3(); }
method3() {}

into

// foo.dart
part 'bar.dart';
part 'baz.dart';
method3() {}

// bar.dart
part of 'foo.dart';
part 'boz.dart';

// boz.dart
part of 'bar.dart';
method1() { method2(); }

// baz.dart
part of 'foo.dart';
method2() { method3(); }

is valid since all files, foo.dart, bar.dart, baz.dart, and boz.dart have access to entities declared in any other files of the same library.

For class-like augmentations this isn't the case because it uses lexical scoping even when the class-like declaration is split into an introductory declaration and one or more augmenting declarations. This means that the splitting of a class-like declaration into several augmentations does not preserve semantics. For instance splitting:

// visitor.dart
class LargeVisitorImpl {
  visitMember1(Member1 node) { helper(); }
  visitMember2(Member2 node) { helper(); }
  ...
  visitMember100(Member100 node) { helper(); }
  visitStatement1(Statement1 node) { helper(); }
  visitStatement2(Statement2 node) { helper(); }
  ...
  visitStatement100(Statement100 node) { helper(); }
  visitExpression1(Expression1 node) { helper(); }
  visitExpression2(Expression2 node) { helper(); }
  ...
  visitExpression100(Expression100 node) { helper(); }
  static helper() {}
}

into

// visitor.dart
part 'member_visitor.dart';
part 'expression_visitor.dart';
part 'statement_visitor.dart';
class LargeVisitorImpl {
  static helper() {}
}

// member_visitor.dart
part of 'visitor.dart';
augment class LargeVisitorImpl {
  visitMember1(Member1 node) { helper(); }
  visitMember2(Member2 node) { helper(); }
  ...
  visitMember100(Member100 node) { helper(); }
}

// statement_visitor.dart
part of 'visitor.dart';
augment class LargeVisitorImpl {
  visitStatement1(Statement1 node) { helper(); }
  visitStatement2(Statement2 node) { helper(); }
  ...
  visitStatement100(Statement100 node) { helper(); }
}

// expression_visitor.dart
part of 'visitor.dart';
augment class LargeVisitorImpl {
  visitExpression1(Expression1 node) { helper(); }
  visitExpression2(Expression2 node) { helper(); }
  ...
  visitExpression100(Expression100 node) { helper(); }
}

results in compile time errors in all calls to helper(); which now have to be replaced with LargeVisitorImpl.helper(); in order to preserve semantics. This would even be the case if all augmentations were in the same file.

I find the two structures to be similar; 1) libraries split into a main library file and several part files, and 2) class-like declarations split into an introductory declaration and several augmenting declarations. They both a have shared responsibility of defining the respective name spaces and to me it therefore seems natural that they should also share the privilege of having simple name access to entities declared in that name space.

[johnniwinther]

@lrhn @jakemac53 @eernstg @munificent @leafpetersen

[lrhn]

I was the one who changed the prior behavior, which was to include the entire scope, to the current specification. My reasoning was that it gives users better ability to control the lexical scope inside a class declaration, augmenting or not, and that I was confused by members that were not in the lexical scope being in the, well, what I thought was a lexical scope. I didn't change how libraries worked, because that would be too breaking (we have libraries with parts already, we don't have classes with augmentations). Also members added by macros in a much later part file won't affect the interpretation of identifiers in an earlier part file.

The only difference is whether you can omit a leading this. from instance members and ClassName. from static members not declared in the same physical class declaration, but therefore also whether you can access something of the same name from a surrounding scope. And ind most cases, you can still omit the this. because the name won't be in the scope at all, so it falls back to this.name anyway. It affects static member access more.

I can see the argument by similarity here. If parts are, like, augmentations for libraries, and augmenting class declartions are augmentations for classes, why can the part see everything declared anywhere in the library directly, and the augmenting class only has its own declarations in its lexical scope. (Because there is no prefix you can write to access things from outside of the part, but you can just write this. or ClassName. for the augmenting class.)

An also reasonable argument is that users expect to be able to write foo instead of this.foo because they're inside the class, and staticFoo directly too inside the class that declared static staticFoo, even if it's in the direct lexical scope of the current fragment of that class. The fraction of normal users who have internalized the distinction between foo not being in the local scope, and foo being in the local scope as an instance member declaration, is ... less than epsilon. For any given epsilon. And most of those probably think of static member access in the same way.

Nothing technical prevents us from switching back, saying that the class scope of a class declaration (augmenting or not) contains an entry for each definition in the fully-augmented class definition.

We can even scan for non-augmenting declarations first and find all the members, before we start applying augmentations (if we want/need to, I'm not sure it's necessary unless we do type inference for bodies during augmentation application).

I can be convinced to switch. I've become more accepting of the maxim that "all fragments of a declaration are, and must be, cooperating". If a macro adds a member to a class, and the class stops compiling, the problem is the entire class, not the macro. The macro generated fragment is not added on top, it exists, and has always existed, at the same level as all the remaining fragments of the declaration.

So including the entire member definition scope of a class definition in the parent scope of every member declaration should not be confusing. All the members were always there, there is just one class definition.

I am also fine with keeping the current behavior. I can argue for it, and I don't think it'll be really be confusing for anything but static member access, and I don't feel so bad about those needing a prefix. (Although it is a little weird to use the current declaration's name as a prefix.)

[jakemac53]

Fwiw I have no strong opinion either way here - I like both approaches for different reasons. I do think it is weird to have to write ClassName.<static-member> when inside that class, but I also like the more "pure" lexical scoping 🤷‍♂️ .