Skip to content

Commit 396e352

Browse files
committed
tools: silence cpplint refs warning for v8 fast api options
cpplint's runtime/references rule flags non-const reference parameters as a style violation. v8::FastApiCallbackOptions& is part of the V8 Fast API contract: V8 dictates the signature and Node.js has no say, so the warning is a false positive on every Fast API callback. Most call sites in src/ also `using v8::FastApiCallbackOptions;` and declare the parameter as the unqualified `FastApiCallbackOptions&`, which a fully qualified pattern would miss. Allowlist both forms in CheckForNonConstReference and add a unit test that exercises both spellings while keeping the rule active for unrelated `Type&` params. Fixes: #45761 Signed-off-by: Maruthan G <[email protected]>
1 parent 58a8e1d commit 396e352

2 files changed

Lines changed: 81 additions & 2 deletions

File tree

tools/cpplint.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6036,6 +6036,11 @@ def _GetTextInside(text, start_pattern):
60366036
)
60376037
# Stream types.
60386038
_RE_PATTERN_REF_STREAM_PARAM = r"(?:.*stream\s*&\s*" + _RE_PATTERN_IDENT + r")"
6039+
# V8 Fast API types whose signatures are dictated by V8 and require a
6040+
# non-const reference parameter; Node.js cannot change these.
6041+
_RE_PATTERN_REF_V8_FAST_API_PARAM = (
6042+
r"(?:.*\b(?:v8::)?FastApiCallbackOptions\s*&\s*" + _RE_PATTERN_IDENT + r")"
6043+
)
60396044

60406045

60416046
def CheckLanguage(
@@ -6585,8 +6590,10 @@ def CheckForNonConstReference(filename, clean_lines, linenum, nesting_state, err
65856590

65866591
decls = re.sub(r"{[^}]*}", " ", line) # exclude function body
65876592
for parameter in re.findall(_RE_PATTERN_REF_PARAM, decls):
6588-
if not re.match(_RE_PATTERN_CONST_REF_PARAM, parameter) and not re.match(
6589-
_RE_PATTERN_REF_STREAM_PARAM, parameter
6593+
if (
6594+
not re.match(_RE_PATTERN_CONST_REF_PARAM, parameter)
6595+
and not re.match(_RE_PATTERN_REF_STREAM_PARAM, parameter)
6596+
and not re.match(_RE_PATTERN_REF_V8_FAST_API_PARAM, parameter)
65906597
):
65916598
error(
65926599
filename,

tools/test_cpplint_fast_api.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
"""Regression tests for cpplint runtime/references and V8 Fast API.
2+
3+
Refs: https://github.com/nodejs/node/issues/45761
4+
"""
5+
6+
import os
7+
import sys
8+
import unittest
9+
10+
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
11+
12+
import cpplint # noqa: E402
13+
14+
15+
def _lint(source):
16+
errors = []
17+
18+
def collect(filename, linenum, category, confidence, message):
19+
errors.append((linenum, category, message))
20+
21+
cpplint.ProcessFileData("test.cc", "cc", source.splitlines(), collect)
22+
return [e for e in errors if e[1] == "runtime/references"]
23+
24+
25+
class FastApiCallbackOptionsRefTest(unittest.TestCase):
26+
def test_unqualified_fast_api_callback_options_not_flagged(self):
27+
# Mirrors src/node_buffer.cc usage after `using v8::FastApiCallbackOptions;`.
28+
src = (
29+
"namespace node {\n"
30+
"void FastFn(Local<Value> recv,\n"
31+
" FastApiCallbackOptions& options) {\n"
32+
"}\n"
33+
"} // namespace node\n"
34+
)
35+
self.assertEqual(_lint(src), [])
36+
37+
def test_qualified_fast_api_callback_options_not_flagged(self):
38+
src = (
39+
"namespace node {\n"
40+
"void FastFn(Local<Value> recv,\n"
41+
" v8::FastApiCallbackOptions& options) {\n"
42+
"}\n"
43+
"} // namespace node\n"
44+
)
45+
self.assertEqual(_lint(src), [])
46+
47+
def test_plain_non_const_reference_still_flagged(self):
48+
src = (
49+
"namespace node {\n"
50+
"void Bad(int& value) {\n"
51+
"}\n"
52+
"} // namespace node\n"
53+
)
54+
errors = _lint(src)
55+
self.assertEqual(len(errors), 1, errors)
56+
self.assertIn("int& value", errors[0][2])
57+
58+
def test_unrelated_type_with_similar_name_still_flagged(self):
59+
# Guard against an over-broad regex that would also accept e.g.
60+
# FastApiCallbackOptionsExtra&.
61+
src = (
62+
"namespace node {\n"
63+
"void Bad(FastApiCallbackOptionsExtra& opts) {\n"
64+
"}\n"
65+
"} // namespace node\n"
66+
)
67+
errors = _lint(src)
68+
self.assertEqual(len(errors), 1, errors)
69+
70+
71+
if __name__ == "__main__":
72+
unittest.main()

0 commit comments

Comments
 (0)