Visitar URL original
fix(indentation_namespace): false positive for MemInitLists · cpplint/cpplint@5ae674c · GitHub
Skip to content

Commit 5ae674c

Browse files
aaronliu0130aaronliu0130
committed
fix(indentation_namespace): false positive for MemInitLists
Fix whitespace/indentation_namespace false positive for member initializer lists (MemInitLists) Adds new _ConstructorInfo, _WrappedInfo, and _MemInitListInfo nesting stack element classes Remember last-popped nesting stack item Update descriptor.pb.cc to 2016-12-16 version (commit 183d31c)
1 parent a046f9c commit 5ae674c

6 files changed

Lines changed: 6949 additions & 7142 deletions

File tree

‎CHANGELOG.rst‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ Changelog
66
===========
77

88
* Python versions less than 3.9 are no longer supported.
9-
* The false positive for indented function parameters in namespaces was eradicated. (https://github.com/cpplint/cpplint/pull/304)
10-
* build/include-what-you-use now recognizes c-style headers, such as <stdio.h> for symbols from <cstdio>. (https://github.com/cpplint/cpplint/pull/319)
9+
* The false positive for indented member initializer lists in namespaces were eradicated. (https://github.com/cpplint/cpplint/pull/353)
1110
* The warning on non-const references (runtime/references) is now disabled by default pursuant to the May 2020 Google style guide update. (https://github.com/cpplint/cpplint/pull/305)
1211

1312
2.0.1 (2025-03-09)

‎cpplint.py‎

Lines changed: 136 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3097,6 +3097,14 @@ def CheckEnd(self, filename, clean_lines, linenum, error):
30973097
)
30983098

30993099

3100+
class _ConstructorInfo(_BlockInfo):
3101+
"""Stores information about a constructor.
3102+
For detecting member initializer lists."""
3103+
3104+
def __init__(self, linenum: int):
3105+
_BlockInfo.__init__(self, linenum, seen_open_brace=False)
3106+
3107+
31003108
class _NamespaceInfo(_BlockInfo):
31013109
"""Stores information about a namespace."""
31023110

@@ -3174,6 +3182,22 @@ def CheckEnd(self, filename, clean_lines, linenum, error):
31743182
)
31753183

31763184

3185+
class _WrappedInfo(_BlockInfo):
3186+
"""Stores information about parentheses, initializer lists, etc.
3187+
Not exactly a block but we do need the same signature.
3188+
Needed to avoid namespace indentation false positives,
3189+
though parentheses tracking would slow us down a lot
3190+
and is effectively already done by open_parentheses."""
3191+
3192+
pass
3193+
3194+
3195+
class _MemInitListInfo(_WrappedInfo):
3196+
"""Stores information about member initializer lists."""
3197+
3198+
pass
3199+
3200+
31773201
class _PreprocessorInfo:
31783202
"""Stores checkpoints of nesting stacks when #if/#else is seen."""
31793203

@@ -3216,6 +3240,9 @@ def __init__(self):
32163240
# We can't use previous_stack_top, a shallow copy whose open_parentheses value is updated.
32173241
self.previous_open_parentheses = 0
32183242

3243+
# The last stack item we popped.
3244+
self.popped_top: _BlockInfo | None = None
3245+
32193246
# Stack of _PreprocessorInfo objects.
32203247
self.pp_stack = []
32213248

@@ -3370,6 +3397,9 @@ def UpdatePreprocessor(self, line):
33703397
# TODO(google): unexpected #endif, issue warning?
33713398
pass
33723399

3400+
def _Pop(self):
3401+
"""Pop the innermost state (top of the stack) and remember the popped item."""
3402+
self.popped_top = self.stack.pop()
33733403

33743404
def _CountOpenParentheses(self, line: str):
33753405
# Count parentheses. This is to avoid adding struct arguments to
@@ -3423,6 +3453,22 @@ def _UpdateNamesapce(self, line: str, linenum: int) -> str | None:
34233453
line = line[line.find("{") + 1 :]
34243454
return line
34253455

3456+
def _UpdateConstructor(self, line: str, linenum: int, class_name: str | None = None):
3457+
"""
3458+
Check if the given line is a constructor.
3459+
Args:
3460+
line: Line to check.
3461+
class_name: If line checked is inside of a class block, a str of the class's name;
3462+
otherwise, None.
3463+
"""
3464+
if not class_name:
3465+
if not re.match(r"\s*(\w*)\s*::\s*\1\s*\(", line):
3466+
return
3467+
elif not re.match(rf"\s*{re.escape(class_name)}\s*\(", line):
3468+
return
3469+
3470+
self.stack.append(_ConstructorInfo(linenum))
3471+
34263472
# TODO(google): Update() is too long, but we will refactor later.
34273473
def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error):
34283474
"""Update nesting state with current line.
@@ -3492,36 +3538,56 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
34923538
if not self.SeenOpenBrace():
34933539
self.stack[-1].CheckBegin(filename, clean_lines, linenum, error)
34943540

3495-
# Update access control if we are inside a class/struct
3541+
# Update access control if we are directly inside a class/struct
34963542
if self.stack and isinstance(self.stack[-1], _ClassInfo):
3497-
classinfo = self.stack[-1]
3498-
access_match = re.match(
3499-
r"^(.*)\b(public|private|protected|signals)(\s+(?:slots\s*)?)?"
3500-
r":(?:[^:]|$)",
3501-
line,
3543+
if self.stack[-1].seen_open_brace:
3544+
classinfo: _ClassInfo = self.stack[-1]
3545+
# Update access control
3546+
if access_match := re.match(
3547+
r"^(.*)\b(public|private|protected|signals)(\s+(?:slots\s*)?)?"
3548+
r":([^:].*|$)",
3549+
line,
3550+
):
3551+
classinfo.access = access_match.group(2)
3552+
3553+
# Check that access keywords are indented +1 space. Skip this
3554+
# check if the keywords are not preceded by whitespaces.
3555+
indent = access_match.group(1)
3556+
if len(indent) != classinfo.class_indent + 1 and re.match(r"^\s*$", indent):
3557+
if classinfo.is_struct:
3558+
parent = "struct " + classinfo.name
3559+
else:
3560+
parent = "class " + classinfo.name
3561+
slots = ""
3562+
if access_match.group(3):
3563+
slots = access_match.group(3)
3564+
error(
3565+
filename,
3566+
linenum,
3567+
"whitespace/indent",
3568+
3,
3569+
f"{access_match.group(2)}{slots}:"
3570+
f" should be indented +1 space inside {parent}",
3571+
)
3572+
3573+
line = access_match.group(4)
3574+
else:
3575+
self._UpdateConstructor(line, linenum, class_name=classinfo.name)
3576+
else: # Not in class
3577+
self._UpdateConstructor(line, linenum)
3578+
3579+
# If brace not open and we just finished a parenthetical definition,
3580+
# check if we're in a member initializer list following a constructor.
3581+
if (
3582+
self.stack
3583+
and (
3584+
isinstance(self.stack[-1], _ConstructorInfo)
3585+
or isinstance(self.previous_stack_top, _ConstructorInfo)
35023586
)
3503-
if access_match:
3504-
classinfo.access = access_match.group(2)
3505-
3506-
# Check that access keywords are indented +1 space. Skip this
3507-
# check if the keywords are not preceded by whitespaces.
3508-
indent = access_match.group(1)
3509-
if len(indent) != classinfo.class_indent + 1 and re.match(r"^\s*$", indent):
3510-
if classinfo.is_struct:
3511-
parent = "struct " + classinfo.name
3512-
else:
3513-
parent = "class " + classinfo.name
3514-
slots = ""
3515-
if access_match.group(3):
3516-
slots = access_match.group(3)
3517-
error(
3518-
filename,
3519-
linenum,
3520-
"whitespace/indent",
3521-
3,
3522-
f"{access_match.group(2)}{slots}:"
3523-
f" should be indented +1 space inside {parent}",
3524-
)
3587+
and not self.stack[-1].seen_open_brace
3588+
and re.search(r"[^:]:[^:]", line)
3589+
):
3590+
self.stack.append(_MemInitListInfo(linenum, seen_open_brace=False))
35253591

35263592
# Consume braces or semicolons from what's left of the line
35273593
while True:
@@ -3532,34 +3598,42 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
35323598

35333599
token = matched.group(1)
35343600
if token == "{":
3535-
# If namespace or class hasn't seen a opening brace yet, mark
3601+
# If namespace or class hasn't seen an opening brace yet, mark
35363602
# namespace/class head as complete. Push a new block onto the
35373603
# stack otherwise.
35383604
if not self.SeenOpenBrace():
3605+
# End of initializer list wrap if present
3606+
if isinstance(self.stack[-1], _MemInitListInfo):
3607+
self._Pop()
35393608
self.stack[-1].seen_open_brace = True
35403609
elif re.match(r'^extern\s*"[^"]*"\s*\{', line):
35413610
self.stack.append(_ExternCInfo(linenum))
35423611
else:
35433612
self.stack.append(_BlockInfo(linenum, True))
35443613
if _MATCH_ASM.match(line):
35453614
self.stack[-1].inline_asm = _BLOCK_ASM
3546-
3547-
elif token in {";", ")"}:
3615+
elif token == ";":
35483616
# If we haven't seen an opening brace yet, but we already saw
35493617
# a semicolon, this is probably a forward declaration. Pop
35503618
# the stack for these.
3551-
#
3619+
if not self.SeenOpenBrace():
3620+
self._Pop()
3621+
elif token == ")":
35523622
# Similarly, if we haven't seen an opening brace yet, but we
35533623
# already saw a closing parenthesis, then these are probably
35543624
# function arguments with extra "class" or "struct" keywords.
35553625
# Also pop these stack for these.
3556-
if not self.SeenOpenBrace():
3557-
self.stack.pop()
3626+
if (
3627+
self.stack
3628+
and not self.stack[-1].seen_open_brace
3629+
and isinstance(self.stack[-1], _ClassInfo)
3630+
):
3631+
self._Pop()
35583632
else: # token == '}'
35593633
# Perform end of block checks and pop the stack.
35603634
if self.stack:
35613635
self.stack[-1].CheckEnd(filename, clean_lines, linenum, error)
3562-
self.stack.pop()
3636+
self._Pop()
35633637
line = matched.group(2)
35643638

35653639
def InnermostClass(self):
@@ -7097,11 +7171,11 @@ def CheckRedundantOverrideOrFinal(filename, clean_lines, linenum, error):
70977171

70987172
# Returns true if we are at a new block, and it is directly
70997173
# inside of a namespace.
7100-
def IsBlockInNameSpace(nesting_state, is_forward_declaration):
7174+
def IsBlockInNameSpace(nesting_state: NestingState, is_forward_declaration: bool): # noqa: FBT001
71017175
"""Checks that the new block is directly in a namespace.
71027176
71037177
Args:
7104-
nesting_state: The _NestingState object that contains info about our state.
7178+
nesting_state: The NestingState object that contains info about our state.
71057179
is_forward_declaration: If the class is a forward declared class.
71067180
Returns:
71077181
Whether or not the new block is directly in a namespace.
@@ -7117,7 +7191,13 @@ def IsBlockInNameSpace(nesting_state, is_forward_declaration):
71177191
if (
71187192
len(nesting_state.stack) > 1
71197193
and isinstance(nesting_state.previous_stack_top, _NamespaceInfo)
7120-
and isinstance(nesting_state.stack[-2], _NamespaceInfo)
7194+
and (
7195+
isinstance(nesting_state.stack[-2], _NamespaceInfo)
7196+
or len(nesting_state.stack) > 2 # Accommodate for WrappedInfo
7197+
and issubclass(type(nesting_state.stack[-1]), _WrappedInfo)
7198+
and not nesting_state.stack[-2].seen_open_brace
7199+
and isinstance(nesting_state.stack[-3], _NamespaceInfo)
7200+
)
71217201
):
71227202
return True
71237203
return False
@@ -7141,6 +7221,10 @@ def ShouldCheckNamespaceIndentation(
71417221
only works for classes and namespaces inside of a namespace.
71427222
"""
71437223

7224+
# Required by all checks involving nesting_state
7225+
if not nesting_state.stack:
7226+
return False
7227+
71447228
is_forward_declaration = IsForwardClassDeclaration(raw_lines_no_comments, linenum)
71457229

71467230
if not (is_namespace_indent_item or is_forward_declaration):
@@ -7154,6 +7238,20 @@ def ShouldCheckNamespaceIndentation(
71547238
if nesting_state.previous_stack_top and nesting_state.previous_open_parentheses > 0:
71557239
return False
71567240

7241+
# Skip if we are extra-indenting a member initializer list.
7242+
if (
7243+
isinstance(nesting_state.previous_stack_top, _ConstructorInfo) # F/N (A::A() : _a(0) {/{})
7244+
and (
7245+
isinstance(nesting_state.stack[-1], _MemInitListInfo)
7246+
or isinstance(nesting_state.popped_top, _MemInitListInfo)
7247+
)
7248+
) or ( # popping constructor after MemInitList on the same line (: _a(a) {})
7249+
isinstance(nesting_state.previous_stack_top, _ConstructorInfo)
7250+
and isinstance(nesting_state.popped_top, _ConstructorInfo)
7251+
and re.search(r"[^:]:[^:]", raw_lines_no_comments[linenum])
7252+
):
7253+
return False
7254+
71577255
return IsBlockInNameSpace(nesting_state, is_forward_declaration)
71587256

71597257

‎cpplint_unittest.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,54 @@ def testNamespaceIndentationIndentedParameter(self):
323323
results = self.GetNamespaceResults(lines)
324324
assert results == ""
325325

326+
def testNamespaceIndentationMemberInitializerList(self):
327+
lines = [
328+
"namespace Opossum {",
329+
"",
330+
"Acme::Acme(const std::shared_ptr<const AbstractOperator> left,",
331+
" const std::shared_ptr<const AbstractOperator> nigh)",
332+
" : _left(left), _behind(nigh) {}",
333+
"",
334+
"} // namespace Opossum",
335+
]
336+
assert self.GetNamespaceResults(lines) == ""
337+
338+
# Multiline member initializer List
339+
lines = [
340+
"namespace Rosenfield {",
341+
"class Crush : public Habitual {",
342+
" public:",
343+
" Crush() : _a(1),",
344+
" _b(2)",
345+
" {}",
346+
"};",
347+
"} // namespace Rosenfield",
348+
]
349+
assert self.GetNamespaceResults(lines) == ""
350+
351+
# Same line as constructor declaration
352+
lines = [
353+
"namespace Boucher {",
354+
" X::X() : _s(2) {}",
355+
" A::A() : _e(12) {",
356+
]
357+
assert self.GetNamespaceResults(lines) == [
358+
"Do not indent within a namespace. [whitespace/indent_namespace] [4]",
359+
"Do not indent within a namespace. [whitespace/indent_namespace] [4]",
360+
]
361+
362+
# Definition on indented line
363+
lines = [
364+
"namespace Store {",
365+
"",
366+
" Color::Color() : my_name_is('b')",
367+
" this_is_true(true) {",
368+
]
369+
assert (
370+
self.GetNamespaceResults(lines)
371+
== "Do not indent within a namespace. [whitespace/indent_namespace] [4]"
372+
)
373+
326374
def testNestingInNamespace(self):
327375
lines = [
328376
"namespace Test {",

‎samples/boost-sample/headers_inspect_exclude.def‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
1
33
3
44
Done processing src/inspect/unnamed_namespace_check.hpp
5-
Total errors found: 50
5+
Total errors found: 49
66

77
src/inspect/unnamed_namespace_check.hpp:0: No #ifndef header guard found, suggested CPP variable is: SAMPLES_BOOST_SAMPLE_SRC_INSPECT_UNNAMED_NAMESPACE_CHECK_HPP_ [build/header_guard] [5]
88
src/inspect/unnamed_namespace_check.hpp:11: Include the directory when naming header files [build/include_subdir] [4]
@@ -18,7 +18,6 @@ src/inspect/unnamed_namespace_check.hpp:28: Do not indent within a namespace.
1818
src/inspect/unnamed_namespace_check.hpp:28: { should almost always be at the end of the previous line [whitespace/braces] [4]
1919
src/inspect/unnamed_namespace_check.hpp:29: Do not indent within a namespace. [whitespace/indent_namespace] [4]
2020
src/inspect/unnamed_namespace_check.hpp:29: Weird number of spaces at line-start. Are you using a 2-space indent? [whitespace/indent] [3]
21-
src/inspect/unnamed_namespace_check.hpp:30: Do not indent within a namespace. [whitespace/indent_namespace] [4]
2221
src/inspect/unnamed_namespace_check.hpp:30: Weird number of spaces at line-start. Are you using a 2-space indent? [whitespace/indent] [3]
2322
src/inspect/unnamed_namespace_check.hpp:30: { should almost always be at the end of the previous line [whitespace/braces] [4]
2423
src/inspect/unnamed_namespace_check.hpp:31: Extra space after ( in function call [whitespace/parens] [4]

0 commit comments

Comments
 (0)