ronantakizawa/codereview-bench huggingface dataset:
Repository: prettier/prettier
File: tests/format/js/nullish-coalescing/nullish_coalesing_operator.js (JavaScript)

Code before:
obj.foo ?? "default";

const x = (foo, bar = foo ?? bar) => {};

foo ? bar ?? foo : baz;

foo ?? (bar ?? baz);
(foo ?? bar) ?? baz;

// Mixing ?? and (&& or ||) requires parens
// It's a syntax error without it.
(foo ?? baz) || baz;
foo ?? (baz || baz);

(foo ?? baz) && baz;
foo ?? (baz && baz);

(foo || baz) ?? baz;
foo || (baz ?? baz);

(foo && baz) ?? baz;
foo && (baz ?? baz);

baz ?? baz ? a : b;


Review comment (suggestion):
Since we are not checking ternary properties, please add test for every possible cases.

```suggestion
foo ?? bar ? a : b;
a ? foo ?? bar : b;
a ? b : foo ?? bar;
```

Code after:
obj.foo ?? "default";

const x = (foo, bar = foo ?? bar) => {};

foo ? bar ?? foo : baz;

foo ?? (bar ?? baz);
(foo ?? bar) ?? baz;

// Mixing ?? and (&& or ||) requires parens
// It's a syntax error without it.
(foo ?? baz) || baz;
foo ?? (baz || baz);

(foo ?? baz) && baz;
foo ?? (baz && baz);

(foo || baz) ?? baz;
foo || (baz ?? baz);

(foo && baz) ?? baz;
foo && (baz ?? baz);

foo ?? bar ? a : b;
a ? foo ?? bar : b;
a ? b : foo ?? bar;
Repository: prettier/prettier
File: src/utils/print-string.js (JavaScript)

Code before:
    options.parser === "jsonc" ||
    // This was added before we have the `jsonc` parser
    // If `{quoteProps: "preserve"}` and `{singleQuote: false}` (default value),
    // and `{parser: "json5"}`, double quotes are always used for strings.
    // This effectively allows using the `json5` parser for “JSON with comments and trailing commas”.
    // See https://github.com/prettier/prettier/pull/10323
    // See https://github.com/prettier/prettier/pull/15831#discussion_r1431010636
    (options.parser === "json5" &&
      options.quoteProps === "preserve" &&
      !options.singleQuote)
      ? '"'
      : options.__isInHtmlAttribute
        ? "'"
        : getPreferredQuote(rawContent, options.singleQuote);

  // It might sound unnecessary to use `makeString` even if the string already
  // is enclosed with `enclosingQuote`, but it isn't. The string could contain
  // unnecessary escapes (such as in `"\'"`). Always using `makeString` makes
  // sure that we consistently output the minimum amount of escaped quotes.
  return makeString(
    rawContent,
    enclosingQuote,
    !(
      options.parser === "css" ||
      options.parser === "less" ||
      options.parser === "scss" ||
      options.__embeddedInHtml
    ),
  );
}

export default printString;


Review comment (suggestion):
WDYT of removing this option completely, since it is always `false`?

This code can be then deleted:
https://github.com/prettier/prettier/blob/250847c2cc44c25f175a521d95daabc4ad804b9b/src/common/util.js#L328-L334

Code after:
    options.parser === "jsonc" ||
    // This was added before we have the `jsonc` parser
    // If `{quoteProps: "preserve"}` and `{singleQuote: false}` (default value),
    // and `{parser: "json5"}`, double quotes are always used for strings.
    // This effectively allows using the `json5` parser for “JSON with comments and trailing commas”.
    // See https://github.com/prettier/prettier/pull/10323
    // See https://github.com/prettier/prettier/pull/15831#discussion_r1431010636
    (options.parser === "json5" &&
      options.quoteProps === "preserve" &&
      !options.singleQuote)
      ? '"'
      : options.__isInHtmlAttribute
        ? "'"
        : getPreferredQuote(rawContent, options.singleQuote);

  // It might sound unnecessary to use `makeString` even if the string already
  // is enclosed with `enclosingQuote`, but it isn't. The string could contain
  // unnecessary escapes (such as in `"\'"`). Always using `makeString` makes
  // sure that we consistently output the minimum amount of escaped quotes.
  return makeString(
    rawContent,
    enclosingQuote,
    // Until Prettier 3.3.3, this option was set to true for most parsers, with some exceptions like CSS.
    // Since Prettier 3.3.4, it is set to false for all parsers.
    // For more details, please see https://github.com/prettier/prettier/issues/16542#issuecomment-2282249280.
    false,
  );
}

