Make NodeListImpl not resizable, update AstBuilder.

Change-Id: I1930b5fe863788198b5d88c0a08828980d935359 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259103 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
2022-09-14 20:12:12 +00:00
parent 88371e4866
commit dc0dbb01f8
5 changed files with 245 additions and 260 deletions
@@ -3768,6 +3768,10 @@ abstract class NodeList<E extends AstNode> implements List<E> {
  /// if the list is empty.
  Token? get endToken;

+  @Deprecated('NodeList cannot be resized')
+  @override
+  set length(int newLength);
+
  /// Return the node that is the parent of each of the elements in the list.
  AstNode get owner;

@@ -3778,6 +3782,26 @@ abstract class NodeList<E extends AstNode> implements List<E> {

  /// Use the given [visitor] to visit each of the nodes in this list.
  void accept(AstVisitor visitor);
+
+  @Deprecated('NodeList cannot be resized')
+  @override
+  void add(E element);
+
+  @Deprecated('NodeList cannot be resized')
+  @override
+  void addAll(Iterable<E> iterable);
+
+  @Deprecated('NodeList cannot be resized')
+  @override
+  void clear();
+
+  @Deprecated('NodeList cannot be resized')
+  @override
+  void insert(int index, E element);
+
+  @Deprecated('NodeList cannot be resized')
+  @override
+  E removeAt(int index);
 }

 /// A formal parameter that is required (is not optional).
@@ -2362,7 +2362,7 @@ class CompilationUnitImpl extends AstNodeImpl implements CompilationUnit {
  int get offset => 0;

  @override
-  ScriptTag? get scriptTag => _scriptTag;
+  ScriptTagImpl? get scriptTag => _scriptTag;

  set scriptTag(ScriptTag? scriptTag) {
    _scriptTag = _becomeParentOf(scriptTag as ScriptTagImpl?);
@@ -8933,7 +8933,7 @@ class NodeListImpl<E extends AstNode> with ListMixin<E> implements NodeList<E> {
  late final AstNodeImpl _owner;

  /// The elements contained in the list.
-  List<E> _elements = <E>[];
+  late final List<E> _elements;

  /// Initialize a newly created list of nodes such that all of the nodes that
  /// are added to the list will have their parent set to the given [owner].
@@ -8962,6 +8962,7 @@ class NodeListImpl<E extends AstNode> with ListMixin<E> implements NodeList<E> {
  @override
  int get length => _elements.length;

+  @Deprecated('NodeList cannot be resized')
  @override
  set length(int newLength) {
    throw UnsupportedError("Cannot resize NodeList.");
@@ -8995,46 +8996,46 @@ class NodeListImpl<E extends AstNode> with ListMixin<E> implements NodeList<E> {
    }
  }

+  @Deprecated('NodeList cannot be resized')
  @override
  void add(E element) {
-    insert(length, element);
+    throw UnsupportedError("Cannot resize NodeList.");
  }

+  @Deprecated('NodeList cannot be resized')
  @override
  void addAll(Iterable<E> iterable) {
-    for (E node in iterable) {
-      _elements.add(node);
-      _owner._becomeParentOf(node as AstNodeImpl);
-    }
+    throw UnsupportedError("Cannot resize NodeList.");
  }

+  @Deprecated('NodeList cannot be resized')
  @override
  void clear() {
-    _elements = <E>[];
+    throw UnsupportedError("Cannot resize NodeList.");
  }

+  @Deprecated('NodeList cannot be resized')
  @override
  void insert(int index, E element) {
-    _elements.insert(index, element);
-    _owner._becomeParentOf(element as AstNodeImpl);
+    throw UnsupportedError("Cannot resize NodeList.");
  }

+  @Deprecated('NodeList cannot be resized')
  @override
  E removeAt(int index) {
-    if (index < 0 || index >= _elements.length) {
-      throw RangeError("Index: $index, Size: ${_elements.length}");
-    }
-    return _elements.removeAt(index);
+    throw UnsupportedError("Cannot resize NodeList.");
  }

  /// Set the [owner] of this container, and populate it with [elements].
  void _initialize(AstNodeImpl owner, List<E>? elements) {
    _owner = owner;
-    if (elements != null) {
+    if (elements == null || elements.isEmpty) {
+      _elements = const <Never>[];
+    } else {
+      _elements = elements.toList(growable: false);
      var length = elements.length;
      for (var i = 0; i < length; i++) {
        var node = elements[i];
-        _elements.add(node);
        owner._becomeParentOf(node as AstNodeImpl);
      }
    }
@@ -9100,11 +9101,6 @@ abstract class NormalFormalParameterImpl extends FormalParameterImpl
  @override
  NodeListImpl<Annotation> get metadata => _metadata;

-  set metadata(List<Annotation> metadata) {
-    _metadata.clear();
-    _metadata.addAll(metadata);
-  }
-
  @override
  Token? get name => _identifier?.token;

@@ -838,10 +838,17 @@ class AstBuilder extends StackListener {
    debugEvent("Cascade");

    var expression = pop() as Expression;
-    var receiver = pop() as CascadeExpression;
+    var cascade = pop() as CascadeExpressionImpl;
    pop(); // Token.
-    receiver.cascadeSections.add(expression);
-    push(receiver);
+    push(
+      CascadeExpressionImpl(
+        target: cascade.target,
+        cascadeSections: <Expression>[
+          ...cascade.cascadeSections,
+          expression,
+        ],
+      ),
+    );
  }

  @override
@@ -2656,18 +2663,49 @@ class AstBuilder extends StackListener {
    var statements = popTypedList2<Statement>(statementCount);
    List<SwitchMember?> members;

+    List<LabelImpl> popLabels() {
+      final labels = <LabelImpl>[];
+      while (peek() is LabelImpl) {
+        labels.insert(0, pop() as LabelImpl);
+        --labelCount;
+      }
+      return labels;
+    }
+
+    SwitchMemberImpl updateSwitchMember({
+      required SwitchMember member,
+      List<Label>? labels,
+      List<Statement>? statements,
+    }) {
+      if (member is SwitchCaseImpl) {
+        return SwitchCaseImpl(
+          labels ?? member.labels,
+          member.keyword,
+          member.expression,
+          member.colon,
+          statements ?? member.statements,
+        );
+      } else if (member is SwitchDefaultImpl) {
+        return SwitchDefaultImpl(
+          labels ?? member.labels,
+          member.keyword,
+          member.colon,
+          statements ?? member.statements,
+        );
+      } else {
+        throw UnimplementedError('(${member.runtimeType}) $member');
+      }
+    }
+
    if (labelCount == 0 && defaultKeyword == null) {
      // Common situation: case with no default and no labels.
      members = popTypedList2<SwitchMember>(expressionCount);
    } else {
      // Labels and case statements may be intertwined
      if (defaultKeyword != null) {
-        SwitchDefault member = ast.switchDefault(
-            <Label>[], defaultKeyword, colonAfterDefault!, <Statement>[]);
-        while (peek() is Label) {
-          member.labels.insert(0, pop() as Label);
-          --labelCount;
-        }
+        final labels = popLabels();
+        final member = ast.switchDefault(
+            labels, defaultKeyword, colonAfterDefault!, <Statement>[]);
        members = List<SwitchMember?>.filled(expressionCount + 1, null);
        members[expressionCount] = member;
      } else {
@@ -2675,17 +2713,23 @@ class AstBuilder extends StackListener {
      }
      for (int index = expressionCount - 1; index >= 0; --index) {
        var member = pop() as SwitchMember;
-        while (peek() is Label) {
-          member.labels.insert(0, pop() as Label);
-          --labelCount;
-        }
-        members[index] = member;
+        final labels = popLabels();
+        members[index] = updateSwitchMember(
+          member: member,
+          labels: labels,
+          statements: null,
+        );
      }
      assert(labelCount == 0);
    }
+
    var members2 = members.whereNotNull().toList();
    if (members2.isNotEmpty) {
-      members2.last.statements.addAll(statements);
+      members2.last = updateSwitchMember(
+        member: members2.last,
+        labels: null,
+        statements: statements,
+      );
    }
    push(members2);
  }
@@ -4320,18 +4364,31 @@ class AstBuilder extends StackListener {
      }
    }
    if (withClause != null) {
-      if (declaration.withClause == null) {
+      final existingClause = declaration.withClause;
+      if (existingClause == null) {
        declaration.withClause = withClause;
      } else {
-        declaration.withClause!.mixinTypes.addAll(withClause.mixinTypes);
+        declaration.withClause = WithClauseImpl(
+          existingClause.withKeyword,
+          [
+            ...existingClause.mixinTypes,
+            ...withClause.mixinTypes,
+          ],
+        );
      }
    }
    if (implementsClause != null) {
-      if (declaration.implementsClause == null) {
+      final existingClause = declaration.implementsClause;
+      if (existingClause == null) {
        declaration.implementsClause = implementsClause;
      } else {
-        declaration.implementsClause!.interfaces
-            .addAll(implementsClause.interfaces);
+        declaration.implementsClause = ImplementsClauseImpl(
+          implementsKeyword: existingClause.implementsKeyword,
+          interfaces: [
+            ...existingClause.interfaces,
+            ...implementsClause.interfaces,
+          ],
+        );
      }
    }
  }
@@ -4344,24 +4401,37 @@ class AstBuilder extends StackListener {
    var combinators = pop() as List<Combinator>?;
    var deferredKeyword = pop(NullValue.Deferred) as Token?;
    var asKeyword = pop(NullValue.As) as Token?;
-    var prefix = pop(NullValue.Prefix) as SimpleIdentifier?;
+    var prefix = pop(NullValue.Prefix) as SimpleIdentifierImpl?;
    var configurations = pop() as List<Configuration>?;

-    var directive = directives.last as ImportDirectiveImpl;
-    if (combinators != null) {
-      directive.combinators.addAll(combinators);
-    }
-    directive.deferredKeyword ??= deferredKeyword;
+    final directive = directives.last as ImportDirectiveImpl;
+
+    // TODO(scheglov) This code would be easier if we used one object.
+    var mergedAsKeyword = directive.asKeyword;
+    var mergedPrefix = directive.prefix;
    if (directive.asKeyword == null && asKeyword != null) {
-      directive.asKeyword = asKeyword;
-      directive.prefix = prefix;
-    }
-    if (configurations != null) {
-      directive.configurations.addAll(configurations);
-    }
-    if (semicolon != null) {
-      directive.semicolon = semicolon;
+      mergedAsKeyword = asKeyword;
+      mergedPrefix = prefix;
    }
+
+    directives.last = ImportDirectiveImpl(
+      comment: directive.documentationComment,
+      metadata: directive.metadata,
+      importKeyword: directive.importKeyword,
+      uri: directive.uri,
+      configurations: [
+        ...directive.configurations,
+        ...?configurations,
+      ],
+      deferredKeyword: directive.deferredKeyword ?? deferredKeyword,
+      asKeyword: mergedAsKeyword,
+      prefix: mergedPrefix,
+      combinators: [
+        ...directive.combinators,
+        ...?combinators,
+      ],
+      semicolon: semicolon ?? directive.semicolon,
+    );
  }

  @override
@@ -4372,19 +4442,31 @@ class AstBuilder extends StackListener {
    var onClause = pop(NullValue.IdentifierList) as OnClauseImpl?;

    if (onClause != null) {
-      if (builder.onClause == null) {
+      final existingClause = builder.onClause;
+      if (existingClause == null) {
        builder.onClause = onClause;
      } else {
-        builder.onClause!.superclassConstraints
-            .addAll(onClause.superclassConstraints);
+        builder.onClause = OnClauseImpl(
+          existingClause.onKeyword,
+          [
+            ...existingClause.superclassConstraints,
+            ...onClause.superclassConstraints,
+          ],
+        );
      }
    }
    if (implementsClause != null) {
-      if (builder.implementsClause == null) {
+      final existingClause = builder.implementsClause;
+      if (existingClause == null) {
        builder.implementsClause = implementsClause;
      } else {
-        builder.implementsClause!.interfaces
-            .addAll(implementsClause.interfaces);
+        builder.implementsClause = ImplementsClauseImpl(
+          implementsKeyword: implementsClause.implementsKeyword,
+          interfaces: [
+            ...existingClause.interfaces,
+            ...implementsClause.interfaces,
+          ],
+        );
      }
    }
  }
@@ -12,7 +12,6 @@ import 'package:analyzer/src/dart/ast/utilities.dart';
 import 'package:analyzer/src/dart/error/syntactic_errors.dart';
 import 'package:analyzer/src/generated/testing/ast_test_factory.dart';
 import 'package:analyzer/src/generated/testing/token_factory.dart';
-import 'package:analyzer/src/summary2/ast_binary_tokens.dart';
 import 'package:analyzer/src/test_utilities/find_node.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -1002,204 +1001,87 @@ void f() {

@reflectiveTest
 class NodeListTest extends ParserDiagnosticsTest {
-  void test_add() {
-    AstNode parent = AstTestFactory.argumentList();
-    AstNode firstNode = true_();
-    AstNode secondNode = false_();
-    NodeList<AstNode> list = astFactory.nodeList<AstNode>(parent);
-    list.insert(0, secondNode);
-    list.insert(0, firstNode);
-    expect(list, hasLength(2));
-    expect(list[0], same(firstNode));
-    expect(list[1], same(secondNode));
-    expect(firstNode.parent, same(parent));
-    expect(secondNode.parent, same(parent));
-    AstNode thirdNode = false_();
-    list.insert(1, thirdNode);
-    expect(list, hasLength(3));
-    expect(list[0], same(firstNode));
-    expect(list[1], same(thirdNode));
-    expect(list[2], same(secondNode));
-    expect(firstNode.parent, same(parent));
-    expect(secondNode.parent, same(parent));
-    expect(thirdNode.parent, same(parent));
-  }
-
-  void test_add_negative() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list.insert(-1, true_());
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
-  }
-
-  void test_add_tooBig() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list.insert(1, true_());
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
-  }
-
-  void test_addAll() {
-    AstNode parent = AstTestFactory.argumentList();
-    List<AstNode> firstNodes = <AstNode>[];
-    AstNode firstNode = true_();
-    AstNode secondNode = false_();
-    firstNodes.add(firstNode);
-    firstNodes.add(secondNode);
-    NodeList<AstNode> list = astFactory.nodeList<AstNode>(parent);
-    list.addAll(firstNodes);
-    expect(list, hasLength(2));
-    expect(list[0], same(firstNode));
-    expect(list[1], same(secondNode));
-    expect(firstNode.parent, same(parent));
-    expect(secondNode.parent, same(parent));
-    List<AstNode> secondNodes = <AstNode>[];
-    AstNode thirdNode = true_();
-    AstNode fourthNode = false_();
-    secondNodes.add(thirdNode);
-    secondNodes.add(fourthNode);
-    list.addAll(secondNodes);
-    expect(list, hasLength(4));
-    expect(list[0], same(firstNode));
-    expect(list[1], same(secondNode));
-    expect(list[2], same(thirdNode));
-    expect(list[3], same(fourthNode));
-    expect(firstNode.parent, same(parent));
-    expect(secondNode.parent, same(parent));
-    expect(thirdNode.parent, same(parent));
-    expect(fourthNode.parent, same(parent));
-  }
-
-  void test_creation() {
-    AstNode owner = AstTestFactory.argumentList();
-    NodeList<AstNode> list = astFactory.nodeList<AstNode>(owner);
-    expect(list, isNotNull);
-    expect(list, hasLength(0));
-    expect(list.owner, same(owner));
-  }
-
-  void test_get_negative() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list[-1];
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
-  }
-
-  void test_get_tooBig() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list[1];
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
-  }
-
  void test_getBeginToken_empty() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    expect(list.beginToken, isNull);
+    final parseResult = parseStringWithErrors(r'''
+final x = f();
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('()');
+    final nodeList = argumentList.arguments;
+    expect(nodeList.beginToken, isNull);
  }

  void test_getBeginToken_nonEmpty() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    AstNode node = AstTestFactory.parenthesizedExpression(true_());
-    list.add(node);
-    expect(list.beginToken, same(node.beginToken));
+    final parseResult = parseStringWithErrors(r'''
+final x = f(0, 1);
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('(0');
+    final nodeList = argumentList.arguments;
+    final first = nodeList[0];
+    expect(nodeList.beginToken, same(first.beginToken));
  }

  void test_getEndToken_empty() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    expect(list.endToken, isNull);
+    final parseResult = parseStringWithErrors(r'''
+final x = f();
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('()');
+    final nodeList = argumentList.arguments;
+    expect(nodeList.endToken, isNull);
  }

  void test_getEndToken_nonEmpty() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    AstNode node = AstTestFactory.parenthesizedExpression(true_());
-    list.add(node);
-    expect(list.endToken, same(node.endToken));
+    final parseResult = parseStringWithErrors(r'''
+final x = f(0, 1);
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('(0');
+    final nodeList = argumentList.arguments;
+    final last = nodeList[nodeList.length - 1];
+    expect(nodeList.endToken, same(last.endToken));
  }

  void test_indexOf() {
-    List<AstNode> nodes = <AstNode>[];
-    AstNode firstNode = true_();
-    AstNode secondNode = false_();
-    AstNode thirdNode = true_();
-    AstNode fourthNode = false_();
-    nodes.add(firstNode);
-    nodes.add(secondNode);
-    nodes.add(thirdNode);
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    list.addAll(nodes);
-    expect(list, hasLength(3));
-    expect(list.indexOf(firstNode), 0);
-    expect(list.indexOf(secondNode), 1);
-    expect(list.indexOf(thirdNode), 2);
-    expect(list.indexOf(fourthNode), -1);
-  }
+    final parseResult = parseStringWithErrors(r'''
+final x = f(0, 1, 2);
+final y = 42;
+''');
+    parseResult.assertNoErrors();

-  void test_remove() {
-    List<AstNode> nodes = <AstNode>[];
-    AstNode firstNode = true_();
-    AstNode secondNode = false_();
-    AstNode thirdNode = true_();
-    nodes.add(firstNode);
-    nodes.add(secondNode);
-    nodes.add(thirdNode);
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    list.addAll(nodes);
-    expect(list, hasLength(3));
-    expect(list.removeAt(1), same(secondNode));
-    expect(list, hasLength(2));
-    expect(list[0], same(firstNode));
-    expect(list[1], same(thirdNode));
-  }
+    final argumentList = parseResult.findNode.argumentList('(0');
+    final nodeList = argumentList.arguments;

-  void test_remove_negative() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list.removeAt(-1);
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
-  }
+    final first = nodeList[0];
+    final second = nodeList[1];
+    final third = nodeList[2];

-  void test_remove_tooBig() {
-    NodeList<AstNode> list =
-        astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
-    try {
-      list.removeAt(1);
-      fail("Expected IndexOutOfBoundsException");
-    } on RangeError {
-      // Expected
-    }
+    expect(nodeList, hasLength(3));
+    expect(nodeList.indexOf(first), 0);
+    expect(nodeList.indexOf(second), 1);
+    expect(nodeList.indexOf(third), 2);
+
+    final notInList = parseResult.findNode.integerLiteral('42');
+    expect(nodeList.indexOf(notInList), -1);
  }

  void test_set_negative() {
-    AstNode node = true_();
-    var list = astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
+    final parseResult = parseStringWithErrors(r'''
+final x = f(0);
+final y = 42;
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('(0');
+    final nodeList = argumentList.arguments;
+
    try {
-      list[-1] = node;
+      nodeList[-1] = nodeList.first;
      fail("Expected IndexOutOfBoundsException");
    } on RangeError {
      // Expected
@@ -1207,29 +1089,21 @@ class NodeListTest extends ParserDiagnosticsTest {
  }

  void test_set_tooBig() {
-    AstNode node = true_();
-    var list = astFactory.nodeList<AstNode>(AstTestFactory.argumentList());
+    final parseResult = parseStringWithErrors(r'''
+final x = f(0);
+final y = 42;
+''');
+    parseResult.assertNoErrors();
+
+    final argumentList = parseResult.findNode.argumentList('(0');
+    final nodeList = argumentList.arguments;
    try {
-      list[1] = node;
+      nodeList[1] = nodeList.first;
      fail("Expected IndexOutOfBoundsException");
    } on RangeError {
      // Expected
    }
  }
-
-  static BooleanLiteralImpl false_() {
-    return BooleanLiteralImpl(
-      literal: Tokens.true_(),
-      value: true,
-    );
-  }
-
-  static BooleanLiteralImpl true_() {
-    return BooleanLiteralImpl(
-      literal: Tokens.true_(),
-      value: true,
-    );
-  }
 }

@reflectiveTest
@@ -199,9 +199,18 @@ part 'foo.dart';
  }

  CompilationUnitImpl _moveFirstDirectiveToEnd(CompilationUnitImpl unit) {
-    unit.directives.add(unit.directives.removeAt(0));
-    unit.beginToken = unit.directives[0].beginToken;
-    return unit;
+    return CompilationUnitImpl(
+      unit.directives.skip(1).first.beginToken,
+      unit.scriptTag,
+      [
+        ...unit.directives.skip(1),
+        unit.directives.first,
+      ],
+      unit.declarations,
+      unit.endToken,
+      unit.featureSet,
+      unit.lineInfo,
+    );
  }

  CompilationUnitImpl _updateBeginToken(CompilationUnitImpl unit) {