ronantakizawa/codereview-bench huggingface dataset:
Repository: babel/babel
File: Makefile.source.mjs (JavaScript)

Code before:
}

function node(args, cwd, inheritStdio) {
  return exec(NODE_PATH, args, cwd, inheritStdio);
}

function env(fun, env) {
  const envBak = process.env;
  process.env = { ...envBak, ...env };
  fun();
  process.env = envBak;
}

/**
 * CLEAN
 */

target["clean-all"] = function () {
  shell.rm("-rf", ["package-lock.json", ".changelog"]);

  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/test/tmp`);
    shell.rm("-rf", `${source}/*/package-lock.json`);
  });

  target.clean();
  target["clean-lib"]();
  target["clean-node-modules"]();
};

target["clean-node-modules"] = function () {
  shell.rm("-rf", "node_modules");
  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/node_modules`);
  });
};

target.clean = function () {
  target["test-clean"]();

  shell.rm("-rf", [
    ".npmrc",
    "coverage",
    "packages/*/npm-debug*",
    "node_modules/.cache",
  ]);
};

target["test-clean"] = function () {
  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/test/tmp`);

Review comment (suggestion):
Could we disable the rule in this file? `target` is used more like a map, and I quite like that we are using string-keyed keys with it.

Code after:
}

function node(args, cwd, inheritStdio) {
  return exec(NODE_PATH, args, cwd, inheritStdio);
}

function env(fun, env) {
  const envBak = process.env;
  process.env = { ...envBak, ...env };
  fun();
  process.env = envBak;
}

/**
 * CLEAN
 */

target["clean-all"] = function () {
  shell.rm("-rf", ["package-lock.json", ".changelog"]);

  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/test/tmp`);
    shell.rm("-rf", `${source}/*/package-lock.json`);
  });

  target["clean"]();
  target["clean-lib"]();
  target["clean-node-modules"]();
};

