ronantakizawa/codereview-bench huggingface dataset:
Repository: babel/babel
File: packages/babel-traverse/test/evaluation.js (JavaScript)

Code before:
        },
      },
    });

    expect(result.confident).toBe(true);
    expect(result.deopt).toBeNull();
    expect(result.value).toEqual(["foo", "bar"]);
  });

  it("should not evaluate vars in child scope", function () {
    const path = getPath(`
      if (typeof Bar != "undefined") {
        var doesExist = true;
      }
      doesExist;
    `);
    const evalResult = path.get("body.1.expression").evaluate();
    expect(evalResult.confident).toBe(false);
  });

  it("should not evaluate vars in child scope 2", function () {
    const path = getPath(`
      {
        var doesExist = true;
        doesExist;
      }
    `);
    const evalResult = path.get("body.0.body.1.expression").evaluate();
    expect(evalResult.confident).toBe(true);
  });

  addDeoptTest("({a:{b}})", "ObjectExpression", "Identifier");
  addDeoptTest("({[a + 'b']: 1})", "ObjectExpression", "Identifier");
  addDeoptTest("[{a}]", "ArrayExpression", "Identifier");
});


Review comment (suggestion):
Can you also add these test cases?
```js
var doesExist = true;
{ doesExist }
```

```js
{
  var doesExist = true;
  { doesExist }
}
```

```js
{ var doesExist = true;
doesExist
```
(this last one _could_ return a confident result, but it's not a big deal if it does not)

Code after:
        },
      },
    });

    expect(result.confident).toBe(true);
    expect(result.deopt).toBeNull();
    expect(result.value).toEqual(["foo", "bar"]);
  });

  it("should not evaluate vars in child scope", function () {
    const path = getPath(`
      if (typeof Bar != "undefined") {
        var doesExist = true;
      }
      doesExist;
    `);
    const evalResult = path.get("body.1.expression").evaluate();
    expect(evalResult.confident).toBe(false);
  });

  it("should not evaluate vars in child scope 2", function () {
    const path = getPath(`
      {
        var doesExist = true;
        doesExist;
      }
    `);
    const evalResult = path.get("body.0.body.1.expression").evaluate();
    expect(evalResult.confident).toBe(true);
  });

  it("should not evaluate vars in child scope 3", function () {
    const path = getPath(`
      var doesExist = true;
      { doesExist }
    `);
    const evalResult = path.get("body.1.body.0.expression").evaluate();
    expect(evalResult.confident).toBe(true);
  });

  it("should not evaluate vars in child scope 4", function () {
    const path = getPath(`
      {
        var doesExist = true;
        { doesExist }
      }
    `);
    const evalResult = path.get("body.0.body.1.body.0.expression").evaluate();
    expect(evalResult.confident).toBe(true);
  });
Repository: babel/babel
File: packages/babel-traverse/src/path/evaluation.ts (TypeScript)

Code before:
    const binding = path.scope.getBinding(path.node.name);

    if (binding) {
      if (
        binding.constantViolations.length > 0 ||
        path.node.start < binding.path.node.end
      ) {
        deopt(binding.path, state);
        return;
      }
      if (binding.kind === "var" && binding.path.scope !== binding.scope) {
        let hasIf = binding.path.scope.path.parentPath.isIfStatement();
        for (
          let scope = binding.path.scope.parent;
          scope;
          scope = scope.parent
        ) {
          if (scope === path.scope) {
            if (hasIf) {
              deopt(binding.path, state);
              return;
            }
            break;
          }
          if (scope.path.parentPath?.isIfStatement()) {
            hasIf = true;
          }
        }
      }
      if (binding.hasValue) {
        return binding.value;
      }
    }

    const name = path.node.name;
    if (Globals.has(name)) {
      if (!binding) {
        return Globals.get(name);
      }
      deopt(binding.path, state);
      return;
    }

    const resolved = path.resolve();
    if (resolved === path) {
      deopt(path, state);
      return;
    } else {
      return evaluateCached(resolved, state);
    }
  }

Review comment (suggestion):
Also while, for, for-of, try-catch. I'd prefer to list "known to be safe" statements (i.e. do-while and block), rather than potentially unsafe ones.

