ronantakizawa/codereview-bench huggingface dataset:
Repository: prettier/prettier
File: src/cli/find-cache-file.js (JavaScript)

Code before:
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import findCacheDirectory from "find-cache-directory";
import { isJson, statSafe } from "./utils.js";

/**
 * Find default cache file (`./node_modules/.cache/prettier/.prettier-cache`) using https://github.com/sindresorhus/find-cache-directory
 */
function findDefaultCacheFile() {
  const cacheDir = findCacheDirectory({ name: "prettier" }) || os.tmpdir();
  const cacheFilePath = path.join(cacheDir, ".prettier-cache");
  return cacheFilePath;
}

async function findCacheFileFromOption(cacheLocation) {
  const cacheFile = path.resolve(cacheLocation);

  const stat = await statSafe(cacheFile);
  if (stat) {
    if (stat.isDirectory()) {
      throw new Error(
        `Resolved --cache-location '${cacheFile}' is a directory`,
      );
    }

    const data = await fs.readFile(cacheFile, "utf8");
    if (!isJson(data)) {
      throw new Error(`'${cacheFile}' isn't a valid JSON file`);
    }
  }

  return cacheFile;
}

/**

Review comment (performance):
```suggestion
  const cacheDir = findCacheDirectory({ name: "prettier" }) ?? os.tmpdir();
```

Code after:
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import findCacheDirectory from "find-cache-directory";
import { isJson, statSafe } from "./utils.js";

/**
 * Find default cache file (`./node_modules/.cache/prettier/.prettier-cache`) using https://github.com/sindresorhus/find-cache-directory
 */
function findDefaultCacheFile() {
  const cacheDir = findCacheDirectory({ name: "prettier" }) ?? os.tmpdir();
  const cacheFilePath = path.join(cacheDir, ".prettier-cache");
  return cacheFilePath;
}

async function findCacheFileFromOption(cacheLocation) {
  const cacheFile = path.resolve(cacheLocation);

  const stat = await statSafe(cacheFile);
  if (stat) {
    if (stat.isDirectory()) {
      throw new Error(
        `Resolved --cache-location '${cacheFile}' is a directory`,
      );
    }

    const data = await fs.readFile(cacheFile, "utf8");
    if (!isJson(data)) {
      throw new Error(`'${cacheFile}' isn't a valid JSON file`);
    }
  }

  return cacheFile;
}

/**
Repository: prettier/prettier
File: src/language-css/print/comma-separated-value-group.js (JavaScript)

Code before:
    if (iNode.value === "with" && isParenGroupNode(iNextNode)) {
      parts = [[fill(parts), " "]];
      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 line suffix
    if (isInlineValueCommentNode(iNextNode) && !iNextNextNode) {
      continue;
    }

    // align value after block comment
    if (
      atRuleAncestorNode === undefined &&
      node.groups
        .slice(0, i + 1)
        .every((group) => group.type === "value-comment") &&
      !iNode.inline
    ) {
      parts.push(dedent(line), "");
      continue;
    }

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

  if (hasInlineComment) {
    parts.push([parts.pop(), 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

Review comment (suggestion):
Move `!iNode.inline` upper? To avoid unnecessary array slice

Code after:
    if (iNode.value === "with" && isParenGroupNode(iNextNode)) {
      parts = [[fill(parts), " "]];
      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 line suffix
    if (isInlineValueCommentNode(iNextNode) && !iNextNextNode) {
      continue;
    }

    // align value after block comment
    if (
      !atRuleAncestorNode &&
      iNode.type === "value-comment" &&
      !iNode.inline &&
      node.groups.slice(0, i).every((group) => group.type === "value-comment")
    ) {
      parts.push(dedent(line), "");
      continue;
    }

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

  if (hasInlineComment) {
    parts.push([parts.pop(), 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)) {
Repository: prettier/prettier
File: src/language-yaml/print/mapping-item.js (JavaScript)

Code before:
      ifBreak(explicitMappingValue, implicitMappingValue, { groupId }),
    ],
  ]);
}

function isMappingKeyBreak(key, options) {
  if (options.proseWrap === "never") {
    return false;
  }

  if (!key || !key.content) {
    return false;
  }

  const { content } = key;

  if (isNode(content, ["flowMapping", "flowSequence"])) {
    return false;
  }

  if (options.proseWrap === "preserve") {
    return content.position.start.line !== content.position.end.line;
  }

  const keyLength = content.position.end.offset - content.position.start.offset;
  return keyLength > options.printWidth;
}

function isAbsolutelyPrintedAsSingleLineNode(node, options) {
  if (!node) {
    return true;
  }

  switch (node.type) {
    case "plain":
    case "quoteSingle":
    case "quoteDouble":
      break;
    case "alias":
      return true;

    default:
      return false;
  }

  if (options.proseWrap === "preserve") {
    return node.position.start.line === node.position.end.line;
  }

  if (
    // backslash-newline

Review comment (bug):
This is incorrect, since it can be a child, which means it will be less room than `printWidth`.

Also, this case absolutely should not in "// force explicit Key" branch.

Code after:
}

function isAbsolutelyPrintedAsSingleLineNode(node, options) {
  if (!node) {
    return true;
  }

  switch (node.type) {
    case "plain":
    case "quoteSingle":
    case "quoteDouble":
      break;
    case "alias":
      return true;

    default:
      return false;
  }

  if (options.proseWrap === "preserve") {
    return node.position.start.line === node.position.end.line;
  }

  if (
    // backslash-newline
    /\\$/mu.test(
      options.originalText.slice(
        node.position.start.offset,
        node.position.end.offset,
      ),
    )
  ) {
    return false;
  }

  switch (options.proseWrap) {
    case "never":
      return !node.value.includes("\n");
    case "always":
      return !/[\n ]/u.test(node.value);
    default:
      /* c8 ignore next */
      return false;
  }
}

