ronantakizawa/codereview-bench huggingface dataset:
Repository: babel/babel
File: packages/babel-helpers/src/helpers/get.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

import _superPropBase from "./superPropBase.ts";

// https://tc39.es/ecma262/multipage/reflection.html#sec-reflect.get
//
//  28.1ak.5 Reflect.get ( target, propertyKey [ , receiver ] )

type Object = {
  __proto__?: any;
} & { [key: string]: unknown };

export default function _get<T extends Object, P extends string | symbol>(
  this: unknown,
  ...args: [target: T, property: P, receiver?: unknown]
): P extends keyof T ? T[P] : any {
  let _getImpl = _get;

  if (typeof Reflect !== "undefined" && Reflect.get) {
    // need a bind because https://github.com/babel/babel/issues/14527
    _getImpl = Reflect.get.bind(null);
  } else {
    _getImpl = function _get(target, property, receiver) {
      var base = _superPropBase(target, property);

      if (!base) return;

      var desc = Object.getOwnPropertyDescriptor(base, property);
      if (desc && desc.get) {
        // STEP 3. If receiver is not present, then set receiver to target.
        return desc.get.call(arguments.length < 3 ? target : receiver);
      }

      return desc ? desc.value : undefined;
    };
  }
  return _getImpl.apply(this, args);
}


Review comment (refactor):
Let's keep the original implementation where we reassign `_get`, and add a `@ts-expect-error` to it.

This is different from the original one because the feature detection would run every single time `_get` is called, rather than just the fist one.

Also for the other helpers.

Code after:
/* @minVersion 7.0.0-beta.0 */

import superPropBase from "./superPropBase.ts";

// https://tc39.es/ecma262/multipage/reflection.html#sec-reflect.get
//
//  28.1ak.5 Reflect.get ( target, propertyKey [ , receiver ] )
export default function _get<T extends object, P extends PropertyKey>(
  target: T,
  property: P,
  receiver?: unknown,
): P extends keyof T ? T[P] : any;
export default function _get(): any {
  if (typeof Reflect !== "undefined" && Reflect.get) {
    // need a bind because https://github.com/babel/babel/issues/14527
    // @ts-expect-error function reassign
    _get = Reflect.get.bind(/* undefined */);
  } else {
    // @ts-expect-error function reassign
    _get = function _get(target, property, receiver) {
      var base = superPropBase(target, property);

      if (!base) return;

      var desc = Object.getOwnPropertyDescriptor(base, property)!;
      if (desc.get) {
        // STEP 3. If receiver is not present, then set receiver to target.
        return desc.get.call(arguments.length < 3 ? target : receiver);
      }

      return desc.value;
    };
  }

  return _get.apply(null, arguments as any as Parameters<typeof Reflect.get>);
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/getPrototypeOf.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

type Object = {
  __proto__?: any;
} & { [key: string]: unknown };

export default function _getPrototypeOf<T extends Object>(o: T) {
  // @ts-expect-error explicitly assign to function
  _getPrototypeOf = Object.setPrototypeOf
    ? // @ts-expect-error -- intentionally omitting the argument
      Object.getPrototypeOf.bind(/* undefined */)
    : function _getPrototypeOf<T extends Object>(o: T) {
        return o.__proto__ || Object.getPrototypeOf(o);
      };
  return _getPrototypeOf(o);
}


Review comment (suggestion):
You don't need a `<T>` here. `Object.getPrototypeOf` doesn't require a generic.

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _getPrototypeOf(o: object) {
  // @ts-expect-error explicitly assign to function
  _getPrototypeOf = Object.setPrototypeOf
    ? // @ts-expect-error -- intentionally omitting the argument
      Object.getPrototypeOf.bind(/* undefined */)
    : function _getPrototypeOf<T extends Object>(o: T) {
        return (o as any).__proto__ || Object.getPrototypeOf(o);
      };
  return _getPrototypeOf(o);
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/getPrototypeOf.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

type Object = {
  __proto__?: any;
} & { [key: string]: unknown };

export default function _getPrototypeOf<T extends Object>(o: T) {
  // @ts-expect-error explicitly assign to function
  _getPrototypeOf = Object.setPrototypeOf
    ? // @ts-expect-error -- intentionally omitting the argument
      Object.getPrototypeOf.bind(/* undefined */)
    : function _getPrototypeOf<T extends Object>(o: T) {
        return o.__proto__ || Object.getPrototypeOf(o);
      };
  return _getPrototypeOf(o);
}


Review comment (suggestion):
Instead of defining `Object` with `__proto__`, we can use `o["__proto__"]` and TS should stop complaining.

Ad an alternative, we can use
```ts
declare global {
    interface Object {
        __proto__: object;
    }
}
```

and then TS will automatically let us use `__proto__` on things of type `object` (with the lowercase `o`)

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _getPrototypeOf(o: object) {
  // @ts-expect-error explicitly assign to function
  _getPrototypeOf = Object.setPrototypeOf
    ? // @ts-expect-error -- intentionally omitting the argument
      Object.getPrototypeOf.bind(/* undefined */)
    : function _getPrototypeOf<T extends Object>(o: T) {
        return (o as any).__proto__ || Object.getPrototypeOf(o);
      };
  return _getPrototypeOf(o);
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/set.ts (TypeScript)

Code before:
        if (!desc.writable) {
          // Setter, getter, and non-writable fall into this.
          return false;
        }

        desc.value = value;
        Object.defineProperty(receiver, property, desc);
      } else {
        // Avoid setters that may be defined on Sub's prototype, but not on
        // the instance.
        defineProperty(receiver, property, value);
      }

      return true;
    };
  }

  return setImpl(target, property, value, receiver);
}

