fix(whitespace/indent_namespace): Specifier & multiline false positives for constructors

aaronliu0130 · aaronliu0130 · commit 322fc9e29fe8 · 2025-11-28T12:46:13.000-05:00
Fix false positive for when a function specifier (e.g. `inline`, `protected`) is provided
Fix false positive for when constructor ends on the same line a MemInitList does, but on a different line from said list's start
Fix ownership assignment of open parentheses
Use search instead of match for some cases
Fix typo &amp; misc refactoring
diff --git a/cpplint.py b/cpplint.py
@@ -3230,7 +3230,17 @@ class _WrappedInfo(_BlockInfo):
 class _MemInitListInfo(_WrappedInfo):
     """Stores information about member initializer lists."""
 
-    pass
+    def __init__(self, linenum):
+        _WrappedInfo.__init__(self, linenum, False)
+        # For `: a(b)`, a is the identifier
+        self.expecting_identifier = True
+        # Whether we are expecting the value that follows the identifier
+        self.expecting_braces = False
+        # Whether we are currently within braces
+        self.in_braces = False
+        # Only used when within braces
+        self.is_curly = False
+        self.open_brace_count = 0
 
 
 class _PreprocessorInfo:
@@ -3384,6 +3394,20 @@ def InTemplateArgumentList(self, clean_lines, linenum, pos):
             pos = end_pos
         return False
 
+    def _BequeathParensAndAppend(self, appendee: _BlockInfo, parens_changed: int):
+        """Pass down number of open parentheses and append to the stack.
+
+        Args:
+          appendee: The object to append to the stack.
+          parens_changed: The number of parentheses opened, as computed by _ChangeParentheses().
+        """
+        # Compensate for adding parentheses to the previous top block before this update
+        if self.stack:
+            self.stack[-1].open_parentheses -= parens_changed
+        appendee.open_parentheses += parens_changed
+
+        self.stack.append(appendee)
+
     def UpdatePreprocessor(self, line):
         """Update preprocessor stack.
 
@@ -3444,9 +3468,15 @@ def _Pop(self):
         """Pop the innermost state (top of the stack) and remember the popped item."""
         self.popped_top = self.stack.pop()
 
-    def _CountOpenParentheses(self, line: str):
-        # Count parentheses.  This is to avoid adding struct arguments to
-        # the nesting stack.
+    def _ChangeParentheses(self, line: str) -> int:
+        """
+        Count parentheses.  This is to avoid adding struct arguments to the nesting stack.
+        Args:
+            line: The line to count parentheses on.
+
+        Returns:
+        The number of parentheses opened on this line.
+        """
         if self.stack:
             inner_block = self.stack[-1]
             depth_change = line.count("(") - line.count(")")
@@ -3469,7 +3499,10 @@ def _CountOpenParentheses(self, line: str):
                 # Exit assembly block
                 inner_block.inline_asm = _END_ASM
 
-    def _UpdateNamesapce(self, line: str, linenum: int) -> str | None:
+            return depth_change
+        return 0
+
+    def _ConsumeNamespace(self, line: str, linenum: int) -> str | None:
         """
         Match start of namespace, append to stack, and consume line
         Args:
@@ -3496,21 +3529,29 @@ def _UpdateNamesapce(self, line: str, linenum: int) -> str | None:
             line = line[line.find("{") + 1 :]
         return line
 
-    def _UpdateConstructor(self, line: str, linenum: int, class_name: str | None = None):
+    def _UpdateConstructor(
+        self, line: str, linenum: int, parens_changed: int, class_name: str | None = None
+    ) -> bool:
         """
         Check if the given line is a constructor.
         Args:
             line: Line to check.
             class_name: If line checked is inside of a class block, a str of the class's name;
                 otherwise, None.
+            parens_changed: The number of parentheses opened on this line.
+
+        Returns:
+        Whether the given line was a constructor.
         """
+        prefix = r"(?:^|public|private|protected|friend|inline|explicit|constexpr)\s*"
         if not class_name:
-            if not re.match(r"\s*(\w*)\s*::\s*\1\s*\(", line):
-                return
-        elif not re.match(rf"\s*{re.escape(class_name)}\s*\(", line):
-            return
+            if not re.search(prefix + r"(\w*)\s*::\s*\1\s*\(", line):
+                return False
+        elif not re.search(prefix + rf"{re.escape(class_name)}\s*\(", line):
+            return False
 
-        self.stack.append(_ConstructorInfo(linenum))
+        self._BequeathParensAndAppend(_ConstructorInfo(linenum))
+        return True
 
     # TODO(google): Update() is too long, but we will refactor later.
     def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error):
@@ -3538,12 +3579,12 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
         # Update pp_stack
         self.UpdatePreprocessor(line)
 
-        self._CountOpenParentheses(line)
+        parens_changed = self._ChangeParentheses(line)
 
         # Consume namespace declaration at the beginning of the line.  Do
         # this in a loop so that we catch same line declarations like this:
         #   namespace proto2 { namespace bridge { class MessageSet; } }
-        while (new_line := self._UpdateNamesapce(line, linenum)) is not None:  # could be empty str
+        while (new_line := self._ConsumeNamespace(line, linenum)) is not None:  # could be empty str
             line = new_line
 
         # Look for a class declaration in whatever is left of the line
