Find var declarations with a parser, and check only classic inline scripts

The fix review showed the var regex flagged the word in comments and strings and
missed var/*c*/x and var {x}. The test now uses the acorn parser Node ships
(--expose-internals; it skips if a Node ever drops it) to find real var
declarations, and reads the page with html.parser so it checks inline classic
scripts only: no src, and no module or JSON data type.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_page_scripts.py

@@ -1,8 +1,8 @@
 """The page's inline scripts share one global scope, so a second top-level function or
 variable with a name already used replaces the first everywhere, silently."""
+import html.parser
 import json
 import pathlib
-import re
 import shutil
 import subprocess
 import unittest
@@ -13,28 +13,75 @@ ROOT = pathlib.Path(__file__).resolve().parents[1]
 # and const, so V8 itself rejects a name declared twice in the shared scope (however it's
 # indented or declared: function, class, let, const, destructuring), while helpers with the
 # same name inside different functions stay legal. var is the exception (declaring one twice
-# is allowed), so the page doesn't use var at all.
+# is allowed), so the page doesn't use var at all: Node's own copy of the acorn parser finds
+# real var declarations, not the word in comments, strings or CSS var().
 CHECK = r'''
 const vm = require('vm');
 const scripts = JSON.parse(require('fs').readFileSync(0, 'utf8'));
-const uses = scripts.join('\n').match(/(^|[^\w$.])var\s+[\w${[]/);
-if (uses) { console.log('declares with var: ' + uses[0].trim()); process.exit(0); }
+let acorn, walk;
+try {
+  acorn = require('internal/deps/acorn/acorn/dist/acorn');
+  walk = require('internal/deps/acorn/acorn-walk/dist/walk');
+} catch { console.log('no parser'); process.exit(0); }
+for (const code of scripts) {
+  let found = null;
+  try {
+    walk.simple(acorn.parse(code, { ecmaVersion: 'latest' }), {
+      VariableDeclaration(n) { if (n.kind === 'var' && !found) found = code.slice(n.start, n.end).slice(0, 40); },
+    });
+  } catch (e) { console.log(e.message); process.exit(0); }
+  if (found) { console.log('declares with var: ' + found); process.exit(0); }
+}
 try { new vm.Script('"use strict"; {\n' + scripts.join('\n;\n') + '\n}'); console.log('ok'); }
 catch (e) { console.log(e.message); }
 '''
 
 
 def check(scripts):
-    r = subprocess.run(['node', '-e', CHECK], input=json.dumps(scripts), capture_output=True, text=True)
-    return r.stdout.strip() or r.stderr.strip()
+    r = subprocess.run(['node', '--expose-internals', '-e', CHECK], input=json.dumps(scripts),
+                       capture_output=True, text=True)
+    out = r.stdout.strip() or r.stderr.strip()
+    if out == 'no parser':
+        raise unittest.SkipTest("this Node doesn't include acorn")
+    return out
+
+
+class ClassicScripts(html.parser.HTMLParser):
+    """The page's inline classic scripts: no src, and no type other than JavaScript (a module
+    has its own scope, and JSON data isn't code)."""
+    JS = {'', 'text/javascript', 'application/javascript'}
+
+    def __init__(self):
+        super().__init__()
+        self.scripts, self.current = [], None
+
+    def handle_starttag(self, tag, attrs):
+        if tag == 'script':
+            a = dict(attrs)
+            inline = 'src' not in a and (a.get('type') or '').strip().lower() in self.JS
+            self.current = [] if inline else None
+
+    def handle_data(self, data):
+        if self.current is not None:
+            self.current.append(data)
+
+    def handle_endtag(self, tag):
+        if tag == 'script' and self.current is not None:
+            self.scripts.append(''.join(self.current))
+        if tag == 'script':
+            self.current = None
+
+
+def classic_scripts(page):
+    p = ClassicScripts()
+    p.feed(page)
+    return p.scripts
 
 
 @unittest.skipUnless(shutil.which('node'), 'Node parses the page scripts')
 class PageScripts(unittest.TestCase):
     def test_no_top_level_name_is_declared_twice(self):
-        page = (ROOT / 'ui/index.html').read_text(encoding='utf-8')
-        # Every inline script, whatever its attributes (not src= ones, which aren't inline).
-        scripts = re.findall(r'<script(?![^>]*\bsrc=)[^>]*>(.*?)</script>', page, re.S)
+        scripts = classic_scripts((ROOT / 'ui/index.html').read_text(encoding='utf-8'))
         self.assertGreaterEqual(len(scripts), 2)
         self.assertEqual(check(scripts), 'ok')
 
@@ -46,13 +93,24 @@ class PageScripts(unittest.TestCase):
             'later declarator': ['let x = 1;', 'const y = 2, x = 3;'],
             'function and const': ['function f() {}', 'const f = 1;'],
         }
-        self.assertIn('var', check(['var x = 1;', 'var x = 2;']))  # legal JavaScript, so: no var
-        self.assertEqual(check(['const css = "color: var(--blue)";']), 'ok')  # CSS var() isn't one
         for what, scripts in twice.items():
             with self.subTest(what):
                 self.assertIn('already been declared', check(scripts))
         helpers = ['function a() { function help() {} }', 'function b() { const help = 1; }']
         self.assertEqual(check(helpers), 'ok')
+        # var: legal to declare twice, so not allowed at all; however it's written.
+        for code in ['var x = 1;', 'var/*c*/x = 1;', 'var {x} = {x: 1};', 'function f() { var y; }']:
+            with self.subTest(code):
+                self.assertIn('declares with var', check([code]))
+        # The word var elsewhere is fine.
+        text = ['// var x\nconst a = "var y", b = `var ${a}`, c = "color: var(--blue)", d = {}.var;']
+        self.assertEqual(check(text), 'ok')
+
+    def test_only_inline_classic_scripts_are_checked(self):
+        page = ('<script>let a;</script><script type="module">export const m = 1;</script>'
+                '<script type="application/json">{"b": 1}</script><script src = "x.js"></script>'
+                '<script data-src="y" type="text/javascript">let c;</script>')
+        self.assertEqual(classic_scripts(page), ['let a;', 'let c;'])
 
 
 if __name__ == '__main__':