ronantakizawa/codereview-bench huggingface dataset:
Repository: microsoft/TypeScript
File: scripts/build/tests.mjs (JavaScript)

Code before:
import { CancelError } from "@esfx/canceltoken";
import chalk from "chalk";
import fs from "fs";
import os from "os";
import path from "path";

import { findUpFile, findUpRoot } from "./findUpDir.mjs";
import cmdLineOptions from "./options.mjs";
import { exec, ExecError } from "./utils.mjs";

const rf = { recursive: true, force: true };
const mochaJs = path.resolve(findUpRoot(), "node_modules", "mocha", "bin", "_mocha");
export const localBaseline = "tests/baselines/local/";
export const refBaseline = "tests/baselines/reference/";
export const localRwcBaseline = "internal/baselines/rwc/local";
export const refRwcBaseline = "internal/baselines/rwc/reference";
export const coverageDir = "coverage";

/**
 * @param {string} runJs
 * @param {string} defaultReporter
 * @param {boolean} runInParallel
 * @param {object} options
 * @param {import("@esfx/canceltoken").CancelToken} [options.token]
 * @param {boolean} [options.watching]
 */
export async function runConsoleTests(runJs, defaultReporter, runInParallel, options = {}) {
    let testTimeout = cmdLineOptions.timeout;
    const tests = cmdLineOptions.tests;
    const inspect = cmdLineOptions.break || cmdLineOptions.inspect;
    const runners = cmdLineOptions.runners;
    const light = cmdLineOptions.light;
    const stackTraceLimit = cmdLineOptions.stackTraceLimit;
    const testConfigFile = "test.config";
    const failed = cmdLineOptions.failed;
    const keepFailed = cmdLineOptions.keepFailed;

Review comment (suggestion):
Though this saves typing (kinda), I'm not totally sure I like this compared to just writing them inline, which this script already does for `fs.promises.mkdir`.

Code after:
import {
    CancelError,
} from "@esfx/canceltoken";
import chalk from "chalk";
import fs from "fs";
import os from "os";
import path from "path";

import {
    findUpFile,
    findUpRoot,
} from "./findUpDir.mjs";
import cmdLineOptions from "./options.mjs";
import {
    exec,
    ExecError,
    rimraf,
} from "./utils.mjs";

const mochaJs = path.resolve(findUpRoot(), "node_modules", "mocha", "bin", "_mocha");
export const localBaseline = "tests/baselines/local/";
export const refBaseline = "tests/baselines/reference/";
export const coverageDir = "coverage";

/**
 * @param {string} runJs
 * @param {string} defaultReporter
 * @param {boolean} runInParallel
 * @param {object} options
 * @param {import("@esfx/canceltoken").CancelToken} [options.token]
 * @param {boolean} [options.watching]
 */
export async function runConsoleTests(runJs, defaultReporter, runInParallel, options = {}) {
    const testTimeout = cmdLineOptions.timeout;
    const tests = cmdLineOptions.tests;
    const inspect = cmdLineOptions.break || cmdLineOptions.inspect;
Repository: microsoft/TypeScript
File: scripts/build/utils.mjs (JavaScript)

Code before:
        }
    }
}

const unset = Symbol();
/**
 * @template T
 * @param {() => T} fn
 * @returns {() => T}
 */
export function memoize(fn) {
    /** @type {T | unset} */
    let value = unset;
    return () => {
        if (value === unset) {
            value = fn();
        }
        return value;
    };
}

/**
 * @param {fs.PathLike} p
 */
export function rimraf(p) {
    // Node's recursive rm is just vendored rimraf, but without a different maxRetries default on Windows.
    return fs.promises.rm(p, { recursive: true, force: true, maxRetries: process.platform === "win32" ? 10 : 0 });
}


Review comment (suggestion):
this should be "with", right?
```suggestion
    // Node's recursive rm is just vendored rimraf, but with a different maxRetries default on Windows.
```

Code after:
        }
    }
}

const unset = Symbol();
/**
 * @template T
 * @param {() => T} fn
 * @returns {() => T}
 */
