ronantakizawa/codereview-bench huggingface dataset:
Repository: babel/babel
File: packages/babel-types/src/definitions/core.ts (TypeScript)

Code before:
          throw new TypeError(
            `Exactly one VariableDeclarator is required in the VariableDeclaration of a ${parent.type}`,
          );
        }
      }
    : undefined,
});

defineType("VariableDeclarator", {
  visitor: ["id", "init"],
  fields: {
    id: {
      validate: (function () {
        if (!process.env.BABEL_TYPES_8_BREAKING) {
          return assertNodeType("LVal");
        }

        const validator = assertNodeType(
          "Identifier",
          "ArrayPattern",
          "ObjectPattern",
        );

        return function (node: t.VariableDeclarator, key, val) {
          validator(node, key, val);
        };
      })(),
    },
    definite: {
      optional: true,
      validate: assertValueType("boolean"),
    },
    init: {
      optional: true,
      validate: assertNodeType("Expression"),
    },
  },
});

defineType("WhileStatement", {
  visitor: ["test", "body"],
  aliases: ["Statement", "BlockParent", "Loop", "While", "Scopable"],
  fields: {
    test: {
      validate: assertNodeType("Expression"),
    },
    body: {
      validate: assertNodeType("Statement"),
    },
  },
});

Review comment (suggestion):
This would have allowed invalid productions such as `var { a, b }`. Could you check if the `parent` (available as the first validate parameter) is a ForXStatement and the parent.left is node? Similar to line 1209.

Code after:
defineType("VariableDeclaration", {
  builder: ["kind", "declarations"],
  visitor: ["declarations"],
  aliases: ["Statement", "Declaration"],
  fields: {
    declare: {
      validate: assertValueType("boolean"),
      optional: true,
    },
    kind: {
      validate: assertOneOf(
        "var",
        "let",
        "const",
        // https://github.com/tc39/proposal-explicit-resource-management
        "using",
        // https://github.com/tc39/proposal-async-explicit-resource-management
        "await using",
      ),
    },
    declarations: {
      validate: chain(
        assertValueType("array"),
        assertEach(assertNodeType("VariableDeclarator")),
      ),
    },
  },
  validate:
    process.env.BABEL_8_BREAKING || process.env.BABEL_TYPES_8_BREAKING
      ? (() => {
          const withoutInit = assertNodeType("Identifier");

          return function (parent, key, node: t.VariableDeclaration) {
            if (is("ForXStatement", parent, { left: node })) {
              if (node.declarations.length !== 1) {
                throw new TypeError(
                  `Exactly one VariableDeclarator is required in the VariableDeclaration of a ${parent.type}`,
                );
              }
            } else {
              node.declarations.forEach(decl => {
                if (!decl.init) withoutInit(decl, "id", decl.id);
              });
            }
          };
        })()
      : undefined,
});

defineType("VariableDeclarator", {
  visitor: ["id", "init"],
Repository: babel/babel
File: packages/babel-parser/src/options.ts (TypeScript)

Code before:
  // When enabled, the parser will support Annex B syntax.
  // https://tc39.es/ecma262/#sec-additional-ecmascript-features-for-web-browsers
  annexB: true,
};

// Interpret and default an options object

export function getOptions(opts?: Options | null): OptionsWithDefaults {
  if (opts == null) {
    return { ...defaultOptions };
  }
  if (opts.annexB != null && opts.annexB !== false) {
    throw new Error("The `annexB` option can only be set to `false`.");
  }

  const options: any = {};
  for (const key of Object.keys(defaultOptions) as (keyof Options)[]) {
    options[key] = opts[key] ?? defaultOptions[key];
  }

  if (options.startColumn > 0 && opts.startIndex == null) {
    if (options.startLine === 1) {
      options.startIndex = options.startColumn;
    } else if (process.env.BABEL_8_BREAKING) {
      throw new Error(
        "When `startColumn` is used with a `startLine > 1` you must also provide a `startIndex`.",
      );
    }
  } else if (options.startIndex > 0 && opts.startColumn == null) {
    if (options.startLine === 1) {
      options.startColumn = options.startIndex;
    } else {
      throw new Error(
        "When `startIndex` is used with a `startLine > 1` you must also provide a `startColumn`.",
      );
    }
  }

  return options;
}


Review comment (suggestion):
When `startLine` is 2 and `startColumn` is 0, we should still throw if `startIndex` is undefined.

Code after:
    attachComment: true,
    // When enabled, the parser will support Annex B syntax.
    // https://tc39.es/ecma262/#sec-additional-ecmascript-features-for-web-browsers
    annexB: true,
  };
}

