ronantakizawa/codereview-bench huggingface dataset:
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/core/Browser.ts (TypeScript)

Code before:
    // SAFETY: A UserContext is always created for the default context.
    return this.#userContexts.get('')!;
  }

  get userContexts(): Iterable<UserContext> {
    return this.#userContexts.values();
  }

  @throwIfDisposed((browser: Browser) => {
    // SAFETY: By definition of `disposed`, `#reason` is defined.
    return browser.#reason!;
  })
  async close(): Promise<void> {
    await this.#session.send('browser.close', {});
    this.#reason = `Browser has already closed.`;
    this.emit('closed', {reason: this.#reason});
    this.removeAllListeners();
  }

  @throwIfDisposed((browser: Browser) => {
    // SAFETY: By definition of `disposed`, `#reason` is defined.
    return browser.#reason!;
  })
  async addPreloadScript(
    functionDeclaration: string,
    options: AddPreloadScriptOptions = {}
  ): Promise<string> {
    const {
      result: {script},
    } = await this.#session.send('script.addPreloadScript', {
      functionDeclaration,
      ...options,
      contexts: options.contexts?.map(context => {
        return context.id;
      }) as [string, ...string[]],
    });
    return script;
  }

  @throwIfDisposed((browser: Browser) => {
    // SAFETY: By definition of `disposed`, `#reason` is defined.
    return browser.#reason!;
  })
  async removePreloadScript(script: string): Promise<void> {
    await this.#session.send('script.removePreloadScript', {
      script,
    });
  }
}


Review comment (bug):
Why is this in finally block? The behavior changed now, previously the browser would not be disposed if the command failed. 

Code after:
  }

  get userContexts(): Iterable<UserContext> {
    return this.#userContexts.values();
  }

  dispose(reason?: string, close?: boolean): void {
    if (this.disposed) {
      return;
    }
    this.#reason = reason ?? `Browser was disposed.`;
    if (close) {
      this.emit('closed', {reason: this.#reason});
    }
    this.emit('disconnect', {reason: this.#reason});
    this.removeAllListeners();
  }

  @throwIfDisposed((browser: Browser) => {
    // SAFETY: By definition of `disposed`, `#reason` is defined.
    return browser.#reason!;
  })
  async close(): Promise<void> {
    try {
      await this.#session.send('browser.close', {});
    } finally {
      this.dispose(`Browser was closed.`, true);
    }
  }

  @throwIfDisposed((browser: Browser) => {
    // SAFETY: By definition of `disposed`, `#reason` is defined.
    return browser.#reason!;
  })
  async addPreloadScript(
    functionDeclaration: string,
    options: AddPreloadScriptOptions = {}
  ): Promise<string> {
    const {
      result: {script},
    } = await this.#session.send('script.addPreloadScript', {
      functionDeclaration,
      ...options,
      contexts: options.contexts?.map(context => {
        return context.id;
      }) as [string, ...string[]],
    });
    return script;
  }

  @throwIfDisposed((browser: Browser) => {
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/core/UserContext.ts (TypeScript)

Code before:
    type: Bidi.BrowsingContext.CreateType,
    options: CreateBrowsingContextOptions = {}
  ): Promise<BrowsingContext> {
    const {
      result: {context: contextId},
    } = await this.#session.send('browsingContext.create', {
      type,
      ...options,
      referenceContext: options.referenceContext?.id,
    });

    const browsingContext = this.#browsingContexts.get(contextId);
    assert(
      browsingContext,
      'The WebDriver BiDi implementation is failing to create a browsing context correctly.'
    );

    // We use an array to avoid the promise from being awaited.
    return browsingContext;
  }

  async remove(): Promise<void> {
    // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289.
    // TODO: Uncomment once we have adopted the new browsing contexts.
    // const promises = [];
    // for (const browsingContext of this.#browsingContexts.values()) {
    //   promises.push(browsingContext.close());
    // }
    // await Promise.all(promises);
    this.emit('destroyed', {userContext: this});
    this.removeAllListeners();
  }
}


Review comment (suggestion):
this should be handled by bidi, I think we do not need to close explicitly but we might need to wait till it is destroyed.

Code after:

  async createBrowsingContext(
    type: Bidi.BrowsingContext.CreateType,
    options: CreateBrowsingContextOptions = {}
  ): Promise<BrowsingContext> {
    const {
      result: {context: contextId},
    } = await this.#session.send('browsingContext.create', {
      type,
      ...options,
      referenceContext: options.referenceContext?.id,
    });

    const browsingContext = this.#browsingContexts.get(contextId);
    assert(
      browsingContext,
      'The WebDriver BiDi implementation is failing to create a browsing context correctly.'
    );

    // We use an array to avoid the promise from being awaited.
    return browsingContext;
  }

  async remove(): Promise<void> {
    // TODO: Call `removeUserContext` once available.
    this.emit('destroyed', {userContext: this});
    this.removeAllListeners();
  }
}
Repository: puppeteer/puppeteer
File: tools/eslint/src/check-license.ts (TypeScript)

