ronantakizawa/codereview-bench huggingface dataset:
Repository: microsoft/TypeScript
File: tests/cases/fourslash/unreachableCodeAfterEdit.ts (TypeScript)

Code before:
//// 
//// export class TransparentCrypto implements ISecretStorageCrypto {
////     async seal(data: string): Promise<string> {
////         return data;
////     }
////     async unseal(data: string): Promise<string> {
////         return data;
////     }
//// }

// Initially, there should be no errors
verify.numberOfErrorsInCurrentFile(0);

// Add a throw statement ABOVE the imports
goTo.marker("before");
edit.insert("throw new Error('foo');\n");

// Now there should be unreachable code errors
verify.numberOfErrorsInCurrentFile(1);

// Remove the throw statement
goTo.marker("before");
edit.deleteAtCaret("throw new Error('foo');\n".length);

// After removing the throw, there should be no errors again
// BUG: This will fail because reportedUnreachableNodes is not cleared properly
verify.numberOfErrorsInCurrentFile(0);


Review comment (bug):
The comment is misleading. After the fix in binder.ts, this test should pass. The comment suggests the test is expected to fail, but it should now succeed. Consider removing this comment or updating it to explain what the test validates (e.g., '// Verify that unreachable code errors are cleared after removing the throw statement').
```suggestion
// Verify that unreachable code errors are cleared after removing the throw statement
```

Code after:
/// <reference path="fourslash.ts" />

// @allowUnreachableCode: false
// @lib: es2015

// @Filename: /base/browser/browser.ts
//// export const isStandalone = true;

// @Filename: /base/browser/dom.ts
//// export function addDisposableListener() {}

// @Filename: /base/browser/window.ts
//// export const mainWindow = {} as Window;

// @Filename: /workbench.ts
//// /*before*/import { isStandalone } from './base/browser/browser';
//// import { addDisposableListener } from './base/browser/dom';
//// import { mainWindow } from './base/browser/window';
//// 
//// interface ISecretStorageCrypto {
////     seal(data: string): Promise<string>;
////     unseal(data: string): Promise<string>;
//// }
//// 
//// export class TransparentCrypto implements ISecretStorageCrypto {
////     async seal(data: string): Promise<string> {
////         return data;
////     }
////     async unseal(data: string): Promise<string> {
////         return data;
////     }
//// }

verify.numberOfErrorsInCurrentFile(0);

goTo.marker("before");
edit.insert("throw new Error('foo');\n");

verify.numberOfErrorsInCurrentFile(1);

goTo.marker("before");
edit.deleteAtCaret("throw new Error('foo');\n".length);

verify.numberOfErrorsInCurrentFile(0);
Repository: microsoft/TypeScript
File: src/harness/harnessUtils.ts (TypeScript)

Code before:
import vm from "vm";
import * as Harness from "./_namespaces/Harness.js";
import * as ts from "./_namespaces/ts.js";

export function encodeString(s: string): string {
    return Buffer.from(s).toString("utf8");
}

export function evalFile(fileContents: string, fileName: string, nodeContext?: any): void {
    if (nodeContext) {
        vm.runInNewContext(fileContents, nodeContext, fileName);
    }
    else {
        vm.runInThisContext(fileContents, fileName);
    }
}

const newlineRegex = /\r\n?|\n/;

/** Splits the given string on \r\n, or on only \n if that fails, or on only \r if *that* fails. */
export function splitContentByNewlines(content: string): string[] {
    // Split up the input file by line
    const lines = content.split(newlineRegex);
    return lines;
}

/** Reads a file under /tests */
export function readTestFile(path: string): string | undefined {
    if (!path.includes("tests")) {
        path = "tests/" + path;
    }

    let content: string | undefined;
    try {
        content = Harness.IO.readFile(Harness.userSpecifiedRoot + path);
    }
    catch {
        return undefined;
    }

    return content;
}

