Macro. Issue 55931. Add new classes first, as AugmentationLibraryBuil…

…der does for code. Bug: #55931 Change-Id: Ice846b41ad4a8b08393346ef973b8ca5410b21ba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373401 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
dart-lang · Jun 26, 2024 · 2653075 · 2653075
1 parent c64cd5f
commit 2653075
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 71 deletions.
diff --git a/pkg/analyzer/lib/src/summary2/macro_merge.dart b/pkg/analyzer/lib/src/summary2/macro_merge.dart
@@ -39,7 +39,10 @@ class MacroElementsMerger {
   void perform({
     required void Function() updateConstants,
   }) {
-    _mergeClasses();
+    // TODO(scheglov): https://github.com/dart-lang/sdk/issues/55931
+    // This is a fix for this specific issue, not a complete implementation.
+    _mergeClasses(isAugmentation: false);
+    _mergeClasses(isAugmentation: true);
     _mergeExtensions();
     _mergeFunctions();
     _mergeUnitPropertyAccessors();
@@ -48,24 +51,28 @@ class MacroElementsMerger {
     _rewriteImportPrefixes();
   }
 
-  void _mergeClasses() {
+  void _mergeClasses({
+    required bool isAugmentation,
+  }) {
     for (var partialUnit in partialUnits) {
       var elementsToAdd = <ClassElementImpl>[];
       for (var element in partialUnit.element.classes) {
-        var reference = element.reference!;
-        var containerRef = element.isAugmentation
-            ? unitReference.getChild('@classAugmentation')
-            : unitReference.getChild('@class');
-        var existingRef = containerRef[element.name];
-        if (existingRef == null) {
-          elementsToAdd.add(element);
-          containerRef.addChildReference(element.name, reference);
-        } else {
-          var existingElement = existingRef.element as ClassElementImpl;
-          if (existingElement.augmentation == element) {
-            existingElement.augmentation = null;
+        if (element.isAugmentation == isAugmentation) {
+          var reference = element.reference!;
+          var containerRef = element.isAugmentation
+              ? unitReference.getChild('@classAugmentation')
+              : unitReference.getChild('@class');
+          var existingRef = containerRef[element.name];
+          if (existingRef == null) {
+            elementsToAdd.add(element);
+            containerRef.addChildReference(element.name, reference);
+          } else {
+            var existingElement = existingRef.element as ClassElementImpl;
+            if (existingElement.augmentation == element) {
+              existingElement.augmentation = null;
+            }
+            _mergeInstanceChildren(existingRef, existingElement, element);
           }
-          _mergeInstanceChildren(existingRef, existingElement, element);
         }
       }
       unitElement.classes = [

diff --git a/pkg/analyzer/test/src/summary/macro_test.dart b/pkg/analyzer/test/src/summary/macro_test.dart
@@ -6153,62 +6153,6 @@ abstract class MacroElementsTest extends MacroElementsBaseTest {
   @override
   bool get retainDataForTesting => true;
 
-  @FailingTest(
-    issue: 'https://github.com/dart-lang/sdk/issues/55931',
-    reason: r'''
-Here we apply `SetExtendsType` first, so `augment class B` is first.
-Then we apply `DeclareType`, so `class C @113` is next.
-But the actual merged code has the different order.
-Note, that the code below has the _expected_ merged code.
-''',
-  )
-  test_addClass_extendsTypeOther() async {
-    var library = await buildLibrary(r'''
-import 'append.dart';
-
-class A {}
-
-@DeclareType('C', 'class C {}')
-@SetExtendsType('{{package:test/test.dart@A}}', [])
-class B {}
-''');
-
-    configuration
-      ..withConstructors = false
-      ..withMetadata = false;
-    checkElementText(library, r'''
-library
-  imports
-    package:test/append.dart
-  definingUnit
-    classes
-      class A @29
-      class B @125
-        augmentation: self::@augmentation::package:test/test.macro.dart::@classAugmentation::B
-        supertype: A
-        augmented
-  augmentationImports
-    package:test/test.macro.dart
-      macroGeneratedCode
----
-augment library 'package:test/test.dart';
-
-import 'package:test/test.dart' as prefix0;
-
-augment class B extends prefix0.A {
-}
-class C {}
----
-      imports
-        package:test/test.dart as prefix0 @78
-      definingUnit
-        classes
-          augment class B @94
-            augmentationTarget: self::@class::B
-          class C @113
-''');
-  }
-
   test_disable_declarationsPhase() async {
     var library = await buildLibrary(r'''
 // @dart = 3.2
@@ -8416,6 +8360,56 @@ library
 ''');
   }
 
+  test_merge_m1_setExtends1_m2_addClass2() async {
+    var library = await buildLibrary(r'''
+import 'append.dart';
+
+class A {}
+
+@DeclareType('C', 'class C {}')
+@SetExtendsType('{{package:test/test.dart@A}}', [])
+class B {}
+''');
+
+    // New `class C` is the first.
+    // Augmented `class B` next.
+    // Even if applications have different order.
+    configuration
+      ..withConstructors = false
+      ..withMetadata = false;
+    checkElementText(library, r'''
+library
+  imports
+    package:test/append.dart
+  definingUnit
+    classes
+      class A @29
+      class B @125
+        augmentation: self::@augmentation::package:test/test.macro.dart::@classAugmentation::B
+        supertype: A
+        augmented
+  augmentationImports
+    package:test/test.macro.dart
+      macroGeneratedCode
+---
+augment library 'package:test/test.dart';
+
+import 'package:test/test.dart' as prefix0;
+
+class C {}
+augment class B extends prefix0.A {
+}
+---
+      imports
+        package:test/test.dart as prefix0 @78
+      definingUnit
+        classes
+          class C @94
+          augment class B @113
+            augmentationTarget: self::@class::B
+''');
+  }
+
   test_notAllowedDeclaration_declarations_class() async {
     if (!keepLinkingLibraries) {
       return;