export function memoize(fn) {
    /** @type {T | unset} */
    let value = unset;
    return () => {
        if (value === unset) {
            value = fn();
        }
        return value;
    };
}

/**
 * @param {fs.PathLike} p
 */
export function rimraf(p) {
    // The rimraf package uses maxRetries=10 on Windows, but Node's fs.rm does not have that special case.
    return fs.promises.rm(p, { recursive: true, force: true, maxRetries: process.platform === "win32" ? 10 : 0 });
}
Repository: microsoft/TypeScript
File: tests/baselines/reference/jsDeclarationsGetterSetter.js (JavaScript)

Code before:
//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsGetterSetter.ts] ////

//// [index.js]
export class A {
    get x() {
        return 12;
    }
}

export class B {
    /**
     * @param {number} _arg
     */
    set x(_arg) {
    }
}

export class C {
    get x() {
        return 12;
    }
    set x(_arg) {
    }
}

export class D {}
Object.defineProperty(D.prototype, "x", {
    get() {
        return 12;
    }
});

export class E {}
Object.defineProperty(E.prototype, "x", {
    /**
     * @param {number} _arg
     */
    set(_arg) {}
});

export class F {}
Object.defineProperty(F.prototype, "x", {
    get() {
        return 12;
    },
    /**
     * @param {number} _arg
     */
    set(_arg) {}
});

export class G {}
Object.defineProperty(G.prototype, "x", {
    /**
     * @param {number[]} args
     */
    set(...args) {}
});


//// [index.js]
export class A {
    get x() {
        return 12;
    }
}
export class B {
    /**
     * @param {number} _arg
     */
    set x(_arg) {
    }
}
export class C {
    get x() {
        return 12;
    }
    set x(_arg) {
    }
}
export class D {
}
Object.defineProperty(D.prototype, "x", {
    get() {
        return 12;
    }
});
export class E {
}
Object.defineProperty(E.prototype, "x", {
    /**
     * @param {number} _arg
     */
    set(_arg) { }
});
export class F {
}
Object.defineProperty(F.prototype, "x", {
    get() {
        return 12;
    },
    /**
     * @param {number} _arg
     */
    set(_arg) { }
});
export class G {
}
Object.defineProperty(G.prototype, "x", {
    /**
     * @param {number[]} args
     */
    set(...args) { }
});


//// [index.d.ts]
export class A {
    get x(): number;
}
export class B {
    /**
     * @param {number} _arg
     */
    set x(_arg: number);
}
export class C {
    set x(_arg: number);
    get x(): number;
}
export class D {
    get x(): number;
}
export class E {
    set x(_arg: number);
}
export class F {
    set x(_arg: number);
    get x(): number;
}
export class G {
    set x(args: number);
}


Review comment (performance):
this could be generated as `args_0` (or smth like that) - but I don't think it's worth the added code complexity

Code after:
//// [index.d.ts]
export class A {
    get x(): number;
}
export class B {
    /**
     * @param {number} _arg
     */
    set x(_arg: number);
}
export class C {
    set x(_arg: number);
    get x(): number;
}
export class D {
    get x(): number;
}
export class E {
    set x(_arg: number);
}
export class F {
    set x(_arg: number);
    get x(): number;
}
export class G {
    set x(args: number);
}
export class H {
    set x(value: any);
}
Repository: microsoft/TypeScript
File: tests/cases/fourslash/stringCompletionsFromGenericConditionalTypesUsingTemplateLiteralTypes.ts (TypeScript)

Code before:
//// type validateString
 = s extends keyword
////     ? s
////     : s extends `${infer left extends keyword}|${infer right}`
////     ? right extends keyword
////         ? s
////         : `${left}|${keyword}`
////     : keyword
////
//// type isUnknown<t> = unknown extends t
////     ? [t] extends [{}]
////         ? false
////         : true
////     : false
////
//// type validate<def> = def extends string
////     ? validateString<def>
////     : isUnknown<def> extends true
////     ? keyword
////     : {
////           [k in keyof def]: validate<def[k]>
////       }
//// const parse = <def>(def: validate<def>) => def
//// const shallowExpression = parse("foo|/*ts*/")
//// const nestedExpression = parse({ prop: "foo|/*ts2*/" })

