tools: fix check-imports.py to match on word boundaries

`check-imports.py` was missing some unused `using` statements as it
was not matching on word boundaries (e.g. `MaybeLocal` was considered
a use of `Local`). Fix that and add some unit tests (which required
the script to be renamed to drop the `-` so it could be imported into
the test script).

PR-URL: #33268
Refs: #29226
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>

Author: richardlau

Makefile

@@ -1322,13 +1322,13 @@ else
 CPPLINT_QUIET = --quiet
 endif
 .PHONY: lint-cpp
-# Lints the C++ code with cpplint.py and check-imports.py.
+# Lints the C++ code with cpplint.py and checkimports.py.
 lint-cpp: tools/.cpplintstamp
 
 tools/.cpplintstamp: $(LINT_CPP_FILES)
 	@echo "Running C++ linter..."
 	@$(PYTHON) tools/cpplint.py $(CPPLINT_QUIET) $?
-	@$(PYTHON) tools/check-imports.py
+	@$(PYTHON) tools/checkimports.py
 	@touch $@
 
 .PHONY: lint-addon-docs

test/fixtures/tools/checkimports/invalid.cc

@@ -0,0 +1,7 @@
+using v8::MaybeLocal;
+using v8::Array;
+using v8::Local;
+
+MaybeLocal<Array> CreateObject() const {
+  return MaybeLocal<Array>();
+}

test/fixtures/tools/checkimports/maybe.cc

@@ -0,0 +1,7 @@
+using v8::Array;
+using v8::Local;
+using v8::MaybeLocal;
+
+MaybeLocal<Array> CreateObject() const {
+  return MaybeLocal<Array>();
+}

test/fixtures/tools/checkimports/unsorted.cc

@@ -0,0 +1,6 @@
+using v8::MaybeLocal;
+using v8::Array;
+
+MaybeLocal<Array> CreateObject() const {
+  return MaybeLocal<Array>();
+}

test/fixtures/tools/checkimports/unused.cc

@@ -0,0 +1,5 @@
+using v8::Context;
+
+static void MyMethod(void) {
+  return;
+}

test/fixtures/tools/checkimports/valid.cc

@@ -0,0 +1,6 @@
+using v8::Array;
+using v8::MaybeLocal;
+
+MaybeLocal<Array> CreateObject() const {
+  return MaybeLocal<Array>();
+}

test/tools/test_checkimports.py

@@ -0,0 +1,77 @@
+import unittest
+import sys
+from contextlib import contextmanager
+from os import path
+sys.path.append(path.abspath(path.join(path.dirname(__file__),
+                                       '..', '..', 'tools')))
+try:
+  from StringIO import StringIO
+except ImportError:
+  from io import StringIO
+
+from checkimports import is_valid
+
+@contextmanager
+def captured_output():
+  tmp_out, tmp_err = StringIO(), StringIO()
+  old_out, old_err = sys.stdout, sys.stderr
+  try:
+    sys.stdout, sys.stderr = tmp_out, tmp_err
+    yield sys.stdout, sys.stderr
+  finally:
+    sys.stdout, sys.stderr = old_out, old_err
+    tmp_out.close()
+    tmp_err.close()
+
+class CheckImportsTest(unittest.TestCase):
+  fixturesDir = path.join(path.dirname(__file__), '..', '..',
+                          'test', 'fixtures', 'tools', 'checkimports')
+
+  def test_unused_and_unsorted(self):
+    with captured_output() as (out, err):
+      self.assertEqual(is_valid(path.join(self.fixturesDir, 'invalid.cc')),
+                       False)
+      output = out.getvalue()
+      self.assertIn('does not use "Local"', output);
+      self.assertIn('using statements aren\'t sorted in', output);
+      self.assertIn('Line 1: Actual: v8::MaybeLocal, Expected: v8::Array',
+                    output);
+      self.assertIn('Line 2: Actual: v8::Array, Expected: v8::Local',
+                    output);
+      self.assertIn('Line 3: Actual: v8::Local, Expected: v8::MaybeLocal',
+                    output);
+
+  def test_unused_complex(self):
+    with captured_output() as (out, err):
+      self.assertEqual(is_valid(path.join(self.fixturesDir, 'maybe.cc')),
+                       False)
+      output = out.getvalue()
+      self.assertIn('does not use "Local"', output);
+
+  def test_unused_simple(self):
+    with captured_output() as (out, err):
+      self.assertEqual(is_valid(path.join(self.fixturesDir, 'unused.cc')),
+                       False)
+      output = out.getvalue()
+      self.assertIn('does not use "Context"', output);
+
+  def test_unsorted(self):
+    with captured_output() as (out, err):
+      self.assertEqual(is_valid(path.join(self.fixturesDir, 'unsorted.cc')),
+                       False)
+      output = out.getvalue()
+      self.assertIn('using statements aren\'t sorted in', output);
+      self.assertIn('Line 1: Actual: v8::MaybeLocal, Expected: v8::Array',
+                    output);
+      self.assertIn('Line 2: Actual: v8::Array, Expected: v8::MaybeLocal',
+                    output);
+
+  def test_valid(self):
+    with captured_output() as (out, err):
+      self.assertEqual(is_valid(path.join(self.fixturesDir, 'valid.cc')),
+                       True)
+      output = out.getvalue()
+      self.assertEqual(output, '');
+
+if __name__ == '__main__':
+  unittest.main()

tools/check-imports.py renamed to tools/checkimports.py

@@ -9,7 +9,7 @@
 
 def do_exist(file_name, lines, imported):
   if not any(not re.match('using \w+::{0};'.format(imported), line) and
-             re.search(imported, line) for line in lines):
+             re.search('\\b{0}\\b'.format(imported), line) for line in lines):
     print('File "{0}" does not use "{1}"'.format(file_name, imported))
     return False
   return True
@@ -40,4 +40,6 @@ def is_valid(file_name):
   else:
     return valid
 
-sys.exit(0 if all(map(is_valid, glob.iglob('src/*.cc'))) else 1)
+if __name__ == '__main__':
+  files = glob.iglob(sys.argv[1] if len(sys.argv) > 1 else 'src/*.cc')
+  sys.exit(0 if all(map(is_valid, files)) else 1)