export default printString;
Repository: prettier/prettier
File: src/language-css/print/comma-separated-value-group.js (JavaScript)

Code before:
    }

    if (
      isAtWordPlaceholderNode(iNode) &&
      isParenGroupNode(iNextNode) &&
      locEnd(iNode) === locStart(iNextNode.open)
    ) {
      parts.push(softline);
      continue;
    }

    if (iNode.value === "with" && isParenGroupNode(iNextNode)) {
      parts.push(" ");
      continue;
    }

    if (
      iNode.value?.endsWith("#") &&
      iNextNode.value === "{" &&
      isParenGroupNode(iNextNode.group)
    ) {
      continue;
    }

    // don't print line when the next node is a comment and last node
    // it will be printed with the comment in a  linesuffix
    if (isInlineValueCommentNode(iNextNode) && !iNextNextNode) {
      continue;
    }

    // Be default all values go through `line`
    parts.push(line);
  }

  if (hasInlineComment) {
    parts.push(breakParent);
  }

  if (didBreak) {
    parts.unshift(hardline);
  }

  if (isControlDirective) {
    return group(indent(parts));
  }

  // Indent is not needed for import url when url is very long
  // and node has two groups
  // when type is value-comma_group
  // example @import url("verylongurl") projection,tv
  if (insideURLFunctionInImportAtRuleNode(path)) {

Review comment (suggestion):
```suggestion
    // it will be printed with the comment in a line suffix
```
should resolve the CI issue

Code after:
      ) {
        parts.push(hardline, "");

        didBreak = true;
      } else {
        parts.push([parts.pop(), " "]);
      }

      continue;
    }

    // Add `space` before next math operation
    // Note: `grip` property have `/` delimiter and it is not math operation, so
    // `grid` property handles above
    if (isNextMathOperator) {
      parts.push([parts.pop(), " "]);

      continue;
    }
    // allow function(returns-list($list)...)
    if (iNextNode?.value === "...") {
      continue;
    }

    if (
      isAtWordPlaceholderNode(iNode) &&
      isAtWordPlaceholderNode(iNextNode) &&
      locEnd(iNode) === locStart(iNextNode)
    ) {
      continue;
    }

    if (
      isAtWordPlaceholderNode(iNode) &&
      isParenGroupNode(iNextNode) &&
      locEnd(iNode) === locStart(iNextNode.open)
    ) {
      parts.push(softline, "");
      continue;
    }

    if (iNode.value === "with" && isParenGroupNode(iNextNode)) {
      parts = [[fill(parts), " "]];
      continue;
    }

    if (
      iNode.value?.endsWith("#") &&
      iNextNode.value === "{" &&
      isParenGroupNode(iNextNode.group)
    ) {
Repository: prettier/prettier
File: tests/format/markdown/splitCjkText/format.test.js (JavaScript)

Code before:
const code = Array.from({ length: 6 })
  // less "文" <= [..., "文...文 ", "文...文", ...] => more "文"
  .map((_, i) => ["文".repeat(39 + (i >> 1)), i & 1 ? "" : " "])
  // ["<Line #1>", "<L2>", ...]
  .reduce(
    (previousInputLines, [base, space]) => [
      ...previousInputLines,
      `${base}${space}\n`,
    ],
    [],
  )
  .join("");
// (39 + 40 + 41) * 2 === 40 * 3 * 2 === 40 * 6
// lineWidth is 80 and "文" is double-width
// This is being saved for after the WebKit-based browser bug is fixed.
const output = /* `${"文".repeat(40)}\n`.repeat(6) */ `${"文".repeat(40 * 6)}\n`;

runFormatTest(
  {
    importMeta: import.meta,
    snippets: [
      {
        code,
        name: "Should ignore single trailing space after han before New Line before han",
        output,
      },
    ],
  },
  ["markdown"],
  { proseWrap: "always" },
);


Review comment (bug):
It's a bit of an inaccuracy to call modern Chrome WebKit based.

```suggestion
// This is being saved for after Chrome and Safari bug is fixed.
```

Code after:
const code = Array.from({ length: 6 })
  // less "文" <= [..., "文...文 ", "文...文", ...] => more "文"
  .map((_, i) => ["文".repeat(39 + (i >> 1)), i & 1 ? "" : " "])
  // ["<Line #1>", "<L2>", ...]
  .reduce(
    (previousInputLines, [base, space]) => [
      ...previousInputLines,
      `${base}${space}\n`,
    ],
    [],
  )
  .join("");
// (39 + 40 + 41) * 2 === 40 * 3 * 2 === 40 * 6
// lineWidth is 80 and "文" is double-width
// This is being saved for after Chrome and Safari bug is fixed.
const output = /* `${"文".repeat(40)}\n`.repeat(6) */ `${"文".repeat(40 * 6)}\n`;

runFormatTest(
  {
    importMeta: import.meta,
    snippets: [
      {
        code,
        name: "Should ignore single trailing space after han before New Line before han",
        output,
      },
    ],
  },
  ["markdown"],
  { proseWrap: "always" },
);
Repository: prettier/prettier
File: src/language-markdown/constants.evaluate.js (JavaScript)

Code before:
import { all as getCjkCharset } from "cjk-regex";
import { Charset } from "regexp-util";
import unicodeRegex from "unicode-regex";

const cjkCharset = new Charset(
  getCjkCharset(),
  unicodeRegex({
    Script_Extensions: ["Han", "Katakana", "Hiragana", "Hangul", "Bopomofo"],
    General_Category: [
      "Other_Letter",
      "Letter_Number",
      "Other_Symbol",
      "Modifier_Letter",
      "Modifier_Symbol",
      "Nonspacing_Mark",
    ],
  }).union(
    // Firefox treats some symbols (U+30A0, U+30FB) in the Katakana block as CJK
    unicodeRegex({
      Block: ["Katakana"],
    }),
  ),
);
const variationSelectorsCharset = unicodeRegex({
  Block: ["Variation_Selectors", "Variation_Selectors_Supplement"],
});

const CJK_REGEXP = new RegExp(
  `(?:${cjkCharset.toString("u")})(?:${variationSelectorsCharset.toString("u")})?`,
  "u",
);

const asciiPunctuationCharacters = [
  "!",
  '"',
  "#",
  "$",
  "%",
  "&",
  "'",
  "(",
  ")",
  "*",
  "+",
  ",",
  "-",
  ".",
  "/",
  ":",

Review comment (suggestion):
Will this work?

```js
const cjkCharset = new Charset(
  getCjkCharset(),
  unicodeRegex({
    Script_Extensions: ["Han", "Katakana", "Hiragana", "Hangul", "Bopomofo"],
    General_Category: [
      "Other_Letter",
      "Letter_Number",
      "Other_Symbol",
      "Modifier_Letter",
      "Modifier_Symbol",
      "Nonspacing_Mark",
    ],
    // Firefox treats some symbols (U+30A0, U+30FB) in the Katakana block as CJK
    Block: ["Katakana"],
  }),
);
```

Code after:
import { all as getCjkCharset } from "cjk-regex";
import { Charset } from "regexp-util";
import unicodeRegex from "unicode-regex";

const cjkCharset = new Charset(
  getCjkCharset(),
  unicodeRegex({
    Script_Extensions: ["Han", "Katakana", "Hiragana", "Hangul", "Bopomofo"],
    General_Category: [
      "Other_Letter",
      "Letter_Number",
      "Other_Symbol",
      "Modifier_Letter",
      "Modifier_Symbol",
      "Nonspacing_Mark",
    ],
    // .union below makes the next Block condition "OR"
    // If it is merged into this object definition, it will be "AND" instead
  }).union(
    // Firefox treats some symbols (U+30A0, U+30FB) in the Katakana block as CJK
    unicodeRegex({
      Block: ["Katakana"],
    }),
  ),
);
const variationSelectorsCharset = unicodeRegex({
  Block: ["Variation_Selectors", "Variation_Selectors_Supplement"],
});

const CJK_REGEXP = new RegExp(
  `(?:${cjkCharset.toString("u")})(?:${variationSelectorsCharset.toString("u")})?`,
  "u",
);

const asciiPunctuationCharacters = [
  "!",
  '"',
  "#",
  "$",
  "%",
  "&",
  "'",
  "(",
  ")",
  "*",
  "+",
  ",",
  "-",
  ".",
Repository: prettier/prettier
File: tests/integration/run-cli.js (JavaScript)

Code before:
    stream.on("end", () => {
      resolve(result);
    });

    stream.on("error", (error) => {
      reject(error);
    });
  });

const removeFinalNewLine = (string) =>
  string.endsWith("\n") ? string.slice(0, -1) : string;

function runCliWorker(dir, args, options) {
  const result = {
    status: undefined,
    stdout: "",
    stderr: "",
    write: [],
  };

  const worker = new Worker(CLI_WORKER_FILE, {
    argv: args,
    execArgv: [
      "--trace-deprecation",
      "--disable-warning=ExperimentalWarning",
      "--experimental-strip-types",
    ],
    stdout: true,
    stderr: true,
    env: {
      ...process.env,
      FORCE_COLOR: 0,
    },
    workerData: {
      dir,
      options,
    },
    trackUnmanagedFds: false,
  });

  worker.on("message", ({ action, data }) => {
    if (action === "write-file") {
      result.write.push(data);
    }
  });

  return new Promise((resolve, reject) => {
    worker.on("exit", async (code) => {
      result.status = code;
      result.stdout = removeFinalNewLine(await streamToString(worker.stdout));
      result.stderr = removeFinalNewLine(await streamToString(worker.stderr));

Review comment (suggestion):
Would this work in Node < 22.6 where this [flag was introduced](https://nodejs.org/en/blog/release/v22.6.0)? We should make sure we don’t get any warnings in all supported Node versions and if the flag is not available, there should be a meaningful error about why `prettier.config.ts` could not load. Docs need to reflect min version for TS config too, unless we want to change the approach and use a built-in transpiler.

Code after:
    stream.on("end", () => {
      resolve(result);
    });

    stream.on("error", (error) => {
      reject(error);
    });
  });

const removeFinalNewLine = (string) =>
  string.endsWith("\n") ? string.slice(0, -1) : string;

const SUPPORTS_DISABLE_WARNING_FLAG =
  Number(process.versions.node.split(".")[0]) >= 20;

function runCliWorker(dir, args, options) {
  const result = {
    status: undefined,
    stdout: "",
    stderr: "",
    write: [],
  };

  const nodeOptions = options?.nodeOptions ?? [];

  const worker = new Worker(CLI_WORKER_FILE, {
    argv: args,
    execArgv: [
      "--trace-deprecation",
      ...(SUPPORTS_DISABLE_WARNING_FLAG
        ? ["--disable-warning=ExperimentalWarning"]
        : []),
      ...nodeOptions,
    ],
    stdout: true,
    stderr: true,
    env: {
      ...process.env,
      FORCE_COLOR: 0,
    },
    workerData: {
      dir,
      options,
    },
    trackUnmanagedFds: false,
  });

  worker.on("message", ({ action, data }) => {
    if (action === "write-file") {
      result.write.push(data);
    }
Repository: prettier/prettier
File: src/config/prettier-config/load-external-config.js (JavaScript)

Code before:
const requireErrorCodesShouldBeIgnored = new Set([
  "MODULE_NOT_FOUND",
  "ERR_REQUIRE_ESM",
  "ERR_PACKAGE_PATH_NOT_EXPORTED",
]);
async function loadExternalConfig(externalConfig, configFile) {
  /*
  Try `require()` first, this is how it works in Prettier v2.
  Kept this because the external config path or package may can't load with `import()`:
  1. is JSON file or package
  2. is CommonJS file without extension
  3. is a dirname with index.js inside
  */
  try {
    const required = requireFromFile(externalConfig, configFile);
    // Since Node.js v23 onwards, it is possible to load ESM using require.
    // If that feature is enabled, it is necessary to return the default.
    // https://github.com/prettier/prettier/issues/16812
    // FIXME: We want to add tests but https://github.com/jestjs/jest/issues/15363 blocks
    // @ts-expect-error
    if (process.features.require_module && required.__esModule) {
      return required.default;
    }
    return required;
  } catch (/** @type {any} */ error) {
    if (!requireErrorCodesShouldBeIgnored.has(error?.code)) {
      throw error;
    }
  }

  const module = await importFromFile(externalConfig, configFile);
  return module.default;
}

export default loadExternalConfig;


Review comment (suggestion):
Maybe simply use `required?.default ?? required`?    https://github.com/babel/babel/issues/16441

Code after:
const requireErrorCodesShouldBeIgnored = new Set([
  "MODULE_NOT_FOUND",
  "ERR_REQUIRE_ESM",
  "ERR_PACKAGE_PATH_NOT_EXPORTED",
  "ERR_REQUIRE_ASYNC_MODULE",
]);
async function loadExternalConfig(externalConfig, configFile) {
  /*
  Try `require()` first, this is how it works in Prettier v2.
  Kept this because the external config path or package may can't load with `import()`:
  1. is JSON file or package
  2. is CommonJS file without extension
  3. is a dirname with index.js inside
  */
  try {
    const required = requireFromFile(externalConfig, configFile);
    // Since Node.js v23 onwards, it is possible to load ESM using require.
    // If that feature is enabled, it is necessary to return the default.
    // https://github.com/prettier/prettier/issues/16812
    // FIXME: We want to add tests but https://github.com/jestjs/jest/issues/15363 blocks
    // @ts-expect-error
    if (process.features.require_module && required.__esModule) {
      return required.default;
    }
    return required;
  } catch (/** @type {any} */ error) {
    if (!requireErrorCodesShouldBeIgnored.has(error?.code)) {
      throw error;
    }
  }

  const module = await importFromFile(externalConfig, configFile);
  return module.default;
}

export default loadExternalConfig;
Repository: prettier/prettier
File: scripts/check-deps.js (JavaScript)

Code before:
import fs from "node:fs/promises";
import chalk from "chalk";

const packageJson = JSON.parse(
  await fs.readFile(new URL("../package.json", import.meta.url)),
);

validateDependencyObject(packageJson.dependencies);
validateDependencyObject(packageJson.devDependencies);

function validateDependencyObject(object) {
  for (const key of Object.keys(object)) {
    if (object[key][0] === "^" || object[key][0] === "~") {
      console.error(
        chalk.red("error"),
        `Dependency "${chalk.bold.red(key)}" should be pinned.`,
      );
      process.exitCode = 1;
    }
  }
}


Review comment (suggestion):
Before:

![image](https://github.com/user-attachments/assets/7086ee1d-68a9-4aaf-b3ee-734c2c3dfb56)

After:

![image](https://github.com/user-attachments/assets/674518e5-0567-45f3-8524-b4f3f263f0c7)


Code after:
import fs from "node:fs/promises";
import styleText from "node-style-text";

const packageJson = JSON.parse(
  await fs.readFile(new URL("../package.json", import.meta.url)),
);

validateDependencyObject(packageJson.dependencies);
validateDependencyObject(packageJson.devDependencies);

function validateDependencyObject(object) {
  for (const [name, version] of Object.entries(object)) {
    if (version[0] === "^" || version[0] === "~") {
      console.error(
        styleText.bgRed.black(" ERROR "),
        `Dependency "${styleText.bold.blue(name)}" should be pinned.`,
      );
      process.exitCode = 1;
    }
  }
}
Repository: prettier/prettier
File: scripts/tools/eslint-plugin-prettier-internal-rules/index.js (JavaScript)

Code before:
"use strict";

module.exports = {
  rules: {
    "await-cli-tests": require("./await-cli-tests.js"),
    "better-parent-property-check-in-needs-parens": require("./better-parent-property-check-in-needs-parens.js"),
    "directly-loc-start-end": require("./directly-loc-start-end.js"),
    "flat-ast-path-call": require("./flat-ast-path-call.js"),
    "jsx-identifier-case": require("./jsx-identifier-case.js"),
    "massage-ast-parameter-names": require("./massage-ast-parameter-names.js"),
    "no-conflicting-comment-check-flags": require("./no-conflicting-comment-check-flags.js"),
    "no-doc-public-import": require("./no-doc-public-import.js"),
    "no-empty-flat-contents-for-if-break": require("./no-empty-flat-contents-for-if-break.js"),
    "no-identifier-n": require("./no-identifier-n.js"),
    "no-legacy-format-test": require("./no-legacy-format-test.js"),
    "no-node-comments": require("./no-node-comments.js"),
    "no-unnecessary-ast-path-call": require("./no-unnecessary-ast-path-call.js"),
    "prefer-ast-path-each": require("./prefer-ast-path-each.js"),
    "prefer-create-type-check-function": require("./prefer-create-type-check-function.js"),
    "prefer-indent-if-break": require("./prefer-indent-if-break.js"),
    "prefer-is-non-empty-array": require("./prefer-is-non-empty-array.js"),
    "prefer-fs-promises-submodule": require("./prefer-fs-promises-submodule.js"),
    "prefer-ast-path-getters": require("./prefer-ast-path-getters.js"),
  },
};


Review comment (suggestion):
I personally don't prefer TLA...But I think this is fine for now. I want `import.sync` proposal

Code after:
import fs from "node:fs/promises";
import path from "node:path";
import packageJson from "./package.json" with { type: "json" };

const rules = {};

for (const dirent of await fs.readdir(import.meta.dirname, {
  withFileTypes: true,
})) {
  const fileName = dirent.name;

  if (
    dirent.isDirectory() ||
    !fileName.endsWith(".js") ||
    fileName === "index.js" ||
    fileName === "test.js"
  ) {
    continue;
  }

  const name = path.basename(fileName, ".js");
  const { default: rule } = await import(
    new URL(fileName, import.meta.url).href
  );

  if (rule.meta?.docs?.url) {
    throw new Error(`Please remove 'meta.docs.url' from '${fileName}'.`);
  }

  rule.meta ??= {};
  rule.meta.docs ??= {};
  rule.meta.docs.url = `https://github.com/prettier/prettier/blob/main/scripts/tools/eslint-plugin-prettier-internal-rules/${fileName}`;

  rules[name] = rule;
Repository: prettier/prettier
File: tests/integration/plugins/languages/is-supported.js (JavaScript)

Code before:
// @ts-check

/** @import {SupportLanguage} from "prettier" */

/** @type {SupportLanguage[]} */
export const languages = [
  {
    name: "language-name-does-not-matter",
    parsers: ["parser-name-inferred-from-language-is-supported"],
    isSupported: (file) =>
      /^\.husky[\\/][^\\/]+/u.test(file) ||
      /([\\/])\.husky\1[^\\/]+$/u.test(file),
  },
];


Review comment (suggestion):
Not important, but I thought we had a rule for this pattern.

```suggestion
      /(?<separator>[\\/])\.husky\k<separator>[^\\/]+$/u.test(file),
```

Code after:
import path from "node:path";
import createPlugin from "../../../config/utils/create-plugin.cjs";

const PARSER_NAME = "parser-name-inferred-from-language-is-supported";
const PRINT_MARK = `formatted by '${PARSER_NAME}' parser`

const languages = [
  {
    name: "language-name-does-not-matter",
    parsers: [PARSER_NAME],
    isSupported(file) {
      if (!path.isAbsolute(file)) {
        throw new Error("Unexpected non absolute path");
      }

      return /(?<separator>[\\/])\.husky\k<separator>[^\\/]+$/u.test(file);
    },
  },
];

export default {
  ...createPlugin({
    name: PARSER_NAME,
    print: (content) => `${content.replace(PRINT_MARK,"").trim()}\n${PRINT_MARK}`,
    finalNewLine: false,
  }),
  languages,
};