verify.completions({ marker: ["ts", "ts2"], exact: ["foo|foo", "foo|bar", "foo|baz"] });


Review comment (suggestion):
This matches Ron's initial expectations from https://github.com/microsoft/TypeScript/pull/54121 .

The other case, ideally, should provide the same completions but currently, the ones based on the contextual types and the ones based on `getCandidateSignaturesForStringLiteralCompletions` work quite differently (and even more so with changes from https://github.com/microsoft/TypeScript/pull/54121 ). I plan to close the gap between both a little bit but I'm leaving that for future PRs.

Code after:
//// type validateString
 = s extends keyword
////     ? s
////     : s extends `${infer left extends keyword}|${infer right}`
////     ? right extends keyword
////         ? s
////         : `${left}|${keyword}`
////     : keyword
////
//// type isUnknown<t> = unknown extends t
////     ? [t] extends [{}]
////         ? false
////         : true
////     : false
////
//// type validate<def> = def extends string
////     ? validateString<def>
////     : isUnknown<def> extends true
////     ? keyword
////     : {
////           [k in keyof def]: validate<def[k]>
////       }
//// const parse = <def>(def: validate<def>) => def
//// const shallowExpression = parse("foo|/*ts*/")
//// const nestedExpression = parse({ prop: "foo|/*ts2*/" })

verify.completions({ marker: ["ts"], exact: ["foo", "bar", "baz", "foo|foo", "foo|bar", "foo|baz"] });
verify.completions({ marker: ["ts2"], exact: ["foo|foo", "foo|bar", "foo|baz"] });
Repository: microsoft/TypeScript
File: tests/cases/conformance/es2020/modules/exportAsNamespace5.ts (TypeScript)

Code before:
// @module: esnext
// @moduleResolution: bundler

// @filename: three.d.ts
export type Named = 0;

// @filename: two.d.ts
export * as default from "./three";

// @filename: one.ts
import ns from "./two";
type Alias = ns.Named;


Review comment (bug):
Please could we also test the value land namespace properties. Either in this test case or as a separate one.

https://www.typescriptlang.org/dev/bug-workbench/?moduleResolution=99&module=199&emitDeclarationOnly=true&ts=5.2.2#code/PTAEAEDMEsBsFMB2BDAtvAXKALgCwE7zwB0AJsdgM4CwAUKfAMazKGiMD2il2oAcmniksABgDcdeAA8ADh3y8A3v0GlQAXwm06ICDAQp0WbAHcOZCjVrS5C0ACpQySqAaRkAV1i9I+DqlAAImJgPEJ4QK0dMCg4JEEsLhIqOmhUW15uUF9-IJDTDki6bmIBdFIxIA

Code after:
// @module: esnext
// @moduleResolution: bundler

// @filename: three.d.ts
export type Named = 0;
declare const Named: 0;

// @filename: two.d.ts
export * as default from "./three";

// @filename: one.ts
import ns from "./two";
type Alias = ns.Named;
ns.Named;
Repository: microsoft/TypeScript
File: src/lib/es2015.iterable.d.ts (TypeScript)

Code before:
interface WeakMapConstructor {
    new <K extends WeakKey = WeakKey, V = any>(iterable?: Iterable<readonly [K, V]> | null): WeakMap<K, V>;
}

interface Set<T> {
    /** Iterates over values in the set. */
    [Symbol.iterator](): IterableIterator<T>;
    /**
     * Returns an iterable of [v,v] pairs for every value `v` in the set.
     */
    entries(): IterableIterator<[T, T]>;
    /**
     * Despite its name, returns an iterable of the values in the set.
     */
    keys(): IterableIterator<T>;

    /**
     * Returns an iterable of values in the set.
     */
    values(): IterableIterator<T>;
}

interface ReadonlySet<T> {
    /** Iterates over values in the set. */
    [Symbol.iterator](): IterableIterator<T>;

    /**
     * Returns an iterable of [v,v] pairs for every value `v` in the set.
     */
    entries(): IterableIterator<[T, T]>;

    /**
     * Despite its name, returns an iterable of the values in the set.
     */
    keys(): IterableIterator<T>;

    /**
     * Returns an iterable of values in the set.
     */
    values(): IterableIterator<T>;
}

interface SetConstructor {
    new <T>(iterable?: Iterable<T> | null): Set<T>;
}

interface WeakSet<T extends WeakKey> {}

interface WeakSetConstructor {
    new <T extends WeakKey = WeakKey>(iterable: Iterable<T>): WeakSet<T>;
}

Review comment (suggestion):
Wouldn't `unknown` be a better default here? It would more closely match the previous behavior (unconstrained type param is implicitly constrained to `unknown`, at least in strict mode)
```suggestion
    new <K extends WeakKey = WeakKey, V = unknown>(iterable?: Iterable<readonly [K, V]> | null): WeakMap<K, V>;

```

Code after:

    /**
     * Returns an iterable of key, value pairs for every entry in the map.
     */
    entries(): MapIterator<[K, V]>;

    /**
     * Returns an iterable of keys in the map
     */
    keys(): MapIterator<K>;

    /**
     * Returns an iterable of values in the map
     */
    values(): MapIterator<V>;
}