Code before:
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: any) {
      if (comment?.range[0] >= 0 && comment?.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header?.value.includes('@license') &&
          header?.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header?.value)
        ) {
          return;
        }

        // Add header license
        if (!header || !header.value.includes('@license')) {
          context.report({
            node: node,
            messageId: 'licenseRule',
          });
        }
      },
    };
  },
});

module.exports = enforceLicenseRule;


Review comment (suggestion):
```suggestion
          header &&
          header.value.includes('@license') &&
          header.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header.value)
```

Code after:
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: TSESTree.Comment) {
      if (comment && comment.range[0] >= 0 && comment.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header &&
          header.value.includes('@license') &&
          header.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header.value)
        ) {
          return;
        }

        // Add header license
        if (!header || !header.value.includes('@license')) {
          const startLoc: [number, number] = [0, 88];
          context.report({
            node: node,
            messageId: 'licenseRule',
            fix(fixer) {
              return fixer.insertTextBeforeRange(startLoc, licenseHeader);
            },
          });
        }
Repository: puppeteer/puppeteer
File: tools/eslint/src/check-license.ts (TypeScript)

Code before:
const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: any) {
      if (comment?.range[0] >= 0 && comment?.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header?.value.includes('@license') &&
          header?.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header?.value)
        ) {
          return;
        }

        // Add header license
        if (!header || !header.value.includes('@license')) {
          context.report({
            node: node,
            messageId: 'licenseRule',
          });
        }
      },

Review comment (suggestion):
```suggestion
    function isHeaderComment(comment: TSESTree.Comment) {
```

Code after:

const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const currentYear = new Date().getFullYear;

const licenseHeader = `/**
 * @license
 * Copyright ${currentYear} Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */`;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'check-license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: TSESTree.Comment) {
      if (comment && comment.range[0] >= 0 && comment.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header &&
          header.value.includes('@license') &&
          header.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header.value)
        ) {
          return;
Repository: puppeteer/puppeteer
File: tools/eslint/src/check-license.ts (TypeScript)

Code before:

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: any) {
      if (comment?.range[0] >= 0 && comment?.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header?.value.includes('@license') &&
          header?.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header?.value)
        ) {
          return;
        }

        // Add header license
        if (!header || !header.value.includes('@license')) {
          context.report({
            node: node,
            messageId: 'licenseRule',
          });
        }
      },
    };

Review comment (suggestion):
```suggestion
      if (comment && comment.range[0] >= 0 && comment.range[1] <= 88) {
```

Code after:
const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const currentYear = new Date().getFullYear;

const licenseHeader = `/**
 * @license
 * Copyright ${currentYear} Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */`;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'check-license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: TSESTree.Comment) {
      if (comment && comment.range[0] >= 0 && comment.range[1] <= 88) {
        return true;
      } else {
        return false;
      }
    }

    return {
      Program(node) {
        if (
          header &&
          header.value.includes('@license') &&
          header.value.includes('SPDX-License-Identifier: Apache-2.0') &&
          copyrightPattern.test(header.value)
        ) {
          return;
        }
Repository: puppeteer/puppeteer
File: tools/eslint/src/check-license.ts (TypeScript)

Code before:
/**
 * @license
 * Copyright 2024 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import {ESLintUtils} from '@typescript-eslint/utils';

const createRule = ESLintUtils.RuleCreator(name => {
  return `https://github.com/puppeteer/puppeteer/tree/main/tools/eslint/${name}.js`;
});

const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]

Review comment (bug):
I think other places are wrong.
```suggestion
  return `https://github.com/puppeteer/puppeteer/tree/main/tools/eslint/${name}.ts`;
```

Code after:
/**
 * @license
 * Copyright 2024 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import type {TSESTree} from '@typescript-eslint/utils';
import {ESLintUtils} from '@typescript-eslint/utils';

const createRule = ESLintUtils.RuleCreator(name => {
  return `https://github.com/puppeteer/puppeteer/tree/main/tools/eslint/${name}.ts`;
});

const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const currentYear = new Date().getFullYear;

const licenseHeader = `/**
 * @license
 * Copyright ${currentYear} Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */`;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'check-license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
Repository: puppeteer/puppeteer
File: tools/eslint/src/check-license.ts (TypeScript)

Code before:
/**
 * @license
 * Copyright 2024 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import {ESLintUtils} from '@typescript-eslint/utils';

const createRule = ESLintUtils.RuleCreator(name => {
  return `https://github.com/puppeteer/puppeteer/tree/main/tools/eslint/${name}.js`;
});

