cpplint · aaronliu0130 · Feb 24, 2024 · Feb 24, 2024 · Mar 17, 2024 · Jul 1, 2024
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -13,8 +13,8 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        # Python 3.7 is the lowest available for actions/setup-python
-        python-version: ['3.7', 'pypy3.10', '3.12']
+        # Python 3.8 is the last non-EOL version
+        python-version: ['3.8', 'pypy3.10', '3.12']
         os: [ubuntu-latest, windows-latest]
       fail-fast: false
 

diff --git a/changelog.rst b/changelog.rst
@@ -7,17 +7,18 @@ Changelog
 
 A bunch of long-overdue modernizations of the codebase!
 
-* Python 2 is no longer supported. Python 3.7 and 3.12 support was added, courtesy of @jayvdb
+* Python 2 is no longer supported. Python 3.12 support was added along with fixed CI for 3.7 and 3.8, courtesy of @jayvdb
    * As a result of all this, setup.py's lint subcommand was removed. Please run the commands directly instead.
 * You can now specify blocks of code that exclude linting with NOLINTBEGIN and NOLINTEND, courtesy of @n3world (https://github.com/cpplint/cpplint/pull/213)
 * The `--filter` option can now be only applied to a specific file or even a specific line through utilizing colons, e.g. `-filter=-whitespace:foo.h,+whitespace/braces:foo.h:418`. Courtesy of @PhilLab (https://github.com/cpplint/cpplint/pull/171)
 * NOLINT and NOLINTNEXTLINE comments now support a comma-separated list of categories, courtesy of @n3world (https://github.com/cpplint/cpplint/pull/220)
 * NOLINT and NOLINTNEXTLINE will now ignore categories known to be from clang-tidy thanks to @xatier (https://github.com/cpplint/cpplint/pull/231)
-* Fix behavior with nested source repositories by @groegeorg (https://github.com/cpplint/cpplint/pull/78)
+* Fixed behavior with nested source repositories by @groegeorg (https://github.com/cpplint/cpplint/pull/78)
 * build/include-what-you-use no longer supports transitive headers from the header for the current module for parity with the style guide by @aaronliu0130
 * build/include-what-you-use now supports a plethora of new functions, courtesy of @geoffviola (https://github.com/cpplint/cpplint/pull/94)
 * build/include-what-you-use will no longer err on similarly-named classes from other namespaces thanks to @geoffviola (https://github.com/cpplint/cpplint/pull/273)
 * Indented functions inside namespaces will now be correctly erred on, courtesy of @Yujinmon (https://github.com/cpplint/cpplint/pull/235)
+* `[[(un)likely]]` no longer clouds readability/braces's super spy−scanning of braces, courtesy of @aaronliu0130 (https://github.com/cpplint/cpplint/pull/265)
 * C++20 headers will no longer be flagged as C headers thanks to @miker2 (https://github.com/cpplint/cpplint/pull/216)
    * Same goes for C++23 and C23 headers, thanks to @aaronliu0130 (https://github.com/cpplint/cpplint/pull/239)
    * "complex.h" will be treated as the C99 header instead of the legacy C++ header by @tkruse (https://github.com/cpplint/cpplint/pull/219)
@@ -29,6 +30,7 @@ A bunch of long-overdue modernizations of the codebase!
 * You can now specify the name of the CPPLINT.cfg file through `--config` as long as it is in the same directory, thanks to @gedankenexperimenter (https://github.com/cpplint/cpplint/pull/198)
 * The new __VA_OPT__(,) will now be recognized by the Whitespace linter as a function thanks to @elrinor (https://github.com/cpplint/cpplint/pull/237)
 * The check for including a source file's header file will now scan all files with the same base name. Thanks to @crogre for figuring out what code needed to be changed and @aaronliu0130 for fixing it (https://github.com/cpplint/cpplint/pull/104)
+* Fixed false positive when an if/else statement has braces everywhere but one of the closing braces before the final block is on a separate line by @aaronliu0130 (https://github.com/cpplint/cpplint/pull/265)
 * Usages of the deprecated sre_compile were refectored by @jspricke (https://github.com/cpplint/cpplint/pull/214)
 * Usages of deprecated unittest aliases were refactored by @tirkarthi (https://github.com/cpplint/cpplint/pull/182), @aaronliu0130 and @jayvdb
 * Typos in this changelog, comments and functions were fixed by @jayvdb (https://github.com/cpplint/cpplint/pull/245), @aaronliu0130 and @tkruse

diff --git a/cpplint.py b/cpplint.py
@@ -4371,11 +4371,13 @@ def CheckBraces(filename, clean_lines, linenum, error):
             '{ should almost always be at the end of the previous line')
 
   # An else clause should be on the same line as the preceding closing brace.
-  if re.match(r'\s*else\b\s*(?:if\b|\{|$)', line):
+  if last_wrong := re.match(r'\s*else\b\s*(?:if\b|\{|$)', line):
     prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0]
     if re.match(r'\s*}\s*$', prevline):
       error(filename, linenum, 'whitespace/newline', 4,
             'An else should appear on the same line as the preceding }')
+    else:
+      last_wrong = False
 
   # If braces come on one side of an else, they should be on both.
   # However, we have to worry about "else if" that spans multiple lines!
@@ -4390,19 +4392,29 @@ def CheckBraces(filename, clean_lines, linenum, error):
       if brace_on_left != brace_on_right:    # must be brace after if
         error(filename, linenum, 'readability/braces', 5,
               'If an else has a brace on one side, it should have it on both')
-  elif re.search(r'}\s*else[^{]*$', line) or re.match(r'[^}]*else\s*{', line):
+  # Prevent detection if statement has { and we detected an improper newline after }
+  elif re.search(r'}\s*else[^{]*$', line) or (re.match(r'[^}]*else\s*{', line) and not last_wrong):
     error(filename, linenum, 'readability/braces', 5,
           'If an else has a brace on one side, it should have it on both')
 
-  # Likewise, an else should never have the else clause on the same line
-  if re.search(r'\belse [^\s{]', line) and not re.search(r'\belse if\b', line):
-    error(filename, linenum, 'whitespace/newline', 4,
-          'Else clause should never be on same line as else (use 2 lines)')
-
-  # In the same way, a do/while should never be on one line
-  if re.match(r'\s*do [^\s{]', line):
-    error(filename, linenum, 'whitespace/newline', 4,
-          'do/while clauses should not be on a single line')
+  # No control clauses with braces should have its contents on the same line
+  # Exclude } which will be covered by empty-block detect
+  # Exclude ; which may be used by while in a do-while
+  if keyword := re.search(
+      r'\b(else if|if|while|for|switch)'  # These have parens
+      r'\s*\(.*\)\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\};]', line):
+    error(filename, linenum, 'whitespace/newline', 5,
+          f'Controlled statements inside brackets of {keyword.group(1)} clause'
+          ' should be on a separate line')
+  elif keyword := re.search(
+      r'\b(else|do|try)'  # These don't have parens
+      r'\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\}]', line):
+    error(filename, linenum, 'whitespace/newline', 5,
+          f'Controlled statements inside brackets of {keyword.group(1)} clause'
+          ' should be on a separate line')
+
+  # TODO: Err on if...else and do...while statements without braces;
+  # style guide has changed since the below comment was written
 
   # Check single-line if/else bodies. The style guide says 'curly braces are not
   # required for single-line statements'. We additionally allow multi-line,
@@ -4422,7 +4434,7 @@ def CheckBraces(filename, clean_lines, linenum, error):
       (endline, endlinenum, endpos) = CloseExpression(clean_lines, linenum, pos)
     # Check for an opening brace, either directly after the if or on the next
     # line. If found, this isn't a single-statement conditional.
-    if (not re.match(r'\s*{', endline[endpos:])
+    if (not re.match(r'\s*(?:\[\[(?:un)?likely\]\]\s*)?{', endline[endpos:])
         and not (re.match(r'\s*$', endline[endpos:])
                  and endlinenum < (len(clean_lines.elided) - 1)
                  and re.match(r'\s*{', clean_lines.elided[endlinenum + 1]))):

diff --git a/cpplint_unittest.py b/cpplint_unittest.py
@@ -43,7 +43,7 @@
 import sys
 import tempfile
 import unittest
-
+from parameterized import parameterized
 import pytest
 
 import cpplint
@@ -229,6 +229,12 @@ def PerformIncludeWhatYouUse(self, code, filename='foo.h', io=codecs):
                                       error_collector, io)
     return error_collector.Results()
 
+  # Perform lint and make sure one of the errors is what we want
+  def TestLintContains(self, code, expected_message):
+    self.assertTrue(expected_message in self.PerformSingleLineLint(code))
+  def TestLintNotContains(self, code, expected_message):
+    self.assertFalse(expected_message in self.PerformSingleLineLint(code))
+
   # Perform lint and compare the error message with "expected_message".
   def TestLint(self, code, expected_message):
     self.assertEqual(expected_message, self.PerformSingleLineLint(code))
@@ -268,7 +274,6 @@ def doTestBlankLinesCheck(self, lines, start_errors, end_errors, extension):
             'Redundant blank line at the end of a code block '
             'should be deleted.  [whitespace/blank_line] [3]'))
 
-
 class CpplintTest(CpplintTestBase):
 
   def GetNamespaceResults(self, lines):
@@ -2217,6 +2222,14 @@ def testBraces(self):
                { { 1, 2 },
                  { 3, 4 } };""",
         '')
+    self.TestMultiLineLint(  # should not claim else should have braces on both sides
+      """if (foo) {
+        bar;
+      }
+      else {
+        baz;
+      }""",
+      'An else should appear on the same line as the preceding }  [whitespace/newline] [4]')
 
   # CHECK/EXPECT_TRUE/EXPECT_FALSE replacements
   def testCheckCheck(self):
@@ -2458,7 +2471,9 @@ def testNonConstReference(self):
                   operand_error_message % 'std::vector<int>& p')
     # Returning an address of something is not prohibited.
     self.TestLint('return &something;', '')
-    self.TestLint('if (condition) {return &something; }', '')
+    self.TestLint('if (condition) {return &something; }',
+                  'Controlled statements inside brackets of if clause should be on a separate line'
+                  '  [whitespace/newline] [5]')
     self.TestLint('if (condition) return &something;', '')
     self.TestLint('if (condition) address = &something;', '')
     self.TestLint('if (condition) result = lhs&rhs;', '')
@@ -2656,8 +2671,8 @@ def testSpacingForFncall(self):
     self.TestLint('for (foo;bar;baz) {', 'Missing space after ;'
                   '  [whitespace/semicolon] [3]')
     # we don't warn about this semicolon, at least for now
-    self.TestLint('if (condition) {return &something; }',
-                  '')
+    self.TestLintNotContains('if (condition) { return &something; }',
+                  'Missing space after ;  [whitespace/semicolon] [3]')
     # seen in some macros
     self.TestLint('DoSth();\\', '')
     # Test that there is no warning about semicolon here.
@@ -2765,7 +2780,7 @@ def testSpacingBeforeBraces(self):
                   '')
 
   def testSemiColonAfterBraces(self):
-    self.TestLint('if (cond) { func(); };',
+    self.TestLintContains('if (cond) { func(); };',
                   'You don\'t need a ; after a }  [readability/braces] [4]')
     self.TestLint('void Func() {};',
                   'You don\'t need a ; after a }  [readability/braces] [4]')
@@ -2854,7 +2869,9 @@ def testBraceInitializerList(self):
                            '  }\n'
                            '};\n', '')
     self.TestMultiLineLint('if (true) {\n'
-                           '  if (false){ func(); }\n'
+                           '  if (false){\n'
+                           '    func();\n'
+                           '  }'
                            '}\n',
                            'Missing space before {  [whitespace/braces] [5]')
     self.TestMultiLineLint('MyClass::MyClass()\n'
@@ -3004,8 +3021,12 @@ def testEmptyBlockBody(self):
                            '  [whitespace/empty_if_body] [4]')
     self.TestMultiLineLint("""if (test)
                              func();""", '')
-    self.TestLint('if (test) { hello; }', '')
-    self.TestLint('if (test({})) { hello; }', '')
+    self.TestLint('if (test) { hello; }',
+                  'Controlled statements inside brackets of if clause should be on a separate line'
+                  '  [whitespace/newline] [5]')
+    self.TestLint('if (test({})) { hello; }',
+                  'Controlled statements inside brackets of if clause should be on a separate line'
+                  '  [whitespace/newline] [5]')
     self.TestMultiLineLint("""if (test) {
                              func();
                            }""", '')
@@ -3707,16 +3728,6 @@ def testEndOfNamespaceComments(self):
         'Namespace should be terminated with "// namespace no_warning"'
         '  [readability/namespace] [5]'))
 
-  def testElseClauseNotOnSameLineAsElse(self):
-    self.TestLint('  else DoSomethingElse();',
-                  'Else clause should never be on same line as else '
-                  '(use 2 lines)  [whitespace/newline] [4]')
-    self.TestLint('  else ifDoSomethingElse();',
-                  'Else clause should never be on same line as else '
-                  '(use 2 lines)  [whitespace/newline] [4]')
-    self.TestLint('  } else if (blah) {', '')
-    self.TestLint('  variable_ends_in_else = true;', '')
-
   def testComma(self):
     self.TestLint('a = f(1,2);',
                   'Missing space after ,  [whitespace/comma] [3]')
@@ -4094,13 +4105,6 @@ def testConditionals(self):
         }""",
         '{ should almost always be at the end of the previous line'
         '  [whitespace/braces] [4]')
-    self.TestMultiLineLint(
-        """
-        if (foo) { \\
-          bar; \\
-          baz; \\
-        }""",
-        '')
     self.TestMultiLineLint(
         """
         void foo() { if (bar) baz; }""",
@@ -4141,6 +4145,33 @@ def testConditionals(self):
         #endif""",
         '')
 
+  @parameterized.expand(['else if', 'if', 'while', 'for', 'switch'])
+  def testControlClauseWithParensNewline(self, keyword):
+    # The % 2 part is pseudorandom whitespace-support testing
+    self.TestLintContains(
+      f'{keyword}{["", " "][len(keyword) % 2]}(condition)'
+        f'{[" ", ""][len(keyword) % 2]}[[unlikely]]'
+        f'{[" ", ""][len(keyword) % 2]}{{'
+        f'{["", " "][len(keyword) % 2]}do_something(); }}',
+      f'Controlled statements inside brackets of {keyword} clause'
+        f' should be on a separate line  [whitespace/newline] [5]'
+    )
+
+  @parameterized.expand(['else', 'do', 'try'])
+  def testControlClauseWithoutParensNewline(self, keyword):
+    # The % 2 part is pseudorandom whitespace-support testing
+    self.TestLintContains(
+      f'{keyword}{["", " "][len(keyword) % 2]}{{'
+        f'{[" ", ""][len(keyword) % 2]}do_something(); }}',
+      f'Controlled statements inside brackets of {keyword} clause'
+        f' should be on a separate line  [whitespace/newline] [5]'
+    )
+
+  def testControlClauseNewlineNameFalsePositives(self):
+    self.TestLint('  else if_condition_do_something();', '')
+    self.TestLint('  } else if (blah) {', '')
+    self.TestLint('  variable_ends_in_else = true;', '')
+
   def testTab(self):
     self.TestLint('\tint a;',
                   'Tab found; better to use spaces  [whitespace/tab] [1]')

diff --git a/samples/cfg-file/simple.def b/samples/cfg-file/simple.def
@@ -2,7 +2,7 @@ src/*.cpp
 1
 3
 Done processing src/sillycode.cpp
-Total errors found: 19
+Total errors found: 18
 
 src/sillycode.cpp:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
 src/sillycode.cpp:3:  Found C system header after C++ system header. Should be: sillycode.h, c system, c++ system, other.  [build/include_order] [4]
@@ -18,7 +18,6 @@ src/sillycode.cpp:123:  Is this a non-const reference? If so, make const or use
 src/sillycode.cpp:171:  Do not use variable-length arrays.  Use an appropriately named ('k' followed by CamelCase) compile-time constant for the size.  [runtime/arrays] [1]
 src/sillycode.cpp:178:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:199:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
-src/sillycode.cpp:202:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
 src/sillycode.cpp:208:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:227:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:228:  Using C-style cast.  Use reinterpret_cast<double*>(...) instead  [readability/casting] [4]

diff --git a/samples/silly-sample/filters.def b/samples/silly-sample/filters.def
@@ -4,9 +4,9 @@
 Done processing src/sillycode.cpp
 Category 'build' errors found: 1
 Category 'legal' errors found: 1
-Category 'readability' errors found: 5
+Category 'readability' errors found: 4
 Category 'runtime' errors found: 12
-Total errors found: 19
+Total errors found: 18
 
 src/sillycode.cpp:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
 src/sillycode.cpp:3:  Found C system header after C++ system header. Should be: sillycode.h, c system, c++ system, other.  [build/include_order] [4]
@@ -22,7 +22,6 @@ src/sillycode.cpp:123:  Is this a non-const reference? If so, make const or use
 src/sillycode.cpp:171:  Do not use variable-length arrays.  Use an appropriately named ('k' followed by CamelCase) compile-time constant for the size.  [runtime/arrays] [1]
 src/sillycode.cpp:178:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:199:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
-src/sillycode.cpp:202:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
 src/sillycode.cpp:208:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:227:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:228:  Using C-style cast.  Use reinterpret_cast<double*>(...) instead  [readability/casting] [4]

diff --git a/samples/silly-sample/includeorder_cfirst.def b/samples/silly-sample/includeorder_cfirst.def
@@ -2,7 +2,7 @@
 1
 3
 Done processing src/sillycode.cpp
-Total errors found: 111
+Total errors found: 110
 
 src/sillycode.cpp:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
 src/sillycode.cpp:8:  public: should be indented +1 space inside class Date  [whitespace/indent] [3]
@@ -79,7 +79,6 @@ src/sillycode.cpp:197:  Tab found; better to use spaces  [whitespace/tab] [1]
 src/sillycode.cpp:197:  At least two spaces is best between code and comments  [whitespace/comments] [2]
 src/sillycode.cpp:199:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
 src/sillycode.cpp:202:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
-src/sillycode.cpp:202:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
 src/sillycode.cpp:208:  Missing space before {  [whitespace/braces] [5]
 src/sillycode.cpp:208:  Static/global string variables are not permitted.  [runtime/string] [4]
 src/sillycode.cpp:209:  Tab found; better to use spaces  [whitespace/tab] [1]

diff --git a/samples/silly-sample/sed.def b/samples/silly-sample/sed.def
@@ -79,7 +79,6 @@ sed -i '208s/\([^ ]\){/\1 {/' src/sillycode.cpp # Missing space before {  [white
 # src/sillycode.cpp:197:  "At least two spaces is best between code and comments"  [whitespace/comments] [2]
 # src/sillycode.cpp:199:  "If an else has a brace on one side, it should have it on both"  [readability/braces] [5]
 # src/sillycode.cpp:202:  "An else should appear on the same line as the preceding }"  [whitespace/newline] [4]
-# src/sillycode.cpp:202:  "If an else has a brace on one side, it should have it on both"  [readability/braces] [5]
 # src/sillycode.cpp:208:  "Static/global string variables are not permitted."  [runtime/string] [4]
 # src/sillycode.cpp:209:  "Tab found; better to use spaces"  [whitespace/tab] [1]
 # src/sillycode.cpp:209:  "At least two spaces is best between code and comments"  [whitespace/comments] [2]