interface MapConstructor {
    new (): Map<any, any>;
    new <K, V>(iterable?: Iterable<readonly [K, V]> | null): Map<K, V>;
}

interface WeakMap<K extends WeakKey, V> {}

interface WeakMapConstructor {
    new <K extends WeakKey = WeakKey, V = any>(iterable?: Iterable<readonly [K, V]> | null): WeakMap<K, V>;
}

interface SetIterator<T> extends IteratorObject<T, BuiltinIteratorReturn, unknown> {
    [Symbol.iterator](): SetIterator<T>;
}

interface Set<T> {
    /** Iterates over values in the set. */
    [Symbol.iterator](): SetIterator<T>;

    /**
     * Returns an iterable of [v,v] pairs for every value `v` in the set.
     */
    entries(): SetIterator<[T, T]>;

    /**
     * Despite its name, returns an iterable of the values in the set.
     */
    keys(): SetIterator<T>;

    /**
     * Returns an iterable of values in the set.
     */
    values(): SetIterator<T>;
}
Repository: microsoft/TypeScript
File: tests/cases/conformance/es2023/intlNumberFormatES2023.ts (TypeScript)

Code before:
// @target: es2022
// @lib: es2022,es2023.intl
// @strict: true

// New / updated resolved options in ES2023, including type change for useGrouping
const { roundingPriority, roundingMode, roundingIncrement, trailingZeroDisplay, useGrouping } =  new Intl.NumberFormat('en-GB').resolvedOptions();

// Empty options
new Intl.NumberFormat('en-GB', {});

// Rounding
new Intl.NumberFormat('en-GB', { roundingPriority: 'lessPrecision', roundingIncrement: 100, roundingMode: 'trunc' });

// Changes to signDisplay
const { signDisplay } = new Intl.NumberFormat('en-GB', { signDisplay: 'negative' }).resolvedOptions();

// Changes to useGrouping
new Intl.NumberFormat('en-GB', { useGrouping: true });
new Intl.NumberFormat('en-GB', { useGrouping: 'true' });
new Intl.NumberFormat('en-GB', { useGrouping: 'always' });

// formatRange
new Intl.NumberFormat('en-GB').formatRange(10, 100);
new Intl.NumberFormat('en-GB').formatRange(10n, 1000n);
new Intl.NumberFormat('en-GB').formatRangeToParts(10, 1000)[0];
new Intl.NumberFormat('en-GB').formatRangeToParts(10n, 1000n)[0];

// Arbitrary-precision string arguments
new Intl.NumberFormat('en-GB').format('-12.3E-4');
new Intl.NumberFormat('en-GB').formatRange('123.4', '567.8');
new Intl.NumberFormat('en-GB').formatRangeToParts('123E-4', '567E8');
new Intl.NumberFormat('en-GB').format('Infinity');
new Intl.NumberFormat('en-GB').format('-Infinity');
new Intl.NumberFormat('en-GB').format('+Infinity');


