Repository: playwright-community/eslint-plugin-playwright (ts)

Issue:
no-unsafe-references for addInitScript
no-unsafe-references rule now only targets page.evaluate method. How about to target addInitScript too? This is the same kind of method and we ran in  mistakes with using variables from closure sometimes.

Pull request:
Support `addInitScript` in `no-unsafe-references`
Fixes #365

Patch:
diff --git a/docs/rules/no-unsafe-references.md b/docs/rules/no-unsafe-references.md
index 09384cf..bac1fc9 100644
--- a/docs/rules/no-unsafe-references.md
+++ b/docs/rules/no-unsafe-references.md
@@ -1,10 +1,10 @@
-## Prevent unsafe variable references in `page.evaluate()` (`no-unsafe-references`)
+## Prevent unsafe variable references in `page.evaluate()` and `page.addInitScript()` (`no-unsafe-references`)
 
-This rule prevents common mistakes when using `page.evaluate()` with variables
-referenced from the parent scope. When referencing variables from the parent
-scope with `page.evaluate()`, you must pass them as an argument so Playwright
-can properly serialize and send them to the browser page where the function
-being evaluated is executed.
+This rule prevents common mistakes when using `page.evaluate()` or
+`page.addInitScript()` with variables referenced from the parent scope. When
+referencing variables from the parent scope with these methods, you must pass
+them as an argument so Playwright can properly serialize and send them to the
+browser page where the function being evaluated is executed.
 
 ## Rule Details
 
@@ -14,14 +14,17 @@ Example of **incorrect** code for this rule:
 const x = 7
 const y = 8
 await page.evaluate(() => Promise.resolve(x * y), [])
+await page.addInitScript(() => Promise.resolve(x * y), [])
 ```
 
 Example of **correct** code for this rule:
 
 ```javascript
 await page.evaluate(([x, y]) => Promise.resolve(x * y), [7, 8])
+await page.addInitScript(([x, y]) => Promise.resolve(x * y), [7, 8])
 
 const x = 7
 const y = 8
 await page.evaluate(([x, y]) => Promise.resolve(x * y), [x, y])
