Skip to content

Commit 337b888

Browse files
norab0130aaronliu0130
norab0130
and
aaronliu0130
authored
Add support for [[likely]] and drop support for Python 3.8 (#277)
1. Fixes #212 2. else statements that have braces on both sides but have the second-to-last } on a separate line will no longer claim we need braces on both sides 3. Fix single-line if statements being yelled at to split their lines, something contradictory to the style guide 4. Fixes relevant tests, including adding the new "TestLintContains" and "TestLintNotContains" methods. (ik the latter may have a grammar error but who cares, this is consistent with the former's name) 5. Add a todo for "This exception does not apply to multi-keyword statements like if ... else or do ... while." --------- Co-authored-by: Aaron Liu <aaronliu0130@gmail.com>
1 parent 80e97a6 commit 337b888

File tree

11 files changed

+93
-53
lines changed

11 files changed

+93
-53
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ jobs:
1313
runs-on: ${{ matrix.os }}
1414
strategy:
1515
matrix:
16-
# Python 3.7 is the lowest available for actions/setup-python
17-
python-version: ['3.7', 'pypy3.10', '3.12']
16+
# Python 3.8 is the last non-EOL version
17+
python-version: ['3.8', 'pypy3.10', '3.12']
1818
os: [ubuntu-latest, windows-latest]
1919
fail-fast: false
2020

‎changelog.rst‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,18 @@ Changelog
77

88
A bunch of long-overdue modernizations of the codebase!
99

10-
* Python 2 is no longer supported. Python 3.7 and 3.12 support was added, courtesy of @jayvdb
10+
* 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
1111
* As a result of all this, setup.py's lint subcommand was removed. Please run the commands directly instead.
1212
* You can now specify blocks of code that exclude linting with NOLINTBEGIN and NOLINTEND, courtesy of @n3world (https://github.com/cpplint/cpplint/pull/213)
1313
* 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)
1414
* NOLINT and NOLINTNEXTLINE comments now support a comma-separated list of categories, courtesy of @n3world (https://github.com/cpplint/cpplint/pull/220)
1515
* NOLINT and NOLINTNEXTLINE will now ignore categories known to be from clang-tidy thanks to @xatier (https://github.com/cpplint/cpplint/pull/231)
16-
* Fix behavior with nested source repositories by @groegeorg (https://github.com/cpplint/cpplint/pull/78)
16+
* Fixed behavior with nested source repositories by @groegeorg (https://github.com/cpplint/cpplint/pull/78)
1717
* 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
1818
* build/include-what-you-use now supports a plethora of new functions, courtesy of @geoffviola (https://github.com/cpplint/cpplint/pull/94)
1919
* 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)
2020
* Indented functions inside namespaces will now be correctly erred on, courtesy of @Yujinmon (https://github.com/cpplint/cpplint/pull/235)
21+
* `[[(un)likely]]` no longer clouds readability/braces's super spy−scanning of braces, courtesy of @aaronliu0130 (https://github.com/cpplint/cpplint/pull/265)
2122
* C++20 headers will no longer be flagged as C headers thanks to @miker2 (https://github.com/cpplint/cpplint/pull/216)
2223
* Same goes for C++23 and C23 headers, thanks to @aaronliu0130 (https://github.com/cpplint/cpplint/pull/239)
2324
* "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!
2930
* 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)
3031
* 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)
3132
* 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)
33+
* 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)
3234
* Usages of the deprecated sre_compile were refectored by @jspricke (https://github.com/cpplint/cpplint/pull/214)
3335
* Usages of deprecated unittest aliases were refactored by @tirkarthi (https://github.com/cpplint/cpplint/pull/182), @aaronliu0130 and @jayvdb
3436
* Typos in this changelog, comments and functions were fixed by @jayvdb (https://github.com/cpplint/cpplint/pull/245), @aaronliu0130 and @tkruse

‎cpplint.py‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4371,11 +4371,13 @@ def CheckBraces(filename, clean_lines, linenum, error):
43714371
'{ should almost always be at the end of the previous line')
43724372

43734373
# An else clause should be on the same line as the preceding closing brace.
4374-
if re.match(r'\s*else\b\s*(?:if\b|\{|$)', line):
4374+
if last_wrong := re.match(r'\s*else\b\s*(?:if\b|\{|$)', line):
43754375
prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0]
43764376
if re.match(r'\s*}\s*$', prevline):
43774377
error(filename, linenum, 'whitespace/newline', 4,
43784378
'An else should appear on the same line as the preceding }')
4379+
else:
4380+
last_wrong = False
43794381

43804382
# If braces come on one side of an else, they should be on both.
43814383
# However, we have to worry about "else if" that spans multiple lines!
@@ -4390,19 +4392,29 @@ def CheckBraces(filename, clean_lines, linenum, error):
43904392
if brace_on_left != brace_on_right: # must be brace after if
43914393
error(filename, linenum, 'readability/braces', 5,
43924394
'If an else has a brace on one side, it should have it on both')
4393-
elif re.search(r'}\s*else[^{]*$', line) or re.match(r'[^}]*else\s*{', line):
4395+
# Prevent detection if statement has { and we detected an improper newline after }
4396+
elif re.search(r'}\s*else[^{]*$', line) or (re.match(r'[^}]*else\s*{', line) and not last_wrong):
43944397
error(filename, linenum, 'readability/braces', 5,
43954398
'If an else has a brace on one side, it should have it on both')
43964399

4397-
# Likewise, an else should never have the else clause on the same line
4398-
if re.search(r'\belse [^\s{]', line) and not re.search(r'\belse if\b', line):
4399-
error(filename, linenum, 'whitespace/newline', 4,
4400-
'Else clause should never be on same line as else (use 2 lines)')
4401-
4402-
# In the same way, a do/while should never be on one line
4403-
if re.match(r'\s*do [^\s{]', line):
4404-
error(filename, linenum, 'whitespace/newline', 4,
4405-
'do/while clauses should not be on a single line')
4400+
# No control clauses with braces should have its contents on the same line
4401+
# Exclude } which will be covered by empty-block detect
4402+
# Exclude ; which may be used by while in a do-while
4403+
if keyword := re.search(
4404+
r'\b(else if|if|while|for|switch)' # These have parens
4405+
r'\s*\(.*\)\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\};]', line):
4406+
error(filename, linenum, 'whitespace/newline', 5,
4407+
f'Controlled statements inside brackets of {keyword.group(1)} clause'
4408+
' should be on a separate line')
4409+
elif keyword := re.search(
4410+
r'\b(else|do|try)' # These don't have parens
4411+
r'\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\}]', line):
4412+
error(filename, linenum, 'whitespace/newline', 5,
4413+
f'Controlled statements inside brackets of {keyword.group(1)} clause'
4414+
' should be on a separate line')
4415+
4416+
# TODO: Err on if...else and do...while statements without braces;
4417+
# style guide has changed since the below comment was written
44064418

44074419
# Check single-line if/else bodies. The style guide says 'curly braces are not
44084420
# required for single-line statements'. We additionally allow multi-line,
@@ -4422,7 +4434,7 @@ def CheckBraces(filename, clean_lines, linenum, error):
44224434
(endline, endlinenum, endpos) = CloseExpression(clean_lines, linenum, pos)
44234435
# Check for an opening brace, either directly after the if or on the next
44244436
# line. If found, this isn't a single-statement conditional.
4425-
if (not re.match(r'\s*{', endline[endpos:])
4437+
if (not re.match(r'\s*(?:\[\[(?:un)?likely\]\]\s*)?{', endline[endpos:])
44264438
and not (re.match(r'\s*$', endline[endpos:])
44274439
and endlinenum < (len(clean_lines.elided) - 1)
44284440
and re.match(r'\s*{', clean_lines.elided[endlinenum + 1]))):

‎cpplint_unittest.py‎

Lines changed: 57 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
import sys
4444
import tempfile
4545
import unittest
46-
46+
from parameterized import parameterized
4747
import pytest
4848

4949
import cpplint
@@ -229,6 +229,12 @@ def PerformIncludeWhatYouUse(self, code, filename='foo.h', io=codecs):
229229
error_collector, io)
230230
return error_collector.Results()
231231

232+
# Perform lint and make sure one of the errors is what we want
233+
def TestLintContains(self, code, expected_message):
234+
self.assertTrue(expected_message in self.PerformSingleLineLint(code))
235+
def TestLintNotContains(self, code, expected_message):
236+
self.assertFalse(expected_message in self.PerformSingleLineLint(code))
237+
232238
# Perform lint and compare the error message with "expected_message".
233239
def TestLint(self, code, expected_message):
234240
self.assertEqual(expected_message, self.PerformSingleLineLint(code))
@@ -268,7 +274,6 @@ def doTestBlankLinesCheck(self, lines, start_errors, end_errors, extension):
268274
'Redundant blank line at the end of a code block '
269275
'should be deleted. [whitespace/blank_line] [3]'))
270276

271-
272277
class CpplintTest(CpplintTestBase):
273278

274279
def GetNamespaceResults(self, lines):
@@ -2217,6 +2222,14 @@ def testBraces(self):
22172222
{ { 1, 2 },
22182223
{ 3, 4 } };""",
22192224
'')
2225+
self.TestMultiLineLint( # should not claim else should have braces on both sides
2226+
"""if (foo) {
2227+
bar;
2228+
}
2229+
else {
2230+
baz;
2231+
}""",
2232+
'An else should appear on the same line as the preceding } [whitespace/newline] [4]')
22202233

22212234
# CHECK/EXPECT_TRUE/EXPECT_FALSE replacements
22222235
def testCheckCheck(self):
@@ -2458,7 +2471,9 @@ def testNonConstReference(self):
24582471
operand_error_message % 'std::vector<int>& p')
24592472
# Returning an address of something is not prohibited.
24602473
self.TestLint('return &something;', '')
2461-
self.TestLint('if (condition) {return &something; }', '')
2474+
self.TestLint('if (condition) {return &something; }',
2475+
'Controlled statements inside brackets of if clause should be on a separate line'
2476+
' [whitespace/newline] [5]')
24622477
self.TestLint('if (condition) return &something;', '')
24632478
self.TestLint('if (condition) address = &something;', '')
24642479
self.TestLint('if (condition) result = lhs&rhs;', '')
@@ -2656,8 +2671,8 @@ def testSpacingForFncall(self):
26562671
self.TestLint('for (foo;bar;baz) {', 'Missing space after ;'
26572672
' [whitespace/semicolon] [3]')
26582673
# we don't warn about this semicolon, at least for now
2659-
self.TestLint('if (condition) {return &something; }',
2660-
'')
2674+
self.TestLintNotContains('if (condition) { return &something; }',
2675+
'Missing space after ; [whitespace/semicolon] [3]')
26612676
# seen in some macros
26622677
self.TestLint('DoSth();\\', '')
26632678
# Test that there is no warning about semicolon here.
@@ -2765,7 +2780,7 @@ def testSpacingBeforeBraces(self):
27652780
'')
27662781

27672782
def testSemiColonAfterBraces(self):
2768-
self.TestLint('if (cond) { func(); };',
2783+
self.TestLintContains('if (cond) { func(); };',
27692784
'You don\'t need a ; after a } [readability/braces] [4]')
27702785
self.TestLint('void Func() {};',
27712786
'You don\'t need a ; after a } [readability/braces] [4]')
@@ -2854,7 +2869,9 @@ def testBraceInitializerList(self):
28542869
' }\n'
28552870
'};\n', '')
28562871
self.TestMultiLineLint('if (true) {\n'
2857-
' if (false){ func(); }\n'
2872+
' if (false){\n'
2873+
' func();\n'
2874+
' }'
28582875
'}\n',
28592876
'Missing space before { [whitespace/braces] [5]')
28602877
self.TestMultiLineLint('MyClass::MyClass()\n'
@@ -3004,8 +3021,12 @@ def testEmptyBlockBody(self):
30043021
' [whitespace/empty_if_body] [4]')
30053022
self.TestMultiLineLint("""if (test)
30063023
func();""", '')
3007-
self.TestLint('if (test) { hello; }', '')
3008-
self.TestLint('if (test({})) { hello; }', '')
3024+
self.TestLint('if (test) { hello; }',
3025+
'Controlled statements inside brackets of if clause should be on a separate line'
3026+
' [whitespace/newline] [5]')
3027+
self.TestLint('if (test({})) { hello; }',
3028+
'Controlled statements inside brackets of if clause should be on a separate line'
3029+
' [whitespace/newline] [5]')
30093030
self.TestMultiLineLint("""if (test) {
30103031
func();
30113032
}""", '')
@@ -3707,16 +3728,6 @@ def testEndOfNamespaceComments(self):
37073728
'Namespace should be terminated with "// namespace no_warning"'
37083729
' [readability/namespace] [5]'))
37093730

3710-
def testElseClauseNotOnSameLineAsElse(self):
3711-
self.TestLint(' else DoSomethingElse();',
3712-
'Else clause should never be on same line as else '
3713-
'(use 2 lines) [whitespace/newline] [4]')
3714-
self.TestLint(' else ifDoSomethingElse();',
3715-
'Else clause should never be on same line as else '
3716-
'(use 2 lines) [whitespace/newline] [4]')
3717-
self.TestLint(' } else if (blah) {', '')
3718-
self.TestLint(' variable_ends_in_else = true;', '')
3719-
37203731
def testComma(self):
37213732
self.TestLint('a = f(1,2);',
37223733
'Missing space after , [whitespace/comma] [3]')
@@ -4094,13 +4105,6 @@ def testConditionals(self):
40944105
}""",
40954106
'{ should almost always be at the end of the previous line'
40964107
' [whitespace/braces] [4]')
4097-
self.TestMultiLineLint(
4098-
"""
4099-
if (foo) { \\
4100-
bar; \\
4101-
baz; \\
4102-
}""",
4103-
'')
41044108
self.TestMultiLineLint(
41054109
"""
41064110
void foo() { if (bar) baz; }""",
@@ -4141,6 +4145,33 @@ def testConditionals(self):
41414145
#endif""",
41424146
'')
41434147

4148+
@parameterized.expand(['else if', 'if', 'while', 'for', 'switch'])
4149+
def testControlClauseWithParensNewline(self, keyword):
4150+
# The % 2 part is pseudorandom whitespace-support testing
4151+
self.TestLintContains(
4152+
f'{keyword}{["", " "][len(keyword) % 2]}(condition)'
4153+
f'{[" ", ""][len(keyword) % 2]}[[unlikely]]'
4154+
f'{[" ", ""][len(keyword) % 2]}{{'
4155+
f'{["", " "][len(keyword) % 2]}do_something(); }}',
4156+
f'Controlled statements inside brackets of {keyword} clause'
4157+
f' should be on a separate line [whitespace/newline] [5]'
4158+
)
4159+
4160+
@parameterized.expand(['else', 'do', 'try'])
4161+
def testControlClauseWithoutParensNewline(self, keyword):
4162+
# The % 2 part is pseudorandom whitespace-support testing
4163+
self.TestLintContains(
4164+
f'{keyword}{["", " "][len(keyword) % 2]}{{'
4165+
f'{[" ", ""][len(keyword) % 2]}do_something(); }}',
4166+
f'Controlled statements inside brackets of {keyword} clause'
4167+
f' should be on a separate line [whitespace/newline] [5]'
4168+
)
4169+
4170+
def testControlClauseNewlineNameFalsePositives(self):
4171+
self.TestLint(' else if_condition_do_something();', '')
4172+
self.TestLint(' } else if (blah) {', '')
4173+
self.TestLint(' variable_ends_in_else = true;', '')
4174+
41444175
def testTab(self):
41454176
self.TestLint('\tint a;',
41464177
'Tab found; better to use spaces [whitespace/tab] [1]')

‎samples/cfg-file/simple.def‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ src/*.cpp
22
1
33
3
44
Done processing src/sillycode.cpp
5-
Total errors found: 19
5+
Total errors found: 18
66

77
src/sillycode.cpp:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
88
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
1818
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]
1919
src/sillycode.cpp:178: Static/global string variables are not permitted. [runtime/string] [4]
2020
src/sillycode.cpp:199: If an else has a brace on one side, it should have it on both [readability/braces] [5]
21-
src/sillycode.cpp:202: If an else has a brace on one side, it should have it on both [readability/braces] [5]
2221
src/sillycode.cpp:208: Static/global string variables are not permitted. [runtime/string] [4]
2322
src/sillycode.cpp:227: Static/global string variables are not permitted. [runtime/string] [4]
2423
src/sillycode.cpp:228: Using C-style cast. Use reinterpret_cast<double*>(...) instead [readability/casting] [4]

‎samples/silly-sample/filters.def‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
Done processing src/sillycode.cpp
55
Category 'build' errors found: 1
66
Category 'legal' errors found: 1
7-
Category 'readability' errors found: 5
7+
Category 'readability' errors found: 4
88
Category 'runtime' errors found: 12
9-
Total errors found: 19
9+
Total errors found: 18
1010

1111
src/sillycode.cpp:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
1212
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
2222
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]
2323
src/sillycode.cpp:178: Static/global string variables are not permitted. [runtime/string] [4]
2424
src/sillycode.cpp:199: If an else has a brace on one side, it should have it on both [readability/braces] [5]
25-
src/sillycode.cpp:202: If an else has a brace on one side, it should have it on both [readability/braces] [5]
2625
src/sillycode.cpp:208: Static/global string variables are not permitted. [runtime/string] [4]
2726
src/sillycode.cpp:227: Static/global string variables are not permitted. [runtime/string] [4]
2827
src/sillycode.cpp:228: Using C-style cast. Use reinterpret_cast<double*>(...) instead [readability/casting] [4]

‎samples/silly-sample/includeorder_cfirst.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/sillycode.cpp
5-
Total errors found: 111
5+
Total errors found: 110
66

77
src/sillycode.cpp:0: No copyright message found. You should have a line: "Copyright [year] <Copyright Owner>" [legal/copyright] [5]
88
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]
7979
src/sillycode.cpp:197: At least two spaces is best between code and comments [whitespace/comments] [2]
8080
src/sillycode.cpp:199: If an else has a brace on one side, it should have it on both [readability/braces] [5]
8181
src/sillycode.cpp:202: An else should appear on the same line as the preceding } [whitespace/newline] [4]
82-
src/sillycode.cpp:202: If an else has a brace on one side, it should have it on both [readability/braces] [5]
8382
src/sillycode.cpp:208: Missing space before { [whitespace/braces] [5]
8483
src/sillycode.cpp:208: Static/global string variables are not permitted. [runtime/string] [4]
8584
src/sillycode.cpp:209: Tab found; better to use spaces [whitespace/tab] [1]

‎samples/silly-sample/sed.def‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ sed -i '208s/\([^ ]\){/\1 {/' src/sillycode.cpp # Missing space before { [white
7979
# src/sillycode.cpp:197: "At least two spaces is best between code and comments" [whitespace/comments] [2]
8080
# src/sillycode.cpp:199: "If an else has a brace on one side, it should have it on both" [readability/braces] [5]
8181
# src/sillycode.cpp:202: "An else should appear on the same line as the preceding }" [whitespace/newline] [4]
82-
# src/sillycode.cpp:202: "If an else has a brace on one side, it should have it on both" [readability/braces] [5]
8382
# src/sillycode.cpp:208: "Static/global string variables are not permitted." [runtime/string] [4]
8483
# src/sillycode.cpp:209: "Tab found; better to use spaces" [whitespace/tab] [1]
8584
# src/sillycode.cpp:209: "At least two spaces is best between code and comments" [whitespace/comments] [2]

0 commit comments

Comments
 (0)