function needsSpaceInFrontOfMappingValue(node) {
  return node.key.content?.type === "alias";
}

function isSingleLineNode(node) {
Repository: prettier/prettier
File: src/utils/make-string.js (JavaScript)

Code before:
/** @import {Quote} from "./get-preferred-quote.js" */

// Matches _any_ escape and unescaped quotes (both single and double).
const REGEX = /(\\.)|(["'])/gsu;

function makeString(rawText, enclosingQuote) {
  const otherQuote = enclosingQuote === '"' ? "'" : '"';

  // Escape and unescape single and double quotes as needed to be able to
  // enclose `rawText` with `enclosingQuote`.
  const raw = rawText.replaceAll(REGEX, (match, escaped, unescaped) => {
    // If we matched an escape, and the escaped character is a quote of the
    // other type than we intend to enclose the string with, there's no need for
    // it to be escaped, so return it _without_ the backslash.
    if (escaped) {
      return escaped[1] === otherQuote ? otherQuote : escaped;
    }

    // If we matched an unescaped quote and it is of the _same_ type as we
    // intend to enclose the string with, it must be escaped, so return it with
    // a backslash.
    return unescaped === enclosingQuote ? "\\" + unescaped : unescaped;
  });

  return enclosingQuote + raw + enclosingQuote;
}

export default makeString;


Review comment (suggestion):
@liuxingbaoyu

Do you understand, why I can't change to `/(\\["'])|(["'])/gu` to only match quote?

Change to `/(\\["'\\])|(["'])/gu` seems work, but I can't understand.



Code after:
/** @import {Quote} from "./get-preferred-quote.js" */

// Matches _any_ escape and unescaped quotes (both single and double).
// Matching `\` in first group, so we won't treat `\\"` as escaped quote.
const REGEX = /\\(["'\\])|(["'])/gu;

/**
 * @param {string} rawText
 * @param {Quote} enclosingQuote
 * @returns {string}
 */
function makeString(rawText, enclosingQuote) {
  const otherQuote = enclosingQuote === '"' ? "'" : '"';

  // Escape and unescape single and double quotes as needed to be able to
  // enclose `rawText` with `enclosingQuote`.
  const raw = rawText.replaceAll(REGEX, (match, escaped, unescaped) => {
    // If we matched an escape, and the escaped character is a quote of the
    // other type than we intend to enclose the string with, there's no need for
    // it to be escaped, so return it _without_ the backslash.
    if (escaped) {
      return escaped === otherQuote ? otherQuote : match;
    }

    // If we matched an unescaped quote and it is of the _same_ type as we
    // intend to enclose the string with, it must be escaped, so return it with
    // a backslash.
    return unescaped === enclosingQuote ? "\\" + unescaped : unescaped;
  });
Repository: prettier/prettier
File: src/language-css/printer-postcss.js (JavaScript)

Code before:
      const grandParent = path.grandparent;
      if (
        parentNode.type === "value-paren_group" &&
        grandParent?.type === "value-func" &&
        grandParent.value === "selector"
      ) {
        const start = locEnd(parentNode.open) + 1;
        const end = locStart(parentNode.close);
        const selector = options.originalText.slice(start, end).trim();

        return lastLineHasInlineComment(selector)
          ? [breakParent, selector]
          : selector;
      }

      return node.value;
    }
    // postcss-values-parser
    case "value-value":
    case "value-root":
      return print("group");

    case "value-comment": {
      const text = options.originalText.slice(locStart(node), locEnd(node));
      const isLineComment =
        node.inline && path.parent?.type === "value-paren_group";
      return isLineComment ? lineSuffix(text) : text;
    }
    case "value-comma_group":
      return printCommaSeparatedValueGroup(path, options, print);

    case "value-paren_group":
      return printParenthesizedValueGroup(path, options, print);

    case "value-func":
      return [
        node.value,
        insideAtRuleNode(path, "supports") && isMediaAndSupportsKeywords(node)
          ? " "
          : "",
        print("group"),
      ];

    case "value-paren":
      return node.value;

    case "value-number":
      return [printCssNumber(node.value), printUnit(node.unit)];

    case "value-operator":
      return node.value;

Review comment (suggestion):
Added `path.parent.type === "value-paren_group"` so it doesn't break the example below.

```less
@var: // comment
{
  preserveCase: 5;
};
```

Code after:
      // Same reason above
      const grandParent = path.grandparent;
      if (
        parentNode.type === "value-paren_group" &&
        grandParent?.type === "value-func" &&
        grandParent.value === "selector"
      ) {
        const start = locEnd(parentNode.open) + 1;
        const end = locStart(parentNode.close);
        const selector = options.originalText.slice(start, end).trim();

        return lastLineHasInlineComment(selector)
          ? [breakParent, selector]
          : selector;
      }

      return node.value;
    }
    // postcss-values-parser
    case "value-value":
    case "value-root":
      return print("group");

    case "value-comment": {
      const text = options.originalText.slice(locStart(node), locEnd(node));
      return node.inline ? lineSuffix(text.trimEnd()) : text;
    }
    case "value-comma_group":
      return printCommaSeparatedValueGroup(path, options, print);

    case "value-paren_group":
      return printParenthesizedValueGroup(path, options, print);

    case "value-func":
      return [
        node.value,
        insideAtRuleNode(path, "supports") && isMediaAndSupportsKeywords(node)
          ? " "
          : "",
        print("group"),
      ];

    case "value-paren":
      return node.value;

    case "value-number":
      return [printCssNumber(node.value), printUnit(node.unit)];

    case "value-operator":
      return node.value;
Repository: prettier/prettier
File: src/language-js/print/comment.js (JavaScript)

Code before:
  }

  if (isBlockComment(comment)) {
    return ["/*", replaceEndOfLine(comment.value), "*/"];
  }

  /* c8 ignore next */
  throw new Error("Not a comment: " + JSON.stringify(comment));
}

function printIndentableBlockComment(comment) {
  /** @type {string[]} */
  const lines = comment.value.split("\n");
  const isJSDoc = comment.value[0] === "*" && comment.value[1] !== "*";

  return [
    "/*",
    lines.map((line, index) => {
      if (index === 0) {
        return [line.trimEnd(), hardline];
      }
      if (index === lines.length - 1) {
        return [" ", line.trimStart()];
      }
      const content = [" ", line.trim()];
      if (isJSDoc && line.endsWith("  ")) {
        return [content, "  ", markAsRoot(literalline)];
      }
      return [content, hardline];
    }),
    "*/",
  ];
}

export { printComment };


Review comment (suggestion):
There's another choice: ignore `  ` after tag (e.g `* @type {any} - blah blah  `).

Code after:
      .trimEnd();
  }

  if (isIndentableBlockComment(comment)) {
    return printIndentableBlockComment(comment);
  }

  if (isBlockComment(comment)) {
    return ["/*", replaceEndOfLine(comment.value), "*/"];
  }

  /* c8 ignore next */
  throw new Error("Not a comment: " + JSON.stringify(comment));
}

