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

Commit 73a57b5

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 33784dd commit 73a57b5

7 files changed

Lines changed: 6951 additions & 7144 deletions

File tree

‎CHANGELOG.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Changelog
77

88
Yet another overdue... hotfix. Sorry this took so long.
99

10-
* The false positive for indented function parameters in namespaces was eradicated. (https://github.com/cpplint/cpplint/pull/304)
10+
* The false positives for indented function parameters (https://github.com/cpplint/cpplint/pull/304) and member initializer lists (https://github.com/cpplint/cpplint/pull/353) in namespaces were eradicated.
1111
* Files that end in ".c", ".C", or ".cu" will now also automatically suppress C++-only categories. Previously, `// NO_LINT_C` was required. (https://github.com/cpplint/cpplint/pull/308)
1212
* 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)
1313
* Ruff was ran on the project to improve performance and reader comprehension thanks to @cclauss.

‎cpplint.py‎

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

31003100

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

@@ -3175,6 +3183,22 @@ def CheckEnd(self, filename, clean_lines, linenum, error):
31753183
)
31763184

31773185

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

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

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

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

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

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

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

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

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

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

35663640
def InnermostClass(self):
@@ -7106,11 +7180,11 @@ def CheckRedundantOverrideOrFinal(filename, clean_lines, linenum, error):
71067180

71077181
# Returns true if we are at a new block, and it is directly
71087182
# inside of a namespace.
7109-
def IsBlockInNameSpace(nesting_state, is_forward_declaration):
7183+
def IsBlockInNameSpace(nesting_state: NestingState, is_forward_declaration: bool): # noqa: FBT001
71107184
"""Checks that the new block is directly in a namespace.
71117185
71127186
Args:
7113-
nesting_state: The _NestingState object that contains info about our state.
7187+
nesting_state: The NestingState object that contains info about our state.
71147188
is_forward_declaration: If the class is a forward declared class.
71157189
Returns:
71167190
Whether or not the new block is directly in a namespace.
@@ -7126,7 +7200,13 @@ def IsBlockInNameSpace(nesting_state, is_forward_declaration):
71267200
if (
71277201
len(nesting_state.stack) > 1
71287202
and isinstance(nesting_state.previous_stack_top, _NamespaceInfo)
7129-
and isinstance(nesting_state.stack[-2], _NamespaceInfo)
7203+
and (
7204+
isinstance(nesting_state.stack[-2], _NamespaceInfo)
7205+
or len(nesting_state.stack) > 2 # Accommodate for WrappedInfo
7206+
and issubclass(type(nesting_state.stack[-1]), _WrappedInfo)
7207+
and not nesting_state.stack[-2].seen_open_brace
7208+
and isinstance(nesting_state.stack[-3], _NamespaceInfo)
7209+
)
71307210
):
71317211
return True
71327212
return False
@@ -7150,6 +7230,10 @@ def ShouldCheckNamespaceIndentation(
71507230
only works for classes and namespaces inside of a namespace.
71517231
"""
71527232

7233+
# Required by all checks involving nesting_state
7234+
if not nesting_state.stack:
7235+
return False
7236+
71537237
is_forward_declaration = IsForwardClassDeclaration(raw_lines_no_comments, linenum)
71547238

71557239
if not (is_namespace_indent_item or is_forward_declaration):
@@ -7163,6 +7247,20 @@ def ShouldCheckNamespaceIndentation(
71637247
if nesting_state.previous_stack_top and nesting_state.previous_open_parentheses > 0:
71647248
return False
71657249

7250+
# Skip if we are extra-indenting a member initializer list.
7251+
if (
7252+
isinstance(nesting_state.previous_stack_top, _ConstructorInfo) # F/N (A::A() : _a(0) {/{})
7253+
and (
7254+
isinstance(nesting_state.stack[-1], _MemInitListInfo)
7255+
or isinstance(nesting_state.popped_top, _MemInitListInfo)
7256+
)
7257+
) or ( # popping constructor after MemInitList on the same line (: _a(a) {})
7258+
isinstance(nesting_state.previous_stack_top, _ConstructorInfo)
7259+
and isinstance(nesting_state.popped_top, _ConstructorInfo)
7260+
and re.search(r"[^:]:[^:]", raw_lines_no_comments[linenum])
7261+
):
7262+
return False
7263+
71667264
return IsBlockInNameSpace(nesting_state, is_forward_declaration)
71677265

71687266

‎cpplint_unittest.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,54 @@ def testNamespaceIndentationIndentedParameter(self):
326326
results = self.GetNamespaceResults(lines)
327327
assert results == ""
328328

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

‎samples/boost-sample/exclude.def‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
--recursive --exclude=headers/* src
1+
--recursive "--exclude=headers/*" src
22
1
33
4
44
Done processing src/inspect/unnamed_namespace_check.hpp
55
Done processing src/tr1/c_policy.hpp
6-
Total errors found: 121
6+
Total errors found: 120
77

88
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]
99
src/inspect/unnamed_namespace_check.hpp:11: Include the directory when naming header files [build/include_subdir] [4]
@@ -19,7 +19,6 @@ src/inspect/unnamed_namespace_check.hpp:28: Do not indent within a namespace.
1919
src/inspect/unnamed_namespace_check.hpp:28: { should almost always be at the end of the previous line [whitespace/braces] [4]
2020
src/inspect/unnamed_namespace_check.hpp:29: Do not indent within a namespace. [whitespace/indent_namespace] [4]
2121
src/inspect/unnamed_namespace_check.hpp:29: Weird number of spaces at line-start. Are you using a 2-space indent? [whitespace/indent] [3]
22-
src/inspect/unnamed_namespace_check.hpp:30: Do not indent within a namespace. [whitespace/indent_namespace] [4]
2322
src/inspect/unnamed_namespace_check.hpp:30: Weird number of spaces at line-start. Are you using a 2-space indent? [whitespace/indent] [3]
2423
src/inspect/unnamed_namespace_check.hpp:30: { should almost always be at the end of the previous line [whitespace/braces] [4]
2524
src/inspect/unnamed_namespace_check.hpp:31: Extra space after ( in function call [whitespace/parens] [4]

‎samples/boost-sample/headers_inspect.def‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ src/inspect/*
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)