+await page.addInitScript(([x, y]) => Promise.resolve(x * y), [x, y])
 ```
diff --git a/package.json b/package.json
index 6925ddc..6e04633 100644
--- a/package.json
+++ b/package.json
@@ -35,8 +35,8 @@
     "lint": "eslint .",
     "fmt": "prettier --write .",
     "fmt:check": "prettier --check .",
-    "test": "vitest",
-    "test:watch": "vitest --reporter=dot",
+    "test": "vitest --run",
+    "test:watch": "vitest --reporter=dot --run",
     "ts": "tsc --noEmit"
   },
   "peerDependencies": {
diff --git a/src/rules/no-unsafe-references.ts b/src/rules/no-unsafe-references.ts
index 1c9721b..2787742 100644
--- a/src/rules/no-unsafe-references.ts
+++ b/src/rules/no-unsafe-references.ts
@@ -19,7 +19,10 @@ function collectVariables(scope: Scope.Scope | null): string[] {
   ]
 }
 
-/** Add the variables from the outer scope as arguments to `page.evaluate()` */
+/**
+ * Add the variables from the outer scope as arguments to `page.evaluate()` or
+ * `page.addInitScript()`.
+ */
 function addArgument(
   fixer: Rule.RuleFixer,
   node: ESTree.CallExpression,
@@ -28,7 +31,7 @@ function addArgument(
   // This should never happen, but just in case
   if (!node.arguments.length) return
 
-  // If there the only one argument to `page.evaluate()` is the function, we
+  // If the only argument to the method is the function, we
   // have to add the references as the second argument.
   if (node.arguments.length === 1) {
     return fixer.insertTextAfter(node.arguments[0], `, [${refs}]`)
@@ -94,7 +97,11 @@ export default createRule({
   create(context) {
     return {
       CallExpression(node) {
-        if (!isPageMethod(node, 'evaluate')) return
+        if (
+          !isPageMethod(node, 'evaluate') &&
+          !isPageMethod(node, 'addInitScript')
+        )
+          return
 
         const [fn] = node.arguments
         if (!fn || !isFunction(fn)) return
@@ -113,8 +120,11 @@ export default createRule({
           })
           .filter((ref) => allRefs.has(ref.identifier.name))
           .forEach((ref, i, arr) => {
+            const methodName = isPageMethod(node, 'evaluate')
+              ? 'evaluate'
+              : 'addInitScript'
             const descriptor: Rule.ReportDescriptor = {
-              data: { variable: ref.identifier.name },
+              data: { method: methodName, variable: ref.identifier.name },
               messageId: 'noUnsafeReference',
               node: ref.identifier,
             }
@@ -143,14 +153,15 @@ export default createRule({
   meta: {
     docs: {
       category: 'Possible Errors',
-      description: 'Prevent unsafe variable references in page.evaluate()',
+      description:
+        'Prevent unsafe variable references in page.evaluate() and page.addInitScript()',
       recommended: true,
       url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md',
     },
     fixable: 'code',
     messages: {
       noUnsafeReference:
-        'Unsafe reference to variable "{{ variable }}" in page.evaluate()',
+        'Unsafe reference to variable "{{ variable }}" in page.{{ method }}()',
     },
     type: 'problem',
   },


Test patch:
diff --git a/src/rules/no-unsafe-references.test.ts b/src/rules/no-unsafe-references.test.ts
index 7379fbd..33c2c6a 100644
--- a/src/rules/no-unsafe-references.test.ts
+++ b/src/rules/no-unsafe-references.test.ts
@@ -20,12 +20,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 3,
           messageId,
         },
       ],
-      name: 'Single argument - missing arg list - arrow function',
+      name: 'Single argument - missing arg list - arrow function - evaluate',
       output: javascript`
         const x = 10
         const result = await page.evaluate(([x]) => {
@@ -33,6 +33,29 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(() => {
+          return Promise.resolve(x);
+        });
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 3,
+          messageId,
+        },
+      ],
+      name: 'Single argument - missing arg list - arrow function - addInitScript',
+      output: javascript`
+        const x = 10
+        const result = await page.addInitScript(([x]) => {
+          return Promise.resolve(x);
+        }, [x]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -43,12 +66,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 3,
           messageId,
         },
       ],
-      name: 'Single argument - missing arg list - function',
+      name: 'Single argument - missing arg list - function - evaluate',
       output: javascript`
         const x = 10
         const result = await page.evaluate(function ([x]) {
@@ -56,6 +79,29 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(function () {
+          return Promise.resolve(x);
+        });
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 3,
+          messageId,
+        },
+      ],
+      name: 'Single argument - missing arg list - function - addInitScript',
+      output: javascript`
+        const x = 10
+        const result = await page.addInitScript(function ([x]) {
+          return Promise.resolve(x);
+        }, [x]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -66,12 +112,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 3,
           messageId,
         },
       ],
-      name: 'Single argument - empty arg list - arrow function',
+      name: 'Single argument - empty arg list - arrow function - evaluate',
       output: javascript`
         const x = 10
         const result = await page.evaluate(([x]) => {
@@ -79,6 +125,29 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(() => {
+          return Promise.resolve(x);
+        }, []);
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 3,
+          messageId,
+        },
+      ],
+      name: 'Single argument - empty arg list - arrow function - addInitScript',
+      output: javascript`
+        const x = 10
+        const result = await page.addInitScript(([x]) => {
+          return Promise.resolve(x);
+        }, [x]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -89,12 +158,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 3,
           messageId,
         },
       ],
-      name: 'Single argument - empty arg list - function',
+      name: 'Single argument - empty arg list - function - evaluate',
       output: javascript`
         const x = 10
         const result = await page.evaluate(function([x]) {
@@ -102,6 +171,29 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(function() {
+          return Promise.resolve(x);
+        }, []);
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 3,
+          messageId,
+        },
+      ],
+      name: 'Single argument - empty arg list - function - addInitScript',
+      output: javascript`
+        const x = 10
+        const result = await page.addInitScript(function([x]) {
+          return Promise.resolve(x);
+        }, [x]);
+      `,
+    },
     {
       code: javascript`
         const foo = 10
@@ -113,20 +205,20 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'foo' },
+          data: { method: 'evaluate', variable: 'foo' },
           endColumn: 29,
           line: 4,
           messageId: 'noUnsafeReference',
         },
         {
           column: 32,
-          data: { variable: 'bar' },
+          data: { method: 'evaluate', variable: 'bar' },
           endColumn: 35,
           line: 4,
           messageId: 'noUnsafeReference',
         },
       ],
-      name: 'Multiple arguments',
+      name: 'Multiple arguments - evaluate',
       output: javascript`
         const foo = 10
         const bar = 20
@@ -135,6 +227,39 @@ runRuleTester('no-unsafe-references', rule, {
         }, [foo, bar]);
       `,
     },
+    {
+      code: javascript`
+        const foo = 10
+        const bar = 20
+        const result = await page.addInitScript(() => {
+          return Promise.resolve(foo + bar);
+        }, []);
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'foo' },
+          endColumn: 29,
+          line: 4,
+          messageId: 'noUnsafeReference',
+        },
+        {
+          column: 32,
+          data: { method: 'addInitScript', variable: 'bar' },
+          endColumn: 35,
+          line: 4,
+          messageId: 'noUnsafeReference',
+        },
+      ],
+      name: 'Multiple arguments - addInitScript',
+      output: javascript`
+        const foo = 10
+        const bar = 20
+        const result = await page.addInitScript(([foo, bar]) => {
+          return Promise.resolve(foo + bar);
+        }, [foo, bar]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -146,12 +271,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 26,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 4,
           messageId,
         },
       ],
-      name: 'Inner and outer variables',
+      name: 'Inner and outer variables - evaluate',
       output: javascript`
         const x = 10
         const result = await page.evaluate(([x]) => {
@@ -160,6 +285,31 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(() => {
+          const y = 20;
+          return Promise.resolve(x + y);
+        });
+      `,
+      errors: [
+        {
+          column: 26,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 4,
+          messageId,
+        },
+      ],
+      name: 'Inner and outer variables - addInitScript',
+      output: javascript`
+        const x = 10
+        const result = await page.addInitScript(([x]) => {
+          const y = 20;
+          return Promise.resolve(x + y);
+        }, [x]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -173,18 +323,18 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 28,
-          data: { variable: 'x' },
+          data: { method: 'evaluate', variable: 'x' },
           line: 5,
           messageId,
         },
         {
           column: 32,
-          data: { variable: 'y' },
+          data: { method: 'evaluate', variable: 'y' },
           line: 5,
           messageId,
         },
       ],
-      name: 'Multi-level scopes',
+      name: 'Multi-level scopes - evaluate',
       output: javascript`
         const x = 10
         test('test', async () => {
@@ -195,6 +345,41 @@ runRuleTester('no-unsafe-references', rule, {
         })
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        test('test', async () => {
+          const y = 10
+          const result = await page.addInitScript(() => {
+            return Promise.resolve(x + y);
+          }, []);
+        })
+      `,
+      errors: [
+        {
+          column: 28,
+          data: { method: 'addInitScript', variable: 'x' },
+          line: 5,
+          messageId,
+        },
+        {
+          column: 32,
+          data: { method: 'addInitScript', variable: 'y' },
+          line: 5,
+          messageId,
+        },
+      ],
+      name: 'Multi-level scopes - addInitScript',
+      output: javascript`
+        const x = 10
+        test('test', async () => {
+          const y = 10
+          const result = await page.addInitScript(([x, y]) => {
+            return Promise.resolve(x + y);
+          }, [x, y]);
+        })
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -206,12 +391,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 30,
-          data: { variable: 'y' },
+          data: { method: 'evaluate', variable: 'y' },
           line: 4,
           messageId,
         },
       ],
-      name: 'Adding to existing arg list',
+      name: 'Adding to existing arg list - evaluate',
       output: javascript`
         const x = 10
         const y = 12
@@ -220,6 +405,31 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x, y]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const y = 12
+        const result = await page.addInitScript(([x]) => {
+          return Promise.resolve(x + y);
+        }, [x]);
+      `,
+      errors: [
+        {
+          column: 30,
+          data: { method: 'addInitScript', variable: 'y' },
+          line: 4,
+          messageId,
+        },
+      ],
+      name: 'Adding to existing arg list - addInitScript',
+      output: javascript`
+        const x = 10
+        const y = 12
+        const result = await page.addInitScript(([x, y]) => {
+          return Promise.resolve(x + y);
+        }, [x, y]);
+      `,
+    },
     {
       code: javascript`
         const x = 10
