chore: add more EsLint rules

package.json

@@ -55,7 +55,6 @@
   "mcpName": "io.github.ChromeDevTools/chrome-devtools-mcp",
   "devDependencies": {
     "@blackwell-systems/gcf": "^2.2.2",
-    "@eslint/js": "^10.0.1",
     "@google/genai": "^2.0.1",
     "@modelcontextprotocol/client": "2.2.0",
     "@modelcontextprotocol/core": "2.1.0",
@@ -75,7 +74,10 @@
     "core-js": "3.50.0",
     "eslint": "^10.7.0",
     "eslint-import-resolver-typescript": "^4.4.4",
+    "eslint-plugin-eslint-plugin": "^7.6.2",
     "eslint-plugin-import": "^2.32.0",
+    "eslint-plugin-jsdoc": "^64.5.4",
+    "eslint-plugin-mocha": "^12.0.2",
     "globals": "^17.0.0",
     "lighthouse": "13.5.0",
     "prettier": "^3.6.2",

rollup.config.js

@@ -16,7 +16,7 @@
  */
 
 /**
- * @fileoverview taken from {@link https://github.com/GoogleChromeLabs/chromium-bidi/blob/main/rollup.config.mjs | chromium-bidi}
+ * @file taken from {@link https://github.com/GoogleChromeLabs/chromium-bidi/blob/main/rollup.config.mjs | chromium-bidi}
  * and modified to specific requirement.
  */
 
@@ -195,7 +195,7 @@ const bundleDependency = (
             const stringifiedDependencies = Array.from(
               seenDependencies.values(),
             ).map(dependency => {
-              let arr = [];
+              const arr = [];
               arr.push(`Name: ${dependency.name ?? 'N/A'}`);
               let url = dependency.homepage ?? dependency.repository;
               if (url !== null && typeof url !== 'string') {

scripts/eslint_rules/check-license-rule.js

@@ -73,7 +73,7 @@ export default {
             nextToken.loc.start.line === header.loc.end.line + 1
           ) {
             context.report({
-              node: node,
+              node,
               loc: header.loc,
               messageId: 'emptyLine',
               fix(fixer) {
@@ -87,7 +87,7 @@ export default {
         // Add header license
         if (!header || !header.value.includes('@license')) {
           context.report({
-            node: node,
+            node,
             messageId: 'licenseRule',
             fix(fixer) {
               return fixer.insertTextAfterRange(insertAfter, licenseHeader);

scripts/eslint_rules/require-parsed-arguments-rule.js

@@ -87,9 +87,8 @@ export default {
                       const output = `${left}: ParsedArguments = ${right}`;
                       if (hasParens) {
                         return fixer.replaceText(firstParam, output);
-                      } else {
-                        return fixer.replaceText(firstParam, `(${output})`);
                       }
+                      return fixer.replaceText(firstParam, `(${output})`);
                     } else {
                       const text = sourceCode.getText(firstParam);
                       paramName = text.split(':')[0].trim();
@@ -101,12 +100,11 @@ export default {
                       firstParam,
                       `${paramName}: ParsedArguments`,
                     );
-                  } else {
-                    return fixer.replaceText(
-                      firstParam,
-                      `(${paramName}: ParsedArguments)`,
-                    );
                   }
+                  return fixer.replaceText(
+                    firstParam,
+                    `(${paramName}: ParsedArguments)`,
+                  );
                 },
               });
             }

scripts/eval_gemini.ts

@@ -15,12 +15,12 @@ import {Client} from '@modelcontextprotocol/client';
 import {StdioClientTransport} from '@modelcontextprotocol/client/stdio';
 import {TestServer} from '../build/tests/server.js';
 
+import type {CapturedFunctionCall, TestScenario} from './eval_result.js';
+import {Result} from './eval_result.js';
+
 const ROOT_DIR = path.resolve(import.meta.dirname, '..');
 const SCENARIOS_DIR = path.join(import.meta.dirname, 'eval_scenarios');
 const SKILL_PATH = path.join(ROOT_DIR, 'skills', 'chrome-devtools', 'SKILL.md');
-
-import type {CapturedFunctionCall, TestScenario} from './eval_result.js';
-import {Result} from './eval_result.js';
 export type {CapturedFunctionCall, TestScenario};
 export {Result};
 

scripts/eval_result.ts

@@ -13,10 +13,10 @@ export interface CapturedFunctionCall {
 
 export class Result {
   private nextCallIndex = 0;
-  public readonly calls: CapturedFunctionCall[];
-  public readonly serverArgs: string[];
+  readonly calls: CapturedFunctionCall[];
+  readonly serverArgs: string[];
   /** The model's final response text. */
-  public readonly text: string;
+  readonly text: string;
 
   constructor(calls: CapturedFunctionCall[], serverArgs: string[], text = '') {
     this.calls = calls;

scripts/eval_scenarios/page_focus_keyboard_test.ts

@@ -46,7 +46,9 @@ Now use the press_key tool to type "a" on Page 1 without selecting it first. You
           pressKeys.length >= 2,
           'Should retry press_key after error recovery',
         );
-        const lastPressKeyIndex = result.calls.lastIndexOf(pressKeys.at(-1)!);
+        const lastPressKey = pressKeys.at(-1);
+        assert.ok(lastPressKey);
+        const lastPressKeyIndex = result.calls.lastIndexOf(lastPressKey);
         assert.ok(
           firstSelectPageIndex < lastPressKeyIndex,
           'select_page should precede the successful press_key',

scripts/test.js

@@ -7,7 +7,7 @@
 // Note: can be converted to ts file once node 20 support is dropped.
 // Node 20 does not support --experimental-strip-types flag.
 
-import {spawn, execSync} from 'node:child_process';
+import {spawn} from 'node:child_process';
 import {readFile} from 'node:fs/promises';
 import path from 'node:path';
 import process from 'node:process';
@@ -37,22 +37,20 @@ if (userArgs.length > 0) {
     }
     files.push(testPath);
   }
-} else {
-  if (flags.includes('--test-only')) {
-    const {glob} = await import('node:fs/promises');
-    for await (const tsFile of glob('tests/**/*.test.ts')) {
-      const content = await readFile(tsFile, 'utf8');
-      if (content.includes('.only(')) {
-        files.push(path.join('build', tsFile.replace(/\.ts$/, '.js')));
-      }
-    }
-    if (files.length === 0) {
-      console.warn('no files contain .only');
-      process.exit(0);
+} else if (flags.includes('--test-only')) {
+  const {glob} = await import('node:fs/promises');
+  for await (const tsFile of glob('tests/**/*.test.ts')) {
+    const content = await readFile(tsFile, 'utf8');
+    if (content.includes('.only(')) {
+      files.push(path.join('build', tsFile.replace(/\.ts$/, '.js')));
     }
-  } else if (files.length === 0) {
-    files.push('build/tests/**/*.test.js');
   }
+  if (files.length === 0) {
+    console.warn('no files contain .only');
+    process.exit(0);
+  }
+} else if (files.length === 0) {
+  files.push('build/tests/**/*.test.js');
 }
 
 const nodeArgs = [
@@ -68,24 +66,11 @@ const nodeArgs = [
   ...files,
 ];
 
-function _installChrome(version) {
-  try {
-    return execSync(
-      `npx puppeteer browsers install chrome@${version} --format "{{path}}"`,
-    )
-      .toString()
-      .trim();
-  } catch (e) {
-    console.error(`Failed to install Chrome ${version}:`, e);
-    process.exit(1);
-  }
-}
-
 async function runTests(attempt) {
   if (attempt > 1) {
     console.log(`\nRun attempt ${attempt}...\n`);
   }
-  return new Promise(resolve => {
+  return await new Promise(resolve => {
     const child = spawn('node', nodeArgs, {
       stdio: 'inherit',
       env: {

scripts/update_metrics.ts

@@ -22,7 +22,7 @@ import {
 } from '../build/src/telemetry/metricsRegistry.js';
 import {createTools} from '../build/src/tools/tools.js';
 
-export function HaveUniqueNames(tools: Array<{name: string}>): boolean {
+export function haveUniqueNames(tools: Array<{name: string}>): boolean {
   const toolNames = tools.map(tool => tool.name);
   const toolNamesSet = new Set(toolNames);
   return toolNamesSet.size === toolNames.length;
@@ -39,7 +39,7 @@ function writeToolCallMetricsConfig() {
   // Avoid 'as ParsedArguments' by using parseArguments
   const allTools = createTools(new ConfigParser('0.0.0', ['', '']).parse());
 
-  if (!HaveUniqueNames(allTools)) {
+  if (!haveUniqueNames(allTools)) {
     throw new Error('Error: Duplicate tool names found.');
   }
 

src/BrowserManager.ts

@@ -141,7 +141,7 @@ export class BrowserManager {
     // a call silently adopt the freshly-rotated token as its own baseline
     // once the lock frees up, defeating the abandonment check entirely.
     const attempt = this.#browserAttempt;
-    using _guard = await this.#mutex.acquire();
+    using guard = await this.#mutex.acquire();
     if (this.#closingCount > 0) {
       throw new Error('Browser was closed while initializing.');
     }
@@ -518,7 +518,7 @@ export class BrowserManager {
   async close(): Promise<void> {
     this.#initPromise = undefined;
     this.#closingCount++;
-    using _guard = await this.#mutex.acquire();
+    using guard = await this.#mutex.acquire();
     try {
       await this.#closeBrowser();
     } finally {

src/McpContext.ts

@@ -142,7 +142,7 @@ export class McpContext implements Context {
     );
   }
 
-  async #init() {
+  async #init(): Promise<void> {
     await this.createPagesSnapshot();
     const workers = this.createWorkersSnapshot();
 
@@ -151,7 +151,7 @@ export class McpContext implements Context {
     this.browser.on('targetdestroyed', this.#onTargetDestroyed);
   }
 
-  dispose() {
+  dispose(): void {
     this.browser.off('targetcreated', this.#onTargetCreated);
     this.browser.off('targetdestroyed', this.#onTargetDestroyed);
 
@@ -169,7 +169,7 @@ export class McpContext implements Context {
     this.#isolatedContexts.clear();
   }
 
-  #onTargetCreated = (target: Target) => {
+  #onTargetCreated = (target: Target): void => {
     try {
       if (!this.#isPageTarget(target)) {
         return;
@@ -180,7 +180,7 @@ export class McpContext implements Context {
     }
   };
 
-  #onTargetDestroyed = (target: Target) => {
+  #onTargetDestroyed = (target: Target): void => {
     try {
       const mcpPage = this.#mcpPages.get(target);
       if (mcpPage) {
@@ -199,7 +199,7 @@ export class McpContext implements Context {
     opts: McpContextOptions,
     /* Let tests use unbundled Locator class to avoid overly strict checks within puppeteer that fail when mixing bundled and unbundled class instances */
     locatorClass: typeof Locator = Locator,
-  ) {
+  ): Promise<McpContext> {
     const context = new McpContext(browser, logger, opts, locatorClass);
     await context.#init();
     return context;
@@ -452,7 +452,7 @@ export class McpContext implements Context {
     if (!targetPage) {
       return undefined;
     }
-    let timeoutId: ReturnType<typeof setTimeout>;
+    let timeoutId: ReturnType<typeof setTimeout> | undefined;
     const timeoutPromise = new Promise<undefined>(resolve => {
       timeoutId = setTimeout(() => resolve(undefined), 500);
     });
@@ -462,7 +462,7 @@ export class McpContext implements Context {
     } catch {
       return undefined;
     } finally {
-      clearTimeout(timeoutId!);
+      clearTimeout(timeoutId);
     }
   }
 
@@ -936,7 +936,7 @@ export class McpContext implements Context {
         if (!response.ok) {
           throw new Error(`Failed to load resource: ${url}`);
         }
-        return response.text();
+        return await response.text();
       }
 
       case 'file:': {