export function memoize<T extends ts.AnyFunction>(f: T, memoKey: (...anything: any[]) => string): T {
    const cache = new Map<string, any>();

Review comment (bug):
The JSDoc comment is now inaccurate. It describes the old conditional splitting behavior ("or on only \n if that fails, or on only \r if *that* fails") but the implementation now uses a single regex that handles all line terminators simultaneously. Update the comment to describe the actual behavior, e.g., "Splits the given string on any line terminator (\r\n, \r, or \n)."
```suggestion
/** Splits the given string on any line terminator (\r\n, \r, or \n). */
```

Code after:
import vm from "vm";
import * as Harness from "./_namespaces/Harness.js";
import * as ts from "./_namespaces/ts.js";

export function encodeString(s: string): string {
    return Buffer.from(s).toString("utf8");
}

export function evalFile(fileContents: string, fileName: string, nodeContext?: any): void {
    if (nodeContext) {
        vm.runInNewContext(fileContents, nodeContext, fileName);
    }
    else {
        vm.runInThisContext(fileContents, fileName);
    }
}

const newlineRegex = /\r?\n/;

/**
 * Splits the given string on the two reasonable line terminators (\r\n or \n).
 * Does NOT split on `\r` alone, \u2028, or \u2029.
 */
export function splitContentByNewlines(content: string): string[] {
    // Split up the input file by line
    const lines = content.split(newlineRegex);
    return lines;
}

/** Reads a file under /tests */
export function readTestFile(path: string): string | undefined {
    if (!path.includes("tests")) {
        path = "tests/" + path;
    }

    let content: string | undefined;
    try {
        content = Harness.IO.readFile(Harness.userSpecifiedRoot + path);
    }
    catch {
        return undefined;
    }

    return content;
}
Repository: microsoft/TypeScript
File: tests/cases/fourslash/deprecatedInheritedJSDocOverload.ts (TypeScript)

Code before:

//// interface PartialObserver<T> {}
//// interface Subscription {}
//// interface Unsubscribable {}
//// 
//// export interface Subscribable<T> {
////   subscribe(observer?: PartialObserver<T>): Unsubscribable;
////   /** @deprecated Base deprecation 1 */
////   subscribe(next: null | undefined, error: null | undefined, complete: () => void): Unsubscribable;
////   /** @deprecated Base deprecation 2 */
////   subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Unsubscribable;
////   /** @deprecated Base deprecation 3 */
////   subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Unsubscribable;
////   subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Unsubscribable;
//// }

//// interface ThingWithDeprecations<T> extends Subscribable<T> {
////    subscribe(observer?: PartialObserver<T>): Subscription;
////    /** @deprecated 'real' deprecation */
////    subscribe(next: null | undefined, error: null | undefined, complete: () => void): Subscription;
////    /** @deprecated 'real' deprecation */
////    subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Subscription;
//// }

//// declare const a: ThingWithDeprecations<void>
//// a.subscribe/**/(() => {

Review comment (suggestion):
This file contains a baseline call (`verify.baselineQuickInfo()` on line 31), but according to the PR description, the script should have "bailed out if the word 'baseline' appeared anywhere in the file." This file should not have received the `// @strict: false` directive addition.

Code after:
// @strict: false

//// interface PartialObserver<T> {}
//// interface Subscription {}
//// interface Unsubscribable {}
//// 
//// export interface Subscribable<T> {
////   subscribe(observer?: PartialObserver<T>): Unsubscribable;
////   /** @deprecated Base deprecation 1 */
////   subscribe(next: null | undefined, error: null | undefined, complete: () => void): Unsubscribable;
////   /** @deprecated Base deprecation 2 */
////   subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Unsubscribable;
////   /** @deprecated Base deprecation 3 */
////   subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Unsubscribable;
////   subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Unsubscribable;
//// }

//// interface ThingWithDeprecations<T> extends Subscribable<T> {
////    subscribe(observer?: PartialObserver<T>): Subscription;
////    /** @deprecated 'real' deprecation */
////    subscribe(next: null | undefined, error: null | undefined, complete: () => void): Subscription;
////    /** @deprecated 'real' deprecation */
////    subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Subscription;
//// }

//// declare const a: ThingWithDeprecations<void>
Repository: microsoft/TypeScript
File: tests/cases/compiler/enumMemberNameNonIdentifier.ts (TypeScript)

Code before:
// @declaration: true

// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {

Review comment (suggestion):
The comment references "ES5 identifiers" but after the changes in this PR (switching from ScriptTarget.ES5 to ScriptTarget.ESNext in checker.ts line 6777), the behavior has changed. The comment should be updated to reflect that the code now checks against the latest standard (ESNext) rather than ES5. Consider updating to "Enum members with names that are not valid identifiers" or specify the current target being used.

Code after:
// @declaration: true

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;
Repository: microsoft/TypeScript
File: tests/baselines/reference/enumMemberNameNonIdentifier.js (JavaScript)

Code before:
//// [tests/cases/compiler/enumMemberNameNonIdentifier.ts] ////

//// [enumMemberNameNonIdentifier.ts]
// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {

Review comment (suggestion):
The comment references "ES5 identifiers" but this is outdated given the PR changes from ScriptTarget.ES5 to ScriptTarget.ESNext. The comment should be updated to reflect that validation is no longer based on ES5 standards.

Code after:
//// [tests/cases/compiler/enumMemberNameNonIdentifier.ts] ////

//// [enumMemberNameNonIdentifier.ts]
export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;


//// [enumMemberNameNonIdentifier.js]
export const a = 1 /* E["hyphen-member"] */;
export const b = 2 /* E["123startsWithNumber"] */;
export const c = 3 /* E["has space"] */;
export const d = 0 /* E.regular */;
export const e = 4 /* E.Ϳ */;


//// [enumMemberNameNonIdentifier.d.ts]
export declare const enum E {
    regular = 0,
Repository: microsoft/TypeScript
File: tests/baselines/reference/enumMemberNameNonIdentifier.js (JavaScript)

Code before:
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {
    return E["hyphen-member"];
}


//// [enumMemberNameNonIdentifier.js]
// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit
// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = 1 /* E["hyphen-member"] */;
export const b = 2 /* E["123startsWithNumber"] */;
export const c = 3 /* E["has space"] */;
// This should use dot notation since "regular" is a valid identifier
export const d = 0 /* E.regular */;
// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = 4 /* E.Ϳ */;
// Function returning specific enum member type
export function getHyphenMember() {
    return 1 /* E["hyphen-member"] */;
}


//// [enumMemberNameNonIdentifier.d.ts]
export declare const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    Ϳ = 4
}
export declare const a = E["hyphen-member"];
export declare const b = E["123startsWithNumber"];

Review comment (suggestion):
The comment references "ES5 identifiers" but this is outdated given the PR changes from ScriptTarget.ES5 to ScriptTarget.ESNext. The comment should be updated to reflect that validation is no longer based on ES5 standards.

Code after:
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;


//// [enumMemberNameNonIdentifier.js]
export const a = 1 /* E["hyphen-member"] */;
export const b = 2 /* E["123startsWithNumber"] */;
export const c = 3 /* E["has space"] */;
export const d = 0 /* E.regular */;
export const e = 4 /* E.Ϳ */;


//// [enumMemberNameNonIdentifier.d.ts]
export declare const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    Ϳ = 4
}
export declare const a = E["hyphen-member"];
export declare const b = E["123startsWithNumber"];
export declare const c = E["has space"];
export declare const d = E.regular;
export declare const e = E.Ϳ;
Repository: microsoft/TypeScript
File: tests/cases/compiler/enumMemberNameNonIdentifier.ts (TypeScript)