@@ -231,12 +441,12 @@ runRuleTester('no-unsafe-references', rule, {
       errors: [
         {
           column: 30,
-          data: { variable: 'y' },
+          data: { method: 'evaluate', variable: 'y' },
           line: 4,
           messageId,
         },
       ],
-      name: 'Converting a single argument to an array',
+      name: 'Converting a single argument to an array - evaluate',
       output: javascript`
         const x = 10
         const y = 12
@@ -245,11 +455,38 @@ runRuleTester('no-unsafe-references', rule, {
         }, [x, y]);
       `,
     },
+    {
+      code: javascript`
+        const x = 10
+        const y = 12
+        const result = await page.addInitScript((x) => {
+          return Promise.resolve(x + y);
+        }, x);
+      `,
+      errors: [
+        {
+          column: 30,
+          data: { method: 'addInitScript', variable: 'y' },
+          line: 4,
+          messageId,
+        },
+      ],
+      name: 'Converting a single argument to an array - addInitScript',
+      output: javascript`
+        const x = 10
+        const y = 12
+        const result = await page.addInitScript(([x, y]) => {
+          return Promise.resolve(x + y);
+        }, [x, y]);
+      `,
+    },
   ],
   valid: [
     { code: 'page.pause()' },
     { code: 'page.evaluate()' },
     { code: 'page.evaluate("1 + 2")' },
+    { code: 'page.addInitScript()' },
+    { code: 'page.addInitScript("1 + 2")' },
     {
       code: javascript`
         const x = 10
@@ -257,7 +494,16 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(12);
         }, []);
       `,
-      name: 'No variables',
+      name: 'No variables - evaluate',
+    },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(() => {
+          return Promise.resolve(12);
+        }, []);
+      `,
+      name: 'No variables - addInitScript',
     },
     {
       code: javascript`
@@ -266,7 +512,16 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(x);
         }, [x]);
       `,
-      name: 'Single argument',
+      name: 'Single argument - evaluate',
+    },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(function (x) {
+          return Promise.resolve(x);
+        }, [x]);
+      `,
+      name: 'Single argument - addInitScript',
     },
     {
       code: javascript`
@@ -276,7 +531,17 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(foo, bar);
         }, [foo, bar]);
       `,
-      name: 'Multiple arguments - same name',
+      name: 'Multiple arguments - same name - evaluate',
+    },
+    {
+      code: javascript`
+        const foo = 10
+        const bar = 20
+        const result = await page.addInitScript(([foo, bar]) => {
+          return Promise.resolve(foo, bar);
+        }, [foo, bar]);
+      `,
+      name: 'Multiple arguments - same name - addInitScript',
     },
     {
       code: javascript`
@@ -286,7 +551,17 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(a + b);
         }, [foo, bar]);
       `,
-      name: 'Multiple arguments - different name',
+      name: 'Multiple arguments - different name - evaluate',
+    },
+    {
+      code: javascript`
+        const foo = 10
+        const bar = 20
+        const result = await page.addInitScript(([a, b]) => {
+          return Promise.resolve(a + b);
+        }, [foo, bar]);
+      `,
+      name: 'Multiple arguments - different name - addInitScript',
     },
     {
       code: javascript`
@@ -296,7 +571,17 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(x);
         }, []);
       `,
-      name: 'Variable shadowing',
+      name: 'Variable shadowing - evaluate',
+    },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript(() => {
+          const x = 20;
+          return Promise.resolve(x);
+        }, []);
+      `,
+      name: 'Variable shadowing - addInitScript',
     },
     {
       code: javascript`
