[cfe] Add Mutable(Declaration)NameSpace

This removes the mutators from the (Declaration)NameSpace interface and
adds concrete subclasses for the various use cases.

Change-Id: I0728acdd93659d2e118aadfe02749cee3a47afb5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427703
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>

Author: johnniwinther

pkg/front_end/lib/src/base/export.dart

@@ -10,8 +10,8 @@ import 'combinator.dart' show CombinatorBuilder;
 import 'uri_offset.dart';
 
 class Export {
-  /// The compilation unit that is exporting [exported];
-  final CompilationUnit exporter;
+  /// The compilation unit that is exporting [exported].
+  final SourceCompilationUnit exporter;
 
   /// The library being exported.
   CompilationUnit exportedCompilationUnit;

pkg/front_end/lib/src/base/name_space.dart

@@ -13,11 +13,6 @@ import 'scope.dart';
 import 'uris.dart';
 
 abstract class NameSpace {
-  void addLocalMember(String name, NamedBuilder member, {required bool setter});
-
-  /// Adds [builder] to the extensions in this name space.
-  void addExtension(ExtensionBuilder builder);
-
   /// Returns the [LookupResult] for the [Builder]s of the given [name] in the
   /// name space.
   ///
@@ -54,11 +49,18 @@ abstract class NameSpace {
       {required bool includeDuplicates});
 }
 
+abstract class MutableNameSpace implements NameSpace {
+  factory MutableNameSpace() = NameSpaceImpl._;
+
+  void addLocalMember(String name, NamedBuilder member, {required bool setter});
+
+  /// Adds [builder] to the extensions in this name space.
+  void addExtension(ExtensionBuilder builder);
+}
+
 abstract class DeclarationNameSpace implements NameSpace {
   MemberBuilder? lookupConstructor(String name);
 
-  void addConstructor(String name, MemberBuilder builder);
-
   /// Returns an iterator of all constructors mapped in this scope,
   /// including duplicate constructors mapped to the same name.
   Iterator<MemberBuilder> get unfilteredConstructorIterator;
@@ -73,12 +75,17 @@ abstract class DeclarationNameSpace implements NameSpace {
       {required bool includeDuplicates});
 }
 
-class NameSpaceImpl implements NameSpace {
+abstract class MutableDeclarationNameSpace
+    implements DeclarationNameSpace, MutableNameSpace {
+  void addConstructor(String name, MemberBuilder builder);
+}
+
+base class NameSpaceImpl implements NameSpace, MutableNameSpace {
   Map<String, NamedBuilder>? _getables;
   Map<String, NamedBuilder>? _setables;
   Set<ExtensionBuilder>? _extensions;
 
-  NameSpaceImpl(
+  NameSpaceImpl._(
       {Map<String, NamedBuilder>? getables,
       Map<String, NamedBuilder>? setables,
       Set<ExtensionBuilder>? extensions})
@@ -139,6 +146,14 @@ class NameSpaceImpl implements NameSpace {
       _extensions?.iterator);
 }
 
+final class SourceLibraryNameSpace extends NameSpaceImpl {
+  SourceLibraryNameSpace(
+      {required Map<String, NamedBuilder> super.getables,
+      required Map<String, NamedBuilder> super.setables,
+      required Set<ExtensionBuilder> super.extensions})
+      : super._();
+}
+
 // Coverage-ignore(suite): Not run.
 /// Returns a string with an error message if [sourceNameSpace] and
 /// [dillNameSpace] are not equivalent.
@@ -218,16 +233,17 @@ String? areNameSpacesEquivalent(
   return sb.toString();
 }
 
-class DeclarationNameSpaceImpl extends NameSpaceImpl
-    implements DeclarationNameSpace {
+abstract base class DeclarationNameSpaceBase extends NameSpaceImpl
+    implements DeclarationNameSpace, MutableDeclarationNameSpace {
   Map<String, MemberBuilder>? _constructors;
 
-  DeclarationNameSpaceImpl(
+  DeclarationNameSpaceBase._(
       {super.getables,
       super.setables,
       super.extensions,
       Map<String, MemberBuilder>? constructors})
-      : _constructors = constructors;
+      : _constructors = constructors,
+        super._();
 
   @override
   void addConstructor(String name, MemberBuilder builder) {
@@ -251,7 +267,21 @@ class DeclarationNameSpaceImpl extends NameSpaceImpl
   }
 }
 
-abstract class LazyNameSpace extends NameSpaceImpl {
+final class SourceDeclarationNameSpace extends DeclarationNameSpaceBase {
+  SourceDeclarationNameSpace(
+      {required Map<String, NamedBuilder> super.getables,
+      required Map<String, NamedBuilder> super.setables,
+      required Map<String, MemberBuilder>? super.constructors})
+      : super._();
+}
+
+final class DillDeclarationNameSpace extends DeclarationNameSpaceBase {
+  DillDeclarationNameSpace() : super._();
+}
+
+abstract base class LazyNameSpace extends NameSpaceImpl {
+  LazyNameSpace() : super._();
+
   /// Override this method to lazily populate the scope before access.
   void ensureNameSpace();
 
@@ -274,7 +304,7 @@ abstract class LazyNameSpace extends NameSpaceImpl {
   }
 }
 
-class DillLibraryNameSpace extends LazyNameSpace {
+final class DillLibraryNameSpace extends LazyNameSpace {
   final DillLibraryBuilder _libraryBuilder;
 
   DillLibraryNameSpace(this._libraryBuilder);
@@ -285,7 +315,7 @@ class DillLibraryNameSpace extends LazyNameSpace {
   }
 }
 
-class DillExportNameSpace extends LazyNameSpace {
+final class DillExportNameSpace extends LazyNameSpace {
   final DillLibraryBuilder _libraryBuilder;
 
   DillExportNameSpace(this._libraryBuilder);

pkg/front_end/lib/src/builder/library_builder.dart

@@ -3,33 +3,28 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_fe_analyzer_shared/src/messages/severity.dart' show Severity;
-import 'package:front_end/src/builder/property_builder.dart';
 import 'package:kernel/ast.dart' show Annotatable, Library, Version;
 import 'package:kernel/reference_from_index.dart';
 
 import '../api_prototype/experimental_flags.dart';
 import '../base/combinator.dart' show CombinatorBuilder;
 import '../base/export.dart' show Export;
 import '../base/loader.dart' show Loader;
-import '../base/lookup_result.dart';
 import '../base/messages.dart'
     show
         FormattedMessage,
         LocatedMessage,
         Message,
         ProblemReporting,
         noLength,
-        templateDuplicatedExport,
         templateInternalProblemConstructorNotFound,
         templateInternalProblemNotFoundIn,
         templateInternalProblemPrivateConstructorAccess;
 import '../base/name_space.dart';
 import '../base/problems.dart' show internalProblem;
 import '../base/scope.dart';
-import '../base/uri_offset.dart';
 import '../fragment/fragment.dart';
 import '../kernel/body_builder_context.dart';
-import '../kernel/load_library_builder.dart';
 import '../source/offset_map.dart';
 import '../source/outline_builder.dart';
 import '../source/source_class_builder.dart';
@@ -83,7 +78,7 @@ sealed class CompilationUnit {
 
   List<Export> get exporters;
 
-  void addExporter(CompilationUnit exporter,
+  void addExporter(SourceCompilationUnit exporter,
       List<CombinatorBuilder>? combinators, int charOffset);
 
   /// Add a problem with a severity determined by the severity of the message.
@@ -376,10 +371,6 @@ abstract class LibraryBuilder implements Builder, ProblemReporting {
   /// Duplicates and augmenting members are _not_ included.
   Iterator<T> fullMemberIterator<T extends NamedBuilder>();
 
-  /// Returns true if the export scope was modified.
-  bool addToExportScope(String name, NamedBuilder member,
-      {required UriOffset uriOffset});
-
   /// Looks up [constructorName] in the class named [className].
   ///
   /// The class is looked up in this library's export scope unless
@@ -478,92 +469,6 @@ abstract class LibraryBuilderImpl extends BuilderImpl
         problemOnLibrary: true);
   }
 
-  /// Computes a builder for the export collision between [declaration] and
-  /// [other]. If [declaration] is declared in [libraryNameSpace] then this is
-  /// returned instead of reporting a collision.
-  NamedBuilder _computeAmbiguousDeclarationForExport(
-      String name, NamedBuilder declaration, NamedBuilder other,
-      {required UriOffset uriOffset}) {
-    // Prefix builders and load library builders are not part of an export
-    // scope.
-    assert(declaration is! PrefixBuilder,
-        "Unexpected prefix builder $declaration.");
-    assert(other is! PrefixBuilder, "Unexpected prefix builder $other.");
-    assert(declaration is! LoadLibraryBuilder,
-        "Unexpected load library builder $declaration.");
-    assert(other is! LoadLibraryBuilder,
-        "Unexpected load library builder $other.");
-
-    if (declaration == other) return declaration;
-    if (declaration is InvalidTypeDeclarationBuilder) return declaration;
-    if (other is InvalidTypeDeclarationBuilder) return other;
-    NamedBuilder? preferred;
-    Uri? uri;
-    Uri? otherUri;
-    if (libraryNameSpace.lookupLocalMember(name)?.getable == declaration) {
-      return declaration;
-    } else {
-      uri = computeLibraryUri(declaration);
-      otherUri = computeLibraryUri(other);
-      if (otherUri.isScheme("dart") && !uri.isScheme("dart")) {
-        preferred = declaration;
-      } else if (uri.isScheme("dart") && !otherUri.isScheme("dart")) {
-        preferred = other;
-      }
-    }
-    if (preferred != null) {
-      return preferred;
-    }
-
-    Uri firstUri = uri;
-    Uri secondUri = otherUri;
-    if (firstUri.toString().compareTo(secondUri.toString()) > 0) {
-      firstUri = secondUri;
-      secondUri = uri;
-    }
-
-    // TODO(ahe): We should probably use a context object here
-    // instead of including URIs in this message.
-    Message message =
-        templateDuplicatedExport.withArguments(name, firstUri, secondUri);
-    addProblem(message, uriOffset.fileOffset, noLength, uriOffset.uri);
-    // We report the error lazily (setting suppressMessage to false) because the
-    // spec 18.1 states that 'It is not an error if N is introduced by two or
-    // more imports but never referred to.'
-    return new InvalidTypeDeclarationBuilder(name,
-        message.withLocation(uriOffset.uri, uriOffset.fileOffset, name.length),
-        suppressMessage: false);
-  }
-
-  @override
-  bool addToExportScope(String name, NamedBuilder member,
-      {required UriOffset uriOffset}) {
-    if (name.startsWith("_")) return false;
-    if (member is PrefixBuilder) return false;
-    bool isSetter = isMappedAsSetter(member);
-    LookupResult? result = exportNameSpace.lookupLocalMember(name);
-    NamedBuilder? existing = isSetter ? result?.setable : result?.getable;
-    if (existing == member) {
-      return false;
-    } else {
-      if (member is MemberBuilder && member.isConflictingSetter) {
-        // TODO(johnniwinther): Remove this case when getables and setables are
-        //  contained in the same map in the name space.
-        exportNameSpace.addLocalMember(name, member, setter: isSetter);
-        return true;
-      } else if (existing != null) {
-        NamedBuilder result = _computeAmbiguousDeclarationForExport(
-            name, existing, member,
-            uriOffset: uriOffset);
-        exportNameSpace.addLocalMember(name, result, setter: isSetter);
-        return result != existing;
-      } else {
-        exportNameSpace.addLocalMember(name, member, setter: isSetter);
-        return true;
-      }
-    }
-  }
-
   @override
   MemberBuilder getConstructor(String className,
       {String? constructorName, bool bypassLibraryPrivacy = false}) {

pkg/front_end/lib/src/builder/prefix_builder.dart

@@ -22,7 +22,7 @@ class PrefixBuilder extends NamedBuilderImpl implements LookupResult {
   @override
   final String name;
 
-  final NameSpace _prefixNameSpace = new NameSpaceImpl();
+  final MutableNameSpace _prefixNameSpace = new MutableNameSpace();
 
   late final LookupScope _prefixScope =
       new NameSpaceLookupScope(_prefixNameSpace, ScopeKind.library);

pkg/front_end/lib/src/dill/dill_class_builder.dart

@@ -36,7 +36,7 @@ class DillClassBuilder extends ClassBuilderImpl
   @override
   final Class cls;
 
-  final DeclarationNameSpace _nameSpace;
+  final MutableDeclarationNameSpace _nameSpace;
 
   List<NominalParameterBuilder>? _typeParameters;
 
@@ -45,7 +45,7 @@ class DillClassBuilder extends ClassBuilderImpl
   List<TypeBuilder>? _interfaceBuilders;
 
   DillClassBuilder(this.cls, this.parent)
-      : _nameSpace = new DeclarationNameSpaceImpl();
+      : _nameSpace = new DillDeclarationNameSpace();
 
   @override
   int get fileOffset => cls.fileOffset;

pkg/front_end/lib/src/dill/dill_extension_builder.dart

@@ -20,13 +20,13 @@ class DillExtensionBuilder extends ExtensionBuilderImpl
   @override
   final Extension extension;
 
-  final DeclarationNameSpace _nameSpace;
+  final MutableDeclarationNameSpace _nameSpace;
 
   List<NominalParameterBuilder>? _typeParameters;
   TypeBuilder? _onType;
 
   DillExtensionBuilder(this.extension, this.libraryBuilder)
-      : _nameSpace = new DeclarationNameSpaceImpl() {
+      : _nameSpace = new DillDeclarationNameSpace() {
     for (ExtensionMemberDescriptor descriptor in extension.memberDescriptors) {
       if (descriptor.isInternalImplementation) continue;
 

pkg/front_end/lib/src/dill/dill_extension_type_declaration_builder.dart

@@ -21,7 +21,7 @@ class DillExtensionTypeDeclarationBuilder
 
   final ExtensionTypeDeclaration _extensionTypeDeclaration;
 
-  final DeclarationNameSpace _nameSpace;
+  final MutableDeclarationNameSpace _nameSpace;
 
   List<NominalParameterBuilder>? _typeParameters;
 
@@ -31,7 +31,7 @@ class DillExtensionTypeDeclarationBuilder
 
   DillExtensionTypeDeclarationBuilder(
       this._extensionTypeDeclaration, this.libraryBuilder)
-      : _nameSpace = new DeclarationNameSpaceImpl() {
+      : _nameSpace = new DillDeclarationNameSpace() {
     for (Procedure procedure in _extensionTypeDeclaration.procedures) {
       String name = procedure.name.text;
       switch (procedure.kind) {