Code before:
// @declaration: true

// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {
    return E["hyphen-member"];
}


Review comment (suggestion):
The comment states "valid identifier in ES2015+ but NOT in ES5" which is technically correct, but should note that after this PR's changes (checker.ts now uses ScriptTarget.ESNext instead of ES5), this character is now treated as a valid identifier by the type checker. The comment should clarify that while the character Ϳ is ES2015+, the code now accepts it as a valid identifier.

Code after:
// @declaration: true

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;
Repository: microsoft/TypeScript
File: tests/cases/compiler/enumMemberNameNonIdentifier.ts (TypeScript)

Code before:
// @declaration: true

// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {
    return E["hyphen-member"];
}


Review comment (suggestion):
The comment states "because the member names are not valid ES5 identifiers" but this is outdated given the PR changes. After switching from ScriptTarget.ES5 to ScriptTarget.ESNext in checker.ts, the validation is no longer based on ES5. Consider updating to simply "because the member names are not valid identifiers" to avoid confusion about which standard is being used.
```suggestion
// because the member names are not valid identifiers
```

Code after:
// @declaration: true

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;
Repository: microsoft/TypeScript
File: tests/cases/compiler/enumMemberNameNonIdentifier.ts (TypeScript)

Code before:
// @declaration: true

// Enum members with names that are not valid ES5 identifiers
// should use indexed access syntax in declaration emit

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

// These should infer types that use indexed access syntax 
// because the member names are not valid ES5 identifiers
export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];

// This should use dot notation since "regular" is a valid identifier
export const d = E.regular;

// Greek Capital Yot (U+037F) - valid in ES2015+ but NOT in ES5
// Because isIdentifierText uses ES5, this will use indexed access syntax
export const e = E.Ϳ;

// Function returning specific enum member type
export function getHyphenMember(): typeof E["hyphen-member"] {
    return E["hyphen-member"];
}


Review comment (bug):
The comment states "Because isIdentifierText uses ES5, this will use indexed access syntax" but this is incorrect after the PR changes. The code in checker.ts line 6777 now uses ScriptTarget.ESNext instead of ES5, so isIdentifierText no longer uses ES5 for validation. The Greek Capital Yot (U+037F) is actually a valid identifier in ESNext, so the declaration file correctly uses dot notation (E.Ϳ) rather than indexed access. The comment should be updated to reflect the actual behavior.
```suggestion
// Greek Capital Yot (U+037F) - valid identifier in ESNext
// isIdentifierText is called with ScriptTarget.ESNext, so this member
// is treated as an identifier and should use dot notation in declarations
```

Code after:
// @declaration: true

export const enum E {
    regular = 0,
    "hyphen-member" = 1,
    "123startsWithNumber" = 2,
    "has space" = 3,
    // Greek Capital Yot (U+037F) - valid identifier in ES2015+ but NOT in ES5
    Ϳ = 4,
}

export const a = E["hyphen-member"];
export const b = E["123startsWithNumber"];
export const c = E["has space"];
export const d = E.regular;
export const e = E.Ϳ;