Move cursor tracking out of the doc print buffer (#18202)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

benchmarks/print-doc-to-string.js

@@ -0,0 +1,33 @@
+import assert from "node:assert/strict";
+import fs from "node:fs/promises";
+import * as prettierProduction from "../node_modules/prettier/index.mjs";
+import * as prettierDevelopment from "../src/index.js";
+import { runBenchmark } from "./utilities.js";
+
+assert.notEqual(prettierProduction.version, prettierDevelopment.version);
+
+const text = await fs.readFile(new URL(import.meta.url), "utf8");
+// @ts-expect-error -- No types
+const doc = await prettierDevelopment.__debug.printToDoc(text, {
+  parser: "babel",
+  cursorOffset: text.indexOf("cursorOffset"),
+});
+
+const run = (prettier) =>
+  prettier.doc.printer.printDocToString(doc, { printWidth: 80, tabWidth: 2 });
+
+const expected = await run(prettierProduction);
+
+await runBenchmark(
+  {
+    name: "printDocToString",
+    assert: (result) => assert.deepEqual(result, expected),
+  },
+  [
+    { name: "Development", prettier: prettierProduction },
+    { name: "Production", prettier: prettierProduction },
+  ].map(({ name, prettier }) => ({
+    name,
+    implementation: () => run(prettier),
+  })),
+);

benchmarks/string-buffer-compress.js

@@ -0,0 +1,51 @@
+import assert from "node:assert/strict";
+import { runBenchmark } from "./utilities.js";
+
+for (const size of [1e1, 1e2, 1e3, 1e4, 1e5]) {
+  const maxLength = 10;
+  const seperator = "|";
+  const strings = Array.from({ length: size }, () => "_");
+  const expected = {
+    length: strings.join(seperator).length,
+    times: Math.floor(size / maxLength) - 1,
+  };
+
+  await runBenchmark(
+    {
+      name: `String buffer compress (${size} strings)`,
+      assert: (result) => assert.deepEqual(result, expected),
+    },
+    {
+      "Array#splice()"() {
+        const result = [];
+        let times = 0;
+
+        for (let index = 0; index < strings.length; index++) {
+          result.push(strings[index]);
+          if (result.length > maxLength) {
+            result.splice(0, Number.POSITIVE_INFINITY, result.join(seperator));
+            assert.equal(result.length, 1);
+            times++;
+          }
+        }
+
+        return { length: result.join(seperator).length, times };
+      },
+      "let reassign"() {
+        let result = [];
+        let times = 0;
+
+        for (let index = 0; index < strings.length; index++) {
+          result.push(strings[index]);
+          if (result.length > maxLength) {
+            result = [result.join(seperator)];
+            assert.equal(result.length, 1);
+            times++;
+          }
+        }
+
+        return { length: result.join(seperator).length, times };
+      },
+    },
+  );
+}

benchmarks/string-concatenation.js

@@ -0,0 +1,30 @@
+import assert from "node:assert/strict";
+import { runBenchmark } from "./utilities.js";
+
+for (const size of [1e1, 1e2, 1e3, 1e4, 1e5]) {
+  const strings = Array.from({ length: size }, () => "_");
+  const expected = strings.join("");
+
+  await runBenchmark(
+    {
+      name: `String concatentation (${size} strings)`,
+      assert: (result) => assert.equal(result, expected),
+    },
+    {
+      "Array#join()"() {
+        const result = [];
+        for (const string of strings) {
+          result.push(string);
+        }
+        return result.join("");
+      },
+      "string +="() {
+        let result = "";
+        for (const string of strings) {
+          result += string;
+        }
+        return result;
+      },
+    },
+  );
+}

benchmarks/trim-indentation.js

@@ -0,0 +1,25 @@
+import assert from "node:assert/strict";
+import { trimTrailingIndentation } from "../src/document/printer/trim-indentation.js";
+import { runBenchmark } from "./utilities.js";
+
+for (const size of [1, 1e1, 1e2, 1e3]) {
+  const trimmed = "foo".repeat(size / 2);
+  const text = trimmed + " \t".repeat(size / 2);
+
+  await runBenchmark(
+    {
+      name: `Trim indentation (${size} spaces)`,
+      assert: (result) => assert.equal(result, trimmed),
+    },
+    {
+      loop: () => trimTrailingIndentation(text),
+      "RegExp (inline)": () => text.replace(/[ \t]*$/u, ""),
+      "RegExp (stored)": (
+        (regexp) => () =>
+          text.replace(regexp, "")
+      )(/[ \t]*$/gu),
+      // eslint-disable-next-line require-unicode-regexp
+      "RegExp (no `u` flag)": () => text.replace(/[ \t]*$/, ""),
+    },
+  );
+}

benchmarks/utilities.js

@@ -0,0 +1,63 @@
+import { Bench } from "tinybench";
+
+/**
+@import {BenchOptions} from "tinybench";
+@typedef {
+  | {
+      name: string,
+      implementation: () => any
+    }[]
+  | Record<string, () => any>
+} Implementations
+*/
+
+/**
+@param {
+  | string
+  | {
+      name: string,
+      assert?: (result: any) => void,
+    } & BenchOptions
+} options
+@param {Implementations} implementations
+*/
+async function runBenchmark(options, implementations) {
+  if (typeof options === "string") {
+    options = { name: options };
+  }
+
+  const { assert, ...benchOptions } = options;
+
+  const bench = new Bench(benchOptions);
+
+  let error;
+
+  for (const { name, implementation } of Array.isArray(implementations)
+    ? implementations
+    : Object.entries(implementations).map(([name, implementation]) => ({
+        name,
+        implementation,
+      }))) {
+    bench.add(name, () => {
+      try {
+        const result = implementation();
+        assert?.(result);
+      } catch (assertionError) {
+        error ??= Object.assign(assertionError, {
+          case: `${benchOptions.name} >> ${name}`,
+        });
+      }
+    });
+  }
+
+  await bench.run();
+
+  console.log(bench.name);
+  console.table(bench.table());
+
+  if (error) {
+    throw error;
+  }
+}
+
+export { runBenchmark };

eslint.config.js

@@ -308,7 +308,11 @@ const configs = [
     },
   },
   {
-    files: ["scripts/**/*", "tests/config/install-prettier.js"],
+    files: [
+      "scripts/**/*",
+      "benchmarks/**",
+      "tests/config/install-prettier.js",
+    ],
     rules: {
       "no-console": "off",
     },

knip.config.js

@@ -15,8 +15,9 @@ const config = {
         "src/experimental-cli/index.js",
         "packages/plugin-oxc/index.js",
         "packages/plugin-hermes/index.js",
+        "benchmarks/**",
       ],
-      project: ["src/**", "scripts/**"],
+      project: ["src/**", "scripts/**", "benchmark/**"],
       ignore: [
         "scripts/build/config.js",
         "scripts/build/build-javascript-module.js",

src/document/printer/printer.js

@@ -22,6 +22,7 @@ import {
 import { getDocType, propagateBreaks } from "../utilities/index.js";
 import InvalidDocError from "../utilities/invalid-doc-error.js";
 import { makeAlign, makeIndent, ROOT_INDENT } from "./indent.js";
+import { trimIndentation } from "./trim-indentation.js";
 
 /**
 @import {EndOfLineOption} from "../../common/end-of-line.js";
@@ -37,53 +38,8 @@ const MODE_BREAK = Symbol("MODE_BREAK");
 /** @type {unique symbol} */
 const MODE_FLAT = Symbol("MODE_FLAT");
 
-const CURSOR_PLACEHOLDER = Symbol("cursor");
-
 const DOC_FILL_PRINTED_LENGTH = Symbol("DOC_FILL_PRINTED_LENGTH");
 
-// Trim `Tab(U+0009)` and `Space(U+0020)` at the end of line
-function trim(out) {
-  let trimCount = 0;
-  let cursorCount = 0;
-  let outIndex = out.length;
-
-  outer: while (outIndex--) {
-    const last = out[outIndex];
-
-    if (last === CURSOR_PLACEHOLDER) {
-      cursorCount++;
-      continue;
-    }
-
-    /* c8 ignore next 3 */
-    if (process.env.NODE_ENV !== "production" && typeof last !== "string") {
-      throw new Error(`Unexpected value in trim: '${typeof last}'`);
-    }
-
-    // Not using a regexp here because regexps for trimming off trailing
-    // characters are known to have performance issues.
-    for (let charIndex = last.length - 1; charIndex >= 0; charIndex--) {
-      const char = last[charIndex];
-      if (char === " " || char === "\t") {
-        trimCount++;
-      } else {
-        out[outIndex] = last.slice(0, charIndex + 1);
-        break outer;
-      }
-    }
-  }
-
-  if (trimCount > 0 || cursorCount > 0) {
-    out.length = outIndex + 1;
-
-    while (cursorCount-- > 0) {
-      out.push(CURSOR_PLACEHOLDER);
-    }
-  }
-
-  return trimCount;
-}
-
 /**
  * @param {Command} next
  * @param {Command[]} restCommands
@@ -149,7 +105,7 @@ function fits(
         break;
 
       case DOC_TYPE_TRIM:
-        width += trim(out);
+        width += trimIndentation(out).count;
         break;
 
       case DOC_TYPE_GROUP: {
@@ -222,11 +178,12 @@ function printDocToString(doc, options) {
   // commands to the array instead of recursively calling `print`.
   /** @type Command[] */
   const commands = [{ indent: ROOT_INDENT, mode: MODE_BREAK, doc }];
-  const out = [];
+  /** @type string[] */
+  let out = [];
   let shouldRemeasure = false;
   /** @type Command[] */
   const lineSuffix = [];
-  let printedCursorCount = 0;
+  const cursorPositions = [];
 
   propagateBreaks(doc);
 
@@ -252,13 +209,15 @@ function printDocToString(doc, options) {
         }
         break;
 
-      case DOC_TYPE_CURSOR:
+      case DOC_TYPE_CURSOR: {
-        if (printedCursorCount >= 2) {
+        if (cursorPositions.length >= 2) {
           throw new Error("There are too many 'cursor' in doc.");
         }
-        out.push(CURSOR_PLACEHOLDER);
+        const text = out.join("");
-        printedCursorCount++;
+        out = [text];
+        cursorPositions.push(text.length - 1);
         break;
+      }
 
       case DOC_TYPE_INDENT:
         commands.push({
@@ -277,7 +236,7 @@ function printDocToString(doc, options) {
         break;
 
       case DOC_TYPE_TRIM:
-        position -= trim(out);
+        trim();
         break;
 
       case DOC_TYPE_GROUP:
@@ -563,7 +522,7 @@ function printDocToString(doc, options) {
                 position = indent.root.length;
               }
             } else {
-              position -= trim(out);
+              trim();
               out.push(newLine + indent.value);
               position = indent.length;
             }
@@ -591,58 +550,67 @@ function printDocToString(doc, options) {
     }
   }
 
-  const cursorPlaceholderIndex = out.indexOf(CURSOR_PLACEHOLDER);
+  const formatted = out.join("");
-  if (cursorPlaceholderIndex !== -1) {
+  if (cursorPositions.length !== 2) {
-    const otherCursorPlaceholderIndex = out.indexOf(
+    // If the doc contained ONE cursor command,
-      CURSOR_PLACEHOLDER,
+    // instead of the expected zero or two. If the doc being printed was
-      cursorPlaceholderIndex + 1,
+    // returned by printAstToDoc, then the only ways this can have happened
-    );
+    // are if:
-
+    // 1. a plugin added a cursor command itself, or
-    if (otherCursorPlaceholderIndex === -1) {
+    // 2. one (but not both) of options.nodeAfterCursor and
-      // If we got here, the doc must have contained ONE cursor command,
+    //    options.nodeAfterCursor pointed to a node within a subtree of the
-      // instead of the expected zero or two. If the doc being printed was
+    //    AST that the printer plugin used in printAstToDoc simply omits from
-      // returned by printAstToDoc, then the only ways this can have happened
+    //    the doc, or that it prints without recursively calling mainPrint,
-      // are if:
+    //    with the consequence that the logic for adding a cursor command in
-      // 1. a plugin added a cursor command itself, or
+    //    callPluginPrintFunction was never called for that node.
-      // 2. one (but not both) of options.nodeAfterCursor and
+    // These are both weird scenarios that should be considered a bug if they
-      //    options.nodeAfterCursor pointed to a node within a subtree of the
+    // ever occur with one of Prettier's built-in plugins. If a third-party
-      //    AST that the printer plugin used in printAstToDoc simply omits from
+    // plugin was used when printing the AST to a doc, the possibility of
-      //    the doc, or that it prints without recursively calling mainPrint,
+    // reaching this scenario MIGHT be reasonable to consider a bug in the
-      //    with the consequence that the logic for adding a cursor command in
+    // plugin. However, we try to at least not crash if this ever happens;
-      //    callPluginPrintFunction was never called for that node.
+    // instead we simply give up on returning a cursorNodeStart or
-      // These are both weird scenarios that should be considered a bug if they
+    // cursorNodeText.
-      // ever occur with one of Prettier's built-in plugins. If a third-party
+    //
-      // plugin was used when printing the AST to a doc, the possibility of
+    // coreFormat has logic specifically to handle this scenario - where it
-      // reaching this scenario MIGHT be reasonable to consider a bug in the
+    // is supposed to preserve the cursor position but the printer gives it
-      // plugin. However, we try to at least not crash if this ever happens;
+    // no information about where the nodes around the cursor ended up -
-      // instead we simply give up on returning a cursorNodeStart or
+    // although that logic is unavoidably slower (and has more potential to
-      // cursorNodeText.
+    // return a perverse result) than the happy path where we help out
-      //
+    // coreFormat by returning a cursorNodeStart and cursorNodeText here.
-      // coreFormat has logic specifically to handle this scenario - where it
+    return { formatted };
-      // is supposed to preserve the cursor position but the printer gives it
-      // no information about where the nodes around the cursor ended up -
-      // although that logic is unavoidably slower (and has more potential to
-      // return a perverse result) than the happy path where we help out
-      // coreFormat by returning a cursorNodeStart and cursorNodeText here.
-      return {
-        formatted: out.filter((char) => char !== CURSOR_PLACEHOLDER).join(""),
-      };
-    }
-
-    const beforeCursor = out.slice(0, cursorPlaceholderIndex).join("");
-    const aroundCursor = out
-      .slice(cursorPlaceholderIndex + 1, otherCursorPlaceholderIndex)
-      .join("");
-    const afterCursor = out.slice(otherCursorPlaceholderIndex + 1).join("");
-
-    return {
-      formatted: beforeCursor + aroundCursor + afterCursor,
-      cursorNodeStart: beforeCursor.length,
-      cursorNodeText: aroundCursor,
-    };
   }
 
-  return { formatted: out.join("") };
+  const cursorNodeStart = cursorPositions[0] + 1;
+  return {
+    formatted,
+    cursorNodeStart,
+    cursorNodeText: formatted.slice(
+      cursorNodeStart,
+      cursorPositions.at(-1) + 1,
+    ),
+  };
+
+  function trim() {
+    const { text, count } = trimIndentation(out);
+    out = [text];
+
+    if (count === 0) {
+      return;
+    }
+
+    position -= count;
+
+    if (cursorPositions.length === 0) {
+      return;
+    }
+
+    for (let index = 0; index < cursorPositions.length; index++) {
+      cursorPositions[index] = Math.min(
+        cursorPositions[index],
+        text.length - 1,
+      );
+    }
+  }
 }
 
 export { printDocToString };

src/document/printer/trim-indentation.js

@@ -0,0 +1,41 @@
+// Not using a regexp here because regexps for trimming off trailing
+// characters are known to have performance issues.
+
+/**
+Trim trailing `Tab(U+0009)` and `Space(U+0020)` from text
+
+@param {string} text
+@returns
+*/
+function trimTrailingIndentation(text) {
+  let end = text.length - 1;
+
+  while (end >= 0) {
+    const character = text[end];
+    if (character !== " " && character !== "\t") {
+      break;
+    }
+    end--;
+  }
+
+  return text.slice(0, end + 1);
+}
+
+/**
+Trim `Tab(U+0009)` and `Space(U+0020)` at the end of line
+
+@param {string[]} buffer
+@returns {{text: string, count: number}}
+*/
+function trimIndentation(buffer) {
+  const text = buffer.join("");
+  const trimmed = trimTrailingIndentation(text);
+
+  return { text: trimmed, count: text.length - trimmed.length };
+}
+
+export {
+  trimIndentation,
+  // Exposed for benchmark test
+  trimTrailingIndentation,
+};

tests/unit/doc-builders.js

@@ -15,6 +15,7 @@ import {
   line,
   lineSuffix,
   markAsRoot,
+  trim,
 } from "../../src/document/index.js";
 
 const invalidDoc = { type: "invalid-type" };
@@ -91,6 +92,8 @@ describe("doc builders", () => {
     ifBreak(validDoc, undefined),
 
     align("any string", validDoc),
+
+    trim,
   ];
 
   describe("Invalid usage", () => {