Review comment (suggestion):
This test file consistently uses single quotes for string literals (e.g. earlier Intl.NumberFormat calls). Consider switching these newly added string literals to single quotes as well to match the existing style and keep baselines stable.

Code after:

// Changes to useGrouping
new Intl.NumberFormat('en-GB', { useGrouping: true });
new Intl.NumberFormat('en-GB', { useGrouping: 'true' });
new Intl.NumberFormat('en-GB', { useGrouping: 'always' });

// formatRange
new Intl.NumberFormat('en-GB').formatRange(10, 100);
new Intl.NumberFormat('en-GB').formatRange(10n, 1000n);
new Intl.NumberFormat('en-GB').formatRangeToParts(10, 1000)[0];
new Intl.NumberFormat('en-GB').formatRangeToParts(10n, 1000n)[0];

// Arbitrary-precision string arguments
new Intl.NumberFormat('en-GB').format('-12.3E-4');
new Intl.NumberFormat('en-GB').formatRange('123.4', '567.8');
new Intl.NumberFormat('en-GB').formatRangeToParts('123E-4', '567E8');
new Intl.NumberFormat('en-GB').format('Infinity');
new Intl.NumberFormat('en-GB').format('-Infinity');
new Intl.NumberFormat('en-GB').format('+Infinity');

// Test approximatelySign part type
const nf = new Intl.NumberFormat("en-US", {
  style: "currency",
  currency: "EUR",
  maximumFractionDigits: 0,
});

const filtered = nf
  .formatRangeToParts(100, 100)
  .filter((part) => part.type !== "approximatelySign")
  .map((part) => part.value)
  .join("");
Repository: microsoft/TypeScript
File: tests/cases/conformance/es2023/intlNumberFormatES2023.ts (TypeScript)

Code before:
// @target: es2022
// @lib: es2022,es2023.intl
// @strict: true

// New / updated resolved options in ES2023, including type change for useGrouping
const { roundingPriority, roundingMode, roundingIncrement, trailingZeroDisplay, useGrouping } =  new Intl.NumberFormat('en-GB').resolvedOptions();

// Empty options
new Intl.NumberFormat('en-GB', {});

// Rounding
new Intl.NumberFormat('en-GB', { roundingPriority: 'lessPrecision', roundingIncrement: 100, roundingMode: 'trunc' });

// Changes to signDisplay
const { signDisplay } = new Intl.NumberFormat('en-GB', { signDisplay: 'negative' }).resolvedOptions();

// Changes to useGrouping
new Intl.NumberFormat('en-GB', { useGrouping: true });
new Intl.NumberFormat('en-GB', { useGrouping: 'true' });
new Intl.NumberFormat('en-GB', { useGrouping: 'always' });

// formatRange
new Intl.NumberFormat('en-GB').formatRange(10, 100);
new Intl.NumberFormat('en-GB').formatRange(10n, 1000n);
new Intl.NumberFormat('en-GB').formatRangeToParts(10, 1000)[0];
new Intl.NumberFormat('en-GB').formatRangeToParts(10n, 1000n)[0];

// Arbitrary-precision string arguments
new Intl.NumberFormat('en-GB').format('-12.3E-4');
new Intl.NumberFormat('en-GB').formatRange('123.4', '567.8');
new Intl.NumberFormat('en-GB').formatRangeToParts('123E-4', '567E8');
new Intl.NumberFormat('en-GB').format('Infinity');
new Intl.NumberFormat('en-GB').format('-Infinity');
new Intl.NumberFormat('en-GB').format('+Infinity');


Review comment (suggestion):
For consistency with the rest of this conformance test (which uses single quotes), consider using single quotes for "approximatelySign" and the join separator here as well.