/**
@param {Comment} comment
@returns {Doc}
*/
function printIndentableBlockComment(comment) {
  const lines = comment.value.split("\n");
  const isJsdoc = comment.value[0] === "*" && comment.value[1] !== "*";

  return [
    "/*",
    lines.map((line, index) => {
      if (index === 0) {
        return [line.trimEnd(), hardline];
      }

      if (index === lines.length - 1) {
        return [" ", line.trimStart()];
      }

      const trimmed = line.trim();
      const content = [" ", trimmed];
      if (isJsdoc && trimmed !== "*" && line.endsWith("  ")) {
        return [content, "  ", markAsRoot(literalline)];
      }

      return [content, hardline];
    }),
    "*/",
  ];
}

export { printComment };
Repository: prettier/prettier
File: tests/format/js/comments/jsdoc.js (JavaScript)

Code before:
 */
function HelloWorld() {
    return (
        <div>
           <span>Test</span>
        </div>
    );
}

/**
 * Trailing double spaces should be preserverd  
 * because it's significant in markdown.
 *
 * Trailing single space should be ignored. 
 *
 * @type {any}
 */
let a;

/***
 * This is not a JSDoc  
 * so trailing double spaces are ignored
 *
 * @type {any}
 */
let b;


Review comment (suggestion):
Since the character is invisible, please use `snippets` to test  https://github.com/prettier/prettier/blob/main/CONTRIBUTING.md#tests,  and assert output directly.

