From 89577e750840c3655e5703004fb774d6564e02b9 Mon Sep 17 00:00:00 2001
From: Bob Nystrom <rnystrom@google.com>
Date: Tue, 15 Oct 2024 15:35:22 -0700
Subject: [PATCH] Allow a factory constructor with metadata before it to split.
 (#1580)

Prior to this PR, if there was metadata before a constructor, it would
be attached to the "header" piece of the constructor -- the leading
keywords and name -- instead of the entire constructor piece.

That in turn meant that the newline after the metadata would lead the
formatter to think a split had occurred inside the header itself,
instead of before the entire constructor. That would in turn confuse
the solver and it ended up trying to disallow splitting anywhere in the
constructor, even if that led to an overflowing line.

This fixes that so that the metadata is attached to the entire
constructor piece, not just the nested header part.
---
 lib/src/front_end/ast_node_visitor.dart | 66 +++++++++++++------------
 test/tall/regression/other/dart.unit    | 11 +++++
 2 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/lib/src/front_end/ast_node_visitor.dart b/lib/src/front_end/ast_node_visitor.dart
index c7e16893..78701316 100644
--- a/lib/src/front_end/ast_node_visitor.dart
+++ b/lib/src/front_end/ast_node_visitor.dart
@@ -407,43 +407,45 @@ final class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
 
   @override
   void visitConstructorDeclaration(ConstructorDeclaration node) {
-    var header = pieces.build(metadata: node.metadata, () {
-      pieces.modifier(node.externalKeyword);
-      pieces.modifier(node.constKeyword);
-      pieces.modifier(node.factoryKeyword);
-      pieces.visit(node.returnType);
-      pieces.token(node.period);
-      pieces.token(node.name);
-    });
+    pieces.withMetadata(node.metadata, () {
+      var header = pieces.build(() {
+        pieces.modifier(node.externalKeyword);
+        pieces.modifier(node.constKeyword);
+        pieces.modifier(node.factoryKeyword);
+        pieces.visit(node.returnType);
+        pieces.token(node.period);
+        pieces.token(node.name);
+      });
 
-    var parameters = nodePiece(node.parameters);
+      var parameters = nodePiece(node.parameters);
 
-    Piece? redirect;
-    Piece? initializerSeparator;
-    Piece? initializers;
-    if (node.redirectedConstructor case var constructor?) {
-      var separator = pieces.build(() {
-        pieces.token(node.separator);
-        pieces.space();
-      });
+      Piece? redirect;
+      Piece? initializerSeparator;
+      Piece? initializers;
+      if (node.redirectedConstructor case var constructor?) {
+        var separator = pieces.build(() {
+          pieces.token(node.separator);
+          pieces.space();
+        });
 
-      redirect = AssignPiece(
-          separator, nodePiece(constructor, context: NodeContext.assignment),
-          canBlockSplitRight: false);
-    } else if (node.initializers.isNotEmpty) {
-      initializerSeparator = tokenPiece(node.separator!);
-      initializers = createCommaSeparated(node.initializers);
-    }
+        redirect = AssignPiece(
+            separator, nodePiece(constructor, context: NodeContext.assignment),
+            canBlockSplitRight: false);
+      } else if (node.initializers.isNotEmpty) {
+        initializerSeparator = tokenPiece(node.separator!);
+        initializers = createCommaSeparated(node.initializers);
+      }
 
-    var body = nodePiece(node.body);
+      var body = nodePiece(node.body);
 
-    pieces.add(ConstructorPiece(header, parameters, body,
-        canSplitParameters: node.parameters.parameters
-            .canSplit(node.parameters.rightParenthesis),
-        hasOptionalParameter: node.parameters.rightDelimiter != null,
-        redirect: redirect,
-        initializerSeparator: initializerSeparator,
-        initializers: initializers));
+      pieces.add(ConstructorPiece(header, parameters, body,
+          canSplitParameters: node.parameters.parameters
+              .canSplit(node.parameters.rightParenthesis),
+          hasOptionalParameter: node.parameters.rightDelimiter != null,
+          redirect: redirect,
+          initializerSeparator: initializerSeparator,
+          initializers: initializers));
+    });
   }
 
   @override
diff --git a/test/tall/regression/other/dart.unit b/test/tall/regression/other/dart.unit
index f0c6c8df..47107b26 100644
--- a/test/tall/regression/other/dart.unit
+++ b/test/tall/regression/other/dart.unit
@@ -54,4 +54,15 @@ class Benchmark {
   // A rate of one run per 2s, with a millisecond of noise.  Some variation is
   // needed for Golem's noise-based filtering and regression detection.
   double measure() => (2000 + Random().nextDouble() - 0.5) * 1000;
+}
+>>>
+class Uint8ClampedList {
+  @patch
+  factory Uint8ClampedList.fromList(List<int> elements) = NativeUint8ClampedList.fromList;
+}
+<<<
+class Uint8ClampedList {
+  @patch
+  factory Uint8ClampedList.fromList(List<int> elements) =
+      NativeUint8ClampedList.fromList;
 }
\ No newline at end of file