Skip to content

Commit c7f6131

Browse files
authored
Don't invalidate solutions from pinned pieces. (#1706)
If a string interpolation contains a splittable expression which in turn contains an expression with a mandatory newline (like a multiline string or `;`), then the solver gets confused. The splittable expression is already pinned to state 0 to prevent there from being newlines in the string interpolation. But now the newline inside the subexpression tries to force the splittable expression to split. That leads the solver to invalidate the solution and then try to find something better. Ultimately, it ends up unnecessarily splitting some piece surrounding the interpolated string. (In the examples I've seen in the wild, it's always a function's argument list.) To avoid that, if a piece is already pinned, then we ignore invalidation from it. This fixes the few rare cases I've seen in a large corpus and doesn't impact anything else. (Also, this fixes an edge case where the formatter in Dart 3.7 behaves differently from the current formatter. The 3.7 formatter doesn't have this bug. Something about the new shape-based solver seems to tickle it. I'm not sure why.)
1 parent eaa3f2d commit c7f6131

File tree

2 files changed

+33
-1
lines changed

2 files changed

+33
-1
lines changed

lib/src/back_end/solution.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,16 @@ final class Solution implements Comparable<Solution> {
193193
///
194194
/// This should only be called by [CodeWriter].
195195
void invalidate(Piece piece) {
196+
// Don't invalidate if the piece is pinned and can't do anything. This only
197+
// comes into play when a mandatory newline inside a string interpolation
198+
// causes some surrounding interpolation expression to try to split but the
199+
// expression is aready pinned to be unsplit to prevent newlines inside
200+
// interpolation.
201+
// TODO(rnystrom): If we come up with a cleaner way to model how newlines
202+
// inside interpolation are handled, we can get rid of this. See the TODO
203+
// comment in visitInterpolationString().
204+
if (piece.pinnedState != null) return;
205+
196206
_isValid = false;
197207

198208
// TODO(rnystrom): It would be good to mark a solution as a dead end here

test/tall/expression/interpolation.stmt

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,26 @@ a + // comment
6868
b} after";
6969
<<<
7070
"before ${a + // comment
71-
b} after";
71+
b} after";
72+
>>> Multiline string inside splittable expression inside interpolation.
73+
var x = f('''
74+
${operand + """
75+
nested
76+
"""}
77+
''');
78+
<<<
79+
var x = f('''
80+
${operand + """
81+
nested
82+
"""}
83+
''');
84+
>>> Block function inside splittable expression inside interpolation.
85+
var x = f('''
86+
${operand + (){;}}
87+
''');
88+
<<<
89+
var x = f('''
90+
${operand + () {
91+
;
92+
}}
93+
''');

0 commit comments

Comments
 (0)