Code after:
/** @type {any} */
const x = (
    <div>
        <div />
    </div>
);

/**
 * @type {object}
 */
() => (
    <div>
        sajdfpoiasdjfpoiasdjfpoiasdjfpoiadsjfpaoisdjfapsdiofjapioisadfaskfaspiofjp
    </div>
);

/**
 * @type {object}
 */
function HelloWorld() {
    return (
        <div>
           <span>Test</span>
        </div>
    );
}
Repository: prettier/prettier
File: src/language-markdown/acorn/parser.js (JavaScript)

Code before:

    // Make `result.parseResult` non-enumerable, so it won't be processed by `micromark-extension-mdx-expression`
    // https://github.com/micromark/micromark-extension-mdx-expression/blob/2891b75ff9e985c6df208a47348e76ced05dbfed/packages/micromark-util-events-to-acorn/dev/lib/index.js#L124
    return Object.defineProperty(
      {
        start: ast.start,
        end: ast.end,
        ...result,
      },
      "parseResult",
      { value: { ast, comments, text: parseOptions.text } },
    );
  };

const parse = createParse({
  transformArguments: (text, options) => ({ text, options }),
  result: { type: "Program", body: [], isProgram: true },
  parse: (acorn, { text, options }) => acorn.parse(text, options),
});

const parseExpressionAt = createParse({
  transformArguments: (text, position, options) => ({
    text,
    position,
    options,
  }),
  result: { type: "ThisExpression", isExpressionRoot: true },
  parse: (acorn, { text, position, options }) =>
    acorn.parseExpressionAt(text, position, options),
});

export { parse, parseExpressionAt };


Review comment (suggestion):
【Out of curiosity】
Any types for expression work fine? Or is `ThisExpression` is special?
I supposed `JsExpressionRoot` looks suitable once but `wrapExpression()` adds it and I'm not sure whether MDX parser can handle unknown type.

Code after:
    // Make `result.parseResult` non-enumerable, so it won't be processed by `micromark-extension-mdx-expression`
    // https://github.com/micromark/micromark-extension-mdx-expression/blob/2891b75ff9e985c6df208a47348e76ced05dbfed/packages/micromark-util-events-to-acorn/dev/lib/index.js#L124
    return Object.defineProperty(
      {
        start: ast.start,
        end: ast.end,
        ...result,
      },
      "parseResult",
      { value: { ast, comments, text: parseOptions.text } },
    );
  };

const parse = createParse({
  transformArguments: (text, options) => ({ text, options }),
  result: { type: "Program", body: [], isProgram: true },
  parse: (acorn, { text, options }) => acorn.parse(text, options),
});

const parseExpressionAt = createParse({
  transformArguments: (text, position, options) => ({
    text,
    position,
    options,
  }),
  result: { type: "ThisExpression", isExpressionRoot: true },
  parse: (acorn, { text, position, options }) =>
    acorn.parseExpressionAt(text, position, options),
});

export { parse, parseExpressionAt };
Repository: prettier/prettier
File: src/language-js/parentheses/needs-parentheses.js (JavaScript)

Code before:
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      if (
        key === "constraint" &&
        isConditionalType(node) &&
        parent.type === "TSTypeParameter"
      ) {
        return true;
      }

      if (
        key === "typeAnnotation" &&
        isConditionalType(node) &&
        parent.type === "TypeAnnotation" &&
        path.grandparent?.type === "TypeParameter"
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {
          return true;
        }
      }


Review comment (suggestion):
Again, we need to assert the current child. The parent should be bound of `TypeParameter`. And the `?.` is unnecessary, it's impossible that `TypeAnnotation` doesn't have a parent.

Code after:
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      // `const foo = <Foo extends (Bar extends Baz ? A : B)>() => true;`
      //                            ^^^^^^^^^^^^^^^^^^^^^^^
      if (
        (key === "constraint" &&
          node.type === "TSConditionalType" &&
          parent.type === "TSTypeParameter") ||
        (key === "typeAnnotation" &&
          node.type === "ConditionalTypeAnnotation" &&
          parent.type === "TypeAnnotation" &&
          path.grandparent.type === "TypeParameter" &&
          path.grandparent.bound === parent &&
          path.grandparent.usesExtendsBound)
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {
          return true;
        }
      }

    // fallthrough