@@ -306,7 +591,17 @@ runRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(x + y);
         }, [x]);
       `,
-      name: 'Inner and outer variables',
+      name: 'Inner and outer variables - evaluate',
+    },
+    {
+      code: javascript`
+        const x = 10
+        const result = await page.addInitScript((x) => {
+          const y = 20;
+          return Promise.resolve(x + y);
+        }, [x]);
+      `,
+      name: 'Inner and outer variables - addInitScript',
     },
     {
       code: javascript`
@@ -318,7 +613,19 @@ runRuleTester('no-unsafe-references', rule, {
           }, [x, y]);
         })
       `,
-      name: 'Multi-level scopes',
+      name: 'Multi-level scopes - evaluate',
+    },
+    {
+      code: javascript`
+        const x = 10
+        test('test', async () => {
+          const y = 10
+          const result = await page.addInitScript(([x, y]) => {
+            return Promise.resolve(x + y);
+          }, [x, y]);
+        })
+      `,
+      name: 'Multi-level scopes - addInitScript',
     },
   ],
 })
@@ -334,7 +641,17 @@ runTSRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(x);
         });
       `,
-      name: 'TypeScript - variable assignment of type',
+      name: 'TypeScript - variable assignment of type - evaluate',
+    },
+    {
+      code: typescript`
+        type X = number;
+        const result = await page.addInitScript(() => {
+          const x = 10 as X;
+          return Promise.resolve(x);
+        });
+      `,
+      name: 'TypeScript - variable assignment of type - addInitScript',
     },
     {
       code: typescript`
@@ -344,7 +661,17 @@ runTSRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(foo(10));
         });
       `,
-      name: 'TypeScript - parameter type',
+      name: 'TypeScript - parameter type - evaluate',
+    },
+    {
+      code: typescript`
+        type X = number;
+        const result = await page.addInitScript(() => {
+          const foo = (bar: X) => bar;
+          return Promise.resolve(foo(10));
+        });
+      `,
+      name: 'TypeScript - parameter type - addInitScript',
     },
     {
       code: typescript`
@@ -354,7 +681,17 @@ runTSRuleTester('no-unsafe-references', rule, {
           return Promise.resolve(x);
         });
       `,
-      name: 'TypeScript - casting',
+      name: 'TypeScript - casting - evaluate',
+    },
+    {
+      code: typescript`
+        type X = number;
+        const result = await page.addInitScript(() => {
+          const x: X = 10;
+          return Promise.resolve(x);
+        });
+      `,
+      name: 'TypeScript - casting - addInitScript',
     },
   ],
 })