Fix comment around nullish coalescing (#19652)

changelog_unreleased/javascript/19652.md

@@ -0,0 +1,13 @@
+#### Fix misplaced type cast comments (#19652 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+foo = a ? b : (/** @type {Foo} */ (foo) ?? bar);
+
+// Prettier stable
+foo = a ? b : /** @type {Foo} */ ((foo) ?? bar);
+
+// Prettier main
+foo = a ? b : (/** @type {Foo} */ (foo) ?? bar);
+```

src/language-js/comments/handle-comments.js

@@ -15,6 +15,7 @@ import { isBlockComment, isLineComment } from "../utilities/comment-types.js";
 import { createTypeCheckFunction } from "../utilities/create-type-check-function.js";
 import { getFunctionParameters } from "../utilities/function-parameters.js";
 import { isMethod } from "../utilities/is-method.js";
+import { isNullishCoalescing } from "../utilities/is-nullish-coalescing.js";
 import { isObjectProperty } from "../utilities/is-object-property.js";
 import { isPrettierIgnoreComment } from "../utilities/is-prettier-ignore-comment.js";
 import { isTypeCastComment } from "../utilities/is-type-cast-comment.js";
@@ -70,6 +71,7 @@ function handleOwnLineComment(context) {
   return [
     handleCommentInEmptyParens,
     handleIgnoreComments,
+    handleClosureTypeCastComments,
     handleConditionalExpressionComments,
     handleLastFunctionParameterComments,
     handleMemberExpressionComments,
@@ -135,6 +137,7 @@ function handleRemainingComment(context) {
   return [
     handleCommentInEmptyParens,
     handleIgnoreComments,
+    handleClosureTypeCastComments,
     handleIfStatementComments,
     handleWhileLikeComments,
     handleSwitchStatementComments,
@@ -152,9 +155,21 @@ function handleRemainingComment(context) {
   ].some((fn) => fn(context));
 }
 
-function handleClosureTypeCastComments({ comment, followingNode }) {
+function handleClosureTypeCastComments({
+  comment,
+  followingNode,
+  enclosingNode,
+}) {
   if (followingNode && isTypeCastComment(comment)) {
-    addLeadingComment(followingNode, comment);
+    addLeadingComment(
+      // We'll add parentheses to the nullish coalescing expression,
+      // The comment need attach to the left side, so the comments can print inside it
+      enclosingNode?.type === "ConditionalExpression" &&
+        isNullishCoalescing(followingNode)
+        ? followingNode.left
+        : followingNode,
+      comment,
+    );
     return true;
   }
   return false;

tests/format/js/comments-closure-typecast/__snapshots__/format.test.js.snap

@@ -566,6 +566,42 @@ const w1 = /** @typefoo Foo */ value;
 ================================================================================
 `;
 
+exports[`nullish-coalescing.js format 1`] = `
+====================================options=====================================
+parsers: ["babel"]
+                                                      printWidth: 80 (default) |
+=====================================input======================================
+target = a
+  ?
+    b
+  : (
+    /** @type {A} */ (c)
+    ?? d
+  )
+
+target =  a
+  ?  b
+  : (/** @type {A} */ (root).head ?? root.ownerDocument.head);
+
+// https://github.com/sveltejs/svelte/blob/602a873b6b82bba4e6edc91b039e2f6defbe3fc4/packages/svelte/src/internal/client/dom/css.js#L15-L17
+target = /** @type {ShadowRoot} */ (root).host
+  ? /** @type {ShadowRoot} */ (root)
+  : (/** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head);
+
+=====================================output=====================================
+target = a ? b : (/** @type {A} */ (c) ?? d);
+
+target = a ? b : (/** @type {A} */ (root).head ?? root.ownerDocument.head);
+
+// https://github.com/sveltejs/svelte/blob/602a873b6b82bba4e6edc91b039e2f6defbe3fc4/packages/svelte/src/internal/client/dom/css.js#L15-L17
+target = /** @type {ShadowRoot} */ (root).host
+  ? /** @type {ShadowRoot} */ (root)
+  : (/** @type {Document} */ (root).head ??
+    /** @type {Document} */ (root.ownerDocument).head);
+
+================================================================================
+`;
+
 exports[`object-with-comment.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel"]

tests/format/js/comments-closure-typecast/nullish-coalescing.js

@@ -0,0 +1,16 @@
+target = a
+  ?
+    b
+  : (
+    /** @type {A} */ (c)
+    ?? d
+  )
+
+target =  a
+  ?  b
+  : (/** @type {A} */ (root).head ?? root.ownerDocument.head);
+
+// https://github.com/sveltejs/svelte/blob/602a873b6b82bba4e6edc91b039e2f6defbe3fc4/packages/svelte/src/internal/client/dom/css.js#L15-L17
+target = /** @type {ShadowRoot} */ (root).host
+  ? /** @type {ShadowRoot} */ (root)
+  : (/** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head);

tests/format/js/nullish-coalescing/__snapshots__/format.test.js.snap

@@ -1,6 +1,119 @@
 // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
 
-exports[`nullish_coalesing_operator.js format 1`] = `
+exports[`comments.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+                                                      printWidth: 80 (default) |
+=====================================input======================================
+errorDetails =
+      errorJSON === '{}'
+        ? // regular JS error case: print stacktrace
+          (error.stack ?? 'N/A')
+        : // MDX error: print extra attributes + stacktrace
+          \`\${errorJSON}\\n\${error.stack}\`;
+
+_ = {
+  children: keepChildren
+        ? instance.children
+        : // $FlowFixMe[incompatible-type] We're not typing immutable instances.
+          (children ?? [])
+}
+
+_ = (
+        <DataTableStoreContext.Provider value={storeContextValue}>
+            <DataTableConfigContext.Provider value={configContextValue}>
+                <OptionalResourceContextProvider value={resource}>
+                    {isPending ? (
+                        loading
+                    ) : data == null || data.length === 0 || total === 0 ? (
+                        /**
+                         * Once loaded, the data for the list may be empty.
+                         * Instead of displaying the table header
+                         * with zero data rows, the DataTable
+                         * displays the empty component.
+                         */
+                        empty ?? null
+                    ) : (
+                        /**
+                         * After the initial load, if the data for the list
+                         * isn't empty, and even if the data is refreshing
+                         * (e.g. after a filter change), the DataTable
+                         * displays the current data.
+                         */
+                        <DataTableSortContext.Provider value={sort}>
+                            <DataTableCallbacksContext.Provider
+                                value={callbacksContextValue}
+                            >
+                                <DataTableSelectedIdsContext.Provider
+                                    value={selectedIds}
+                                >
+                                    <DataTableDataContext.Provider value={data}>
+                                        {children}
+                                    </DataTableDataContext.Provider>
+                                </DataTableSelectedIdsContext.Provider>
+                            </DataTableCallbacksContext.Provider>
+                        </DataTableSortContext.Provider>
+                    )}
+                </OptionalResourceContextProvider>
+            </DataTableConfigContext.Provider>
+        </DataTableStoreContext.Provider>
+    );
+
+=====================================output=====================================
+errorDetails =
+  errorJSON === "{}"
+    ? // regular JS error case: print stacktrace
+      (error.stack ?? "N/A")
+    : // MDX error: print extra attributes + stacktrace
+      \`\${errorJSON}\\n\${error.stack}\`;
+
+_ = {
+  children: keepChildren
+    ? instance.children
+    : // $FlowFixMe[incompatible-type] We're not typing immutable instances.
+      (children ?? []),
+};
+
+_ = (
+  <DataTableStoreContext.Provider value={storeContextValue}>
+    <DataTableConfigContext.Provider value={configContextValue}>
+      <OptionalResourceContextProvider value={resource}>
+        {isPending ? (
+          loading
+        ) : data == null || data.length === 0 || total === 0 ? (
+          /**
+           * Once loaded, the data for the list may be empty.
+           * Instead of displaying the table header
+           * with zero data rows, the DataTable
+           * displays the empty component.
+           */
+          (empty ?? null)
+        ) : (
+          /**
+           * After the initial load, if the data for the list
+           * isn't empty, and even if the data is refreshing
+           * (e.g. after a filter change), the DataTable
+           * displays the current data.
+           */
+          <DataTableSortContext.Provider value={sort}>
+            <DataTableCallbacksContext.Provider value={callbacksContextValue}>
+              <DataTableSelectedIdsContext.Provider value={selectedIds}>
+                <DataTableDataContext.Provider value={data}>
+                  {children}
+                </DataTableDataContext.Provider>
+              </DataTableSelectedIdsContext.Provider>
+            </DataTableCallbacksContext.Provider>
+          </DataTableSortContext.Provider>
+        )}
+      </OptionalResourceContextProvider>
+    </DataTableConfigContext.Provider>
+  </DataTableStoreContext.Provider>
+);
+
+================================================================================
+`;
+
+exports[`nullish-coalesing-operator.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]
                                                       printWidth: 80 (default) |

tests/format/js/nullish-coalescing/comments.js

@@ -0,0 +1,53 @@
+errorDetails =
+      errorJSON === '{}'
+        ? // regular JS error case: print stacktrace
+          (error.stack ?? 'N/A')
+        : // MDX error: print extra attributes + stacktrace
+          `${errorJSON}\n${error.stack}`;
+
+_ = {
+  children: keepChildren
+        ? instance.children
+        : // $FlowFixMe[incompatible-type] We're not typing immutable instances.
+          (children ?? [])
+}
+
+_ = (
+        <DataTableStoreContext.Provider value={storeContextValue}>
+            <DataTableConfigContext.Provider value={configContextValue}>
+                <OptionalResourceContextProvider value={resource}>
+                    {isPending ? (
+                        loading
+                    ) : data == null || data.length === 0 || total === 0 ? (
+                        /**
+                         * Once loaded, the data for the list may be empty.
+                         * Instead of displaying the table header
+                         * with zero data rows, the DataTable
+                         * displays the empty component.
+                         */
+                        empty ?? null
+                    ) : (
+                        /**
+                         * After the initial load, if the data for the list
+                         * isn't empty, and even if the data is refreshing
+                         * (e.g. after a filter change), the DataTable
+                         * displays the current data.
+                         */
+                        <DataTableSortContext.Provider value={sort}>
+                            <DataTableCallbacksContext.Provider
+                                value={callbacksContextValue}
+                            >
+                                <DataTableSelectedIdsContext.Provider
+                                    value={selectedIds}
+                                >
+                                    <DataTableDataContext.Provider value={data}>
+                                        {children}
+                                    </DataTableDataContext.Provider>
+                                </DataTableSelectedIdsContext.Provider>
+                            </DataTableCallbacksContext.Provider>
+                        </DataTableSortContext.Provider>
+                    )}
+                </OptionalResourceContextProvider>
+            </DataTableConfigContext.Provider>
+        </DataTableStoreContext.Provider>
+    );