target["clean-node-modules"] = function () {
  shell.rm("-rf", "node_modules");
  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/node_modules`);
  });
};

target["clean"] = function () {
  target["test-clean"]();

  shell.rm("-rf", [
    ".npmrc",
    "coverage",
    "packages/*/npm-debug*",
    "node_modules/.cache",
  ]);
};

target["test-clean"] = function () {
  SOURCES.forEach(source => {
    shell.rm("-rf", `${source}/*/test/tmp`);
Repository: babel/babel
File: scripts/integration-tests/e2e-angular-cli.sh (Shell)

Code before:
cd "$(dirname "$0")" || exit

dir="$PWD"

source utils/local-registry.sh
source utils/cleanup.sh

# Echo every command being executed
set -x

#==============================================================================#
#                                   TEST                                       #
#==============================================================================#

startLocalRegistry "$PWD"/verdaccio-config.yml

# Create and build a new angular project
mkdir tmp && cd tmp
npx -p @angular/cli ng new --defaults ngx --package-manager yarn --skip-git --skip-install
cd ngx
node "$dir"/utils/bump-babel-dependencies.js resolutions
touch yarn.lock
yarn set version stable
YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn
# Remove this when https://github.com/fb55/css-select/issues/1591 is fixed
yarn add css-select@5.1.0
yarn run build
yarn run ng test --watch=false --browsers ChromeHeadless

cleanup


Review comment (suggestion):
```suggestion
```
This issue has been fixed upstream.

Code after:
cd "$(dirname "$0")" || exit

dir="$PWD"

source utils/local-registry.sh
source utils/cleanup.sh

# Echo every command being executed
set -x

#==============================================================================#
#                                   TEST                                       #
#==============================================================================#

startLocalRegistry "$PWD"/verdaccio-config.yml

# Create and build a new angular project
mkdir tmp && cd tmp
npx -p @angular/cli ng new --defaults ngx --package-manager yarn --skip-git --skip-install
cd ngx
node "$dir"/utils/bump-babel-dependencies.js resolutions
touch yarn.lock
yarn set version stable
YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn
yarn run build
yarn run ng test --watch=false --browsers ChromeHeadless

cleanup
Repository: babel/babel
File: packages/babel-standalone/src/transformScriptTags.ts (TypeScript)

Code before:
 * Builds the Babel options for transforming the specified script, using some
 * sensible default presets and plugins if none were explicitly provided.
 */
function buildBabelOptions(script: CompilationResult, filename: string) {
  let presets = script.presets;
  if (!presets) {
    if (script.type === "module") {
      presets = [
        "react",
        [
          "env",
          {
            targets: {
              esmodules: true,
            },
            modules: false,
          },
        ],
      ];
    } else {
      presets = ["react", "env"];
    }
  }

  let plugins = script.plugins;
  if (!plugins && process.env.BABEL_8_BREAKING) {
    plugins = [
      "transform-class-properties",
      "transform-object-rest-spread",
      "transform-flow-strip-types",
    ];
  }

  return {
    filename,
    presets,
    plugins,
    sourceMaps: "inline" as const,
    sourceFileName: filename,
  };
}

/**
 * Appends a script element at the end of the <head> with the content of code,
 * after transforming it.
 */
function run(transformFn: typeof transform, script: CompilationResult) {
  const scriptEl = document.createElement("script");
  if (script.type) {
    scriptEl.setAttribute("type", script.type);
  }

Review comment (suggestion):
```suggestion
  if (!process.env.BABEL_8_BREAKING && !plugins) {
```

The `BABEL_8_BREAKING` flag will be true for the Babel 8 release, we would like the plugin inject logic for Babel 7 only.

Code after:
 * Builds the Babel options for transforming the specified script, using some
 * sensible default presets and plugins if none were explicitly provided.
 */
function buildBabelOptions(script: CompilationResult, filename: string) {
  let presets = script.presets;
  if (!presets) {
    if (script.type === "module") {
      presets = [
        "react",
        [
          "env",
          {
            targets: {
              esmodules: true,
            },
            modules: false,
          },
        ],
      ];
    } else {
      presets = ["react", "env"];
    }
  }

  let plugins = script.plugins;
  if (!process.env.BABEL_8_BREAKING && !plugins) {
    plugins = [
      "transform-class-properties",
      "transform-object-rest-spread",
      "transform-flow-strip-types",
    ];
  }

  return {
    filename,
    presets,
    plugins,
    sourceMaps: "inline" as const,
    sourceFileName: filename,
  };
}

/**
 * Appends a script element at the end of the <head> with the content of code,
 * after transforming it.
 */
function run(transformFn: typeof transform, script: CompilationResult) {
  const scriptEl = document.createElement("script");
  if (script.type) {
    scriptEl.setAttribute("type", script.type);
  }
Repository: babel/babel
File: packages/babel-core/src/transformation/normalize-opts.ts (TypeScript)

Code before:

    // @ts-expect-error We should have passed `sourceFilename` here
    // pending https://github.com/babel/babel/issues/15917#issuecomment-2789278964
    sourceFileName: filename,
    plugins: [],
    ...opts.parserOpts,
  };

  const options: NormalizedOptions = {
    ...opts,

    parserOpts: parserOpts,

    generatorOpts: {
      // General generator flags.
      filename,

      auxiliaryCommentBefore: opts.auxiliaryCommentBefore,
      auxiliaryCommentAfter: opts.auxiliaryCommentAfter,
      retainLines: opts.retainLines,
      comments,
      shouldPrintComment: opts.shouldPrintComment,
      compact,
      minified: opts.minified,

      // Source-map generation flags.
      // babel-generator does not differentiate between `true`, `"inline"` or `"both"`
      sourceMaps: !!sourceMaps,
      sourceRoot,
      sourceFileName,

      ...opts.generatorOpts,
    },
  };

  for (const plugins of config.passes) {
    for (const plugin of plugins) {
      if (plugin.manipulateOptions) {
        plugin.manipulateOptions(options, options.parserOpts);
      }
    }
  }

  return options;
}


Review comment (suggestion):
Here we cast `generatorOpts.sourceMaps` to boolean as required by the `@babel/generator`.

Code after:
    parserOpts: {
      sourceType:
        path.extname(filenameRelative) === ".mjs" ? "module" : sourceType,

      // @ts-expect-error We should have passed `sourceFilename` here
      // pending https://github.com/babel/babel/issues/15917#issuecomment-2789278964
      sourceFileName: filename,
      plugins: [],
      ...opts.parserOpts,
    },

    generatorOpts: {
      // General generator flags.
      filename,

      auxiliaryCommentBefore: opts.auxiliaryCommentBefore,
      auxiliaryCommentAfter: opts.auxiliaryCommentAfter,
      retainLines: opts.retainLines,
      comments,
      shouldPrintComment: opts.shouldPrintComment,
      compact,
      minified: opts.minified,

      // Source-map generation flags.
      // babel-generator does not differentiate between `true`, `"inline"` or `"both"`
      sourceMaps: !!sourceMaps,
      sourceRoot,
      sourceFileName,

      ...opts.generatorOpts,
    },
  };

  for (const plugins of config.passes) {
    for (const plugin of plugins) {
      if (plugin.manipulateOptions) {
        plugin.manipulateOptions(options, options.parserOpts);
      }
    }
  }

  return options;
}
Repository: babel/babel
File: packages/babel-traverse/src/path/modification.ts (TypeScript)

