From 622489d906c3f1078fcc5a49608324e79f3e9cce Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sat, 20 May 2023 20:40:47 -0400 Subject: [PATCH 1/2] tools: update cpplint to 1.6.1 --- tools/cpplint.py | 179 +++++++++++++++-------------------------------- 1 file changed, 56 insertions(+), 123 deletions(-) diff --git a/tools/cpplint.py b/tools/cpplint.py index 06fc45abf1a0eb..8158eb7cd5106f 100755 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -64,9 +64,10 @@ # if empty, use defaults _valid_extensions = set([]) -__VERSION__ = '1.6.0' +__VERSION__ = '1.6.1' try: + # -- pylint: disable=used-before-assignment xrange # Python 2 except NameError: # -- pylint: disable=redefined-builtin @@ -97,9 +98,10 @@ certain of the problem, and 1 meaning it could be a legitimate construct. This will miss some errors, and is not a substitute for a code review. - To suppress false-positive errors of a certain category, add a - 'NOLINT(category)' comment to the line. NOLINT or NOLINT(*) - suppresses errors of all categories on that line. + To suppress false-positive errors of certain categories, add a + 'NOLINT(category[, category...])' comment to the line. NOLINT or NOLINT(*) + suppresses errors of all categories on that line. To suppress categories + on the next line use NOLINTNEXTLINE instead of NOLINT. The files passed in will be linted; at least one file must be provided. Default linted extensions are %s. @@ -300,7 +302,6 @@ 'build/include', 'build/include_subdir', 'build/include_alpha', - 'build/include_inline', 'build/include_order', 'build/include_what_you_use', 'build/namespaces_headers', @@ -316,13 +317,11 @@ 'readability/constructors', 'readability/fn_size', 'readability/inheritance', - 'readability/pointer_notation', 'readability/multiline_comment', 'readability/multiline_string', 'readability/namespace', 'readability/nolint', 'readability/nul', - 'readability/null_usage', 'readability/strings', 'readability/todo', 'readability/utf8', @@ -342,7 +341,6 @@ 'runtime/string', 'runtime/threadsafe_fn', 'runtime/vlog', - 'runtime/v8_persistent', 'whitespace/blank_line', 'whitespace/braces', 'whitespace/comma', @@ -377,6 +375,12 @@ 'readability/function', ] +# These prefixes for categories should be ignored since they relate to other +# tools which also use the NOLINT syntax, e.g. clang-tidy. +_OTHER_NOLINT_CATEGORY_PREFIXES = [ + 'clang-analyzer', + ] + # The default state of the category filter. This is overridden by the --filter= # flag. By default all errors are on, so only add here categories that should be # off by default (i.e., categories that must be enabled by the --filter= flags). @@ -405,7 +409,7 @@ 'alloc.h', 'builtinbuf.h', 'bvector.h', - 'complex.h', + # 'complex.h', collides with System C header "complex.h" 'defalloc.h', 'deque.h', 'editbuf.h', @@ -517,6 +521,22 @@ 'optional', 'string_view', 'variant', + # 17.6.1.2 C++20 headers + 'barrier', + 'bit', + 'compare', + 'concepts', + 'coroutine', + 'format', + 'latch' + 'numbers', + 'ranges', + 'semaphore', + 'source_location', + 'span', + 'stop_token', + 'syncstream', + 'version', # 17.6.1.2 C++ headers for C library facilities 'cassert', 'ccomplex', @@ -851,14 +871,6 @@ 'Missing space after ,': r's/,\([^ ]\)/, \1/g', } -_NULL_TOKEN_PATTERN = re.compile(r'\bNULL\b') - -_V8_PERSISTENT_PATTERN = re.compile(r'\bv8::Persistent\b') - -_RIGHT_LEANING_POINTER_PATTERN = re.compile(r'[^=|(,\s><);&?:}]' - r'(? "suppress all" + categories = matched.group(2) + if categories in (None, '(*)'): # => "suppress all" _error_suppressions.setdefault(None, set()).add(suppressed_line) - else: - if category.startswith('(') and category.endswith(')'): - category = category[1:-1] + elif categories.startswith('(') and categories.endswith(')'): + for category in set(map(lambda c: c.strip(), categories[1:-1].split(','))): if category in _ERROR_CATEGORIES: _error_suppressions.setdefault(category, set()).add(suppressed_line) + elif any(c for c in _OTHER_NOLINT_CATEGORY_PREFIXES if category.startswith(c)): + # Ignore any categories from other tools. + pass elif category not in _LEGACY_ERROR_CATEGORIES: error(filename, linenum, 'readability/nolint', 5, 'Unknown NOLINT error category: %s' % category) @@ -1099,11 +1115,10 @@ class _IncludeState(object): # needs to move backwards, CheckNextIncludeOrder will raise an error. _INITIAL_SECTION = 0 _MY_H_SECTION = 1 - _OTHER_H_SECTION = 2 - _OTHER_SYS_SECTION = 3 - _C_SECTION = 4 - _CPP_SECTION = 5 - + _C_SECTION = 2 + _CPP_SECTION = 3 + _OTHER_SYS_SECTION = 4 + _OTHER_H_SECTION = 5 _TYPE_NAMES = { _C_SYS_HEADER: 'C system header', @@ -2540,21 +2555,6 @@ def CheckForBadCharacters(filename, lines, error): error(filename, linenum, 'readability/nul', 5, 'Line contains NUL byte.') -def CheckInlineHeader(filename, include_state, error): - """Logs an error if both a header and its inline variant are included.""" - - all_headers = dict(item for sublist in include_state.include_list - for item in sublist) - bad_headers = set('%s.h' % name[:-6] for name in all_headers.keys() - if name.endswith('-inl.h')) - bad_headers &= set(all_headers.keys()) - - for name in bad_headers: - err = '%s includes both %s and %s-inl.h' % (filename, name, name) - linenum = all_headers[name] - error(filename, linenum, 'build/include_inline', 5, err) - - def CheckForNewlineAtEOF(filename, lines, error): """Logs an error if there is no newline char at the end of the file. @@ -3578,7 +3578,7 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, """Reports for long function bodies. For an overview why this is done, see: - https://google.github.io/styleguide/cppguide.html#Write_Short_Functions + https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Write_Short_Functions Uses a simplistic algorithm assuming other style guidelines (especially spacing) are followed. @@ -4805,71 +4805,6 @@ def CheckAltTokens(filename, clean_lines, linenum, error): 'Use operator %s instead of %s' % ( _ALT_TOKEN_REPLACEMENT[match.group(1)], match.group(1))) -def CheckNullTokens(filename, clean_lines, linenum, error): - """Check NULL usage. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - error: The function to call with any errors found. - """ - line = clean_lines.elided[linenum] - - # Avoid preprocessor lines - if Match(r'^\s*#', line): - return - - if line.find('/*') >= 0 or line.find('*/') >= 0: - return - - for match in _NULL_TOKEN_PATTERN.finditer(line): - error(filename, linenum, 'readability/null_usage', 2, - 'Use nullptr instead of NULL') - -def CheckV8PersistentTokens(filename, clean_lines, linenum, error): - """Check v8::Persistent usage. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - error: The function to call with any errors found. - """ - line = clean_lines.elided[linenum] - - # Avoid preprocessor lines - if Match(r'^\s*#', line): - return - - if line.find('/*') >= 0 or line.find('*/') >= 0: - return - - for match in _V8_PERSISTENT_PATTERN.finditer(line): - error(filename, linenum, 'runtime/v8_persistent', 2, - 'Use v8::Global instead of v8::Persistent') - -def CheckLeftLeaningPointer(filename, clean_lines, linenum, error): - """Check for left-leaning pointer placement. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - error: The function to call with any errors found. - """ - line = clean_lines.elided[linenum] - - # Avoid preprocessor lines - if Match(r'^\s*#', line): - return - - if '/*' in line or '*/' in line: - return - - for match in _RIGHT_LEANING_POINTER_PATTERN.finditer(line): - error(filename, linenum, 'readability/pointer_notation', 2, - 'Use left leaning pointer instead of right leaning') def GetLineWidth(line): """Determines the width of the line in column positions. @@ -5024,9 +4959,6 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) - CheckNullTokens(filename, clean_lines, linenum, error) - CheckV8PersistentTokens(filename, clean_lines, linenum, error) - CheckLeftLeaningPointer(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() if classinfo: CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error) @@ -5108,7 +5040,8 @@ def _ClassifyInclude(fileinfo, include, used_angle_brackets, include_order="defa or Search(r'(?:%s)\/.*\.h' % "|".join(C_STANDARD_HEADER_FOLDERS), include)) # Headers with C++ extensions shouldn't be considered C system headers - is_system = used_angle_brackets and not os.path.splitext(include)[1] in ['.hpp', '.hxx', '.h++'] + include_ext = os.path.splitext(include)[1] + is_system = used_angle_brackets and not include_ext in ['.hh', '.hpp', '.hxx', '.h++'] if is_system: if is_cpp_header: @@ -5183,7 +5116,7 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): match = _RE_PATTERN_INCLUDE.search(line) if match: include = match.group(2) - used_angle_brackets = (match.group(1) == '<') + used_angle_brackets = match.group(1) == '<' duplicate_line = include_state.FindHeader(include) if duplicate_line >= 0: error(filename, linenum, 'build/include', 4, @@ -5214,10 +5147,11 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): include_state.include_list[-1].append((include, linenum)) # We want to ensure that headers appear in the right order: - # 1) for foo.cc, foo.h - # 2) other project headers - # 3) c system files - # 4) cpp system files + # 1) for foo.cc, foo.h (preferred location) + # 2) c system files + # 3) cpp system files + # 4) for foo.cc, foo.h (deprecated location) + # 5) other google headers # # We classify each include statement as one of those 5 types # using a number of techniques. The include_state object keeps @@ -5480,7 +5414,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, and line[-1] != '\\'): error(filename, linenum, 'build/namespaces_headers', 4, 'Do not use unnamed namespaces in header files. See ' - 'https://google.github.io/styleguide/cppguide.html#Namespaces' + 'https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces' ' for more information.') @@ -5947,7 +5881,8 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): return False # operator++(int) and operator--(int) - if context.endswith(' operator++') or context.endswith(' operator--'): + if (context.endswith(' operator++') or context.endswith(' operator--') or + context.endswith('::operator++') or context.endswith('::operator--')): return False # A single unnamed argument for a function tends to look like old style cast. @@ -6602,8 +6537,6 @@ def ProcessFileData(filename, file_extension, lines, error, CheckForNewlineAtEOF(filename, lines, error) - CheckInlineHeader(filename, include_state, error) - def ProcessConfigOverrides(filename): """ Loads the configuration files and processes the config overrides. @@ -6622,13 +6555,13 @@ def ProcessConfigOverrides(filename): if not base_name: break # Reached the root directory. - cfg_file = os.path.join(abs_path, ".cpplint") + cfg_file = os.path.join(abs_path, "CPPLINT.cfg") abs_filename = abs_path if not os.path.isfile(cfg_file): continue try: - with open(cfg_file, encoding='utf-8') as file_handle: + with codecs.open(cfg_file, 'r', 'utf8', 'replace') as file_handle: for line in file_handle: line, _, _ = line.partition('#') # Remove comments. if not line.strip(): From 55c0f575ec836da91e82b425f6b00bfb30e3868c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 9 Oct 2020 06:00:00 -0700 Subject: [PATCH 2/2] tools: refloat 7 Node.js patches to cpplint.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick 12c8b4d15471cb6211b39c3a2ca5b10fa4b9f12b Original commit message: This commit is a suggestion for adding a rule for NULL usages in the code base. This will currently report a number of errors which could be ignored using // NOLINT (readability/null_usage) PR-URL: https://github.com/nodejs/node/pull/17373 Reviewed-By: Jon Moss Reviewed-By: Anna Henningsen Reviewed-By: Timothy Gu Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Tobias Nießen Refs: https://github.com/nodejs/node/commit/12c8b4d15471cb6211b39c3a2ca5b10fa4b9f12b Cherry-pick fc81e801913de3e3f3c0c8e26c105f983a74e539 Original commit message: Update cpplint.py to check for inline headers when the corresponding header is already included. PR-URL: https://github.com/nodejs/node/pull/21521 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Refs: https://github.com/nodejs/node/commit/fc81e801913de3e3f3c0c8e26c105f983a74e539 Cherry-pick cbc3dd997eb90d629d1b9912b7a5a40eb82343df Original commit message: src, tools: add check for left leaning pointers This commit adds a rule to cpplint to check that pointers in the code base lean to the left and not right, and also fixes the violations reported. PR-URL: https://github.com/nodejs/node/pull/21010 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Refs: https://github.com/nodejs/node/commit/cbc3dd997eb90d629d1b9912b7a5a40eb82343df Cherry-pick 902998190a55d6915b881936f6dd5b6e9cca6ad8 Original commit message: tools: fix cpplint.py header rules THIS COMMIT SHOULD GO WITH THE NEXT. IT WILL FIND NEW LINT. PR-URL: https://github.com/nodejs/node/pull/26306 Reviewed-By: Gireesh Punathil Refs: https://github.com/nodejs/node/commit/902998190a55d6915b881936f6dd5b6e9cca6ad8 Cherry-pick 0a25ace9c35b62ece4d32fd90b326d8063265109 Original commit message: tools: move cpplint configuration to .cpplint PR-URL: https://github.com/nodejs/node/pull/27098 Reviewed-By: Joyee Cheung Reviewed-By: Daniel Bevenius Refs: https://github.com/nodejs/node/commit/0a25ace9c35b62ece4d32fd90b326d8063265109 Cherry-pick afa9a7206c26a29a2af226696c145c924a6d3754 Original commit message: tools: refloat update link to google styleguide for cpplint This commit updates two old links to Google's C++ styleguide which currently result in a 404 when accessed. PR-URL: https://github.com/nodejs/node/pull/30876 Reviewed-By: Michaël Zasso Reviewed-By: David Carlier Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau Reviewed-By: Rich Trott Refs: https://github.com/nodejs/node/commit/afa9a7206c26a29a2af226696c145c924a6d3754 Cherry-pick e23bf8f771aa0bd60e25ff079985fc29b5846403 Original commit message: tools,src: refloat forbid usage of v8::Persistent `v8::Persistent` comes with the surprising catch that it requires manual cleanup. `v8::Global` doesn’t, making it easier to use, and additionally provides move semantics. New code should always use `v8::Global`. PR-URL: https://github.com/nodejs/node/pull/31018 Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau Reviewed-By: James M Snell Reviewed-By: David Carlier Reviewed-By: Rich Trott Reviewed-By: Gus Caplan Reviewed-By: Joyee Cheung Reviewed-By: Ben Noordhuis Reviewed-By: Stephen Belanger PR-URL: https://github.com/nodejs/node/pull/35569 Reviewed-By: Richard Lau Reviewed-By: Daijiro Wachi Reviewed-By: Jiawen Geng PR-URL: https://github.com/nodejs/node/pull/35719 Reviewed-By: Antoine du Hamel PR-URL: https://github.com/nodejs/node/pull/35866 PR-URL: https://github.com/nodejs/node/pull/36213 Reviewed-By: Franziska Hinkelmann PR-URL: https://github.com/nodejs/node/pull/36235 Reviewed-By: Luigi Pinca PR-URL: https://github.com/nodejs/node/pull/36324 Reviewed-By: Beth Griggs PR-URL: https://github.com/nodejs/node/pull/38851 Reviewed-By: Khaidi Chu PR-URL: https://github.com/nodejs/node/pull/42416 Reviewed-By: Mestery Reviewed-By: Darshan Sen --- tools/cpplint.py | 121 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 109 insertions(+), 12 deletions(-) diff --git a/tools/cpplint.py b/tools/cpplint.py index 8158eb7cd5106f..fd0940ef2ef5cf 100755 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -302,6 +302,7 @@ 'build/include', 'build/include_subdir', 'build/include_alpha', + 'build/include_inline', 'build/include_order', 'build/include_what_you_use', 'build/namespaces_headers', @@ -317,11 +318,13 @@ 'readability/constructors', 'readability/fn_size', 'readability/inheritance', + 'readability/pointer_notation', 'readability/multiline_comment', 'readability/multiline_string', 'readability/namespace', 'readability/nolint', 'readability/nul', + 'readability/null_usage', 'readability/strings', 'readability/todo', 'readability/utf8', @@ -341,6 +344,7 @@ 'runtime/string', 'runtime/threadsafe_fn', 'runtime/vlog', + 'runtime/v8_persistent', 'whitespace/blank_line', 'whitespace/braces', 'whitespace/comma', @@ -871,6 +875,14 @@ 'Missing space after ,': r's/,\([^ ]\)/, \1/g', } +_NULL_TOKEN_PATTERN = re.compile(r'\bNULL\b') + +_V8_PERSISTENT_PATTERN = re.compile(r'\bv8::Persistent\b') + +_RIGHT_LEANING_POINTER_PATTERN = re.compile(r'[^=|(,\s><);&?:}]' + r'(?= 0 or line.find('*/') >= 0: + return + + for match in _NULL_TOKEN_PATTERN.finditer(line): + error(filename, linenum, 'readability/null_usage', 2, + 'Use nullptr instead of NULL') + +def CheckV8PersistentTokens(filename, clean_lines, linenum, error): + """Check v8::Persistent usage. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Avoid preprocessor lines + if Match(r'^\s*#', line): + return + + if line.find('/*') >= 0 or line.find('*/') >= 0: + return + + for match in _V8_PERSISTENT_PATTERN.finditer(line): + error(filename, linenum, 'runtime/v8_persistent', 2, + 'Use v8::Global instead of v8::Persistent') + +def CheckLeftLeaningPointer(filename, clean_lines, linenum, error): + """Check for left-leaning pointer placement. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Avoid preprocessor lines + if Match(r'^\s*#', line): + return + + if '/*' in line or '*/' in line: + return + + for match in _RIGHT_LEANING_POINTER_PATTERN.finditer(line): + error(filename, linenum, 'readability/pointer_notation', 2, + 'Use left leaning pointer instead of right leaning') def GetLineWidth(line): """Determines the width of the line in column positions. @@ -4959,6 +5052,9 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) + CheckNullTokens(filename, clean_lines, linenum, error) + CheckV8PersistentTokens(filename, clean_lines, linenum, error) + CheckLeftLeaningPointer(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() if classinfo: CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error) @@ -5147,11 +5243,10 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): include_state.include_list[-1].append((include, linenum)) # We want to ensure that headers appear in the right order: - # 1) for foo.cc, foo.h (preferred location) - # 2) c system files - # 3) cpp system files - # 4) for foo.cc, foo.h (deprecated location) - # 5) other google headers + # 1) for foo.cc, foo.h + # 2) other project headers + # 3) c system files + # 4) cpp system files # # We classify each include statement as one of those 5 types # using a number of techniques. The include_state object keeps @@ -5414,7 +5509,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, and line[-1] != '\\'): error(filename, linenum, 'build/namespaces_headers', 4, 'Do not use unnamed namespaces in header files. See ' - 'https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces' + 'https://google.github.io/styleguide/cppguide.html#Namespaces' ' for more information.') @@ -6537,6 +6632,8 @@ def ProcessFileData(filename, file_extension, lines, error, CheckForNewlineAtEOF(filename, lines, error) + CheckInlineHeader(filename, include_state, error) + def ProcessConfigOverrides(filename): """ Loads the configuration files and processes the config overrides. @@ -6555,7 +6652,7 @@ def ProcessConfigOverrides(filename): if not base_name: break # Reached the root directory. - cfg_file = os.path.join(abs_path, "CPPLINT.cfg") + cfg_file = os.path.join(abs_path, ".cpplint") abs_filename = abs_path if not os.path.isfile(cfg_file): continue