ronantakizawa/codereview-bench huggingface dataset:
Repository: prettier/prettier
File: src/language-html/utils/index.js (JavaScript)

Code before:
  if (!node.parent) {
    return false;
  }

  if (!node.prev) {
    return false;
  }

  return isPrettierIgnore(node.prev);
}

function isPrettierIgnore(node) {
  return node.type === "comment" && node.value.trim() === "prettier-ignore";
}

/** there's no opening/closing tag or it's considered not breakable */
function isTextLikeNode(node) {
  return node.type === "text" || node.type === "comment";
}

function isScriptLikeTag(node) {
  return (
    node.type === "element" &&
    (node.fullName === "script" ||
      node.fullName === "style" ||
      node.fullName === "mj-style" ||
      node.fullName === "svg:style" ||
      node.fullName === "svg:script" ||
      (isUnknownNamespace(node) &&
        (node.name === "script" || node.name === "style" || node.name === "mj-style")))
  );
}

function canHaveInterpolation(node) {
  return node.children && !isScriptLikeTag(node);
}

function isWhitespaceSensitiveNode(node) {
  return (
    isScriptLikeTag(node) ||
    node.type === "interpolation" ||
    isIndentationSensitiveNode(node)
  );
}

function isIndentationSensitiveNode(node) {
  return getNodeCssStyleWhiteSpace(node).startsWith("pre");
}

function isLeadingSpaceSensitiveNode(node, options) {
  const isLeadingSpaceSensitive = _isLeadingSpaceSensitiveNode();

Review comment (suggestion):
I think it should only work if ~the parser is `mjml`~, sorry, I thought we has this parser..., we should check `filepath` instead.

Code after:
  if (!node.parent) {
    return false;
  }

  if (!node.prev) {
    return false;
  }

  return isPrettierIgnore(node.prev);
}

function isPrettierIgnore(node) {
  return node.type === "comment" && node.value.trim() === "prettier-ignore";
}

/** there's no opening/closing tag or it's considered not breakable */
function isTextLikeNode(node) {
  return node.type === "text" || node.type === "comment";
}

function isScriptLikeTag(node, options) {
  return (
    node.type === "element" &&
    (node.fullName === "script" ||
      node.fullName === "style" ||
      node.fullName === "svg:style" ||
      node.fullName === "svg:script" ||
      (node.fullName === "mj-style" && options.parser === "mjml") ||
      (isUnknownNamespace(node) &&
        (node.name === "script" || node.name === "style")))
  );
}

function canHaveInterpolation(node, options) {
  return node.children && !isScriptLikeTag(node, options);
}

function isWhitespaceSensitiveNode(node, options) {
  return (
    isScriptLikeTag(node, options) ||
    node.type === "interpolation" ||
    isIndentationSensitiveNode(node)
  );
}

function isIndentationSensitiveNode(node) {
  return getNodeCssStyleWhiteSpace(node).startsWith("pre");
}

