FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tools: fix check-imports.py to match on word boundaries · nodejs/node@3d05e3d · GitHub

/ node Public

Commit 3d05e3d

Browse files
richardlau
authored and
codebytere
committed
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 <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 103479a commit 3d05e3d

8 files changed

Lines changed: 114 additions & 4 deletions

File tree

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,13 +1322,13 @@ else
13221322
CPPLINT_QUIET = --quiet
13231323
endif
13241324
.PHONY: lint-cpp
1325-
# Lints the C++ code with cpplint.py and check-imports.py.
1325+
# Lints the C++ code with cpplint.py and checkimports.py.
13261326
lint-cpp: tools/.cpplintstamp
13271327

13281328
tools/.cpplintstamp: $(LINT_CPP_FILES)
13291329
@echo "Running C++ linter..."
13301330
@$(PYTHON) tools/cpplint.py $(CPPLINT_QUIET) $?
1331-
@$(PYTHON) tools/check-imports.py
1331+
@$(PYTHON) tools/checkimports.py
13321332
@touch $@
13331333

13341334
.PHONY: lint-addon-docs

‎test/fixtures/tools/checkimports/invalid.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
using v8::MaybeLocal;
2+
using v8::Array;
3+
using v8::Local;
4+
5+
MaybeLocal<Array> CreateObject() const {
6+
return MaybeLocal<Array>();
7+
}

‎test/fixtures/tools/checkimports/maybe.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
using v8::Array;
2+
using v8::Local;
3+
using v8::MaybeLocal;
4+
5+
MaybeLocal<Array> CreateObject() const {
6+
return MaybeLocal<Array>();
7+
}

‎test/fixtures/tools/checkimports/unsorted.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
using v8::MaybeLocal;
2+
using v8::Array;
3+
4+
MaybeLocal<Array> CreateObject() const {
5+
return MaybeLocal<Array>();
6+
}

‎test/fixtures/tools/checkimports/unused.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
using v8::Context;
2+
3+
static void MyMethod(void) {
4+
return;
5+
}

‎test/fixtures/tools/checkimports/valid.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
using v8::Array;
2+
using v8::MaybeLocal;
3+
4+
MaybeLocal<Array> CreateObject() const {
5+
return MaybeLocal<Array>();
6+
}

‎test/tools/test_checkimports.py‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import unittest
2+
import sys
3+
from contextlib import contextmanager
4+
from os import path
5+
sys.path.append(path.abspath(path.join(path.dirname(__file__),
6+
'..', '..', 'tools')))
7+
try:
8+
from StringIO import StringIO
9+
except ImportError:
10+
from io import StringIO
11+
12+
from checkimports import is_valid
13+
14+
@contextmanager
15+
def captured_output():
16+
tmp_out, tmp_err = StringIO(), StringIO()
17+
old_out, old_err = sys.stdout, sys.stderr
18+
try:
19+
sys.stdout, sys.stderr = tmp_out, tmp_err
20+
yield sys.stdout, sys.stderr
21+
finally:
22+
sys.stdout, sys.stderr = old_out, old_err
23+
tmp_out.close()
24+
tmp_err.close()
25+
26+
class CheckImportsTest(unittest.TestCase):
27+
fixturesDir = path.join(path.dirname(__file__), '..', '..',
28+
'test', 'fixtures', 'tools', 'checkimports')
29+
30+
def test_unused_and_unsorted(self):
31+
with captured_output() as (out, err):
32+
self.assertEqual(is_valid(path.join(self.fixturesDir, 'invalid.cc')),
33+
False)
34+
output = out.getvalue()
35+
self.assertIn('does not use "Local"', output);
36+
self.assertIn('using statements aren\'t sorted in', output);
37+
self.assertIn('Line 1: Actual: v8::MaybeLocal, Expected: v8::Array',
38+
output);
39+
self.assertIn('Line 2: Actual: v8::Array, Expected: v8::Local',
40+
output);
41+
self.assertIn('Line 3: Actual: v8::Local, Expected: v8::MaybeLocal',
42+
output);
43+
44+
def test_unused_complex(self):
45+
with captured_output() as (out, err):
46+
self.assertEqual(is_valid(path.join(self.fixturesDir, 'maybe.cc')),
47+
False)
48+
output = out.getvalue()
49+
self.assertIn('does not use "Local"', output);
50+
51+
def test_unused_simple(self):
52+
with captured_output() as (out, err):
53+
self.assertEqual(is_valid(path.join(self.fixturesDir, 'unused.cc')),
54+
False)
55+
output = out.getvalue()
56+
self.assertIn('does not use "Context"', output);
57+
58+
def test_unsorted(self):
59+
with captured_output() as (out, err):
60+
self.assertEqual(is_valid(path.join(self.fixturesDir, 'unsorted.cc')),
61+
False)
62+
output = out.getvalue()
63+
self.assertIn('using statements aren\'t sorted in', output);
64+
self.assertIn('Line 1: Actual: v8::MaybeLocal, Expected: v8::Array',
65+
output);
66+
self.assertIn('Line 2: Actual: v8::Array, Expected: v8::MaybeLocal',
67+
output);
68+
69+
def test_valid(self):
70+
with captured_output() as (out, err):
71+
self.assertEqual(is_valid(path.join(self.fixturesDir, 'valid.cc')),
72+
True)
73+
output = out.getvalue()
74+
self.assertEqual(output, '');
75+
76+
if __name__ == '__main__':
77+
unittest.main()

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
def do_exist(file_name, lines, imported):
1111
if not any(not re.match('using \w+::{0};'.format(imported), line) and
12-
re.search(imported, line) for line in lines):
12+
re.search('\\b{0}\\b'.format(imported), line) for line in lines):
1313
print('File "{0}" does not use "{1}"'.format(file_name, imported))
1414
return False
1515
return True
@@ -40,4 +40,6 @@ def is_valid(file_name):
4040
else:
4141
return valid
4242

43-
sys.exit(0 if all(map(is_valid, glob.iglob('src/*.cc'))) else 1)
43+
if __name__ == '__main__':
44+
files = glob.iglob(sys.argv[1] if len(sys.argv) > 1 else 'src/*.cc')
45+
sys.exit(0 if all(map(is_valid, files)) else 1)

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL