Skip to content

tools: silence cpplint refs warning for v8 fast api options#62825

Open
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:tools/issue-45761-cpplint-fast-api
Open

tools: silence cpplint refs warning for v8 fast api options#62825
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:tools/issue-45761-cpplint-fast-api

Conversation

@maruthang
Copy link
Copy Markdown

What

Allowlist FastApiCallbackOptions& (with or without the v8:: prefix)
in cpplint's CheckForNonConstReference so that V8 Fast API callbacks
no longer trigger spurious runtime/references warnings.

Why

cpplint's runtime/references rule requires non-const references to be
replaced with pointers or const references. V8 Fast API callbacks
declare a non-const v8::FastApiCallbackOptions& parameter, and the
signature is dictated by V8 — Node.js cannot change it. Running
cpplint against a minimal callback reproduces the false positive:

void FastFn(v8::Local<v8::Value> recv,
            v8::FastApiCallbackOptions& options) {}
repro.cc:2:  Is this a non-const reference? If so, make const or use a
pointer: v8::FastApiCallbackOptions& options  [runtime/references] [2]

Several real users in src/ (e.g. node_buffer.cc) already carry
// NOLINT(runtime/references) to silence the warning; others bring
the type into scope with using v8::FastApiCallbackOptions; and use
the unqualified spelling.

Fix

Add _RE_PATTERN_REF_V8_FAST_API_PARAM matching both
FastApiCallbackOptions& and v8::FastApiCallbackOptions& and skip
the runtime/references error for parameters that match it. Genuine
non-const references (e.g. int& value, SomeType& thing) and
similarly-named-but-distinct types (e.g. FastApiCallbackOptionsExtra&)
continue to be flagged.

A unittest-based regression test is added at
tools/test_cpplint_fast_api.py and locks in both the allowlist and
the negative cases.

Relationship to #62585

PR #62585 takes a similar approach but matches only the fully
qualified v8::FastApiCallbackOptions&. Most existing call sites in
src/ (node_buffer.cc, node_url.cc, node_debug.cc,
crypto/crypto_timing.cc, node_wasi.cc) write the parameter as the
unqualified FastApiCallbackOptions& after a using declaration, so
those sites would still warn. This PR extends the pattern to cover the
unqualified form and adds tests; happy to defer or rebase if #62585
lands first.

Validation

  • py tools/cpplint.py --filter=-,+runtime/references against
    src/node_buffer.cc, src/node_url.cc, src/node_debug.cc,
    src/crypto/crypto_timing.cc, src/node_wasi.cc, src/node_zlib.cc
    — zero warnings before and after, with no regressions.
  • py -m unittest tools.test_cpplint_fast_api — 4/4 tests pass with
    the fix; 2/4 fail without it (verified by stashing the cpplint
    change), confirming the test exercises the bug.
  • No Node binary build was required; this is a tooling-only change.

Fixes: #45761

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: nodejs#45761
Signed-off-by: Maruthan G <maruthang4@gmail.com>
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cpplint produces false positives for FastApiOptions

2 participants