Code before:

export function updateSiblingKeys(
  this: NodePath,
  fromIndex: number,
  incrementBy: number,
) {
  if (!this.parent) return;

  const paths = getCachedPaths(this);
  if (!paths) return;

  for (const [, path] of paths) {
    if (
      typeof path.key === "number" &&
      path.container === this.container &&
      path.key >= fromIndex
    ) {
      path.key += incrementBy;
    }
  }
}

export function _verifyNodeList<N extends t.Node | null>(
  this: NodePath,
  nodes: N | N[],
) {
  if (!nodes) {
    return [];
  }

  if (!Array.isArray(nodes)) {
    nodes = [nodes];
  }

  for (let i = 0; i < nodes.length; i++) {
    const node = nodes[i];
    let msg;

    if (!node) {
      msg = "has falsy node";
    } else if (typeof node !== "object") {
      msg = "contains a non-object node";
    } else if (!node.type) {
      msg = "without a type";
    } else if (node instanceof NodePath) {
      msg = "has a NodePath when it expected a raw object";
    }

    if (msg) {
      const type = Array.isArray(node) ? "array" : typeof node;
      throw new Error(

Review comment (suggestion):
The `_verifyNodeList` currently does not accept `null` in `nodes`. However, I think we should eventually accept `null`, because `null` represents a hole / empty pattern in `ArrayExpression#elements` or `ArrayPattern#elements`. The API should not have prevent users from adding them to the `elements`.

Code after:
/**
 * Update all sibling node paths after `fromIndex` by `incrementBy`.
 */

export function updateSiblingKeys(
  this: NodePath,
  fromIndex: number,
  incrementBy: number,
) {
  if (!this.parent) return;

  const paths = getCachedPaths(this);
  if (!paths) return;

  for (const [, path] of paths) {
    if (
      typeof path.key === "number" &&
      path.container === this.container &&
      path.key >= fromIndex
    ) {
      path.key += incrementBy;
    }
  }
}

export function _verifyNodeList<N extends t.Node | null>(
  this: NodePath,
  nodes: N | N[],
) {
  if (!nodes) {
    return [];
  }

  if (!Array.isArray(nodes)) {
    nodes = [nodes];
  }

  for (let i = 0; i < nodes.length; i++) {
    const node = nodes[i];
    let msg;

    if (!node) {
      msg = "has falsy node";
    } else if (typeof node !== "object") {
      msg = "contains a non-object node";
    } else if (!node.type) {
      msg = "without a type";
    } else if (node instanceof NodePath) {
      msg = "has a NodePath when it expected a raw object";
    }
Repository: babel/babel
File: packages/babel-generator/src/token-map.ts (TypeScript)

Code before:
import type { Token } from "@babel/parser";

import { traverseFast, VISITOR_KEYS } from "@babel/types";

export class TokenMap {
  _tokens: Token[];
  _source: string;

  _nodesToTokenIndexes: Map<t.Node, number[]> = new Map();
  _nodesOccurrencesCountCache: Map<
    t.Node,
    { test: string; count: number; i: number }
  > = new Map();

  _tokensCache = new Map<t.Node, { first: number; last: number }>();

  constructor(ast: t.Node, tokens: Token[], source: string) {
    this._tokens = tokens;
    this._source = source;

    traverseFast(ast, node => {
      const indexes = this._getTokensIndexesOfNode(node);
      if (indexes.length > 0) this._nodesToTokenIndexes.set(node, indexes);
    });
  }

  has(node: t.Node): boolean {
    return this._nodesToTokenIndexes.has(node);
  }

  getIndexes(node: t.Node): readonly number[] | undefined {
    return this._nodesToTokenIndexes.get(node);
  }

  find(
    node: t.Node,
    condition: (token: Token, index: number) => boolean,
  ): Token | null {
    const indexes = this._nodesToTokenIndexes.get(node);
    if (indexes) {
      for (let k = 0; k < indexes.length; k++) {
        const index = indexes[k];
        const tok = this._tokens[index];
        if (condition(tok, index)) return tok;
      }
    }
    return null;
  }

  findLastIndex(
    node: t.Node,

Review comment (performance):
Lets keep a `this._tokensCache.clear()` at least? To free up memory.

Code after:
import type { Token } from "@babel/parser";

import { traverseFast, VISITOR_KEYS } from "@babel/types";

export class TokenMap {
  _tokens: Token[];
  _source: string;

  _nodesToTokenIndexes: Map<t.Node, number[]> = new Map();
  _nodesOccurrencesCountCache: Map<
    t.Node,
    { test: string; count: number; i: number }
  > = new Map();

  _tokensCache = new Map<t.Node, { first: number; last: number }>();

  constructor(ast: t.Node, tokens: Token[], source: string) {
    this._tokens = tokens;
    this._source = source;

    traverseFast(ast, node => {
      const indexes = this._getTokensIndexesOfNode(node);
      if (indexes.length > 0) this._nodesToTokenIndexes.set(node, indexes);
    });

    this._tokensCache.clear();
  }

  has(node: t.Node): boolean {
    return this._nodesToTokenIndexes.has(node);
  }

  getIndexes(node: t.Node): readonly number[] | undefined {
    return this._nodesToTokenIndexes.get(node);
  }

  find(
    node: t.Node,
    condition: (token: Token, index: number) => boolean,
  ): Token | null {
    const indexes = this._nodesToTokenIndexes.get(node);
    if (indexes) {
      for (let k = 0; k < indexes.length; k++) {
        const index = indexes[k];
        const tok = this._tokens[index];
        if (condition(tok, index)) return tok;
      }
    }
    return null;
  }
Repository: babel/babel
File: packages/babel-generator/src/node/whitespace.ts (TypeScript)

Code before:
  return (
    isLiteral(node) ||
    isObjectExpression(node) ||
    isArrayExpression(node) ||
    isIdentifier(node) ||
    isMemberExpression(node)
  );
}

/**
 * Tests for node types that need whitespace.
 */

export const nodes: NodeHandlers<WhitespaceFlag> = {
  /**
   * Test if AssignmentExpression needs whitespace.
   */

  AssignmentExpression(node: t.AssignmentExpression): WhitespaceFlag {
    const state = crawl(node.right);
    if ((state.hasCall && state.hasHelper) || state.hasFunction) {
      return state.hasFunction
        ? WhitespaceFlag.before | WhitespaceFlag.after
        : WhitespaceFlag.after;
    }
    return 0;
  },

  /**
   * Test if SwitchCase needs whitespace.
   */

  SwitchCase(node: t.SwitchCase, parent: t.SwitchStatement): WhitespaceFlag {
    return (
      (!!node.consequent.length || parent.cases[0] === node
        ? WhitespaceFlag.before
        : 0) |
      (!node.consequent.length && parent.cases[parent.cases.length - 1] === node
        ? WhitespaceFlag.after
        : 0)
    );
  },

  /**
   * Test if LogicalExpression needs whitespace.
   */

  LogicalExpression(node: t.LogicalExpression): WhitespaceFlag {
    if (isFunction(node.left) || isFunction(node.right)) {
      return WhitespaceFlag.after;
    }

Review comment (suggestion):

```suggestion
    return WhitespaceFlag.none;
```

and in other places

Code after:
  return (
    isLiteral(node) ||
    isObjectExpression(node) ||
    isArrayExpression(node) ||
    isIdentifier(node) ||
    isMemberExpression(node)
  );
}

/**
 * Tests for node types that need whitespace.
 */

export const nodes: NodeHandlers<WhitespaceFlag> = {
  /**
   * Test if AssignmentExpression needs whitespace.
   */

  AssignmentExpression(node: t.AssignmentExpression): WhitespaceFlag {
    const state = crawl(node.right);
    if ((state.hasCall && state.hasHelper) || state.hasFunction) {
      return state.hasFunction
        ? WhitespaceFlag.before | WhitespaceFlag.after
        : WhitespaceFlag.after;
    }
    return WhitespaceFlag.none;
  },

  /**
   * Test if SwitchCase needs whitespace.
   */

  SwitchCase(node: t.SwitchCase, parent: t.SwitchStatement): WhitespaceFlag {
    return (
      (!!node.consequent.length || parent.cases[0] === node
        ? WhitespaceFlag.before
        : WhitespaceFlag.none) |
      (!node.consequent.length && parent.cases[parent.cases.length - 1] === node
        ? WhitespaceFlag.after
        : WhitespaceFlag.none)
    );
  },

  /**
   * Test if LogicalExpression needs whitespace.
   */

  LogicalExpression(node: t.LogicalExpression): WhitespaceFlag {
    if (isFunction(node.left) || isFunction(node.right)) {
      return WhitespaceFlag.after;
    }
Repository: babel/babel
File: packages/babel-parser/src/plugins/jsx/xhtml.ts (TypeScript)

Code before:
const entities: { [name: string]: string } = {
  // @ts-expect-error expected
  __proto__: null,
  quot: "\u0022",
  amp: "&",
  apos: "\u0027",
  lt: "<",
  gt: ">",
  nbsp: "\u00A0",
  iexcl: "\u00A1",
  cent: "\u00A2",
  pound: "\u00A3",
  curren: "\u00A4",
  yen: "\u00A5",
  brvbar: "\u00A6",
  sect: "\u00A7",
  uml: "\u00A8",
  copy: "\u00A9",
  ordf: "\u00AA",
  laquo: "\u00AB",
  not: "\u00AC",
  shy: "\u00AD",
  reg: "\u00AE",
  macr: "\u00AF",
  deg: "\u00B0",
  plusmn: "\u00B1",
  sup2: "\u00B2",

Review comment (suggestion):
```suggestion
  // @ts-expect-error __proto__ is not an actual property: https://github.com/microsoft/TypeScript/issues/38385
```

Code after:
const entities: { [name: string]: string } = {
  // @ts-expect-error __proto__ is not an actual property: https://github.com/microsoft/TypeScript/issues/38385
  __proto__: null,
  quot: "\u0022",
  amp: "&",
  apos: "\u0027",
  lt: "<",
  gt: ">",
  nbsp: "\u00A0",
  iexcl: "\u00A1",
  cent: "\u00A2",
  pound: "\u00A3",
  curren: "\u00A4",
  yen: "\u00A5",
  brvbar: "\u00A6",
  sect: "\u00A7",
  uml: "\u00A8",
  copy: "\u00A9",
  ordf: "\u00AA",
  laquo: "\u00AB",
  not: "\u00AC",
  shy: "\u00AD",
  reg: "\u00AE",
  macr: "\u00AF",
  deg: "\u00B0",
  plusmn: "\u00B1",
  sup2: "\u00B2",
Repository: babel/babel
File: packages/babel-traverse/src/path/index.ts (TypeScript)

Code before:
  ): asserts this is NodePath_Final<
    (
      | t.Loop
      | t.WithStatement
      | t.Function
      | t.LabeledStatement
      | t.CatchClause
    ) & { body: t.BlockStatement }
  >;
  /**
   * @see ./introspection.ts for implementation.
   */
  isStatementOrBlock(
    this: NodePath_Final,
  ): this is NodePath_Final<t.Statement | t.Block>;
}

type NodePathMixins = Omit<typeof methods, keyof NodePathOverwrites>;

interface NodePath<
  N extends t.Node | null,
  T extends t.Node["type"] = NonNullable<N>["type"],
> extends InstanceType<typeof NodePath_Final>,
    NodePathAssertions,
    NodePathValidators,
    NodePathMixins,
    NodePathOverwrites {
  type: T | null;
  node: N;
  // .parent is only null for File nodes, which are not traversed by @babel/traverse
  // You can technically create a path that contains one, but it's so rare that
  // we can ignore it to avoid having non-null assertions everywhere.
  parent: NonNullable<t.ParentMaps[T]>;
  parentPath: NodePath_Final<NonNullable<t.ParentMaps[T]>>;
}

// This trick is necessary so that
// NodePath_Final<A | B> is the same as NodePath_Final<A> | NodePath_Final<B>
type NodePath_Final<T extends t.Node | null = t.Node> = T extends any
  ? NodePath<T>
  : never;

export { NodePath_Final as default, type NodePath as NodePath_Internal };


Review comment (suggestion):
`NodePath<null>` was introduced here, but it was not used more in `path.get` because it would be very cumbersome to use if we used strict null and non-null.
Suggestions welcome!

Code after:
  ): asserts this is NodePath_Final<
    (
      | t.Loop
      | t.WithStatement
      | t.Function
      | t.LabeledStatement
      | t.CatchClause
    ) & { body: t.BlockStatement }
  >;
  /**
   * @see ./introspection.ts for implementation.
   */
  isStatementOrBlock(
    this: NodePath_Final,
  ): this is NodePath_Final<t.Statement | t.Block>;
}

type NodePathMixins = Omit<typeof methods, keyof NodePathOverwrites>;

interface NodePath<
  N extends t.Node | null,
  T extends t.Node["type"] | null = N extends null
    ? null
    : NonNullable<N>["type"],
  P extends t.Node = NonNullable<t.ParentMaps[NonNullable<T>]>,
> extends InstanceType<typeof NodePath_Final>,
    NodePathAssertions,
    NodePathValidators,
    NodePathMixins,
    NodePathOverwrites {
  type: T;
  node: N;
  // .parent is only null for File nodes, which are not traversed by @babel/traverse
  // You can technically create a path that contains one, but it's so rare that
  // we can ignore it to avoid having non-null assertions everywhere.
  parent: P;
  parentPath: NodePath_Final<P>;
}

// This trick is necessary so that
// NodePath_Final<A | B> is the same as NodePath_Final<A> | NodePath_Final<B>
type NodePath_Final<T extends t.Node | null = t.Node> = T extends any
  ? NodePath<T>
  : never;

export { NodePath_Final as default, type NodePath as NodePath_Internal };