adjacent concat strings in arg lists lost indention - Regression with 1.2.5 (SDK 2.2.1-dev.3.0)

[kevmoo]

 @ShouldThrow(
   'Could not generate `toJson` code for `watch`.\n'
-      'None of the provided `TypeHelper` instances support the defined type.',
+  'None of the provided `TypeHelper` instances support the defined type.',
   configurations: ['default'],
 )

[jacob314]

not a bug... a feature!

The new output is much better for this case.

[munificent]

It is much worse for other cases though:

function(
    "one",
    "two",
    "three",
    "four"
    "five",
    "six",
    "seven");

Spot the bug.

[jacob314]

It didn't even take me fourfive seconds to spot the bug.
That said, it could be reasonable to use the previous style if there are multiple string arguments on subsequent lines.

function(
    "one\n"
    "two\n",
    "three",
    4,
    5,
 );

seems fine.

[munificent]

Did some more digging. This is only a regression on some adjacent strings.

• In an argument list without a trailing comma, adjacent strings were not indented.
• In a collection literal, subsequent adjacent strings were always indented +4.
• In an argument list with a trailing comma, subsequent adjacent strings were always indented +4.

e0a7cd3 changed the behavior of strings in trailing comma argument lists. Instead of matching collection literals (indent), they changed to match argument lists (no indent). So, the scale of the regression isn't that big, which is good. (This also explains how it slipped through, since we don't have two copies of every single test that involves an argument list, one with and one without a trailing comma.

The argument list behavior is dubious in examples like the one I gave above. Adjacent string formatting has never been great: #188, #409, #684.

I'll see if I can do something smarter, like you suggest, @jacob314. At the very least, I don't think it makes sense to treat adjacent strings differently from ones in argument lists. I don't think that behavior was ever deliberate.

[jacob314]

As long as this behavior is maintained if the trailing comma style is used I'm happy.

[a14n]

I tried a change to always split without indent the adjacent strings:

  visitAdjacentStrings(AdjacentStrings node) {
    builder.startRule(Rule.hard());
    visitNodes(node.strings, before: splitOrNewline, between: splitOrNewline);
    builder.endRule();
  }

On flutter examples the output looks better to me.

  // current format
  var name = "the first string"
      "the second string"
      "the third string";
  
  // after my change
  var name =
      "the first string"
      "the second string"
      "the third string";

[kevmoo]

Now hitting this on VERY stable, old Dart code – https://travis-ci.org/dart-lang/csslib/jobs/515487047

[kwalrath]

@munificent when will this fix make it into the dev SDK? The issue is still happening for me in 2.3.0-dev.0.1. Our site builds on dev are failing due to dartfmt wanting to make the following changes (or should I change the source code, and how?):

diff --git a/examples/misc/lib/effective_dart/usage_bad.dart b/examples/misc/lib/effective_dart/usage_bad.dart
index 2bf3b5e..2650ffa 100644
--- a/examples/misc/lib/effective_dart/usage_bad.dart
+++ b/examples/misc/lib/effective_dart/usage_bad.dart
@@ -23,7 +23,7 @@ void miscDeclAnalyzedButNotTested() {
     return
         // #docregion string-interpolation-avoid-curly
         'Hi, ${name}!'
-        "Wear your wildest ${decade}'s outfit."
+            "Wear your wildest ${decade}'s outfit."
         // #enddocregion string-interpolation-avoid-curly
         ;
   };
diff --git a/examples/misc/lib/effective_dart/usage_good.dart b/examples/misc/lib/effective_dart/usage_good.dart
index ce026f7..08582fa 100644
--- a/examples/misc/lib/effective_dart/usage_good.dart
+++ b/examples/misc/lib/effective_dart/usage_good.dart
@@ -30,8 +30,8 @@ void miscDeclAnalyzedButNotTested() {
     return
         // #docregion string-interpolation-avoid-curly
         'Hi, $name!'
-        "Wear your wildest $decade's outfit."
-        'Wear your wildest ${decade}s outfit.'
+            "Wear your wildest $decade's outfit."
+            'Wear your wildest ${decade}s outfit.'
         // #enddocregion string-interpolation-avoid-curly
         ;
   };
diff --git a/examples/misc/test/language_tour/built_in_types_test.dart b/examples/misc/test/language_tour/built_in_types_test.dart
index 5f9fa6d..0d6d1f4 100644
--- a/examples/misc/test/language_tour/built_in_types_test.dart
+++ b/examples/misc/test/language_tour/built_in_types_test.dart
@@ -50,7 +50,7 @@ void main() {
         " works even over line breaks.";
     assert(s1 ==
         'String concatenation works even over '
-        'line breaks.');
+            'line breaks.');
 
     var s2 = 'The + operator ' + 'works, as well.';
     assert(s2 == 'The + operator works, as well.');
diff --git a/examples/misc/test/library_tour/convert_test.dart b/examples/misc/test/library_tour/convert_test.dart
index 17aaea2..29fcc3d 100644
--- a/examples/misc/test/library_tour/convert_test.dart
+++ b/examples/misc/test/library_tour/convert_test.dart
@@ -36,8 +36,8 @@ void main() {
     var jsonText = jsonEncode(scores);
     assert(jsonText ==
         '[{"score":40},{"score":80},'
-        '{"score":100,"overtime":true,'
-        '"special_guest":null}]');
+            '{"score":100,"overtime":true,'
+            '"special_guest":null}]');
     // #enddocregion json-encode
   });

[munificent]

The fix wasn't to go back exactly to the previous formatting. In your examples here, I'm not too surprised the output is different. The code is a little odd because of things like the marker comment, so it's not the kind of cases I was looking at when I tweaked the indentation rules for adjacent strings.

I'd just accept the changes it suggests, if that's OK with you.

[kwalrath]

I guess I can live with them. :) Thanks for the quick response.