export default function _set(
  target: Object,
  property: string | symbol,
  value: any,
  receiver?: any,
  isStrict: boolean = false,
) {
  var s = set(target, property, value, receiver || target);
  if (!s && isStrict) {
    throw new TypeError("failed to set property");
  }

  return value;
}


Review comment (suggestion):
```suggestion
  isStrict?: boolean,
```

This file needs to be compatible with old browsers, so let's not use default params :)

Code after:
          return false;
        }

        desc.value = value;
        Object.defineProperty(receiver, property, desc);
      } else {
        // Avoid setters that may be defined on Sub's prototype, but not on
        // the instance.
        defineProperty(receiver, property, value);
      }

      return true;
    };
  }

  return set(target, property, value, receiver);
}

export default function _set(
  target: Object,
  property: PropertyKey,
  value: any,
  receiver?: any,
  isStrict?: boolean,
) {
  var s = set(target, property, value, receiver || target);
  if (!s && isStrict) {
    throw new TypeError("failed to set property");
  }

  return value;
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/superPropBase.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

import getPrototypeOf from "./getPrototypeOf.ts";

type Object = {
  __proto__?: any;
} & { [key: string]: unknown };

export default function _superPropBase(
  object: Object,
  property: string | symbol,
) {
  // Yes, this throws if object is null to being with, that's on purpose.
  while (!Object.prototype.hasOwnProperty.call(object, property)) {
    object = getPrototypeOf(object);
    if (object === null) break;
  }
  return object;
}


Review comment (suggestion):
Use `PropertyKey` here. You are typing arguments, not typing declared variables.

Code after:
/* @minVersion 7.0.0-beta.0 */

import getPrototypeOf from "./getPrototypeOf.ts";

export default function _superPropBase(object: object, property: PropertyKey) {
  // Yes, this throws if object is null to being with, that's on purpose.
  while (!Object.prototype.hasOwnProperty.call(object, property)) {
    object = getPrototypeOf(object);
    if (object === null) break;
  }
  return object;
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/assertThisInitialized.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

export default function _assertThisInitialized<T>(self: T): T {
  if (self === void 0) {
    throw new ReferenceError(
      "this hasn't been initialised - super() hasn't been called",
    );
  }
  return self;
}


Review comment (suggestion):
```suggestion
export default function _assertThisInitialized<T>(self: T | undefined): T {
```

`_assertThisInitialized` does accept `undefined` though.

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _assertThisInitialized<T>(self: T | undefined): T {
  if (self === void 0) {
    throw new ReferenceError(
      "this hasn't been initialised - super() hasn't been called",
    );
  }
  return self;
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/createClass.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

// @ts-expect-error Migrate in another PR
import toPropertyKey from "toPropertyKey.ts";

interface Prop extends PropertyDescriptor {
  key: PropertyKey;
}

function _defineProperties(target: object, props: Prop[]): void {
  for (var i = 0; i < props.length; i++) {
    var descriptor = props[i];
    descriptor.enumerable = descriptor.enumerable || false;
    descriptor.configurable = true;
    if ("value" in descriptor) descriptor.writable = true;
    Object.defineProperty(target, toPropertyKey(descriptor.key), descriptor);
  }
}

export default function _createClass<T extends new (...args: any[]) => any>(
  Constructor: T,
  protoProps?: Prop[],
  staticProps?: Prop[],
): T {
  if (protoProps) _defineProperties(Constructor.prototype, protoProps);
  if (staticProps) _defineProperties(Constructor, staticProps);
  Object.defineProperty(Constructor, "prototype", { writable: false });
  return Constructor;
}

Review comment (suggestion):
```suggestion
import toPropertyKey from "./toPropertyKey.ts";
```

(and it's already written in TS :) )

Code after:
/* @minVersion 7.0.0-beta.0 */

import toPropertyKey from "./toPropertyKey.ts";

interface Prop extends PropertyDescriptor {
  key: PropertyKey;
}

function _defineProperties(target: object, props: Prop[]): void {
  for (var i = 0; i < props.length; i++) {
    var descriptor = props[i];
    descriptor.enumerable = descriptor.enumerable || false;
    descriptor.configurable = true;
    if ("value" in descriptor) descriptor.writable = true;
    Object.defineProperty(target, toPropertyKey(descriptor.key), descriptor);
  }
}

export default function _createClass<T extends new (...args: any[]) => any>(
  Constructor: T,
  protoProps?: Prop[],
  staticProps?: Prop[],
): T {
  if (protoProps) _defineProperties(Constructor.prototype, protoProps);
  if (staticProps) _defineProperties(Constructor, staticProps);
  Object.defineProperty(Constructor, "prototype", { writable: false });
  return Constructor;
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/possibleConstructorReturn.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

import assertThisInitialized from "./assertThisInitialized.ts";

export default function _possibleConstructorReturn<
  T extends object | undefined,
>(self: T, call: T | ((...args: any[]) => T) | undefined) {
  if (call && (typeof call === "object" || typeof call === "function")) {
    return call;
  } else if (call !== void 0) {
    throw new TypeError(
      "Derived constructors may only return object or undefined",
    );
  }

  return assertThisInitialized(self);
}


Review comment (suggestion):
While we are here, let's rename `call` to `value`. Calling it `call` makes absolutely no sense 😅 

It's the value passed to `return` in a derived class constructor:
```js
class A extends B {
  constructor() {
    return 1;
  }
}
```

"call" is `1` in this case.

(and probably it's type should just be `any` or `unknown`)

Code after:
/* @minVersion 7.0.0-beta.0 */

import assertThisInitialized from "./assertThisInitialized.ts";

export default function _possibleConstructorReturn(
  self: object | undefined,
  value: unknown,
) {
  if (value && (typeof value === "object" || typeof value === "function")) {
    return value;
  } else if (value !== void 0) {
    throw new TypeError(
      "Derived constructors may only return object or undefined",
    );
  }

  return assertThisInitialized(self);
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/iterableToArray.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArray<T>(
  // Note the '@@iterator' property is not recognized here by TypeScript.
  // In fact, it looks like it's used only as the spec name to denote the symbol.
  // We may be able to remove.
  // https://stackoverflow.com/a/29672061/11711383
  // https://262.ecma-international.org/6.0/#sec-well-known-symbols
  iter: Iterable<T>,
) {
  if (
    (typeof Symbol !== "undefined" && iter[Symbol.iterator] != null) ||
    (iter as any)["@@iterator"] != null
  ) {
    return Array.from(iter);
  }
}


Review comment (suggestion):
`@@iterator` is used for compatibility with some `Symbol.iterator` polyfills (and some old Firefox versions)

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArray<T>(iter: Iterable<T>) {
  if (
    (typeof Symbol !== "undefined" && iter[Symbol.iterator] != null) ||
    (iter as any)["@@iterator"] != null
  ) {
    return Array.from(iter);
  }
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/iterableToArrayLimitLoose.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArrayLimitLoose<T>(
  arr: Array<T>,
  i: number,
) {
  var iterator: Iterator<T> & Function =
    arr &&
    ((typeof Symbol !== "undefined" && arr[Symbol.iterator]) ||
      (arr as any)["@@iterator"]);
  if (iterator == null) return;

  var _arr: T[] = [];
  var step;
  for (
    iterator = iterator.call(arr);
    arr.length < i && !(step = iterator.next()).done;

  ) {
    _arr.push(step.value);
  }
  return _arr;
}


Review comment (suggestion):
```suggestion
  iterator = iterator.call(arr);
  while (arr.length < i && !(step = iterator.next()).done) {
    _arr.push(step.value);
  }
```

we can also slightly improve readability here, to avoid the confusing empty for "update" segment.

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArrayLimitLoose<T>(
  arr: Array<T> | string,
  i: number,
) {
  var iterator: Iterator<T> & Function =
    arr &&
    ((typeof Symbol !== "undefined" && arr[Symbol.iterator]) ||
      (arr as any)["@@iterator"]);
  if (iterator == null) return;

  var _arr: T[] = [];
  var step;
  iterator = iterator.call(arr);
  while (arr.length < i && !(step = iterator.next()).done) {
    _arr.push(step.value);
  }
  return _arr;
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/maybeArrayLike.ts (TypeScript)

Code before:
/* @minVersion 7.9.0 */

import arrayLikeToArray from "./arrayLikeToArray.ts";

export default function _maybeArrayLike<T>(
  next: (arr: ArrayLike<T>, i: number) => T[] | undefined,
  arr: ArrayLike<T>,
  i: number,
) {
  if (arr && !Array.isArray(arr) && typeof arr.length === "number") {
    var len = arr.length;
    return arrayLikeToArray<T>(arr, i !== void 0 && i < len ? i : len);
  }
  return next(arr, i);
}


Review comment (suggestion):
While we are at this, let's rename `next` to `orElse` to make it more clear what it does. It is called when `arr` is _not_ an ArrayLike.

Code after:
/* @minVersion 7.9.0 */

import arrayLikeToArray from "./arrayLikeToArray.ts";

export default function _maybeArrayLike<T>(
  orElse: (arr: any, i: number) => T[] | undefined,
  arr: ArrayLike<T>,
  i: number,
) {
  if (arr && !Array.isArray(arr) && typeof arr.length === "number") {
    var len = arr.length;
    return arrayLikeToArray<T>(arr, i !== void 0 && i < len ? i : len);
  }
  return orElse(arr, i);
}
Repository: babel/babel
File: packages/babel-helpers/src/helpers/iterableToArrayLimitLoose.ts (TypeScript)

Code before:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArrayLimitLoose<T>(
  arr: Array<T>,
  i: number,
) {
  var iterator: Iterator<T> & Function =
    arr &&
    ((typeof Symbol !== "undefined" && arr[Symbol.iterator]) ||
      (arr as any)["@@iterator"]);
  if (iterator == null) return;

  var _arr: T[] = [];
  var step;
  iterator = iterator.call(arr);
  while (arr.length < i && !(step = iterator.next()).done) {
    _arr.push(step.value);
  }
  return _arr;
}


Review comment (suggestion):
The function argument `arr` accepts `Array<T>` here as we are accessing the `.length` property, which doesn't exist on normal `Iterator`s.

Code after:
/* @minVersion 7.0.0-beta.0 */

export default function _iterableToArrayLimitLoose<T>(
  arr: Array<T> | string,
  i: number,
) {
  var iterator: Iterator<T> & Function =
    arr &&
    ((typeof Symbol !== "undefined" && arr[Symbol.iterator]) ||
      (arr as any)["@@iterator"]);
  if (iterator == null) return;

  var _arr: T[] = [];
  var step;
  iterator = iterator.call(arr);
  while (arr.length < i && !(step = iterator.next()).done) {
    _arr.push(step.value);
  }
  return _arr;
}
Repository: babel/babel
File: packages/babel-traverse/test/scope.js (JavaScript)

Code before:
      expect(body[1].body[1].expression.left.properties[0]).toStrictEqual(
        originalPropertyMatcher,
      );
      expect(body[1].body[2].expression.right.properties[0]).toStrictEqual(
        originalPropertyMatcher,
      );

      expect(String(program)).toMatchInlineSnapshot(`
        "const b = 1;
        {
          const {
            b
          } = 2;
          ({
            b
          } = 3);
          _ = {
            b
          };
        }"
      `);
    });
  });

  describe("constantViolations", () => {
    it("should not treat as constantViolations 1", () => {
      const program = getPath(`
        function v() { }
        const G2 = () => {
          var a = 1;
          var {
            currentPoint: a,
            centp: v,
          } = {};
        }
      `);
      const a = program.scope.getBinding("v");
      expect(a.constantViolations).toHaveLength(0);
    });
  });
});


Review comment (suggestion):
```suggestion
    it("var redeclarations should not be treated as constantViolations", () => {
```

Code after:
              // eslint-disable-next-line jest/no-conditional-expect
              extra: expect.objectContaining({ shorthand: true }),
            }),
        key: expect.objectContaining({ name: "b" }),
        value: expect.objectContaining({ name: "b" }),
      });

      const { body } = program.node;
      expect(body[1].body[0].declarations[0].id.properties[0]).toStrictEqual(
        originalPropertyMatcher,
      );
      expect(body[1].body[1].expression.left.properties[0]).toStrictEqual(
        originalPropertyMatcher,
      );
      expect(body[1].body[2].expression.right.properties[0]).toStrictEqual(
        originalPropertyMatcher,
      );

      expect(String(program)).toMatchInlineSnapshot(`
        "const b = 1;
        {
          const {
            b
          } = 2;
          ({
            b
          } = 3);
          _ = {
            b
          };
        }"
      `);
    });
  });

  describe("constantViolations", () => {
    it("var redeclarations should not be treated as constantViolations", () => {
      const program = getPath(`
        function v() { }
        function f() {
          var a = 1;
          var {
            currentPoint: a,
            centp: v,
          } = {};
        }
      `);

      const bindingV = program.scope.getBinding("v");
      expect(bindingV.constantViolations).toHaveLength(0);
Repository: babel/babel
File: packages/babel-helpers/src/helpers/interopRequireWildcard.ts (TypeScript)

Code before:
/* @minVersion 7.14.0 */

export default function _interopRequireWildcard(
  obj: any,
  nodeInterop: boolean,
) {
  if (typeof WeakMap === "function") {
    var cacheBabelInterop = new WeakMap();
    var cacheNodeInterop = new WeakMap();
  }

  // @ts-expect-error: assign to function
  return (_interopRequireWildcard = function (obj: any, nodeInterop: boolean) {
    if (!nodeInterop && obj && obj.__esModule) {
      return obj;
    }
    // Temporary variable for output size
    var _: any = nodeInterop ? cacheNodeInterop : cacheBabelInterop;
    var newObj: { [key: string]: any } = { __proto__: null, default: obj };
    var desc: PropertyDescriptor | undefined;

    if (
      obj === null ||
      (typeof obj !== "object" && typeof obj !== "function")
    ) {
      return newObj;
    }

    if (_) {
      if (_.has(obj)) return _.get(obj);
      _.set(obj, newObj);
    }

    for (const key in obj) {
      if (key !== "default" && {}.hasOwnProperty.call(obj, key)) {
        desc =
          (_ = Object.defineProperty) &&
          Object.getOwnPropertyDescriptor(obj, key);
        if (desc && (desc.get || desc.set)) {
          _(newObj, key, desc);
        } else {
          newObj[key] = obj[key];
        }
      }
    }
    return newObj;
  })(obj, nodeInterop);
}


Review comment (performance):
I didn't use `Object(v) === v` here because of possible performance reasons.

Code after:
/* @minVersion 7.14.0 */

export default function _interopRequireWildcard(
  obj: any,
  nodeInterop: boolean,
) {
  if (typeof WeakMap === "function") {
    var cacheBabelInterop = new WeakMap();
    var cacheNodeInterop = new WeakMap();
  }

  // @ts-expect-error: assign to function
  return (_interopRequireWildcard = function (obj: any, nodeInterop: boolean) {
    if (!nodeInterop && obj && obj.__esModule) {
      return obj;
    }
    // Temporary variable for output size
    var _;
    var newObj: { [key: string]: any } = { __proto__: null, default: obj };
    var desc: PropertyDescriptor | undefined;

    if (
      obj === null ||
      (typeof obj !== "object" && typeof obj !== "function")
    ) {
      return newObj;
    }

    _ = nodeInterop ? cacheNodeInterop : cacheBabelInterop;
    if (_) {
      if (_.has(obj)) return _.get(obj);
      _.set(obj, newObj);
    }

    for (const key in obj) {
      if (key !== "default" && {}.hasOwnProperty.call(obj, key)) {
        desc =
          (_ = Object.defineProperty) &&
          Object.getOwnPropertyDescriptor(obj, key);
        if (desc && (desc.get || desc.set)) {
          _(newObj, key, desc);
        } else {
          newObj[key] = obj[key];
        }
      }
    }
    return newObj;
  })(obj, nodeInterop);
}