Code after:
// formatRange
new Intl.NumberFormat('en-GB').formatRange(10, 100);
new Intl.NumberFormat('en-GB').formatRange(10n, 1000n);
new Intl.NumberFormat('en-GB').formatRangeToParts(10, 1000)[0];
new Intl.NumberFormat('en-GB').formatRangeToParts(10n, 1000n)[0];

// Arbitrary-precision string arguments
new Intl.NumberFormat('en-GB').format('-12.3E-4');
new Intl.NumberFormat('en-GB').formatRange('123.4', '567.8');
new Intl.NumberFormat('en-GB').formatRangeToParts('123E-4', '567E8');
new Intl.NumberFormat('en-GB').format('Infinity');
new Intl.NumberFormat('en-GB').format('-Infinity');
new Intl.NumberFormat('en-GB').format('+Infinity');

// Test approximatelySign part type
const nf = new Intl.NumberFormat("en-US", {
  style: "currency",
  currency: "EUR",
  maximumFractionDigits: 0,
});

const filtered = nf
  .formatRangeToParts(100, 100)
  .filter((part) => part.type !== "approximatelySign")
  .map((part) => part.value)
  .join("");
Repository: microsoft/TypeScript
File: tests/baselines/reference/tsserver/typingsInstaller/discover-typings-should-return-node-for-core-modules.js (JavaScript)

Code before:
TI:: [hh:mm:ss:mss] Finished typings discovery:
    {
      "cachedTypingPaths": [],
      "newTypingNames": [
        "node",
        "somename"
      ],
      "filesToWatch": [
        "/home/src/projects/project/bower_components",
        "/home/src/projects/project/node_modules"
      ]
    }

ts.JsTyping.discoverTypings::
    {
      "fileNames": [
        "/home/src/projects/project/app.js"
      ],
      "projectRootPath": "/home/src/projects/project",
      "safeList": {},
      "packageNameToTypingLocation": {},
      "typeAcquisition": {
        "enable": true
      },
      "unresolvedImports": [
        "node:worker_threads",
        "somename"
      ],
      "typesRegistry": {},
      "compilerOptions": {}
    }
TI:: [hh:mm:ss:mss] Inferred typings from unresolved imports: ["node","somename"]
TI:: [hh:mm:ss:mss] Finished typings discovery:
    {
      "cachedTypingPaths": [],
      "newTypingNames": [
        "node",
        "somename"
      ],
      "filesToWatch": [
        "/home/src/projects/project/bower_components",
        "/home/src/projects/project/node_modules"
      ]
    }

ts.JsTyping.discoverTypings::
    {
      "fileNames": [
        "/home/src/projects/project/app.js"
      ],
      "projectRootPath": "/home/src/projects/project",

Review comment (suggestion):
I was very confused why everything in this list got changed until I realized it's because one module was removed and one was added, so the diff makes it look like the code shifted

Code after:
TI:: [hh:mm:ss:mss] Finished typings discovery:
    {
      "cachedTypingPaths": [],
      "newTypingNames": [
        "node",
        "somename"
      ],
      "filesToWatch": [
        "/home/src/projects/project/bower_components",
        "/home/src/projects/project/node_modules"
      ]
    }

ts.JsTyping.discoverTypings::
    {
      "fileNames": [
        "/home/src/projects/project/app.js"
      ],
      "projectRootPath": "/home/src/projects/project",
      "safeList": {},
      "packageNameToTypingLocation": {},
      "typeAcquisition": {
        "enable": true
      },
      "unresolvedImports": [
        "node:quic",
        "somename"
      ],
      "typesRegistry": {},
      "compilerOptions": {}
    }
TI:: [hh:mm:ss:mss] Inferred typings from unresolved imports: ["node","somename"]
TI:: [hh:mm:ss:mss] Finished typings discovery:
    {
      "cachedTypingPaths": [],
      "newTypingNames": [
        "node",
        "somename"
      ],
      "filesToWatch": [
        "/home/src/projects/project/bower_components",
        "/home/src/projects/project/node_modules"
      ]
    }

ts.JsTyping.discoverTypings::
    {
      "fileNames": [
        "/home/src/projects/project/app.js"
      ],
      "projectRootPath": "/home/src/projects/project",