Fix #5599, long yaml keys trigger explicit mapping (#10874)

Fix #5599, long yaml keys trigger explicit mapping

changelog_unreleased/yaml/10874.md

@@ -0,0 +1,66 @@
+#### Don't switch to explicit mappings needlessly (#10874 by @pdavies)
+
+Previously, Prettier printed long YAML keys using explicit mapping syntax. The intent was to use that syntax to accommodate multiline keys, but it had the unintended effect of using it even for long keys that couldn’t be made multiline – for example, because `--prose-wrap` was set to `preserve` or the key didn’t contain whitespace. This is bad because:
+
+- Doing so has the effect of further increasing the line length.
+- Explicit mappings are obscure and most people have never seen them. It causes confusion to introduce them needlessly.
+
+<!-- prettier-ignore -->
+```yaml
+# Input
+- stage: Process
+  jobs:
+    - template: Process.yml
+      parameters:
+        ${{ if in(parameters.BuildType, 'a') }}:
+          BuildArtifacts:
+            - input: Build.Release.x86
+              output: Processed.Release.x86
+        ${{ if in(parameters.BuildType, 'b                                 ') }}:
+          BuildArtifacts:
+            - input: Build.Release
+              output: Processed.Release
+
+# Prettier stable
+- stage: Process
+  jobs:
+    - template: Process.yml
+      parameters:
+        ${{ if in(parameters.BuildType, 'a') }}:
+          BuildArtifacts:
+            - input: Build.Release.x86
+              output: Processed.Release.x86
+        ? ${{ if in(parameters.BuildType, 'b                                 ') }}
+        : BuildArtifacts:
+            - input: Build.Release
+              output: Processed.Release
+
+# Prettier main
+- stage: Process
+  jobs:
+    - template: Process.yml
+      parameters:
+        ${{ if in(parameters.BuildType, 'a') }}:
+          BuildArtifacts:
+            - input: Build.Release.x86
+              output: Processed.Release.x86
+        ${{ if in(parameters.BuildType, 'b                                 ') }}:
+          BuildArtifacts:
+            - input: Build.Release
+              output: Processed.Release
+
+# Prettier main (with --prose-wrap always)
+- stage: Process
+  jobs:
+    - template: Process.yml
+      parameters:
+        ${{ if in(parameters.BuildType, 'a') }}:
+          BuildArtifacts:
+            - input: Build.Release.x86
+              output: Processed.Release.x86
+        ? ${{ if in(parameters.BuildType, 'b                                 ')
+          }}
+        : BuildArtifacts:
+            - input: Build.Release
+              output: Processed.Release
+```

src/language-yaml/print/mapping-item.js

@@ -88,9 +88,17 @@ function printMappingItem(node, parentNode, path, print, options) {
     ifBreak("? "),
     group(alignWithSpaces(2, printedKey), { id: groupId }),
   ]);
-  const breakValue = [hardline, ": ", alignWithSpaces(2, printedValue)];
+
+  // Construct both explicit and implicit mapping values.
+  const explicitMappingValue = [
+    hardline,
+    ": ",
+    alignWithSpaces(2, printedValue),
+  ];
   /** @type {Doc[]} */
-  const flatValueParts = [spaceBeforeColon, ":"];
+  // In the implicit case, it's convenient to treat everything from the key's colon
+  // as part of the mapping value
+  const implicitMappingValueParts = [spaceBeforeColon, ":"];
   if (
     hasLeadingComments(value.content) ||
     (hasEndComments(value) &&
@@ -103,15 +111,34 @@ function printMappingItem(node, parentNode, path, print, options) {
       value.content.tag === null &&
       value.content.anchor === null)
   ) {
-    flatValueParts.push(hardline);
+    implicitMappingValueParts.push(hardline);
   } else if (value.content) {
-    flatValueParts.push(line);
+    implicitMappingValueParts.push(line);
   }
-  flatValueParts.push(printedValue);
-  const flatValue = alignWithSpaces(options.tabWidth, flatValueParts);
+  implicitMappingValueParts.push(printedValue);
+  const implicitMappingValue = alignWithSpaces(
+    options.tabWidth,
+    implicitMappingValueParts
+  );
 
+  // If a key is definitely single-line, forcibly use implicit style to avoid edge cases (very long
+  // keys) that would otherwise trigger explicit style as if it was multiline.
+  // In those cases, explicit style makes the line even longer and causes confusion.
+  if (
+    isAbsolutelyPrintedAsSingleLineNode(key.content, options) &&
+    !hasLeadingComments(key.content) &&
+    !hasMiddleComments(key.content) &&
+    !hasEndComments(key)
+  ) {
+    return conditionalGroup([[printedKey, implicitMappingValue]]);
+  }
+
+  // Use explicit mapping syntax if the key breaks, implicit otherwise
   return conditionalGroup([
-    [groupedKey, ifBreak(breakValue, flatValue, { groupId })],
+    [
+      groupedKey,
+      ifBreak(explicitMappingValue, implicitMappingValue, { groupId }),
+    ],
   ]);
 }
 
@@ -127,6 +154,7 @@ function isAbsolutelyPrintedAsSingleLineNode(node, options) {
       break;
     case "alias":
       return true;
+
     default:
       return false;
   }

tests/format/yaml/comment/__snapshots__/jsfmt.spec.js.snap

@@ -6,9 +6,9 @@ parsers: ["yaml"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
-a:
+a: # a.trailingComment
   123
-  # impicitMappginValue
+  # implicitMappingValue
 
 ? b
   # explicitMappingKey
@@ -37,9 +37,9 @@ empty_content:
   # hello world
 
 =====================================output=====================================
-a:
+a: # a.trailingComment
   123
-  # impicitMappginValue
+  # implicitMappingValue
 
 ? b
   # explicitMappingKey

tests/format/yaml/comment/collection.yml

@@ -1,6 +1,6 @@
-a:
+a: # a.trailingComment
   123
-  # impicitMappginValue
+  # implicitMappingValue
 
 ? b
   # explicitMappingKey

tests/format/yaml/mapping/__snapshots__/jsfmt.spec.js.snap

@@ -366,11 +366,25 @@ tabWidth: 4
 : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 : value
+? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
+: # Comment
+  foo: bar
+? multiline
+  scalar
+  key
+: value
 
 =====================================output=====================================
 key1: value
 key2: longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: value
+solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong:
+    # Comment
+    foo: bar
+? multiline
+  scalar
+  key
+: value
 
 ================================================================================
 `;
@@ -387,11 +401,25 @@ printWidth: 80
 : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 : value
+? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
+: # Comment
+  foo: bar
+? multiline
+  scalar
+  key
+: value
 
 =====================================output=====================================
 key1: value
 key2: longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: value
+solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong:
+  # Comment
+  foo: bar
+? multiline
+  scalar
+  key
+: value
 
 ================================================================================
 `;

tests/format/yaml/mapping/explicit-key.yml

@@ -4,3 +4,10 @@
 : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
 : value
+? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
+: # Comment
+  foo: bar
+? multiline
+  scalar
+  key
+: value