Macro. Fix for 'add constructor', 'augment constructor' sequence.

Change-Id: If9a5a07cefc7dcd782db636ada2dfa02d7b26e4f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/339044
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analysis_server/test/verify_no_solo_test.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/dart/analysis/analysis_context.dart';
 import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/analysis/session.dart';
@@ -9,6 +10,7 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
+import 'package:analyzer/src/utilities/extensions/file_system.dart';
 import 'package:analyzer_utilities/package_root.dart';
 import 'package:test/test.dart';
 
@@ -18,7 +20,16 @@ void main() {
   });
 
   group('analyzer', () {
-    buildTests(packagePath: 'analyzer');
+    buildTests(
+      packagePath: 'analyzer',
+      analysisContextPredicate: (analysisContext) {
+        final root = analysisContext.contextRoot.root;
+        if (root.endsWithNames(['macro', 'single'])) {
+          return false;
+        }
+        return true;
+      },
+    );
   });
 
   group('analyzer_cli', () {
@@ -30,7 +41,10 @@ void main() {
   });
 }
 
-void buildTests({required String packagePath}) {
+void buildTests({
+  required String packagePath,
+  bool Function(AnalysisContext)? analysisContextPredicate,
+}) {
   var provider = PhysicalResourceProvider.INSTANCE;
   var pkgRootPath = provider.pathContext.normalize(packageRoot);
 
@@ -40,14 +54,18 @@ void buildTests({required String packagePath}) {
     includedPaths: <String>[testsPath],
     resourceProvider: provider,
   );
-  var contexts = collection.contexts;
-  if (contexts.length != 1) {
+
+  final singleAnalysisContext = collection.contexts
+      .where(analysisContextPredicate ?? (_) => true)
+      .toList()
+      .singleOrNull;
+  if (singleAnalysisContext == null) {
     fail('The directory $testsPath contains multiple analysis contexts.');
   }
 
   test('no @soloTest', () async {
     var failures = <String>[];
-    await buildTestsIn(contexts[0].currentSession, testsPath,
+    await buildTestsIn(singleAnalysisContext.currentSession, testsPath,
         provider.getFolder(testsPath), failures);
 
     if (failures.isNotEmpty) {

pkg/analyzer/lib/src/summary2/macro_merge.dart

@@ -108,15 +108,32 @@ class MacroElementsMerger {
       }
     }
 
-    final containerRef = existingRef.getChild('@method');
-    for (final element in newElement.methods) {
-      final reference = element.reference!;
-      containerRef.addChildReference(element.name, reference);
+    {
+      final containerRef = existingRef.getChild('@method');
+      for (final element in newElement.methods) {
+        final reference = element.reference!;
+        containerRef.addChildReference(element.name, reference);
+      }
+      existingElement.methods = [
+        ...existingElement.methods,
+        ...newElement.methods,
+      ].toFixedList();
+    }
+
+    if (existingElement is InterfaceElementImpl &&
+        newElement is InterfaceElementImpl) {
+      for (final element in newElement.constructors) {
+        final reference = element.reference!;
+        final containerRef = element.isAugmentation
+            ? existingRef.getChild('@constructorAugmentation')
+            : existingRef.getChild('@constructor');
+        containerRef.addChildReference(element.name, reference);
+      }
+      existingElement.constructors = [
+        ...existingElement.constructors,
+        ...newElement.constructors,
+      ].toFixedList();
     }
-    existingElement.methods = [
-      ...existingElement.methods,
-      ...newElement.methods,
-    ].toFixedList();
 
     // TODO(scheglov): accessors, fields
   }

pkg/analyzer/lib/src/summary2/types_builder.dart

@@ -454,14 +454,15 @@ class TypesBuilder {
         toDeclaration.mapInterfaceTypes(element.interfaces),
       );
 
-      augmented.constructors.addAll(
-        element.constructors.notAugmented.map((element) {
+      augmented.constructors = [
+        ...augmented.constructors.notAugmented,
+        ...element.constructors.notAugmented.map((element) {
           if (toDeclaration.map.isEmpty) {
             return element;
           }
           return ConstructorMember(typeProvider, element, toDeclaration, false);
         }),
-      );
+      ];
     }
 
     if (element is MixinElementImpl && augmented is AugmentedMixinElementImpl) {

pkg/analyzer/lib/src/utilities/extensions/collection.dart

@@ -46,6 +46,21 @@ extension ListExtension<E> on List<E> {
     }
   }
 
+  bool endsWith(List<E> expected) {
+    var thisIndex = length - expected.length;
+    if (thisIndex < 0) {
+      return false;
+    }
+
+    var expectedIndex = 0;
+    for (; expectedIndex < expected.length;) {
+      if (this[thisIndex++] != expected[expectedIndex++]) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   E? nextOrNull(E element) {
     final index = indexOf(element);
     if (index >= 0 && index < length - 1) {

pkg/analyzer/lib/src/utilities/extensions/file_system.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
+import 'package:analyzer/src/utilities/extensions/collection.dart';
 import 'package:path/path.dart';
 
 extension FolderExtension on Folder {
@@ -43,4 +44,8 @@ extension ResourceExtension on Resource {
       return path;
     }
   }
+
+  bool endsWithNames(List<String> expected) {
+    return provider.pathContext.split(path).endsWith(expected);
+  }
 }

pkg/analyzer/test/src/summary/macro/single/README.md

@@ -0,0 +1 @@
+Each macro file in this directory is used by a test with the  same name.

pkg/analyzer/test/src/summary/macro/single/analysis_options.yaml

@@ -0,0 +1,6 @@
+include: package:lints/recommended.yaml
+
+analyzer:
+  errors:
+    # We use camel case file names in this directory.
+    file_names: ignore

pkg/analyzer/test/src/summary/macro/single/class_addConstructor_augmentConstructor.dart

@@ -0,0 +1,36 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+
+/*macro*/ class AddConstructor implements ClassDeclarationsMacro {
+  const AddConstructor();
+
+  @override
+  buildDeclarationsForClass(clazz, builder) async {
+    // ignore: deprecated_member_use
+    final identifier = await builder.resolveIdentifier(
+      Uri.parse('package:test/a.dart'),
+      'AugmentConstructor',
+    );
+    builder.declareInType(
+      DeclarationCode.fromParts([
+        '  @',
+        identifier,
+        '()\n  A.named();',
+      ]),
+    );
+  }
+}
+
+/*macro*/ class AugmentConstructor implements ConstructorDefinitionMacro {
+  const AugmentConstructor();
+
+  @override
+  buildDefinitionForConstructor(constructor, builder) {
+    builder.augment(
+      body: FunctionBodyCode.fromString('{ print(42); }'),
+    );
+  }
+}

pkg/analyzer/test/src/summary/macro_test.dart

@@ -44,6 +44,8 @@ main() {
     defineReflectiveTests(MacroTypesTest_fromBytes);
     defineReflectiveTests(MacroDeclarationsTest_keepLinking);
     defineReflectiveTests(MacroDeclarationsTest_fromBytes);
+    defineReflectiveTests(MacroDefinitionTest_keepLinking);
+    defineReflectiveTests(MacroDefinitionTest_fromBytes);
     defineReflectiveTests(MacroElementsTest_keepLinking);
     defineReflectiveTests(MacroElementsTest_fromBytes);
     defineReflectiveTests(MacroApplicationOrderTest);
@@ -2169,6 +2171,84 @@ class MacroDeclarationsTest_keepLinking extends MacroDeclarationsTest {
   bool get keepLinkingLibraries => true;
 }
 
+abstract class MacroDefinitionTest extends MacroElementsBaseTest {
+  test_class_addConstructor_augmentConstructor() async {
+    _addSingleMacro('class_addConstructor_augmentConstructor.dart');
+
+    var library = await buildLibrary(r'''
+import 'a.dart';
+
+@AddConstructor()
+class A {}
+''');
+
+    configuration
+      ..withMetadata = false
+      ..withReferences = true;
+    checkElementText(library, r'''
+library
+  reference: self
+  imports
+    package:test/a.dart
+  definingUnit
+    reference: self
+    classes
+      class A @42
+        reference: self::@class::A
+        augmentation: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A
+        augmented
+          constructors
+            self::@augmentation::package:test/test.macro.dart::@classAugmentation::A::@constructorAugmentation::named
+  augmentationImports
+    package:test/test.macro.dart
+      reference: self::@augmentation::package:test/test.macro.dart
+      macroGeneratedCode
+---
+library augment 'test.dart';
+
+import 'package:test/a.dart' as prefix0;
+
+augment class A {
+  @prefix0.AugmentConstructor()
+  A.named();
+  augment A.named() { print(42); }
+}
+---
+      imports
+        package:test/a.dart as prefix0 @62
+      definingUnit
+        reference: self::@augmentation::package:test/test.macro.dart
+        classes
+          augment class A @86
+            reference: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A
+            augmentationTarget: self::@class::A
+            constructors
+              named @126
+                reference: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A::@constructor::named
+                periodOffset: 125
+                nameEnd: 131
+                augmentation: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A::@constructorAugmentation::named
+              augment named @147
+                reference: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A::@constructorAugmentation::named
+                periodOffset: 146
+                nameEnd: 152
+                augmentationTarget: self::@augmentation::package:test/test.macro.dart::@classAugmentation::A::@constructor::named
+''');
+  }
+}
+
+@reflectiveTest
+class MacroDefinitionTest_fromBytes extends MacroDefinitionTest {
+  @override
+  bool get keepLinkingLibraries => false;
+}
+
+@reflectiveTest
+class MacroDefinitionTest_keepLinking extends MacroDefinitionTest {
+  @override
+  bool get keepLinkingLibraries => true;
+}
+
 abstract class MacroElementsBaseTest extends ElementsBaseTest {
   @override
   Future<void> setUp() async {
@@ -2185,6 +2265,12 @@ abstract class MacroElementsBaseTest extends ElementsBaseTest {
     );
   }
 
+  /// Adds `a.dart` with the content from `single/` directory.
+  void _addSingleMacro(String fileName) {
+    final code = _getMacroCode('single/$fileName');
+    newFile('$testPackageLibPath/a.dart', code);
+  }
+
   /// Verifies the code of the macro generated augmentation.
   void _assertMacroCode(LibraryElementImpl library, String expected) {
     final actual = library.augmentations.single.macroGenerated!.code;

pkg/analyzer/test/src/utilities/extensions/collection_test.dart

@@ -50,6 +50,22 @@ class ListExtensionTest {
     expect([0, 1].elementAtOrNull2(2), isNull);
   }
 
+  test_endsWith() {
+    expect([0, 1, 2].endsWith([]), isTrue);
+
+    expect([0, 1, 2].endsWith([2]), isTrue);
+    expect([0, 1, 2].endsWith([1]), isFalse);
+    expect([0, 1, 2].endsWith([0]), isFalse);
+
+    expect([0, 1, 2].endsWith([1, 2]), isTrue);
+    expect([0, 1, 2].endsWith([0, 2]), isFalse);
+
+    expect([0, 1, 2].endsWith([0, 1, 2]), isTrue);
+    expect([0, 1, 2].endsWith([0, 0, 2]), isFalse);
+
+    expect([0, 1, 2].endsWith([-1, 0, 1, 2]), isFalse);
+  }
+
   test_nextOrNull() {
     var elements = [0, 1, 2];
     expect(elements.nextOrNull(0), 1);

pkg/analyzer/test/verify_tests_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
+import 'package:analyzer/src/utilities/extensions/file_system.dart';
 import 'package:analyzer_utilities/package_root.dart' as package_root;
 import 'package:analyzer_utilities/verify_tests.dart';
 import 'package:path/path.dart' as path;
@@ -13,7 +14,15 @@ main() {
   var packageRoot = provider.pathContext.normalize(package_root.packageRoot);
   var pathToAnalyze = provider.pathContext.join(packageRoot, 'analyzer');
   var testDirPath = provider.pathContext.join(pathToAnalyze, 'test');
-  _VerifyTests(testDirPath).build();
+  _VerifyTests(testDirPath).build(
+    analysisContextPredicate: (analysisContext) {
+      final root = analysisContext.contextRoot.root;
+      if (root.endsWithNames(['macro', 'single'])) {
+        return false;
+      }
+      return true;
+    },
+  );
 }
 
 class _VerifyTests extends VerifyTests {

pkg/analyzer_utilities/lib/verify_tests.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/dart/analysis/analysis_context.dart';
 import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/analysis/session.dart';
@@ -23,18 +24,23 @@ class VerifyTests {
   VerifyTests(this.testDirPath, {this.excludedPaths});
 
   /// Build tests.
-  void build() {
+  void build({
+    bool Function(AnalysisContext)? analysisContextPredicate,
+  }) {
     var provider = PhysicalResourceProvider.INSTANCE;
     var collection = AnalysisContextCollection(
         resourceProvider: provider,
         includedPaths: <String>[testDirPath],
         excludedPaths: excludedPaths);
-    var contexts = collection.contexts;
-    if (contexts.length != 1) {
+    final singleAnalysisContext = collection.contexts
+        .where(analysisContextPredicate ?? (_) => true)
+        .toList()
+        .singleOrNull;
+    if (singleAnalysisContext == null) {
       fail('The test directory contains multiple analysis contexts.');
     }
 
-    _buildTestsIn(contexts[0].currentSession, testDirPath,
+    _buildTestsIn(singleAnalysisContext.currentSession, testDirPath,
         provider.getFolder(testDirPath));
   }