// Interpret and default an options object

export function getOptions(opts?: Options | null): OptionsWithDefaults {
  // https://github.com/babel/babel/pull/16918
  // `options` is accessed frequently, please make sure it is a fast object.
  // `%ToFastProperties` can make it a fast object, but the performance is the same as the slow object.
  const options: any = createDefaultOptions();

  if (opts == null) {
    return options;
  }
  if (opts.annexB != null && opts.annexB !== false) {
    throw new Error("The `annexB` option can only be set to `false`.");
  }

  for (const key of Object.keys(options) as (keyof Options)[]) {
    if (opts[key] != null) options[key] = opts[key];
  }

  if (options.startLine === 1) {
    if (opts.startIndex == null && options.startColumn > 0) {
      options.startIndex = options.startColumn;
    } else if (opts.startColumn == null && options.startIndex > 0) {
      options.startColumn = options.startIndex;
    }
  } else if (opts.startColumn == null || opts.startIndex == null) {
    if (opts.startIndex != null || process.env.BABEL_8_BREAKING) {
      throw new Error(
        "With a `startLine > 1` you must also specify `startIndex` and `startColumn`.",
      );
    }
  }

  return options;
}
Repository: babel/babel
File: packages/babel-core/src/transformation/plugin-pass.ts (TypeScript)

Code before:
import type * as t from "@babel/types";
import type File from "./file/file.ts";

export default class PluginPass<Options = object> {
  _map: Map<unknown, unknown> = new Map();
  key: string | undefined | null;
  file: File;
  opts: Partial<Options>;

  /**
   * The working directory that Babel's programmatic options are loaded
   * relative to.
   */
  cwd: string;

  /** The absolute path of the file being compiled. */
  filename: string | void;

  /**
   * Is Babel executed in async mode or not.
   */
  isAsync: boolean | undefined;

  constructor(
    file: File,
    key?: string | null,
    options?: Options,
    isAsync?: boolean,
  ) {
    this.key = key;
    this.file = file;
    this.opts = options || {};
    this.cwd = file.opts.cwd;
    this.filename = file.opts.filename;
    this.isAsync = isAsync;
  }

  set(key: unknown, val: unknown) {
    this._map.set(key, val);
  }