function isLeadingSpaceSensitiveNode(node, options) {
  const isLeadingSpaceSensitive = _isLeadingSpaceSensitiveNode();
Repository: prettier/prettier
File: src/language-css/loc.js (JavaScript)

Code before:
import isNonEmptyArray from "../utils/is-non-empty-array.js";
import lineColumnToIndex from "../utils/line-column-to-index.js";
import { skipEverythingButNewLine } from "../utils/skip.js";

function fixValueWordLoc(node, originalIndex) {
  if (node.value.startsWith("--")) {
    if (node.value.length > 2) {
      return originalIndex - 2;
    }
  } else if (node.value.startsWith("-") && node.value.length > 1) {
    return originalIndex - 1;
  }
  return originalIndex;
}

function calculateLocStart(node, text) {
  // `postcss>=8`
  if (typeof node.source?.start?.offset === "number") {
    return node.source.start.offset;
  }

  // value-* nodes have this
  if (typeof node.sourceIndex === "number") {
    if (node.type === "value-word") {
      return fixValueWordLoc(node, node.sourceIndex);
    }
    return node.sourceIndex;
  }

  if (node.source?.start) {
    return lineColumnToIndex(node.source.start, text);
  }

  /* c8 ignore next */
  throw Object.assign(new Error("Can not locate node."), { node });

Review comment (suggestion):
Why don't we destruct `value` here. Maybe also return early if `value === '' || !value.startsWith('-')` then we can check `value.charAt(1)` instead.

Code after:
import isNonEmptyArray from "../utils/is-non-empty-array.js";
import lineColumnToIndex from "../utils/line-column-to-index.js";
import { skipEverythingButNewLine } from "../utils/skip.js";

function fixValueWordLoc(node, originalIndex) {
  const { value } = node;
  if (value === "-" || value === "--" || value.charAt(0) !== "-") {
    return originalIndex;
  }
  return originalIndex - (value.charAt(1) === "-" ? 2 : 1);
}

function calculateLocStart(node, text) {
  // `postcss>=8`
  if (typeof node.source?.start?.offset === "number") {
    return node.source.start.offset;
  }

  // value-* nodes have this
  if (typeof node.sourceIndex === "number") {
    if (node.type === "value-word") {
      return fixValueWordLoc(node, node.sourceIndex);
    }
    return node.sourceIndex;
  }

  if (node.source?.start) {
    return lineColumnToIndex(node.source.start, text);
  }

  /* c8 ignore next */
  throw Object.assign(new Error("Can not locate node."), { node });
}

function calculateLocEnd(node, text) {
Repository: prettier/prettier
File: src/language-css/loc.js (JavaScript)

Code before:
import isNonEmptyArray from "../utils/is-non-empty-array.js";
import lineColumnToIndex from "../utils/line-column-to-index.js";
import { skipEverythingButNewLine } from "../utils/skip.js";

function fixValueWordLoc(node, originalIndex) {
  if (node.value.startsWith("--")) {
    if (node.value.length > 2) {
      return originalIndex - 2;
    }
  } else if (node.value.startsWith("-") && node.value.length > 1) {
    return originalIndex - 1;
  }
  return originalIndex;
}

function calculateLocStart(node, text) {
  // `postcss>=8`
  if (typeof node.source?.start?.offset === "number") {
    return node.source.start.offset;
  }

  // value-* nodes have this
  if (typeof node.sourceIndex === "number") {
    if (node.type === "value-word") {
      return fixValueWordLoc(node, node.sourceIndex);
    }
    return node.sourceIndex;
  }

  if (node.source?.start) {
    return lineColumnToIndex(node.source.start, text);
  }

  /* c8 ignore next */
  throw Object.assign(new Error("Can not locate node."), { node });
}

function calculateLocEnd(node, text) {
  if (node.type === "css-comment" && node.inline) {

Review comment (suggestion):
```suggestion
function fixValueWordLoc(node, originalIndex) {
  const {value} = node;
  if (value.charAt(0) !== "-") {
    return originalIndex;
  }
  return originalIndex - (value.charAt(1) === "-" ? 2 : 1);
}


Code after:
import isNonEmptyArray from "../utils/is-non-empty-array.js";
import lineColumnToIndex from "../utils/line-column-to-index.js";
import { skipEverythingButNewLine } from "../utils/skip.js";

function fixValueWordLoc(node, originalIndex) {
  const { value } = node;
  if (value === "-" || value === "--" || value.charAt(0) !== "-") {
    return originalIndex;
  }
  return originalIndex - (value.charAt(1) === "-" ? 2 : 1);
}

function calculateLocStart(node, text) {
  // `postcss>=8`
  if (typeof node.source?.start?.offset === "number") {
    return node.source.start.offset;
  }

  // value-* nodes have this
  if (typeof node.sourceIndex === "number") {
    if (node.type === "value-word") {
      return fixValueWordLoc(node, node.sourceIndex);
    }
    return node.sourceIndex;
  }

  if (node.source?.start) {
    return lineColumnToIndex(node.source.start, text);
  }

  /* c8 ignore next */
  throw Object.assign(new Error("Can not locate node."), { node });
}

function calculateLocEnd(node, text) {
  if (node.type === "css-comment" && node.inline) {
    return skipEverythingButNewLine(text, node.source.startOffset);
  }
Repository: prettier/prettier
File: src/utils/infer-parser.js (JavaScript)

Code before:
  }

  const interpreter = getInterpreter(file);

  if (!interpreter) {
    return;
  }

  return languagesWithInterpreters.find(({ interpreters }) =>
    interpreters.includes(interpreter),
  );
}

/**
 * @param {SupportLanguage[]} languages
 * @param {string | URL | undefined} [file]
 * @returns {SupportLanguage | undefined}
 */
function getLanguageByIsSupported(languages, file) {
  if (!file) {
    return;
  }

  if (
    (file instanceof URL && file.protocol === "file:") ||
    (typeof file === "string" && file.startsWith("file:"))
  ) {
    try {
      file = toPath(file);
    } catch {
      return;
    }
  } else {
    file = String(file);
  }

  return languages.find(({ isSupported }) => isSupported?.({ filepath: file }));
}

/**
 * @param {Options} options
 * @param {{physicalFile?: string | URL | undefined, file?: string | URL | undefined, language?: string | undefined}} fileInfo
 * @returns {string | undefined} matched parser name if found
 */
function inferParser(options, fileInfo) {
  const languages = options.plugins.flatMap(
    (plugin) =>
      // @ts-expect-error -- Safe
      plugin.languages ?? [],
  );


Review comment (suggestion):
nits: Is the following closer to the original meaning?

```suggestion
    (String(file).startsWith("file:"))
```

Code after:
  }

  const interpreter = getInterpreter(file);

  if (!interpreter) {
    return;
  }

  return languagesWithInterpreters.find(({ interpreters }) =>
    interpreters.includes(interpreter),
  );
}

/**
 * @param {SupportLanguage[]} languages
 * @param {string | URL | undefined} [file]
 * @returns {SupportLanguage | undefined}
 */
function getLanguageByIsSupported(languages, file) {
  if (!file) {
    return;
  }

  // Ideally, we should only allow `URL` with `file:` protocol and
  // string starts with `file:`, but `URL` is missing in some environments
  // eg: `node:vm`
  if (String(file).startsWith("file:")) {
    try {
      file = toPath(file);
    } catch {
      return;
    }
  }

  if (typeof file !== "string") {
    return;
  }

  return languages.find(({ isSupported }) => isSupported?.({ filepath: file }));
}

/**
 * @param {Options} options
 * @param {{physicalFile?: string | URL | undefined, file?: string | URL | undefined, language?: string | undefined}} fileInfo
 * @returns {string | undefined} matched parser name if found
 */
function inferParser(options, fileInfo) {
  const languages = options.plugins.flatMap(
    (plugin) =>
      // @ts-expect-error -- Safe
      plugin.languages ?? [],
Repository: prettier/prettier
File: src/utils/infer-parser.js (JavaScript)

Code before:
    return;
  }

  const interpreter = getInterpreter(file);

  if (!interpreter) {
    return;
  }

  return languagesWithInterpreters.find(({ interpreters }) =>
    interpreters.includes(interpreter),
  );
}

/**
 * @param {SupportLanguage[]} languages
 * @param {string | URL | undefined} [file]
 * @returns {SupportLanguage | undefined}
 */
function getLanguageByIsSupported(languages, file) {
  if (!file) {
    return;
  }

  if (
    (typeof URL !== "undefined" && // In `node:vm`, `URL` is not available
      file instanceof URL &&
      file.protocol === "file:") ||
    (typeof file === "string" && file.startsWith("file:"))
  ) {
    try {
      file = toPath(file);
    } catch {
      return;
    }
  }

  if (typeof file !== "string") {
    return;
  }

  return languages.find(({ isSupported }) => isSupported?.({ filepath: file }));
}

/**
 * @param {Options} options
 * @param {{physicalFile?: string | URL | undefined, file?: string | URL | undefined, language?: string | undefined}} fileInfo
 * @returns {string | undefined} matched parser name if found
 */
function inferParser(options, fileInfo) {
  const languages = options.plugins.flatMap(

Review comment (suggestion):
So it would not work when using `import { URL } from 'node:url'`? Then maybe `file.toString()` should still be used.

Code after:
    return;
  }

  const interpreter = getInterpreter(file);

  if (!interpreter) {
    return;
  }

  return languagesWithInterpreters.find(({ interpreters }) =>
    interpreters.includes(interpreter),
  );
}

/**
 * @param {SupportLanguage[]} languages
 * @param {string | URL | undefined} [file]
 * @returns {SupportLanguage | undefined}
 */
function getLanguageByIsSupported(languages, file) {
  if (!file) {
    return;
  }

  // Ideally, we should only allow `URL` with `file:` protocol and
  // string starts with `file:`, but `URL` is missing in some environments
  // eg: `node:vm`
  if (String(file).startsWith("file:")) {
    try {
      file = toPath(file);
    } catch {
      return;
    }
  }

  if (typeof file !== "string") {
    return;
  }

  return languages.find(({ isSupported }) => isSupported?.({ filepath: file }));
}

/**
 * @param {Options} options
 * @param {{physicalFile?: string | URL | undefined, file?: string | URL | undefined, language?: string | undefined}} fileInfo
 * @returns {string | undefined} matched parser name if found
 */
function inferParser(options, fileInfo) {
  const languages = options.plugins.flatMap(
    (plugin) =>
      // @ts-expect-error -- Safe
Repository: prettier/prettier
File: src/common/get-file-info.js (JavaScript)

Code before:
import { resolveConfig } from "../config/resolve-config.js";
import { isIgnored } from "../utils/ignore.js";
import inferParser from "../utils/infer-parser.js";

import {
  clearCache as clearPluginCache,
  loadBuiltinPlugins,
  loadPlugins,
} from "../main/plugins/index.js";

/**
 * @typedef {{ ignorePath?: string | URL | (string | URL)[], withNodeModules?: boolean, plugins: object, resolveConfig?: boolean }} FileInfoOptions
 * @typedef {{ ignored: boolean, inferredParser: string | null }} FileInfoResult
 */

/**
 * @param {string | URL} file
 * @param {FileInfoOptions} options
 * @returns {Promise<FileInfoResult>}
 *
 * Please note that prettier.getFileInfo() expects options.plugins to be an array of paths,
 * not an object.
 */
async function getFileInfo(file, options = {}) {
  if (typeof file !== "string" && !(file instanceof URL)) {
    throw new TypeError(
      `expect \`file\` to be a string or URL, got \`${typeof file}\``,
    );
  }

  let { ignorePath, withNodeModules } = options;
  // In API we allow single `ignorePath`
  if (!Array.isArray(ignorePath)) {
    ignorePath = [ignorePath];
  }

  const ignored = await isIgnored(file, { ignorePath, withNodeModules });
  let inferredParser;
  if (!ignored) {
    inferredParser = options.parser ?? (await getParser(file, options));
  }

  return {

Review comment (bug):
The original comment is incorrect, `withPlugins` doesn't support object type of `plugins`, only standalone version does, but `getFileInfo` is not supported in standalone version.

Code after:
import { resolveConfig } from "../config/resolve-config.js";
import { loadBuiltinPlugins, loadPlugins } from "../main/plugins/index.js";
import { isIgnored } from "../utils/ignore.js";
import inferParser from "../utils/infer-parser.js";

/**
 * @typedef {{ ignorePath?: string | URL | (string | URL)[], withNodeModules?: boolean, plugins: object, resolveConfig?: boolean }} FileInfoOptions
 * @typedef {{ ignored: boolean, inferredParser: string | null }} FileInfoResult
 */

/**
 * @param {string | URL} file
 * @param {FileInfoOptions} options
 * @returns {Promise<FileInfoResult>}
 *
 * Please note that prettier.getFileInfo() expects options.plugins to be an array of paths,
 * not an object.
 */
async function getFileInfo(file, options = {}) {
  if (typeof file !== "string" && !(file instanceof URL)) {
    throw new TypeError(
      `expect \`file\` to be a string or URL, got \`${typeof file}\``,
    );
  }

  let { ignorePath, withNodeModules } = options;
  // In API we allow single `ignorePath`
  if (!Array.isArray(ignorePath)) {
    ignorePath = [ignorePath];
  }

  const ignored = await isIgnored(file, { ignorePath, withNodeModules });
  let inferredParser;
  if (!ignored) {
    inferredParser = options.parser ?? (await getParser(file, options));
  }

  return {
    ignored,
    inferredParser: inferredParser ?? null,
  };
}
Repository: prettier/prettier
File: src/language-js/print/jsx.js (JavaScript)

Code before:
  const shouldBreak = shouldBreakJsxElement(path);
  const needsParens = pathNeedsParens(path, options);

  return group(
    [
      needsParens ? "" : ifBreak("("),
      indent([softline, elem]),
      softline,
      needsParens ? "" : ifBreak(")"),
    ],
    { shouldBreak },
  );
}

function shouldBreakJsxElement(path) {
  const { parent, grandparent } = path;
  if (parent?.type !== "ArrowFunctionExpression") {
    return false;
  }
  if (!isCallExpression(grandparent)) {
    return false;
  }
  const callParent = path.getParentNode(
    path.getParentNode(2).type === "ChainExpression" ? 3 : 2,
  );
  return callParent?.type === "JSXExpressionContainer";
}

function printJsxAttribute(path, options, print) {
  const { node } = path;
  const parts = [];
  parts.push(print("name"));

  if (node.value) {
    let res;
    if (isStringLiteral(node.value)) {
      const raw = getRaw(node.value);
      // Remove enclosing quotes and unescape
      // all quotes so we get an accurate preferred quote
      let final = raw
        .slice(1, -1)
        .replaceAll("&apos;", "'")
        .replaceAll("&quot;", '"');
      const quote = getPreferredQuote(final, options.jsxSingleQuote);
      final =
        quote === '"'
          ? final.replaceAll('"', "&quot;")
          : final.replaceAll("'", "&apos;");
      res = path.call(
        () =>
          printComments(path, replaceEndOfLine(quote + final + quote), options),

Review comment (suggestion):
```suggestion
  if (parent.type !== "ArrowFunctionExpression") {
    return false;
  }
  if (!isCallExpression(grandparent)) {
    return false;
  }
  const callParent = path.getParentNode(
    grandparent.type === "ChainExpression" ? 3 : 2,
  );
  return callParent.type === "JSXExpressionContainer";
```

Code after:
  const shouldBreak = shouldBreakJsxElement(path);
  const needsParens = pathNeedsParens(path, options);

  return group(
    [
      needsParens ? "" : ifBreak("("),
      indent([softline, elem]),
      softline,
      needsParens ? "" : ifBreak(")"),
    ],
    { shouldBreak },
  );
}

function shouldBreakJsxElement(path) {
  return (
    path.match(
      undefined,
      (node) => node.type === "ArrowFunctionExpression",
      isCallExpression,
    ) &&
    // Babel
    (path.match(
      undefined,
      undefined,
      undefined,
      (node) => node.type === "JSXExpressionContainer",
    ) ||
      // Estree
      path.match(
        undefined,
        undefined,
        undefined,
        (node) => node.type === "ChainExpression",
        (node) => node.type === "JSXExpressionContainer",
      ))
  );
}

function printJsxAttribute(path, options, print) {
  const { node } = path;
  const parts = [];
  parts.push(print("name"));

  if (node.value) {
    let res;
    if (isStringLiteral(node.value)) {
      const raw = getRaw(node.value);
      // Remove enclosing quotes and unescape
      // all quotes so we get an accurate preferred quote
      let final = raw
Repository: prettier/prettier
File: src/language-js/print/jsx.js (JavaScript)

Code before:

const NO_WRAP_PARENTS = new Set([
  "ArrayExpression",
  "JSXAttribute",
  "JSXElement",
  "JSXExpressionContainer",
  "JSXFragment",
  "ExpressionStatement",
  "CallExpression",
  "OptionalCallExpression",
  "ConditionalExpression",
  "JsExpressionRoot",
]);
function maybeWrapJsxElementInParens(path, elem, options) {
  const { parent } = path;

  if (NO_WRAP_PARENTS.has(parent.type)) {
    return elem;
  }

  const shouldBreak = shouldBreakJsxElement(path);
  const needsParens = pathNeedsParens(path, options);

  return group(
    [
      needsParens ? "" : ifBreak("("),
      indent([softline, elem]),
      softline,
      needsParens ? "" : ifBreak(")"),
    ],
    { shouldBreak },
  );
}

function shouldBreakJsxElement(path) {
  const { parent, grandparent } = path;
  if (parent?.type !== "ArrowFunctionExpression") {
    return false;
  }
  if (!isCallExpression(grandparent)) {
    return false;
  }
  const callParent = path.getParentNode(
    path.getParentNode(2).type === "ChainExpression" ? 3 : 2,
  );
  return callParent?.type === "JSXExpressionContainer";
}

function printJsxAttribute(path, options, print) {
  const { node } = path;
  const parts = [];

Review comment (bug):
This seems incorrect, it only match

```
{a.b(() => <div/>)}
```

```
{a?.b(() => <div/>)}
```

```
{a.b?.(() => <div/>)}
```

But not longer chain?

Code after:

const NO_WRAP_PARENTS = new Set([
  "ArrayExpression",
  "JSXAttribute",
  "JSXElement",
  "JSXExpressionContainer",
  "JSXFragment",
  "ExpressionStatement",
  "CallExpression",
  "OptionalCallExpression",
  "ConditionalExpression",
  "JsExpressionRoot",
]);
function maybeWrapJsxElementInParens(path, elem, options) {
  const { parent } = path;

  if (NO_WRAP_PARENTS.has(parent.type)) {
    return elem;
  }

  const shouldBreak = shouldBreakJsxElement(path);
  const needsParens = pathNeedsParens(path, options);

  return group(
    [
      needsParens ? "" : ifBreak("("),
      indent([softline, elem]),
      softline,
      needsParens ? "" : ifBreak(")"),
    ],
    { shouldBreak },
  );
}

function shouldBreakJsxElement(path) {
  return (
    path.match(
      undefined,
      (node) => node.type === "ArrowFunctionExpression",
      isCallExpression,
    ) &&
    // Babel
    (path.match(
      undefined,
      undefined,
      undefined,
      (node) => node.type === "JSXExpressionContainer",
    ) ||
      // Estree
      path.match(
        undefined,
Repository: prettier/prettier
File: tests/integration/plugins/preprocess/plugin.cjs (JavaScript)

Code before:
"use strict";

module.exports = {
  languages: [
    {
      name: "foo",
      parsers: ["foo-parser"],
      extensions: [".foo"],
      since: "1.0.0",
    },
  ],
  parsers: {
    "foo-parser": {
      preprocess: (text) => Promise.resolve(`preprocessed:${text}`),
      parse: (text) => ({ text }),
      astFormat: "foo-ast",
    },
  },
  printers: {
    "foo-ast": {
      print: ({ node }) => node.text,
    },
  },
};


Review comment (suggestion):
If we don't have a test for non-promise besides this one, I think we should add a new test instead of modifying this one.

Code after:
"use strict";

module.exports = {
  languages: [
    {
      name: "foo",
      parsers: ["foo-parser"],
      extensions: [".foo"],
      since: "1.0.0",
    },
  ],
  parsers: {
    "foo-parser": {
      preprocess: (text) => `preprocessed:${text}`,
      parse: (text) => ({ text }),
      astFormat: "foo-ast",
    },
  },
  printers: {
    "foo-ast": {
      print: ({ node }) => node.text,
    },
  },
};