Code after:
    const binding = path.scope.getBinding(path.node.name);

    if (binding) {
      if (
        binding.constantViolations.length > 0 ||
        path.node.start < binding.path.node.end
      ) {
        deopt(binding.path, state);
        return;
      }
      const bindingPathScope = binding.path.scope;
      if (binding.kind === "var" && bindingPathScope !== binding.scope) {
        let hasUnsafeBlock =
          !bindingPathScope.path.parentPath.isBlockStatement();
        for (let scope = bindingPathScope.parent; scope; scope = scope.parent) {
          if (scope === path.scope) {
            if (hasUnsafeBlock) {
              deopt(binding.path, state);
              return;
            }
            break;
          }
          if (scope.path.parentPath?.isBlockStatement()) {
            hasUnsafeBlock = true;
          }
        }
      }
      if (binding.hasValue) {
        return binding.value;
      }
    }

    const name = path.node.name;
    if (Globals.has(name)) {
      if (!binding) {
        return Globals.get(name);
      }
      deopt(binding.path, state);
      return;
    }

    const resolved = path.resolve();
    if (resolved === path) {
      deopt(path, state);
      return;
    } else {
      return evaluateCached(resolved, state);
    }
  }

  if (path.isUnaryExpression({ prefix: true })) {
Repository: babel/babel
File: packages/babel-traverse/src/scope/binding.ts (TypeScript)

Code before:
  scope: Scope;
  path: NodePath;
  kind: BindingKind;

  constructor({
    identifier,
    scope,
    path,
    kind,
  }: {
    identifier: t.Identifier;
    scope: Scope;
    path: NodePath;
    kind: BindingKind;
  }) {
    this.identifier = identifier;
    this.scope = scope;
    this.path = path;
    this.kind = kind;

    if ((kind === "var" || kind === "hoisted") && isDeclaredInLoop(path)) {
      if (
        !path.isVariableDeclarator() ||
        (path.parentPath.parentPath.isForXStatement() &&
          path.parentPath.parentKey === "left") ||
        path.node.init
      ) {
        this.reassign(path);
      }
    }

    this.clearValue();
  }

  constantViolations: Array<NodePath> = [];
  constant: boolean = true;

  referencePaths: Array<NodePath> = [];
  referenced: boolean = false;
  references: number = 0;

  declare hasDeoptedValue: boolean;
  declare hasValue: boolean;
  declare value: any;

  deoptValue() {
    this.clearValue();
    this.hasDeoptedValue = true;
  }

  setValue(value: any) {

Review comment (refactor):
Can we move this check into `isDeclaredInLoop`? Since it already performed the ForXStatement check there.

Code after:
  scope: Scope;
  path: NodePath;
  kind: BindingKind;

  constructor({
    identifier,
    scope,
    path,
    kind,
  }: {
    identifier: t.Identifier;
    scope: Scope;
    path: NodePath;
    kind: BindingKind;
  }) {
    this.identifier = identifier;
    this.scope = scope;
    this.path = path;
    this.kind = kind;

    if ((kind === "var" || kind === "hoisted") && isInitInLoop(path)) {
      this.reassign(path);
    }

    this.clearValue();
  }

  constantViolations: Array<NodePath> = [];
  constant: boolean = true;

  referencePaths: Array<NodePath> = [];
  referenced: boolean = false;
  references: number = 0;

  declare hasDeoptedValue: boolean;
  declare hasValue: boolean;
  declare value: any;

  deoptValue() {
    this.clearValue();
    this.hasDeoptedValue = true;
  }

  setValue(value: any) {
    if (this.hasDeoptedValue) return;
    this.hasValue = true;
    this.value = value;
  }

  clearValue() {
    this.hasDeoptedValue = false;
Repository: babel/babel
File: packages/babel-types/src/traverse/traverseFast.ts (TypeScript)

Code before:
import { VISITOR_KEYS } from "../definitions/index.ts";
import type * as t from "../index.ts";

/**
 * A prefix AST traversal implementation meant for simple searching
 * and processing.
 */
export default function traverseFast<Options = object>(
  node: t.Node | null | undefined,
  enter: (node: t.Node, opts?: Options) => void | "skip" | "stop",
  opts?: Options,
): boolean {
  if (!node) return false;

  const keys = VISITOR_KEYS[node.type];
  if (!keys) return false;

  opts = opts || ({} as Options);
  const ret = enter(node, opts);
  if (ret !== undefined) {
    switch (ret) {
      case "skip":
        return false;
      case "stop":
        return true;
    }
  }

  for (const key of keys) {
    const subNode: t.Node | undefined | null =
      // @ts-expect-error key must present in node
      node[key];

    if (!subNode) continue;

    if (Array.isArray(subNode)) {
      for (const node of subNode) {
        if (traverseFast(node, enter, opts)) return true;
      }
    } else {
      if (traverseFast(subNode, enter, opts)) return true;
    }
  }
  return false;
}

Review comment (suggestion):
Please document what the `true`/`false` return types mean, in the JSDoc comment.

Code after:
import { VISITOR_KEYS } from "../definitions/index.ts";
import type * as t from "../index.ts";

const _skip = Symbol();
const _stop = Symbol();

/**
 * A prefix AST traversal implementation meant for simple searching and processing.
 * @param enter The callback can return `traverseFast.skip` to skip the subtree of the current node, or `traverseFast.stop` to stop the traversal.
 * @returns `true` if the traversal was stopped by callback, `false` otherwise.
 */
export default function traverseFast<Options = object>(
  node: t.Node | null | undefined,
  enter: (
    node: t.Node,
    opts?: Options,
  ) => void | typeof traverseFast.skip | typeof traverseFast.stop,
  opts?: Options,
): boolean {
  if (!node) return false;

  const keys = VISITOR_KEYS[node.type];
  if (!keys) return false;

  opts = opts || ({} as Options);
  const ret = enter(node, opts);
  if (ret !== undefined) {
    switch (ret) {
      case _skip:
        return false;
      case _stop:
        return true;
    }
  }

  for (const key of keys) {
    const subNode: t.Node | undefined | null =
      // @ts-expect-error key must present in node
      node[key];

    if (!subNode) continue;

    if (Array.isArray(subNode)) {
      for (const node of subNode) {
        if (traverseFast(node, enter, opts)) return true;
Repository: babel/babel
File: packages/babel-types/src/traverse/traverseFast.ts (TypeScript)

Code before:
import { VISITOR_KEYS } from "../definitions/index.ts";
import type * as t from "../index.ts";

/**
 * A prefix AST traversal implementation meant for simple searching
 * and processing.
 */
export default function traverseFast<Options = object>(
  node: t.Node | null | undefined,
  enter: (node: t.Node, opts?: Options) => void | "skip" | "stop",
  opts?: Options,
): boolean {
  if (!node) return false;

  const keys = VISITOR_KEYS[node.type];
  if (!keys) return false;

  opts = opts || ({} as Options);
  const ret = enter(node, opts);
  if (ret !== undefined) {
    switch (ret) {
      case "skip":
        return false;
      case "stop":
        return true;
    }
  }

  for (const key of keys) {
    const subNode: t.Node | undefined | null =
      // @ts-expect-error key must present in node
      node[key];

    if (!subNode) continue;


Review comment (suggestion):
Wdyt about returning `t.traferseFast.skip` and `t.traverseFast.stop` symbols instead of strings? We usually don't have this type of magic strings to control behavior in our public API.

Code after:
import { VISITOR_KEYS } from "../definitions/index.ts";
import type * as t from "../index.ts";

const _skip = Symbol();
const _stop = Symbol();

/**
 * A prefix AST traversal implementation meant for simple searching and processing.
 * @param enter The callback can return `traverseFast.skip` to skip the subtree of the current node, or `traverseFast.stop` to stop the traversal.
 * @returns `true` if the traversal was stopped by callback, `false` otherwise.
 */
export default function traverseFast<Options = object>(
  node: t.Node | null | undefined,
  enter: (
    node: t.Node,
    opts?: Options,
  ) => void | typeof traverseFast.skip | typeof traverseFast.stop,
  opts?: Options,
): boolean {
  if (!node) return false;

  const keys = VISITOR_KEYS[node.type];
  if (!keys) return false;

  opts = opts || ({} as Options);
  const ret = enter(node, opts);
  if (ret !== undefined) {
    switch (ret) {
      case _skip:
        return false;
      case _stop:
        return true;
    }
  }
Repository: babel/babel
File: packages/babel-runtime/test/unittests/wrapRegExp.test.js (JavaScript)

Code before:
import wrapRegExp from "../../helpers/esm/wrapRegExp.js";

describe("wrapRegExp", () => {
  it("should handle maliciously crafted substitutions", () => {
    const pattern = "(foo)";
    const groups = { group: 1 };
    const myRegExp = wrapRegExp(pattern, groups);
    const targetStr = "foofoo";
    const str = "$<".repeat(1e5) + "group";
    const startTime = Date.now();
    myRegExp[Symbol.replace](targetStr, str);
    // This test will fail when 1000ms is passed
    const timeTaken = Date.now() - startTime;
    expect(timeTaken).toBeLessThan(1000);
  });
});


Review comment (bug):
```suggestion
    const str = "[$<".repeat(1e5) + "group>]";
    const startTime = Date.now();
    const result = myRegExp[Symbol.replace](targetStr, str);
    
    // This test will fail when 1000ms is passed
    const timeTaken = Date.now() - startTime;
    expect(timeTaken).toBeLessThan(1000);
    
    expect(result).toBe("[]foo");
```

Note: there is currently a bug on `main` that causes `result` to be `[$undefined]foo` instead of `[]foo`.

I didn't try this PR, but if it causes the result to be `[$<$<$<$<$<....foo]foo`, it's a regression.

Code after:
import wrapRegExp from "../../helpers/wrapRegExp.js";

describe("wrapRegExp", () => {
  describe("should handle maliciously crafted substitutions", () => {
    it("$<$<...$<group", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foofoo";
      const replacement = "$<".repeat(1e5) + "group";
      const startTime = Date.now();
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      // This test will fail if 2000ms is passed
      const timeTaken = Date.now() - startTime;
      expect(timeTaken).toBeLessThan(2000);
      expect(result).toBe(replacement + "foo");
    });

    it("$<$<...$<group>", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foofoo";
      const replacement = "$<".repeat(1e5) + "group>";
      const startTime = Date.now();
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      // This test will fail if 2000ms is passed
      const timeTaken = Date.now() - startTime;
      expect(timeTaken).toBeLessThan(2000);
      expect(result).toBe("foo");
    });

    it("$<g$<g...$<group", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foofoo";
      const replacement = "$<g".repeat(1e5) + "group";
      const startTime = Date.now();
Repository: babel/babel
File: packages/babel-runtime/test/unittests/wrapRegExp.test.js (JavaScript)

Code before:
      const timeTaken = Date.now() - startTime;
      expect(timeTaken).toBeLessThan(1000);
      expect(result).toBe("foo");
    });

    it("$<_$>", () => {
      const pattern = "(foo)";
      const groups = { _$: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<_$>$<_$>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("foofoobar");
    });

    it.skip("$<hasOwnProperty>", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<hasOwnProperty>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("bar");
    });

    it.todo("$<__proto__>");
  });
});


Review comment (suggestion):
These two tests are skipped because we will have to adjust the emit part of the named-captured transform.

Code after:
      const timeTaken = Date.now() - startTime;
      expect(timeTaken).toBeLessThan(2000);
      expect(result).toBe("foo");
    });

    it("$<_$>", () => {
      const pattern = "(foo)";
      const groups = { _$: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<_$>$<_$>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("foofoobar");
    });

    it("$<hasOwnProperty>", () => {
      const pattern = "(foo)";
      const groups = { hasOwnProperty: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<hasOwnProperty>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("foobar");
    });

    it("$<__proto__>", () => {
      const pattern = "(foo)";
      const groups = { ["__proto__"]: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<__proto__>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("foobar");
    });
  });
  describe("substitutions", () => {
    it("unknown group", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
      const replacement = "$<UNKNOWN>";
      const result = myRegExp[Symbol.replace](targetStr, replacement);
      expect(result).toBe("bar");
    });

    it("$<hasOwnProperty> - unknown group", () => {
      const pattern = "(foo)";
      const groups = { group: 1 };
      const myRegExp = wrapRegExp(pattern, groups);
      const targetStr = "foobar";
Repository: babel/babel
File: packages/babel-core/test/async.js (JavaScript)

Code before:
import path from "path";
import { fileURLToPath } from "url";
import * as babel from "../lib/index.js";

import {
  spawnTransformAsync,
  spawnTransformAsyncParallel,
  spawnTransformSync,
  supportsESM,
} from "./helpers/esm.js";

import { itGte, itESM, itNegate, itSatisfies } from "$repo-utils";

// "minNodeVersion": "8.0.0" <-- For Ctrl+F when dropping node 6
const nodeGte8 = itGte("8.0.0");
const nodeGte14 = itGte("14.8.0");

// "minNodeVersion": "22.0.0" <-- For Ctrl+F when dropping node 20
const nodeGte22_12OrHat20_19 = itSatisfies("^20.19.0 || >= 22.12.0");

describe("asynchronicity", () => {
  const base = path.join(
    path.dirname(fileURLToPath(import.meta.url)),
    "fixtures",
    "async",
  );
  let cwd;

  beforeEach(function () {
    cwd = process.cwd();
    process.chdir(base);
  });

  afterEach(function () {
    process.chdir(cwd);
  });

  describe("config file", () => {
    describe("async function", () => {
      nodeGte8("called synchronously", () => {
        process.chdir("config-file-async-function");

        expect(() =>
          babel.transformSync(""),

Review comment (suggestion):
We could also use `process.features.require_module` instead of version checks

Code after:
import path from "path";
import { fileURLToPath } from "url";
import * as babel from "../lib/index.js";

import {
  spawnTransformAsync,
  spawnTransformAsyncParallel,
  spawnTransformSync,
  supportsESM,
} from "./helpers/esm.js";

import { itGte, itESM, itNegate, itSatisfies } from "$repo-utils";

// "minNodeVersion": "8.0.0" <-- For Ctrl+F when dropping node 6
const nodeGte8 = itGte("8.0.0");
const nodeGte14 = itGte("14.8.0");

// "minNodeVersion": "22.0.0" <-- For Ctrl+F when dropping node 20
const versionHasRequireESM = "^20.19.0 || >= 22.12.0";

describe("asynchronicity", () => {
  const base = path.join(
    path.dirname(fileURLToPath(import.meta.url)),
    "fixtures",
    "async",
  );
  let cwd;

  beforeEach(function () {
    cwd = process.cwd();
    process.chdir(base);
  });

  afterEach(function () {
    process.chdir(cwd);
  });

  describe("config file", () => {
    describe("async function", () => {
      nodeGte8("called synchronously", () => {
        process.chdir("config-file-async-function");

        expect(() =>
          babel.transformSync(""),
Repository: babel/babel
File: test/runtime-integration/node.cjs (JavaScript)

Code before:
      ? "expected-cjs-absolute-13.0.txt"
      : major === 13 && minor <= 3
        ? "expected-cjs-absolute-13.2.txt"
        : major < 16 || (major === 16 && minor <= 5)
          ? "expected-cjs-absolute-16.0.txt"
          : major < 22 || (major === 22 && minor <= 11)
            ? "expected-cjs-absolute-22.11.txt"
            : major < 24
              ? "expected-cjs-absolute-23.10.txt"
              : "expected-cjs-absolute.txt";

test(
  "CJS - absoluteRuntime",
  "./src/absolute/main-cjs.cjs",
  expectedCjsAbsolute
);

function test(title, command, expectedName) {
  const expectedPath = path.join(__dirname, expectedName);
  const expected = fs.readFileSync(expectedPath, "utf8");

  console.log(`Testing with Node.js ${process.version} - ${title}`);
  const out = normalize(
    cp.execSync(
      `node ${
        (major > 22 && major < 24) || (major === 22 && minor >= 12)
          ? "--disable-warning=ExperimentalWarning "
          : ""
      }${command}`,
      {
        cwd: __dirname,
        encoding: "utf8",
      }
    )
  );

  if (expected === out) {
    console.log("OK");
  } else if (process.env.OVERWRITE) {
    fs.writeFileSync(expectedPath, out);
    console.log("UPDATED");
  } else {
    console.error("FAILED\n\n");
    console.error(out);
    process.exitCode = 1;
  }
  console.log("\n");
}

function normalize(output) {
  const root = path.resolve(__dirname, "../..");

Review comment (suggestion):
```suggestion
        major === 23 || (major === 22 && minor >= 12)
```

Code after:
      ? "expected-cjs-absolute-13.0.txt"
      : major === 13 && minor <= 3
        ? "expected-cjs-absolute-13.2.txt"
        : major < 16 || (major === 16 && minor <= 5)
          ? "expected-cjs-absolute-16.0.txt"
          : major < 22 || (major === 22 && minor <= 11)
            ? "expected-cjs-absolute-22.11.txt"
            : major < 24
              ? "expected-cjs-absolute-23.10.txt"
              : "expected-cjs-absolute.txt";

test(
  "CJS - absoluteRuntime",
  "./src/absolute/main-cjs.cjs",
  expectedCjsAbsolute
);

function test(title, command, expectedName) {
  const expectedPath = path.join(__dirname, expectedName);
  const expected = fs.readFileSync(expectedPath, "utf8");

  console.log(`Testing with Node.js ${process.version} - ${title}`);
  const out = normalize(
    cp.execSync(
      `node ${
        major === 23 || (major === 22 && minor >= 12)
          ? "--disable-warning=ExperimentalWarning "
          : ""
      }${command}`,
      {
        cwd: __dirname,
        encoding: "utf8",
      }
    )
  );

  if (expected === out) {
    console.log("OK");
  } else if (process.env.OVERWRITE) {
    fs.writeFileSync(expectedPath, out);
    console.log("UPDATED");
  } else {
    console.error("FAILED\n\n");
    console.error(out);
    process.exitCode = 1;
  }
  console.log("\n");
}

function normalize(output) {
  const root = path.resolve(__dirname, "../..");