diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f740d1d..0fcd906 100644 --- 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 index 7182d32..4d04e8e 100644 --- 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 index d98c659..7723d00 100755 --- 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 index 142a495..894f911 100755 --- 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& 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 index 77f5bd6..85f618c 100644 --- 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] " [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(...) instead [readability/casting] [4] diff --git a/samples/silly-sample/filters.def b/samples/silly-sample/filters.def index 43029f1..be4cfab 100644 --- 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] " [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(...) instead [readability/casting] [4] diff --git a/samples/silly-sample/includeorder_cfirst.def b/samples/silly-sample/includeorder_cfirst.def index b7ea51d..b8483cd 100644 --- 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] " [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 index a9e7026..6e394e9 100644 --- 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] diff --git a/samples/silly-sample/simple.def b/samples/silly-sample/simple.def index adcd44e..f0233a3 100644 --- a/samples/silly-sample/simple.def +++ b/samples/silly-sample/simple.def @@ -2,7 +2,7 @@ src/*.cpp 1 3 Done processing src/sillycode.cpp -Total errors found: 112 +Total errors found: 111 src/sillycode.cpp:0: No copyright message found. You should have a line: "Copyright [year] " [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] @@ -80,7 +80,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/setup.py b/setup.py index a567f02..cf4dbe4 100755 --- a/setup.py +++ b/setup.py @@ -31,7 +31,6 @@ def read_without_comments(filename): 'License :: OSI Approved :: BSD License', 'Natural Language :: English', 'Programming Language :: Python :: 3 :: Only', - 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', 'Programming Language :: Python :: 3.10', diff --git a/test-requirements b/test-requirements index e048cd7..9c9abb5 100644 --- a/test-requirements +++ b/test-requirements @@ -5,3 +5,4 @@ pytest pytest-cov pytest-timeout testfixtures +parameterized