@@ -3616,8 +3657,11 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
                     line = access_match.group(4)
                 else:
                     self._UpdateConstructor(line, linenum, class_name=classinfo.name)
+                    self._UpdateConstructor(
+                        line, linenum, parens_changed, class_name=classinfo.name
+                    )
         else:  # Not in class
-            self._UpdateConstructor(line, linenum)
+            self._UpdateConstructor(line, linenum, parens_changed)
 
         # If brace not open and we just finished a parenthetical definition,
         # check if we're in a member initializer list following a constructor.
@@ -3630,16 +3674,16 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
             and not self.stack[-1].seen_open_brace
             and re.search(r"[^:]:[^:]", line)
         ):
-            self.stack.append(_MemInitListInfo(linenum, seen_open_brace=False))
+            self._BequeathParensAndAppend(_MemInitListInfo(linenum), parens_changed)
 
         # Consume braces or semicolons from what's left of the line
         while True:
             # Match first brace, semicolon, or closed parenthesis.
-            matched = re.match(r"^[^{;)}]*([{;)}])(.*)$", line)
-            if not matched:
+            searched = re.search(r"([{;)}])", line)
+            if not searched:
                 break
 
-            token = matched.group(1)
+            token = searched.group(1)
             if token == "{":
                 # If namespace or class hasn't seen an opening brace yet, mark
                 # namespace/class head as complete.  Push a new block onto the
@@ -3677,7 +3721,7 @@ def Update(self, filename: str, clean_lines: CleansedLines, linenum: int, error)
                 if self.stack:
                     self.stack[-1].CheckEnd(filename, clean_lines, linenum, error)
                     self._Pop()
-            line = matched.group(2)
+            line = line[searched.end(0) :]
 
     def InnermostClass(self):
         """Get class info on the top of the stack.
@@ -7294,15 +7338,22 @@ def ShouldCheckNamespaceIndentation(
 
     # Skip if we are extra-indenting a member initializer list.
     if (
-        isinstance(nesting_state.previous_stack_top, _ConstructorInfo)  # F/N (A::A() : _a(0) {/{})
-        and (
-            isinstance(nesting_state.stack[-1], _MemInitListInfo)
-            or isinstance(nesting_state.popped_top, _MemInitListInfo)
+        (
+            isinstance(nesting_state.previous_stack_top, _ConstructorInfo)  # F/N (A::A() : _a(0){)
+            and (
+                isinstance(nesting_state.stack[-1], _MemInitListInfo)
+                or isinstance(nesting_state.popped_top, _MemInitListInfo)
+            )
+        )
+        or (  # empty constructor in multiline list
+            isinstance(nesting_state.previous_stack_top, _MemInitListInfo)
+            and isinstance(nesting_state.popped_top, _ConstructorInfo)
+        )
+        or (  # popping constructor after MemInitList on the same line (: _a(a) {})
+            isinstance(nesting_state.previous_stack_top, _ConstructorInfo)
+            and isinstance(nesting_state.popped_top, _ConstructorInfo)
+            and re.search(r"[^:]:[^:]", raw_lines_no_comments[linenum])
         )
-    ) or (  # popping constructor after MemInitList on the same line (: _a(a) {})
-        isinstance(nesting_state.previous_stack_top, _ConstructorInfo)
-        and isinstance(nesting_state.popped_top, _ConstructorInfo)
-        and re.search(r"[^:]:[^:]", raw_lines_no_comments[linenum])
     ):
         return False
 
diff --git a/cpplint_unittest.py b/cpplint_unittest.py
@@ -327,27 +327,39 @@ def testNamespaceIndentationMemberInitializerList(self):
         lines = [
             "namespace Opossum {",
             "",
-            "Acme::Acme(const std::shared_ptr<const AbstractOperator> left,",
-            "           const std::shared_ptr<const AbstractOperator> nigh)",
+            "protected Acme::Acme(const std::shared_ptr<const AbstractOperator> left,",
+            "                     const std::shared_ptr<const AbstractOperator> nigh)",
             "    : _left(left), _behind(nigh) {}",
             "",
             "}  // namespace Opossum",
         ]
         assert self.GetNamespaceResults(lines) == ""
 
-        # Multiline member initializer List
+        # Multiline member initializer list; constructor on different line from list
         lines = [
             "namespace Rosenfield {",
             "class Crush : public Habitual {",
             " public:",
-            "  Crush() : _a(1),",
+            "  explicit Crush() : _a(1),",
             "            _b(2)",
             "       {}",
             "};",
             "}  // namespace Rosenfield",
         ]
         assert self.GetNamespaceResults(lines) == ""
 
+        # Inline + multiline, ends on the same line as a list's end
+        lines = [
+            "namespace Humpety {",
+            "Dumpety::Dumpety(const int a)",
+            "    : number(a),",
+            "      looooooong_attr1(0),",
+            "      looooooong_attr2(0),",
+            "      looooooong_attr3(0),",
+            "      looooooong_attr4(0) {}",
+        ]
+        assert self.GetNamespaceResults(lines) == ""
+
         # Same line as constructor declaration
         lines = [
             "namespace Boucher {",
@@ -363,7 +375,7 @@ def testNamespaceIndentationMemberInitializerList(self):
         lines = [
             "namespace Store {",
             "",
-            "   Color::Color() : my_name_is('b')",
+            "   Color::Color() : my_name_is('b'),",
             "       this_is_true(true) {",
         ]
         assert (