  get(key: unknown): any {

Review comment (suggestion):
With `/**` style of comment it's a jsdoc description which could be used as quick documentation in IDE. 

Code after:
import type * as t from "@babel/types";
import type File from "./file/file.ts";

export default class PluginPass<Options = object> {
  _map: Map<unknown, unknown> = new Map();
  key: string | undefined | null;
  file: File;
  opts: Partial<Options>;

  /**
   * The working directory that Babel's programmatic options are loaded
   * relative to.
   */
  cwd: string;

  /** The absolute path of the file being compiled. */
  filename: string | void;

  /**
   * Is Babel executed in async mode or not.
   */
  isAsync: boolean;

  constructor(
    file: File,
    key: string | null,
    options: Options | undefined,
    isAsync: boolean,
  ) {
    this.key = key;
    this.file = file;
    this.opts = options || {};
    this.cwd = file.opts.cwd;
    this.filename = file.opts.filename;
    this.isAsync = isAsync;
  }

  set(key: unknown, val: unknown) {
    this._map.set(key, val);
  }

  get(key: unknown): any {
Repository: babel/babel
File: packages/babel-core/src/transformation/plugin-pass.ts (TypeScript)

Code before:

export default class PluginPass<Options = object> {
  _map: Map<unknown, unknown> = new Map();
  key: string | undefined | null;
  file: File;
  opts: Partial<Options>;

  /**
   * The working directory that Babel's programmatic options are loaded
   * relative to.
   */
  cwd: string;

  /** The absolute path of the file being compiled. */
  filename: string | void;

  /**
   * Is Babel executed in async mode or not.
   */
  isAsync: boolean | undefined;

  constructor(
    file: File,
    key?: string | null,
    options?: Options,
    isAsync?: boolean,
  ) {
    this.key = key;
    this.file = file;
    this.opts = options || {};
    this.cwd = file.opts.cwd;
    this.filename = file.opts.filename;
    this.isAsync = isAsync;
  }

  set(key: unknown, val: unknown) {
    this._map.set(key, val);
  }

  get(key: unknown): any {
    return this._map.get(key);
  }

  availableHelper(name: string, versionRange?: string | null) {
    return this.file.availableHelper(name, versionRange);
  }

  addHelper(name: string) {
    return this.file.addHelper(name);
  }


Review comment (suggestion):
i've added `isAsync` as optional argument to the last position, so it shouldn't be a breaking change to anyone who uses `PluginPass` constructor directly. However, i would like to add it as required argument after the `file`.  Let me know if that change is acceptable. 

Code after:

export default class PluginPass<Options = object> {
  _map: Map<unknown, unknown> = new Map();
  key: string | undefined | null;
  file: File;
  opts: Partial<Options>;

  /**
   * The working directory that Babel's programmatic options are loaded
   * relative to.
   */
  cwd: string;

  /** The absolute path of the file being compiled. */
  filename: string | void;

  /**
   * Is Babel executed in async mode or not.
   */
  isAsync: boolean;

  constructor(
    file: File,
    key: string | null,
    options: Options | undefined,
    isAsync: boolean,
  ) {
    this.key = key;
    this.file = file;
    this.opts = options || {};
    this.cwd = file.opts.cwd;
    this.filename = file.opts.filename;
    this.isAsync = isAsync;
  }

  set(key: unknown, val: unknown) {
    this._map.set(key, val);
  }

  get(key: unknown): any {
    return this._map.get(key);
  }

  availableHelper(name: string, versionRange?: string | null) {
    return this.file.availableHelper(name, versionRange);
  }

  addHelper(name: string) {
    return this.file.addHelper(name);
  }
Repository: babel/babel
File: packages/babel-core/src/config/full.ts (TypeScript)

Code before:
  },
);

/**
 * Generate a config object that will act as the root of a new nested config.
 */
function* loadPresetDescriptor(
  descriptor: UnloadedDescriptor<PresetAPI>,
  context: Context.FullPreset,
): Handler<{
  chain: ConfigChain | null;
  externalDependencies: ReadonlyDeepArray<string>;
}> {
  const preset = instantiatePreset(
    yield* presetDescriptorLoader(descriptor, context),
  );
  validatePreset(preset, context, descriptor);
  return {
    chain: yield* buildPresetChain(preset, context),
    externalDependencies: preset.externalDependencies,
  };
}

function chain<Args extends any[]>(
  a: undefined | ((...args: Args) => void),
  b: undefined | ((...args: Args) => void),
) {
  const fns = [a, b].filter(Boolean);
  if (fns.length <= 1) return fns[0];

  return function (this: unknown, ...args: unknown[]) {
    for (const fn of fns) {
      fn.apply(this, args);
    }
  };
}


Review comment (suggestion):
Here we assume `a` and `b` share the same return type, however, it is also possible that `a` is sync while `b` is async. I suggest we remove the `R` constraint and type them as `(...args: Args) => void | Promise<void>`. We can probably get rid of the final type cast as well.

For situations where both `a` and `b` are sync, we can add a type overload so that the chained function is still sync.

Code after:
  },
);

/**
 * Generate a config object that will act as the root of a new nested config.
 */
function* loadPresetDescriptor(
  descriptor: UnloadedDescriptor<PresetAPI>,
  context: Context.FullPreset,
): Handler<{
  chain: ConfigChain | null;
  externalDependencies: ReadonlyDeepArray<string>;
}> {
  const preset = instantiatePreset(
    yield* presetDescriptorLoader(descriptor, context),
  );
  validatePreset(preset, context, descriptor);
  return {
    chain: yield* buildPresetChain(preset, context),
    externalDependencies: preset.externalDependencies,
  };
}

function chainMaybeAsync<Args extends any[], R extends void | Promise<void>>(
  a: undefined | ((...args: Args) => R),
  b: undefined | ((...args: Args) => R),
): (...args: Args) => R {
  if (!a) return b;
  if (!b) return a;

  return function (this: unknown, ...args: Args) {
    const res = a.apply(this, args);
    if (res && typeof res.then === "function") {
      return res.then(() => b.apply(this, args));
    }
    return b.apply(this, args);
  } as (...args: Args) => R;
}
Repository: babel/babel
File: eslint/babel-eslint-tests/test/integration/eslint/verify.js (JavaScript)

Code before:
    });

    describe("accessor declarations", () => {
      it("should not be undefined", () => {
        verifyAndAssertMessages(
          `
              class C {
                accessor d = 1;
              }
          `,
          { "no-undef": 1 },
        );
      });

      it("should not be unused", () => {
        verifyAndAssertMessages(
          `
              export class C {
                accessor d = 1;
              }
          `,
          { "no-unused-vars": 1 },
        );
      });

      it("no-use-before-define allows referencing the class in a accessor", () => {
        verifyAndAssertMessages(
          `
            class C {
              accessor d = C.name;
            }
          `,
          { "no-use-before-define": 1 },
        );
      });
    });

    describe("private methods", () => {
      it("should not be undefined", () => {
        verifyAndAssertMessages(
          `
              class C {
                #d() {};
              }
          `,
          { "no-undef": 1 },
        );
      });

      it("should not be unused", () => {
        verifyAndAssertMessages(

Review comment (suggestion):
```suggestion
      it("no-use-before-define allows referencing the class in an accessor", () => {
```

Code after:
    });

    describe("accessor declarations", () => {
      it("should not be undefined", () => {
        verifyAndAssertMessages(
          `
              class C {
                accessor d = 1;
              }
          `,
          { "no-undef": 1 },
        );
      });

      it("should not be unused", () => {
        verifyAndAssertMessages(
          `
              export class C {
                accessor d = 1;
              }
          `,
          { "no-unused-vars": 1 },
        );
      });

      it("no-use-before-define allows referencing the class in an accessor", () => {
        verifyAndAssertMessages(
          `
            class C {
              accessor d = C.name;
            }
          `,
          { "no-use-before-define": 1 },
        );
      });
    });

    describe("private methods", () => {
      it("should not be undefined", () => {
        verifyAndAssertMessages(
          `
              class C {
                #d() {};
              }
          `,
          { "no-undef": 1 },
        );
      });

      it("should not be unused", () => {
        verifyAndAssertMessages(
Repository: babel/babel
File: packages/babel-plugin-transform-exponentiation-operator/src/index.ts (TypeScript)

Code before:
import { declare } from "@babel/helper-plugin-utils";
import type { types as t, Scope } from "@babel/core";

export default declare(api => {
  api.assertVersion(REQUIRED_VERSION(7));

  const { types: t } = api;

  function build(left: t.Expression, right: t.Expression) {
    return t.callExpression(
      t.memberExpression(t.identifier("Math"), t.identifier("pow")),
      [left, right],
    );
  }

  function maybeMemoize<T extends t.Expression | t.Super>(
    node: T,
    scope: Scope,
  ) {
    if (scope.isPure(node) || t.isSuper(node)) {
      return { assign: node, ref: t.cloneNode(node) };
    }

    const id = scope.generateUidIdentifierBasedOnNode(node);
    scope.push({ id });
    return {
      assign: t.assignmentExpression("=", t.cloneNode(id), node),
      ref: t.cloneNode(id),
    };
  }

  return {
    name: "transform-exponentiation-operator",

    visitor: {
      AssignmentExpression(path) {
        const { node, scope } = path;
        if (node.operator !== "**=") return;

        if (t.isMemberExpression(node.left)) {
          let member1: t.Expression;
          let member2: t.Expression;

          const object = maybeMemoize(node.left.object, scope);
          const { property, computed } = node.left;

Review comment (bug):
In the nullish transform we use `scope.isStatic` for the memoization purpose:

https://github.com/babel/babel/blob/b01932768b978bcaa4aa0517d4bdba29ff3966f8/packages/babel-plugin-transform-nullish-coalescing-operator/src/index.ts#L28-L41

Can you also add a new test and see if we can handle the edge case `function (a, b = a.b.c ** 2) {}` here?

Code after:
import { declare } from "@babel/helper-plugin-utils";
import type { types as t, Scope } from "@babel/core";

export default declare(api => {
  api.assertVersion(REQUIRED_VERSION(7));

  const { types: t, template } = api;

  function build(left: t.Expression, right: t.Expression) {
    return t.callExpression(
      t.memberExpression(t.identifier("Math"), t.identifier("pow")),
      [left, right],
    );
  }

  function maybeMemoize<T extends t.Expression | t.Super>(
    node: T,
    scope: Scope,
  ) {
    if (scope.isStatic(node)) {
      return { assign: node, ref: t.cloneNode(node) };
    }

    if (scope.path.isPattern()) {
      // We cannot inject a temp var in function arguments.
      return null;
    }

    const id = scope.generateUidIdentifierBasedOnNode(node);
    scope.push({ id });
    return {
      assign: t.assignmentExpression(
        "=",
        t.cloneNode(id),
        // This is not t.Super, because otherwise the .isStatic check above
        // would have returned true.
        // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
        node as t.Expression,
      ),
      ref: t.cloneNode(id),
    };
  }

  return {
    name: "transform-exponentiation-operator",
Repository: babel/babel
File: packages/babel-parser/src/options.ts (TypeScript)

Code before:
  errorRecovery: false,
  // When enabled, comments will be attached to adjacent AST nodes as one of
  // `leadingComments`, `trailingComments` and `innerComments`. The comment attachment
  // is vital to preserve comments after transform. If you don't print AST back,
  // consider set this option to `false` for performance
  attachComment: true,
  // When enabled, the parser will support Annex B syntax.
  // https://tc39.es/ecma262/#sec-additional-ecmascript-features-for-web-browsers
  annexB: true,
};

// Interpret and default an options object

export function getOptions(opts?: Options | null): OptionsWithDefaults {
  if (opts == null) {
    return { ...defaultOptions };
  }
  if (opts.annexB != null && opts.annexB !== false) {
    throw new Error("The `annexB` option can only be set to `false`.");
  }

  const options: any = {};
  for (const key of Object.keys(defaultOptions) as (keyof Options)[]) {
    options[key] = opts[key] ?? defaultOptions[key];
  }

  // Here we use a trick to make it a fast object.
  // So that it can be accessed frequently later.
  // `%ToFastProperties` can make it a fast object, but the performance is the same as the slow object.
  return { ...options };
}


Review comment (suggestion):
Here we start from `{}` and then change the shape for ~20 times. Can we start the new options from `{ ...defaultOptions }` and assign the values if `opts[key]` is not nullish? Doing so should also avoid the shape migration during initialization.

Code after:
  errorRecovery: false,
  // When enabled, comments will be attached to adjacent AST nodes as one of
  // `leadingComments`, `trailingComments` and `innerComments`. The comment attachment
  // is vital to preserve comments after transform. If you don't print AST back,
  // consider set this option to `false` for performance
  attachComment: true,
  // When enabled, the parser will support Annex B syntax.
  // https://tc39.es/ecma262/#sec-additional-ecmascript-features-for-web-browsers
  annexB: true,
};

// Interpret and default an options object

export function getOptions(opts?: Options | null): OptionsWithDefaults {
  // https://github.com/babel/babel/pull/16918
  // `options` is accessed frequently, please make sure it is a fast object.
  // `%ToFastProperties` can make it a fast object, but the performance is the same as the slow object.
  const options: any = { ...defaultOptions };

  if (opts == null) {
    return options;
  }
  if (opts.annexB != null && opts.annexB !== false) {
    throw new Error("The `annexB` option can only be set to `false`.");
  }

  for (const key of Object.keys(defaultOptions) as (keyof Options)[]) {
    if (opts[key] != null) options[key] = opts[key];
  }

  return options;
}