Skip to content

fix: handle multiline function templates in namespaces #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
janmarsino98 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: handle multiline function templates in namespaces #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Changelog
TBA
===

* Function template declarations with multiline template parameters no longer trigger ``whitespace/indent_namespace`` on their continuation lines.

2.0.2 (2025-04-08)
===========

Expand Down
33 changes: 33 additions & 0 deletions cpplint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4161,6 +4161,9 @@ def CheckForNamespaceIndentation(filename, nesting_state, clean_lines, line, err
or (isinstance(nesting_state.previous_stack_top, _NamespaceInfo))
)

if IsMultilineFunctionTemplateDeclaration(clean_lines, line):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be part of ShouldCheckNamespaceIndentation()

return

if ShouldCheckNamespaceIndentation(
nesting_state, is_namespace_indent_item, clean_lines.elided, line
):
Expand Down Expand Up @@ -7384,6 +7387,36 @@ def IsBlockInNameSpace(nesting_state: NestingState, is_forward_declaration: bool
return False


def IsMultilineFunctionTemplateDeclaration(clean_lines, linenum):
"""Checks whether a line continues a function template declaration."""
for start_line in range(linenum - 1, -1, -1):
line = clean_lines.elided[start_line]
if re.search(r"[;{}]", line):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not

Suggested change
if re.search(r"[;{}]", line):
if any((c in set(";{}")) for c in line):

return False

if template := re.search(r"\btemplate\s*<", line):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems more pythonic to have an early exit instead of an implicit else to me

_, end_line, end_pos = CloseExpression(clean_lines, start_line, template.end() - 1)
if not start_line < linenum <= end_line or end_pos < 0:
return False

declaration = clean_lines.elided[end_line][end_pos:]

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably strip here instead of at 7408.

for next_line in range(end_line + 1, clean_lines.NumLines()):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider iterating over a slice

declaration += " " + clean_lines.elided[next_line].strip()
if re.search(r"[;{}]", declaration):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see https://github.com/cpplint/cpplint/pull/447/changes#r3666301777

break

declaration = declaration.strip()
if not declaration or "{" in declaration.split(";", 1)[0]:

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of false positive are we avoiding here? Wouldn't this stop functions with both template and definition from being exempted?

return False
if re.match(r"(?:class|enum|struct|using)\b", declaration):
return False

function = re.search(r"\b(?:operator\s*\S+|~?[A-Za-z_]\w*)\s*\(", declaration)
return function is not None and "=" not in declaration[: function.start()]
Comment on lines +7408 to +7415

@coderabbitai coderabbitai Bot Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix false positives for function template definitions.

The current logic correctly exempts function template declarations but fails for function template definitions (which have a { before any ;). Because "{" in declaration.split(";", 1)[0] evaluates to True for definitions, the function returns False, causing the continuation lines of a function template definition's parameters to still trigger false positive indentation warnings.

To support both declarations and definitions, we can split on both ; and { to isolate the signature, and then run the existing checks on it.

🐛 Proposed fix 
             declaration = declaration.strip()
-            if not declaration or "{" in declaration.split(";", 1)[0]:
+            if not declaration:
                 return False
-            if re.match(r"(?:class|enum|struct|using)\b", declaration):
+
+            decl_head = re.split(r"[;{]", declaration, 1)[0].strip()
+            if re.match(r"(?:class|enum|struct|using)\b", decl_head):
                 return False
 
-            function = re.search(r"\b(?:operator\s*\S+|~?[A-Za-z_]\w*)\s*\(", declaration)
-            return function is not None and "=" not in declaration[: function.start()]
+            function = re.search(r"\b(?:operator\s*\S+|~?[A-Za-z_]\w*)\s*\(", decl_head)
+            return function is not None and "=" not in decl_head[: function.start()]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
declaration = declaration.strip()
if not declaration or "{" in declaration.split(";", 1)[0]:
return False
if re.match(r"(?:class|enum|struct|using)\b", declaration):
return False
function = re.search(r"\b(?:operator\s*\S+|~?[A-Za-z_]\w*)\s*\(", declaration)
return function is not None and "=" not in declaration[: function.start()]
declaration = declaration.strip()
if not declaration:
return False
decl_head = re.split(r"[;{]", declaration, 1)[0].strip()
if re.match(r"(?:class|enum|struct|using)\b", decl_head):
return False
function = re.search(r"\b(?:operator\s*\S+|~?[A-Za-z_]\w*)\s*\(", decl_head)
return function is not None and "=" not in decl_head[: function.start()]
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint.py` around lines 7408 - 7415, Update the declaration handling around
the existing function-template detection to isolate the signature by splitting
on either “;” or “{” before checking for braces and matching the function name.
Preserve the existing exemptions for empty declarations, class/enum/struct/using
declarations, and assignment operators while allowing function template
definitions to return the same result as declarations.


return False


def ShouldCheckNamespaceIndentation(
nesting_state: NestingState, is_namespace_indent_item, raw_lines_no_comments, linenum
):
Expand Down
32 changes: 32 additions & 0 deletions cpplint_unittest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,38 @@ def testNamespaceIndentationIndentedParameter(self):
results = self.GetNamespaceResults(lines)
assert results == ""

def testNamespaceIndentationMultilineFunctionTemplateDeclaration(self):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have a test for definition too.

lines = [
"namespace Test {",
"template <typename Type1,",
" typename Type2>",
"void TestFunc(const Type1 &var1, Type2 &var2);",
"} // namespace Test",
]
assert self.GetNamespaceResults(lines) == ""

lines = [
"namespace Test {",
"template <typename T,",
" typename Callback = void (*)(T)>",
"void Register(Callback callback);",
"} // namespace Test",
]
assert self.GetNamespaceResults(lines) == ""

def testNamespaceIndentationIndentedMultilineFunctionTemplateDeclaration(self):

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method split seems unnecessary.

lines = [
"namespace Test {",
" template <typename Type1,",
" typename Type2>",
" void TestFunc(const Type1 &var1, Type2 &var2);",
"} // namespace Test",
]
assert self.GetNamespaceResults(lines) == [
"Do not indent within a namespace. [whitespace/indent_namespace] [4]",
"Do not indent within a namespace. [whitespace/indent_namespace] [4]",
]
Comment on lines +346 to +356

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does what you think it does. If it was erroring for truly indented, we would see three errors instead of two. It is a compelling design decision (performance for a pretty niche case, and we'd have errored on the starting line already) whether we even want to check for this kind of false negative (see also the Boost sample's F- above), but this makes codereaders thought it still emits on the F- when it does not.


def testNamespaceIndentationMemberInitializerList(self):
lines = [
"namespace Opossum {",
Expand Down
4 changes: 1 addition & 3 deletions samples/boost-sample/simple.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ include/boost/math/*
1
3
Done processing include/boost/math/octonion.hpp
Total errors found: 2900
Total errors found: 2898

include/boost/math/octonion.hpp:11: #ifndef header guard has wrong style, please use: SAMPLES_BOOST_SAMPLE_INCLUDE_BOOST_MATH_OCTONION_HPP_ [build/header_guard] [5]
include/boost/math/octonion.hpp:4250: #endif line should be "#endif // SAMPLES_BOOST_SAMPLE_INCLUDE_BOOST_MATH_OCTONION_HPP_" [build/header_guard] [5]
Expand Down Expand Up @@ -290,8 +290,6 @@ include/boost/math/octonion.hpp:672: { should almost always be at the end of th
include/boost/math/octonion.hpp:673: Do not indent within a namespace. [whitespace/indent_namespace] [4]
include/boost/math/octonion.hpp:673: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
include/boost/math/octonion.hpp:674: Do not indent within a namespace. [whitespace/indent_namespace] [4]
include/boost/math/octonion.hpp:675: Do not indent within a namespace. [whitespace/indent_namespace] [4]
include/boost/math/octonion.hpp:676: Do not indent within a namespace. [whitespace/indent_namespace] [4]

@aaronliu0130 aaronliu0130 Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False negative.

include/boost/math/octonion.hpp:677: Do not indent within a namespace. [whitespace/indent_namespace] [4]
include/boost/math/octonion.hpp:678: Do not indent within a namespace. [whitespace/indent_namespace] [4]
include/boost/math/octonion.hpp:679: Do not indent within a namespace. [whitespace/indent_namespace] [4]
Expand Down