const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
    const header =
      comments[0]?.type === 'Block' && isHeaderComment(comments[0])
        ? comments[0]
        : null;

    function isHeaderComment(comment: any) {
      if (comment?.range[0] >= 0 && comment?.range[1] <= 88) {
        return true;
      } else {

Review comment (suggestion):
Name should match filename.
```suggestion
  name: 'check-license',
```

Code after:
/**
 * @license
 * Copyright 2024 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import type {TSESTree} from '@typescript-eslint/utils';
import {ESLintUtils} from '@typescript-eslint/utils';

const createRule = ESLintUtils.RuleCreator(name => {
  return `https://github.com/puppeteer/puppeteer/tree/main/tools/eslint/${name}.ts`;
});

const copyrightPattern = /Copyright ([0-9]{4}) Google Inc\./;

const currentYear = new Date().getFullYear;

const licenseHeader = `/**
 * @license
 * Copyright ${currentYear} Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */`;

const enforceLicenseRule = createRule<[], 'licenseRule'>({
  name: 'check-license',
  meta: {
    type: 'layout',
    docs: {
      description: 'Validate existence of license header',
      requiresTypeChecking: false,
    },
    fixable: 'code',
    schema: [],
    messages: {
      licenseRule: 'Add license header.',
    },
  },
  defaultOptions: [],
  create(context) {
    const sourceCode = context.sourceCode;
    const comments = sourceCode.getAllComments();
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/common/Errors.ts (TypeScript)

Code before:
/**
 * @license
 * Copyright 2018 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

/**
 * All Puppeteer Error should extend this one
 *
 * @public
 */
export class PuppeteerError extends Error {
  /**
   * @internal
   */
  constructor(message?: string) {
    super(message);
    this.name = this.constructor.name;
  }

  /**
   * @internal
   */
  get [Symbol.toStringTag](): string {
    return this.constructor.name;
  }
}

/**
 * TimeoutError is emitted whenever certain operations are terminated due to
 * timeout.
 *
 * @remarks

Review comment (suggestion):
```suggestion
 * The base class for all Puppeteer-specific errors
```

Code after:
/**
 * @license
 * Copyright 2018 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

/**
 * The base class for all Puppeteer-specific errors
 *
 * @public
 */
export class PuppeteerError extends Error {
  /**
   * @internal
   */
  constructor(message?: string) {
    super(message);
    this.name = this.constructor.name;
  }

  /**
   * @internal
   */
  get [Symbol.toStringTag](): string {
    return this.constructor.name;
  }
}

/**
 * TimeoutError is emitted whenever certain operations are terminated due to
 * timeout.
 *
 * @remarks
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/api/Target.ts (TypeScript)

Code before:
/**
 * @license
 * Copyright 2023 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import type {Browser} from './Browser.js';
import type {BrowserContext} from './BrowserContext.js';
import type {CDPSession} from './CDPSession.js';
import type {Page} from './Page.js';
import type {WebWorker} from './WebWorker.js';

/**
 * @public
 */
export enum TargetType {
  PAGE = 'page',
  BACKGROUND_PAGE = 'background_page',
  SERVICE_WORKER = 'service_worker',
  SHARED_WORKER = 'shared_worker',
  WORKER = 'worker',
  BROWSER = 'browser',
  WEBVIEW = 'webview',
  OTHER = 'other',
  /**
   * @internal
   */
  TAB = 'tab',
}

/**
 * Target represents a
 * {@link https://chromedevtools.github.io/devtools-protocol/tot/Target/ | CDP target}.
 * In CDP a target is something that can be debugged such a frame, a page or a
 * worker.
 * @public
 */
export abstract class Target {
  /**
   * @internal
   */
  protected constructor() {}

  /**
   * If the target is not of type `"service_worker"` or `"shared_worker"`, returns `null`.
   */

Review comment (suggestion):
I think we need to either start supporting this type for CDP targets (which could be a breaking change) or declare the target type as Other for BiDi workers for now.

Code after:
/**
 * @license
 * Copyright 2023 Google Inc.
 * SPDX-License-Identifier: Apache-2.0
 */

import type {Browser} from './Browser.js';
import type {BrowserContext} from './BrowserContext.js';
import type {CDPSession} from './CDPSession.js';
import type {Page} from './Page.js';
import type {WebWorker} from './WebWorker.js';

/**
 * @public
 */
export enum TargetType {
  PAGE = 'page',
  BACKGROUND_PAGE = 'background_page',
  SERVICE_WORKER = 'service_worker',
  SHARED_WORKER = 'shared_worker',
  BROWSER = 'browser',
  WEBVIEW = 'webview',
  OTHER = 'other',
  /**
   * @internal
   */
  TAB = 'tab',
}

/**
 * Target represents a
 * {@link https://chromedevtools.github.io/devtools-protocol/tot/Target/ | CDP target}.
 * In CDP a target is something that can be debugged such a frame, a page or a
 * worker.
 * @public
 */
export abstract class Target {
  /**
   * @internal
   */
  protected constructor() {}

  /**
   * If the target is not of type `"service_worker"` or `"shared_worker"`, returns `null`.
   */
  async worker(): Promise<WebWorker | null> {