From 0120560f137703165280ecc6e026a52c74dc0a9c Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Thu, 28 Jul 2016 22:50:46 +0200 Subject: [PATCH 1/6] update to pypi cpplint 1.3.0, fix linter errors --- CppCoreGuidelines.md | 9 +- scripts/python/Makefile.in | 2 +- scripts/python/cpplint.py | 1435 +++++++++++++++++++----------------- 3 files changed, 745 insertions(+), 701 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 1c92c21..8fc4bf8 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -12175,7 +12175,7 @@ Not all member functions can be called. // if elem!=nullptr then elem points to sz doubles public: vector() : elem{nullptr}, sz{0}{} - vctor(int s) : elem{new double},sz{s} { /* initialize elements */ } + vector(int s) : elem{new double}, sz{s} { /* initialize elements */ } ~vector() { delete elem; } double& operator[](int s) { return elem[s]; } @@ -12874,9 +12874,9 @@ A not uncommon technique is to gather cleanup at the end of the function to avoi // ... exit: - if (g1.valid()) cleanup(g1); - if (g1.valid()) cleanup(g2); - return {res, err}; + if (g1.valid()) cleanup(g1); + if (g1.valid()) cleanup(g2); + return {res, err}; } The larger the function, the more tempting this technique becomes. @@ -16511,6 +16511,7 @@ Also, `std::array<>::fill()` or `std::fill()` or even an empty initializer are b fill(b, 0); // std::fill() + Ranges TS if ( a == b ) { + // ... } } diff --git a/scripts/python/Makefile.in b/scripts/python/Makefile.in index da38464..46e809f 100644 --- a/scripts/python/Makefile.in +++ b/scripts/python/Makefile.in @@ -14,5 +14,5 @@ CXX_LINT := ${CXX_SRCS:.cpp=.lint} cpplint-all: $(CXX_LINT) %.lint: %.cpp - @python ../../python/cpplint.py --verbose=0 --linelength=100 --filter=-legal/copyright,-build/include_order,-build/c++11,-build/namespaces,-readability/inheritance,-readability/function,-readability/casting,-readability/namespace,-readability/alt_tokens,-readability/braces,-readability/fn_size,-whitespace/comments,-whitespace/braces,-whitespace/empty_loop_body,-whitespace/indent,-whitespace/newline,-runtime/explicit,-runtime/arrays,-runtime/int,-runtime/references,-runtime/string,-runtime/operator $< || (cat $< | nl -ba | grep -v 'md-split' && false) + @python ../../python/cpplint.py --verbose=0 --linelength=100 --filter=-legal/copyright,-build/include_order,-build/c++11,-build/namespaces,-build/class,-build/include,-build/include_subdir,-readability/inheritance,-readability/function,-readability/casting,-readability/namespace,-readability/alt_tokens,-readability/braces,-readability/fn_size,-whitespace/comments,-whitespace/braces,-whitespace/empty_loop_body,-whitespace/indent,-whitespace/newline,-runtime/explicit,-runtime/arrays,-runtime/int,-runtime/references,-runtime/string,-runtime/operator $< || (cat $< | nl -ba | grep -v 'md-split' && false) diff --git a/scripts/python/cpplint.py b/scripts/python/cpplint.py index 7fae2f0..95c0c32 100755 --- a/scripts/python/cpplint.py +++ b/scripts/python/cpplint.py @@ -44,6 +44,7 @@ same line, but it is far from perfect (in either direction). import codecs import copy import getopt +import glob import itertools import math # for log import os @@ -52,6 +53,7 @@ import sre_compile import string import sys import unicodedata +import xml.etree.ElementTree # if empty, use defaults _header_extensions = set([]) @@ -80,19 +82,18 @@ def GetNonHeaderExtensions(): return GetAllExtensions().difference(GetHeaderExtensions()) -# files with this suffix before the extension will be treated as test files -_test_suffixes = set(['_unittest', '_test', '_regtest']) - _USAGE = """ -Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] - [--counting=total|toplevel|detailed] [--root=subdir] - [--linelength=digits] +Syntax: cpplint.py [--verbose=#] [--output=emacs|eclipse|vs7|junit] + [--filter=-x,+y,...] + [--counting=total|toplevel|detailed] [--repository=path] + [--root=subdir] [--linelength=digits] [--recursive] + [--exclude=path] [--headers=ext1,ext2] - [--extensions=ext1,ext2] + [--extensions=hpp,cpp,...] [file] ... The style guidelines this tries to follow are those in - https://github.com/google/styleguide + https://google.github.io/styleguide/cppguide.html Every problem is given a confidence score from 1-5, with 5 meaning we are certain of the problem, and 1 meaning it could be a legitimate construct. @@ -109,12 +110,20 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] Flags: - output=vs7 - By default, the output is formatted to ease emacs parsing. Visual Studio - compatible output (vs7) may also be used. Other formats are unsupported. + output=emacs|eclipse|vs7|junit + By default, the output is formatted to ease emacs parsing. Output + compatible with eclipse (eclipse), Visual Studio (vs7), and JUnit + XML parsers such as those used in Jenkins and Bamboo may also be + used. Other formats are unsupported. verbose=# Specify a number 0-5 to restrict errors to certain verbosity levels. + Errors with lower verbosity levels have lower confidence and are more + likely to be false positives. + + quiet + Supress output other than linting errors, such as information about + which files have been processed and excluded. filter=-x,+y,... Specify a comma-separated list of category-filters to apply: only @@ -138,17 +147,40 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] also be printed. If 'detailed' is provided, then a count is provided for each category like 'build/class'. - root=subdir - The root directory used for deriving header guard CPP variable. - By default, the header guard CPP variable is calculated as the relative - path to the directory that contains .git, .hg, or .svn. When this flag - is specified, the relative path is calculated from the specified - directory. If the specified directory does not exist, this flag is - ignored. + repository=path + The top level directory of the repository, used to derive the header + guard CPP variable. By default, this is determined by searching for a + path that contains .git, .hg, or .svn. When this flag is specified, the + given path is used instead. This option allows the header guard CPP + variable to remain consistent even if members of a team have different + repository root directories (such as when checking out a subdirectory + with SVN). In addition, users of non-mainstream version control systems + can use this flag to ensure readable header guard CPP variables. Examples: - Assuming that src/.git exists, the header guard CPP variables for - src/chrome/browser/ui/browser.h are: + Assuming that Alice checks out ProjectName and Bob checks out + ProjectName/trunk and trunk contains src/chrome/ui/browser.h, then + with no --repository flag, the header guard CPP variable will be: + + Alice => TRUNK_SRC_CHROME_BROWSER_UI_BROWSER_H_ + Bob => SRC_CHROME_BROWSER_UI_BROWSER_H_ + + If Alice uses the --repository=trunk flag and Bob omits the flag or + uses --repository=. then the header guard CPP variable will be: + + Alice => SRC_CHROME_BROWSER_UI_BROWSER_H_ + Bob => SRC_CHROME_BROWSER_UI_BROWSER_H_ + + root=subdir + The root directory used for deriving header guard CPP variables. This + directory is relative to the top level directory of the repository which + by default is determined by searching for a directory that contains .git, + .hg, or .svn but can also be controlled with the --repository flag. If + the specified directory does not exist, this flag is ignored. + + Examples: + Assuming that src is the top level directory of the repository, the + header guard CPP variables for src/chrome/browser/ui/browser.h are: No flag => CHROME_BROWSER_UI_BROWSER_H_ --root=chrome => BROWSER_UI_BROWSER_H_ @@ -161,6 +193,23 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] Examples: --linelength=120 + recursive + Search for files to lint recursively. Each directory given in the list + of files to be linted is replaced by all files that descend from that + directory. Files with extensions not in the valid extensions list are + excluded. + + exclude=path + Exclude the given path from the list of files to be linted. Relative + paths are evaluated relative to the current directory and shell globbing + is performed. This flag can be provided multiple times to exclude + multiple files. + + Examples: + --exclude=one.cc + --exclude=src/*.cc + --exclude=src/*.cc --exclude=test/*.cc + extensions=extension,extension,... The allowed file extensions that cpplint will check @@ -183,6 +232,7 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] filter=+filter1,-filter2,... exclude_files=regex linelength=80 + root=subdir "set noparent" option prevents cpplint from traversing directory tree upwards looking for more .cfg files in parent directories. This option @@ -194,22 +244,28 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] "exclude_files" allows to specify a regular expression to be matched against a file name. If the expression matches, the file is skipped and not run - through liner. + through the linter. - "linelength" allows to specify the allowed line length for the project. + "linelength" specifies the allowed line length for the project. + + The "root" option is similar in function to the --root flag (see example + above). CPPLINT.cfg has an effect on files in the same directory and all - sub-directories, unless overridden by a nested configuration file. + subdirectories, unless overridden by a nested configuration file. Example file: filter=-build/include_order,+build/include_alpha - exclude_files=.*\.cc + exclude_files=.*\\.cc The above example disables build/include_order warning and enables build/include_alpha as well as excludes all .cc from being processed by linter, in the current directory (where the .cfg - file is located) and all sub-directories. -""" % (list(GetAllExtensions()), ','.join(list(GetAllExtensions())), GetHeaderExtensions(), ','.join(GetHeaderExtensions())) + file is located) and all subdirectories. +""" % (list(GetAllExtensions()), + ','.join(list(GetAllExtensions())), + GetHeaderExtensions(), + ','.join(GetHeaderExtensions())) # We categorize each error message we print. Here are the categories. # We want an explicit list so we can list them all in cpplint --filter=. @@ -218,15 +274,19 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] _ERROR_CATEGORIES = [ 'build/class', 'build/c++11', + 'build/c++14', + 'build/c++tr1', 'build/deprecated', 'build/endif_comment', 'build/explicit_make_pair', 'build/forward_decl', 'build/header_guard', 'build/include', + 'build/include_subdir', 'build/include_alpha', 'build/include_order', 'build/include_what_you_use', + 'build/namespaces_literals', 'build/namespaces', 'build/printf_format', 'build/storage_class', @@ -237,7 +297,6 @@ _ERROR_CATEGORIES = [ 'readability/check', 'readability/constructors', 'readability/fn_size', - 'readability/function', 'readability/inheritance', 'readability/multiline_comment', 'readability/multiline_string', @@ -268,6 +327,7 @@ _ERROR_CATEGORIES = [ 'whitespace/comma', 'whitespace/comments', 'whitespace/empty_conditional_body', + 'whitespace/empty_if_body', 'whitespace/empty_loop_body', 'whitespace/end_of_line', 'whitespace/ending_newline', @@ -286,6 +346,7 @@ _ERROR_CATEGORIES = [ # compatibility they may still appear in NOLINT comments. _LEGACY_ERROR_CATEGORIES = [ 'readability/streams', + 'readability/function', ] # The default state of the category filter. This is overridden by the --filter= @@ -294,6 +355,16 @@ _LEGACY_ERROR_CATEGORIES = [ # All entries here should start with a '-' or '+', as in the --filter= flag. _DEFAULT_FILTERS = ['-build/include_alpha'] +# The default list of categories suppressed for C (not C++) files. +_DEFAULT_C_SUPPRESSED_CATEGORIES = [ + 'readability/casting', + ] + +# The default list of categories suppressed for Linux Kernel files. +_DEFAULT_KERNEL_SUPPRESSED_CATEGORIES = [ + 'whitespace/tab', + ] + # We used to check for high-bit characters, but after much discussion we # decided those were OK, as long as they were in UTF-8 and didn't represent # hard-coded international strings, which belong in a separate i18n file. @@ -387,6 +458,7 @@ _CPP_HEADERS = frozenset([ 'random', 'ratio', 'regex', + 'scoped_allocator', 'set', 'sstream', 'stack', @@ -434,6 +506,19 @@ _CPP_HEADERS = frozenset([ 'cwctype', ]) +# Type names +_TYPES = re.compile( + r'^(?:' + # [dcl.type.simple] + r'(char(16_t|32_t)?)|wchar_t|' + r'bool|short|int|long|signed|unsigned|float|double|' + # [support.types] + r'(ptrdiff_t|size_t|max_align_t|nullptr_t)|' + # [cstdint.syn] + r'(u?int(_fast|_least)?(8|16|32|64)_t)|' + r'(u?int(max|ptr)_t)|' + r')$') + # These headers are excluded from [build/include] and [build/include_order] # checks: @@ -443,20 +528,23 @@ _CPP_HEADERS = frozenset([ _THIRD_PARTY_HEADERS_PATTERN = re.compile( r'^(?:[^/]*[A-Z][^/]*\.h|lua\.h|lauxlib\.h|lualib\.h)$') +# Pattern for matching FileInfo.BaseName() against test file name +_test_suffixes = ['_test', '_regtest', '_unittest'] +_TEST_FILE_SUFFIX = '(' + '|'.join(_test_suffixes) + r')$' + +# Pattern that matches only complete whitespace, possibly across multiple lines. +_EMPTY_CONDITIONAL_BODY_PATTERN = re.compile(r'^\s*$', re.DOTALL) # Assertion macros. These are defined in base/logging.h and -# testing/base/gunit.h. Note that the _M versions need to come first -# for substring matching to work. +# testing/base/public/gunit.h. _CHECK_MACROS = [ 'DCHECK', 'CHECK', - 'EXPECT_TRUE_M', 'EXPECT_TRUE', - 'ASSERT_TRUE_M', 'ASSERT_TRUE', - 'EXPECT_FALSE_M', 'EXPECT_FALSE', - 'ASSERT_FALSE_M', 'ASSERT_FALSE', + 'EXPECT_TRUE', 'ASSERT_TRUE', + 'EXPECT_FALSE', 'ASSERT_FALSE', ] # Replacement macros for CHECK/DCHECK/EXPECT_TRUE/EXPECT_FALSE -_CHECK_REPLACEMENT = dict([(m, {}) for m in _CHECK_MACROS]) +_CHECK_REPLACEMENT = dict([(macro_var, {}) for macro_var in _CHECK_MACROS]) for op, replacement in [('==', 'EQ'), ('!=', 'NE'), ('>=', 'GE'), ('>', 'GT'), @@ -465,16 +553,12 @@ for op, replacement in [('==', 'EQ'), ('!=', 'NE'), _CHECK_REPLACEMENT['CHECK'][op] = 'CHECK_%s' % replacement _CHECK_REPLACEMENT['EXPECT_TRUE'][op] = 'EXPECT_%s' % replacement _CHECK_REPLACEMENT['ASSERT_TRUE'][op] = 'ASSERT_%s' % replacement - _CHECK_REPLACEMENT['EXPECT_TRUE_M'][op] = 'EXPECT_%s_M' % replacement - _CHECK_REPLACEMENT['ASSERT_TRUE_M'][op] = 'ASSERT_%s_M' % replacement for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'), ('>=', 'LT'), ('>', 'LE'), ('<=', 'GT'), ('<', 'GE')]: _CHECK_REPLACEMENT['EXPECT_FALSE'][op] = 'EXPECT_%s' % inv_replacement _CHECK_REPLACEMENT['ASSERT_FALSE'][op] = 'ASSERT_%s' % inv_replacement - _CHECK_REPLACEMENT['EXPECT_FALSE_M'][op] = 'EXPECT_%s_M' % inv_replacement - _CHECK_REPLACEMENT['ASSERT_FALSE_M'][op] = 'ASSERT_%s_M' % inv_replacement # Alternative tokens and their replacements. For full list, see section 2.5 # Alternative tokens [lex.digraph] in the C++ standard. @@ -523,6 +607,12 @@ _MATCH_ASM = re.compile(r'^\s*(?:asm|_asm|__asm|__asm__)' r'(?:\s+(volatile|__volatile__))?' r'\s*[{(]') +# Match strings that indicate we're working on a C (not C++) file. +_SEARCH_C_FILE = re.compile(r'\b(?:LINT_C_FILE|' + r'vim?:\s*.*(\s*|:)filetype=c(\s*|:|$))') + +# Match string that indicates we're working on a Linux Kernel file. +_SEARCH_KERNEL_FILE = re.compile(r'\b(?:LINT_KERNEL_FILE)') _regexp_compile_cache = {} @@ -534,36 +624,64 @@ _error_suppressions = {} # This is set by --root flag. _root = None +# The top level repository directory. If set, _root is calculated relative to +# this directory instead of the directory containing version control artifacts. +# This is set by the --repository flag. +_repository = None + +# Files to exclude from linting. This is set by the --exclude flag. +_excludes = None + +# Whether to supress PrintInfo messages +_quiet = False + # The allowed line length of files. # This is set by --linelength flag. _line_length = 80 try: - xrange + xrange(1, 0) except NameError: - xrange = range + # -- pylint: disable=redefined-builtin + xrange = range try: unicode except NameError: + # -- pylint: disable=redefined-builtin basestring = unicode = str +try: + long(2) +except NameError: + # -- pylint: disable=redefined-builtin + long = int + if sys.version_info < (3,): - def u(x): - return codecs.unicode_escape_decode(x)[0] - # BINARY_TYPE = str - itervalues = dict.itervalues - iteritems = dict.iteritems + # -- pylint: disable=no-member + # BINARY_TYPE = str + itervalues = dict.itervalues + iteritems = dict.iteritems else: - def u(x): - return x - # BINARY_TYPE = bytes - itervalues = dict.values - iteritems = dict.items + # BINARY_TYPE = bytes + itervalues = dict.values + iteritems = dict.items + +def unicode_escape_decode(x): + if sys.version_info < (3,): + return codecs.unicode_escape_decode(x)[0] + else: + return x + +# {str, bool}: a map from error categories to booleans which indicate if the +# category should be suppressed for every line. +_global_error_suppressions = {} + + def ParseNolintSuppressions(filename, raw_line, linenum, error): - """Updates the global list of error-suppressions. + """Updates the global list of line error-suppressions. Parses any NOLINT comments on the current line, updating the global error_suppressions store. Reports an error if the NOLINT comment @@ -594,24 +712,45 @@ def ParseNolintSuppressions(filename, raw_line, linenum, error): 'Unknown NOLINT error category: %s' % category) +def ProcessGlobalSuppresions(lines): + """Updates the list of global error suppressions. + + Parses any lint directives in the file that have global effect. + + Args: + lines: An array of strings, each representing a line of the file, with the + last element being empty if the file is terminated with a newline. + """ + for line in lines: + if _SEARCH_C_FILE.search(line): + for category in _DEFAULT_C_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + if _SEARCH_KERNEL_FILE.search(line): + for category in _DEFAULT_KERNEL_SUPPRESSED_CATEGORIES: + _global_error_suppressions[category] = True + + def ResetNolintSuppressions(): """Resets the set of NOLINT suppressions to empty.""" _error_suppressions.clear() + _global_error_suppressions.clear() def IsErrorSuppressedByNolint(category, linenum): """Returns true if the specified error category is suppressed on this line. Consults the global error_suppressions map populated by - ParseNolintSuppressions/ResetNolintSuppressions. + ParseNolintSuppressions/ProcessGlobalSuppresions/ResetNolintSuppressions. Args: category: str, the category of the error. linenum: int, the current line number. Returns: - bool, True iff the error should be suppressed due to a NOLINT comment. + bool, True iff the error should be suppressed due to a NOLINT comment or + global suppression. """ - return (linenum in _error_suppressions.get(category, set()) or + return (_global_error_suppressions.get(category, False) or + linenum in _error_suppressions.get(category, set()) or linenum in _error_suppressions.get(None, set())) @@ -650,6 +789,11 @@ def Search(pattern, s): return _regexp_compile_cache[pattern].search(s) +def _IsSourceExtension(s): + """File extension (excluding dot) matches a source file extension.""" + return s in GetNonHeaderExtensions() + + class _IncludeState(object): """Tracks line numbers for includes, and the order in which includes appear. @@ -687,6 +831,8 @@ class _IncludeState(object): def __init__(self): self.include_list = [[]] + self._section = None + self._last_header = None self.ResetSection('') def FindHeader(self, header): @@ -830,9 +976,16 @@ class _CppLintState(object): # output format: # "emacs" - format that emacs can parse (default) + # "eclipse" - format that eclipse can parse # "vs7" - format that Microsoft Visual Studio 7 can parse + # "junit" - format that Jenkins, Bamboo, etc can parse self.output_format = 'emacs' + # For JUnit output, save errors and failures until the end so that they + # can be written into the XML + self._junit_errors = [] + self._junit_failures = [] + def SetOutputFormat(self, output_format): """Sets the output format for errors.""" self.output_format = output_format @@ -901,11 +1054,69 @@ class _CppLintState(object): def PrintErrorCounts(self): """Print a summary of errors by category, and the total.""" - for category, count in iteritems(self.errors_by_category): - sys.stderr.write('Category \'%s\' errors found: %d\n' % + for category, count in sorted(iteritems(self.errors_by_category)): + self.PrintInfo('Category \'%s\' errors found: %d\n' % (category, count)) - if self.error_count > 0 and self.verbose_level > 0: - sys.stderr.write('Total errors found: %d\n' % self.error_count) + if self.error_count > 0: + self.PrintInfo('Total errors found: %d\n' % self.error_count) + + def PrintInfo(self, message): + if not _quiet and self.output_format != 'junit': + sys.stderr.write(message) + + def PrintError(self, message): + if self.output_format == 'junit': + self._junit_errors.append(message) + else: + sys.stderr.write(message) + + def AddJUnitFailure(self, filename, linenum, message, category, confidence): + self._junit_failures.append((filename, linenum, message, category, + confidence)) + + def FormatJUnitXML(self): + num_errors = len(self._junit_errors) + num_failures = len(self._junit_failures) + + testsuite = xml.etree.ElementTree.Element('testsuite') + testsuite.attrib['name'] = 'cpplint' + testsuite.attrib['errors'] = str(num_errors) + testsuite.attrib['failures'] = str(num_failures) + + if num_errors == 0 and num_failures == 0: + testsuite.attrib['tests'] = str(1) + xml.etree.ElementTree.SubElement(testsuite, 'testcase', name='passed') + + else: + testsuite.attrib['tests'] = str(num_errors + num_failures) + if num_errors > 0: + testcase = xml.etree.ElementTree.SubElement(testsuite, 'testcase') + testcase.attrib['name'] = 'errors' + error = xml.etree.ElementTree.SubElement(testcase, 'error') + error.text = '\n'.join(self._junit_errors) + if num_failures > 0: + # Group failures by file + failed_file_order = [] + failures_by_file = {} + for failure in self._junit_failures: + failed_file = failure[0] + if failed_file not in failed_file_order: + failed_file_order.append(failed_file) + failures_by_file[failed_file] = [] + failures_by_file[failed_file].append(failure) + # Create a testcase for each file + for failed_file in failed_file_order: + failures = failures_by_file[failed_file] + testcase = xml.etree.ElementTree.SubElement(testsuite, 'testcase') + testcase.attrib['name'] = failed_file + failure = xml.etree.ElementTree.SubElement(testcase, 'failure') + template = '{0}: {1} [{2}] [{3}]' + texts = [template.format(f[1], f[2], f[3], f[4]) for f in failures] + failure.text = '\n'.join(texts) + + xml_decl = '\n' + return xml_decl + xml.etree.ElementTree.tostring(testsuite, 'utf-8').decode('utf-8') + _cpplint_state = _CppLintState() @@ -1006,6 +1217,9 @@ class _FunctionState(object): filename: The name of the current file. linenum: The number of the line to check. """ + if not self.in_a_function: + return + if Match(r'T(EST|est)', self.current_function): base_trigger = self._TEST_TRIGGER else: @@ -1048,7 +1262,7 @@ class FileInfo(object): return os.path.abspath(self._filename).replace('\\', '/') def RepositoryName(self): - """FullName after removing the local path to the repository. + r"""FullName after removing the local path to the repository. If we have a real absolute path name here we can try to do something smart: detecting the root of the checkout and truncating /path/to/checkout from @@ -1062,6 +1276,20 @@ class FileInfo(object): if os.path.exists(fullname): project_dir = os.path.dirname(fullname) + # If the user specified a repository path, it exists, and the file is + # contained in it, use the specified repository path + if _repository: + repo = FileInfo(_repository).FullName() + root_dir = project_dir + while os.path.exists(root_dir): + # allow case insensitive compare on Windows + if os.path.normcase(root_dir) == os.path.normcase(repo): + return os.path.relpath(fullname, root_dir).replace('\\', '/') + one_up_dir = os.path.dirname(root_dir) + if one_up_dir == root_dir: + break + root_dir = one_up_dir + if os.path.exists(os.path.join(project_dir, ".svn")): # If there's a .svn file in the current directory, we recursively look # up the directory tree for the top of the SVN checkout @@ -1076,12 +1304,13 @@ class FileInfo(object): # Not SVN <= 1.6? Try to find a git, hg, or svn top level directory by # searching up from the current path. - root_dir = os.path.dirname(fullname) - while (root_dir != os.path.dirname(root_dir) and - not os.path.exists(os.path.join(root_dir, ".git")) and - not os.path.exists(os.path.join(root_dir, ".hg")) and - not os.path.exists(os.path.join(root_dir, ".svn"))): - root_dir = os.path.dirname(root_dir) + root_dir = current_dir = os.path.dirname(fullname) + while current_dir != os.path.dirname(current_dir): + if (os.path.exists(os.path.join(current_dir, ".git")) or + os.path.exists(os.path.join(current_dir, ".hg")) or + os.path.exists(os.path.join(current_dir, ".svn"))): + root_dir = current_dir + current_dir = os.path.dirname(current_dir) if (os.path.exists(os.path.join(root_dir, ".git")) or os.path.exists(os.path.join(root_dir, ".hg")) or @@ -1120,7 +1349,7 @@ class FileInfo(object): def IsSource(self): """File has a source file extension.""" - return self.Extension()[1:] in GetAllExtensions() + return _IsSourceExtension(self.Extension()[1:]) def _ShouldPrintError(category, confidence, linenum): @@ -1176,15 +1405,18 @@ def Error(filename, linenum, category, confidence, message): if _ShouldPrintError(category, confidence, linenum): _cpplint_state.IncrementErrorCount(category) if _cpplint_state.output_format == 'vs7': - sys.stderr.write('%s(%s): warning: %s [%s] [%d]\n' % ( + _cpplint_state.PrintError('%s(%s): warning: %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence)) elif _cpplint_state.output_format == 'eclipse': sys.stderr.write('%s:%s: warning: %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence)) + elif _cpplint_state.output_format == 'junit': + _cpplint_state.AddJUnitFailure(filename, linenum, message, category, + confidence) else: - m = '%s:%s: %s [%s] [%d]\n' % ( + final_message = '%s:%s: %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence) - sys.stderr.write(m) + sys.stderr.write(final_message) # Matches standard C++ escape sequences per 2.13.2.3 of the C++ standard. _RE_PATTERN_CLEANSE_LINE_ESCAPES = re.compile( @@ -1266,8 +1498,18 @@ def CleanseRawStrings(raw_lines): while delimiter is None: # Look for beginning of a raw string. # See 2.14.15 [lex.string] for syntax. - matched = Match(r'^(.*)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) - if matched: + # + # Once we have matched a raw string, we check the prefix of the + # line to make sure that the line is not part of a single line + # comment. It's done this way because we remove raw strings + # before removing comments as opposed to removing comments + # before removing raw strings. This is because there are some + # cpplint checks that requires the comments to be preserved, but + # we don't want to check comments that are inside raw strings. + matched = Match(r'^(.*?)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line) + if (matched and + not Match(r'^([^\'"]|\'(\\.|[^\'])*\'|"(\\.|[^"])*")*//', + matched.group(1))): delimiter = ')' + matched.group(2) + '"' end = matched.group(3).find(delimiter) @@ -1732,7 +1974,12 @@ def GetHeaderGuardCPPVariable(filename): fileinfo = FileInfo(filename) file_path_from_root = fileinfo.RepositoryName() if _root: - file_path_from_root = re.sub('^' + _root + os.sep, '', file_path_from_root) + suffix = os.sep + # On Windows using directory separator will leave us with + # "bogus escape error" unless we properly escape regex. + if suffix == '\\': + suffix += '\\' + file_path_from_root = re.sub('^' + _root + suffix, '', file_path_from_root) return re.sub(r'[^a-zA-Z0-9]', '_', file_path_from_root).upper() + '_' @@ -1759,6 +2006,11 @@ def CheckForHeaderGuard(filename, clean_lines, error): if Search(r'//\s*NOLINT\(build/header_guard\)', i): return + # Allow pragma once instead of header guards + for i in raw_lines: + if Search(r'^\s*#pragma\s+once', i): + return + cppvar = GetHeaderGuardCPPVariable(filename) ifndef = '' @@ -1838,12 +2090,13 @@ def CheckHeaderFileIncluded(filename, include_state, error): """Logs an error if a source file does not include its header.""" # Do not check test files - if _IsTestFilename(filename): + fileinfo = FileInfo(filename) + if Search(_TEST_FILE_SUFFIX, fileinfo.BaseName()): return - fileinfo = FileInfo(filename) for ext in GetHeaderExtensions(): - headerfile = filename[:filename.rfind('.') + 1] + ext + basefilename = filename[0:len(filename) - len(fileinfo.Extension())] + headerfile = basefilename + '.' + ext if not os.path.exists(headerfile): continue headername = FileInfo(headerfile).RepositoryName() @@ -1878,7 +2131,7 @@ def CheckForBadCharacters(filename, lines, error): error: The function to call with any errors found. """ for linenum, line in enumerate(lines): - if u('\ufffd') in line: + if unicode_escape_decode('\ufffd') in line: error(filename, linenum, 'readability/utf8', 5, 'Line contains invalid UTF-8 (or Unicode replacement character).') if '\0' in line: @@ -2060,7 +2313,8 @@ def IsForwardClassDeclaration(clean_lines, linenum): class _BlockInfo(object): """Stores information about a generic block of code.""" - def __init__(self, seen_open_brace): + def __init__(self, linenum, seen_open_brace): + self.starting_linenum = linenum self.seen_open_brace = seen_open_brace self.open_parentheses = 0 self.inline_asm = _NO_ASM @@ -2109,17 +2363,16 @@ class _BlockInfo(object): class _ExternCInfo(_BlockInfo): """Stores information about an 'extern "C"' block.""" - def __init__(self): - _BlockInfo.__init__(self, True) + def __init__(self, linenum): + _BlockInfo.__init__(self, linenum, True) class _ClassInfo(_BlockInfo): """Stores information about a class.""" def __init__(self, name, class_or_struct, clean_lines, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name - self.starting_linenum = linenum self.is_derived = False self.check_namespace_indentation = True if class_or_struct == 'struct': @@ -2187,9 +2440,8 @@ class _NamespaceInfo(_BlockInfo): """Stores information about a namespace.""" def __init__(self, name, linenum): - _BlockInfo.__init__(self, False) + _BlockInfo.__init__(self, linenum, False) self.name = name or '' - self.starting_linenum = linenum self.check_namespace_indentation = True def CheckEnd(self, filename, clean_lines, linenum, error): @@ -2208,7 +2460,7 @@ class _NamespaceInfo(_BlockInfo): # deciding what these nontrivial things are, so this check is # triggered by namespace size only, which works most of the time. if (linenum - self.starting_linenum < 10 - and not Match(r'};*\s*(//|/\*).*\bnamespace\b', line)): + and not Match(r'^\s*};*\s*(//|/\*).*\bnamespace\b', line)): return # Look for matching comment at end of namespace. @@ -2225,18 +2477,18 @@ class _NamespaceInfo(_BlockInfo): # expected namespace. if self.name: # Named namespace - if not Match((r'};*\s*(//|/\*).*\bnamespace\s+' + re.escape(self.name) + - r'[\*/\.\\\s]*$'), + if not Match((r'^\s*};*\s*(//|/\*).*\bnamespace\s+' + + re.escape(self.name) + r'[\*/\.\\\s]*$'), line): error(filename, linenum, 'readability/namespace', 5, 'Namespace should be terminated with "// namespace %s"' % self.name) else: # Anonymous namespace - if not Match(r'};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): + if not Match(r'^\s*};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): # If "// namespace anonymous" or "// anonymous namespace (more text)", # mention "// anonymous namespace" as an acceptable form - if Match(r'}.*\b(namespace anonymous|anonymous namespace)\b', line): + if Match(r'^\s*}.*\b(namespace anonymous|anonymous namespace)\b', line): error(filename, linenum, 'readability/namespace', 5, 'Anonymous namespace should be terminated with "// namespace"' ' or "// anonymous namespace"') @@ -2575,9 +2827,9 @@ class NestingState(object): if not self.SeenOpenBrace(): self.stack[-1].seen_open_brace = True elif Match(r'^extern\s*"[^"]*"\s*\{', line): - self.stack.append(_ExternCInfo()) + self.stack.append(_ExternCInfo(linenum)) else: - self.stack.append(_BlockInfo(True)) + self.stack.append(_BlockInfo(linenum, True)) if _MATCH_ASM.match(line): self.stack[-1].inline_asm = _BLOCK_ASM @@ -2689,7 +2941,8 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, r'\s+(register|static|extern|typedef)\b', line): error(filename, linenum, 'build/storage_class', 5, - 'Storage class (static, extern, typedef, etc) should be first.') + 'Storage-class specifier (static, extern, typedef, etc) should be ' + 'at the beginning of the declaration.') if Match(r'\s*#\s*endif\s*[^/\s]+', line): error(filename, linenum, 'build/endif_comment', 5, @@ -2728,9 +2981,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, base_classname = classinfo.name.split('::')[-1] # Look for single-argument constructors that aren't marked explicit. - # Technically a valid construct, but against style. Also look for - # non-single-argument constructors which are also technically valid, but - # strongly suggest something is wrong. + # Technically a valid construct, but against style. explicit_constructor_match = Match( r'\s+(?:inline\s+)?(explicit\s+)?(?:inline\s+)?%s\s*' r'\(((?:[^()]|\([^()]*\))*)\)' @@ -2795,10 +3046,6 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, if noarg_constructor: error(filename, linenum, 'runtime/explicit', 5, 'Zero-parameter constructors should not be marked explicit.') - else: - error(filename, linenum, 'runtime/explicit', 0, - 'Constructors that require multiple arguments ' - 'should not be marked explicit.') def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): @@ -2853,6 +3100,7 @@ def CheckSpacingForFunctionCall(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/parens', 2, 'Extra space after (') if (Search(r'\w\s+\(', fncall) and + not Search(r'_{0,2}asm_{0,2}\s+_{0,2}volatile_{0,2}\s+\(', fncall) and not Search(r'#\s*define|typedef|using\s+\w+\s*=', fncall) and not Search(r'\w\s+\((\w+::)*\*\w+\)\(', fncall) and not Search(r'\bcase\s+\(', fncall)): @@ -2911,7 +3159,7 @@ def CheckForFunctionLengths(filename, clean_lines, linenum, """Reports for long function bodies. For an overview why this is done, see: - http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#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. @@ -2990,9 +3238,7 @@ def CheckComment(line, filename, linenum, next_line_start, error): commentpos = line.find('//') if commentpos != -1: # Check if the // may be in quotes. If so, ignore it - # Comparisons made explicit for clarity -- pylint: disable=g-explicit-bool-comparison - if (line.count('"', 0, commentpos) - - line.count('\\"', 0, commentpos)) % 2 == 0: # not in quotes + if re.sub(r'\\.', '', line[0:commentpos]).count('"') % 2 == 0: # Allow one space for new scopes, two spaces otherwise: if (not (Match(r'^.*{ *//', line) and next_line_start == commentpos) and ((commentpos >= 1 and @@ -3241,8 +3487,8 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # macro context and don't do any checks. This avoids false # positives. # - # Note that && is not included here. Those are checked separately - # in CheckRValueReference + # Note that && is not included here. This is because there are too + # many false positives due to RValue references. match = Search(r'[^<>=!\s](==|!=|<=|>=|\|\|)[^<>=!\s,;\)]', line) if match: error(filename, linenum, 'whitespace/operators', 3, @@ -3276,7 +3522,7 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # # We also allow operators following an opening parenthesis, since # those tend to be macros that deal with operators. - match = Search(r'(operator|[^\s(<])(?:L|UL|ULL|l|ul|ull)?<<([^\s,=<])', line) + match = Search(r'(operator|[^\s(<])(?:L|UL|LL|ULL|l|ul|ll|ull)?<<([^\s,=<])', line) if (match and not (match.group(1).isdigit() and match.group(2).isdigit()) and not (match.group(1) == 'operator' and match.group(2) == ';')): error(filename, linenum, 'whitespace/operators', 3, @@ -3380,22 +3626,90 @@ def CheckCommaSpacing(filename, clean_lines, linenum, error): 'Missing space after ;') -def CheckBracesSpacing(filename, clean_lines, linenum, error): +def _IsType(clean_lines, nesting_state, expr): + """Check if expression looks like a type name, returns true if so. + + Args: + clean_lines: A CleansedLines instance containing the file. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. + expr: The expression to check. + Returns: + True, if token looks like a type. + """ + # Keep only the last token in the expression + last_word = Match(r'^.*(\b\S+)$', expr) + if last_word: + token = last_word.group(1) + else: + token = expr + + # Match native types and stdint types + if _TYPES.match(token): + return True + + # Try a bit harder to match templated types. Walk up the nesting + # stack until we find something that resembles a typename + # declaration for what we are looking for. + typename_pattern = (r'\b(?:typename|class|struct)\s+' + re.escape(token) + + r'\b') + block_index = len(nesting_state.stack) - 1 + while block_index >= 0: + if isinstance(nesting_state.stack[block_index], _NamespaceInfo): + return False + + # Found where the opening brace is. We want to scan from this + # line up to the beginning of the function, minus a few lines. + # template + # class C + # : public ... { // start scanning here + last_line = nesting_state.stack[block_index].starting_linenum + + next_block_start = 0 + if block_index > 0: + next_block_start = nesting_state.stack[block_index - 1].starting_linenum + first_line = last_line + while first_line >= next_block_start: + if clean_lines.elided[first_line].find('template') >= 0: + break + first_line -= 1 + if first_line < next_block_start: + # Didn't find any "template" keyword before reaching the next block, + # there are probably no template things to check for this block + block_index -= 1 + continue + + # Look for typename in the specified range + for i in xrange(first_line, last_line + 1, 1): + if Search(typename_pattern, clean_lines.elided[i]): + return True + block_index -= 1 + + return False + + +def CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error): """Checks for horizontal spacing near commas. Args: filename: The name of the current file. clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. + nesting_state: A NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: The function to call with any errors found. """ line = clean_lines.elided[linenum] # Except after an opening paren, or after another opening brace (in case of # an initializer list, for instance), you should have spaces before your - # braces. And since you should never have braces at the beginning of a line, - # this is an easy test. + # braces when they are delimiting blocks, classes, namespaces etc. + # And since you should never have braces at the beginning of a line, + # this is an easy test. Except that braces used for initialization don't + # follow the same rule; we often don't want spaces before those. match = Match(r'^(.*[^ ({>]){', line) + if match: # Try a bit harder to check for brace initialization. This # happens in one of the following forms: @@ -3425,6 +3739,7 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): # There is a false negative with this approach if people inserted # spurious semicolons, e.g. "if (cond){};", but we will catch the # spurious semicolon with a separate check. + leading_text = match.group(1) (endline, endlinenum, endpos) = CloseExpression( clean_lines, linenum, len(match.group(1))) trailing_text = '' @@ -3433,7 +3748,11 @@ def CheckBracesSpacing(filename, clean_lines, linenum, error): for offset in xrange(endlinenum + 1, min(endlinenum + 3, clean_lines.NumLines() - 1)): trailing_text += clean_lines.elided[offset] - if not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text): + # We also suppress warnings for `uint64_t{expression}` etc., as the style + # guide recommends brace initialization for integral types to avoid + # overflow/truncation. + if (not Match(r'^[\s}]*[{.;,)<>\]:]', trailing_text) + and not _IsType(clean_lines, nesting_state, leading_text)): error(filename, linenum, 'whitespace/braces', 5, 'Missing space before {') @@ -3476,406 +3795,6 @@ def IsDecltype(clean_lines, linenum, column): return True return False - -def IsTemplateParameterList(clean_lines, linenum, column): - """Check if the token ending on (linenum, column) is the end of template<>. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is end of a template parameter list, False otherwise. - """ - (_, startline, startpos) = ReverseCloseExpression( - clean_lines, linenum, column) - if (startpos > -1 and - Search(r'\btemplate\s*$', clean_lines.elided[startline][0:startpos])): - return True - return False - - -def IsRValueType(typenames, clean_lines, nesting_state, linenum, column): - """Check if the token ending on (linenum, column) is a type. - - Assumes that text to the right of the column is "&&" or a function - name. - - Args: - typenames: set of type names from template-argument-list. - clean_lines: A CleansedLines instance containing the file. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - linenum: the number of the line to check. - column: end column of the token to check. - Returns: - True if this token is a type, False if we are not sure. - """ - prefix = clean_lines.elided[linenum][0:column] - - # Get one word to the left. If we failed to do so, this is most - # likely not a type, since it's unlikely that the type name and "&&" - # would be split across multiple lines. - match = Match(r'^(.*)(\b\w+|[>*)&])\s*$', prefix) - if not match: - return False - - # Check text following the token. If it's "&&>" or "&&," or "&&...", it's - # most likely a rvalue reference used inside a template. - suffix = clean_lines.elided[linenum][column:] - if Match(r'&&\s*(?:[>,]|\.\.\.)', suffix): - return True - - # Check for known types and end of templates: - # int&& variable - # vector&& variable - # - # Because this function is called recursively, we also need to - # recognize pointer and reference types: - # int* Function() - # int& Function() - if (match.group(2) in typenames or - match.group(2) in ['char', 'char16_t', 'char32_t', 'wchar_t', 'bool', - 'short', 'int', 'long', 'signed', 'unsigned', - 'float', 'double', 'void', 'auto', '>', '*', '&']): - return True - - # If we see a close parenthesis, look for decltype on the other side. - # decltype would unambiguously identify a type, anything else is - # probably a parenthesized expression and not a type. - if match.group(2) == ')': - return IsDecltype( - clean_lines, linenum, len(match.group(1)) + len(match.group(2)) - 1) - - # Check for casts and cv-qualifiers. - # match.group(1) remainder - # -------------- --------- - # const_cast< type&& - # const type&& - # type const&& - if Search(r'\b(?:const_cast\s*<|static_cast\s*<|dynamic_cast\s*<|' - r'reinterpret_cast\s*<|\w+\s)\s*$', - match.group(1)): - return True - - # Look for a preceding symbol that might help differentiate the context. - # These are the cases that would be ambiguous: - # match.group(1) remainder - # -------------- --------- - # Call ( expression && - # Declaration ( type&& - # sizeof ( type&& - # if ( expression && - # while ( expression && - # for ( type&& - # for( ; expression && - # statement ; type&& - # block { type&& - # constructor { expression && - start = linenum - line = match.group(1) - match_symbol = None - while start >= 0: - # We want to skip over identifiers and commas to get to a symbol. - # Commas are skipped so that we can find the opening parenthesis - # for function parameter lists. - match_symbol = Match(r'^(.*)([^\w\s,:&*])[\w\s,:&*]*$', line) - if match_symbol: - break - start -= 1 - line = clean_lines.elided[start] - - if not match_symbol: - # Probably the first statement in the file is an rvalue reference - return True - - if match_symbol.group(2) == '}': - # Found closing brace, probably an indicate of this: - # block{} type&& - return True - - if match_symbol.group(2) == ';': - # Found semicolon, probably one of these: - # for(; expression && - # statement; type&& - - # Look for the previous 'for(' in the previous lines. - before_text = match_symbol.group(1) - for i in xrange(start - 1, max(start - 6, 0), -1): - before_text = clean_lines.elided[i] + before_text - if Search(r'for\s*\([^{};]*$', before_text): - # This is the condition inside a for-loop - return False - - # Did not find a for-init-statement before this semicolon, so this - # is probably a new statement and not a condition. - return True - - if match_symbol.group(2) == '{': - # Found opening brace, probably one of these: - # block{ type&& = ... ; } - # constructor{ expression && expression } - - # Look for a closing brace or a semicolon. If we see a semicolon - # first, this is probably a rvalue reference. - line = clean_lines.elided[start][0:len(match_symbol.group(1)) + 1] - end = start - depth = 1 - while True: - for ch in line: - if ch == ';': - return True - elif ch == '{': - depth += 1 - elif ch == '}': - depth -= 1 - if depth == 0: - return False - end += 1 - if end >= clean_lines.NumLines(): - break - line = clean_lines.elided[end] - # Incomplete program? - return False - - if match_symbol.group(2) == '(': - # Opening parenthesis. Need to check what's to the left of the - # parenthesis. Look back one extra line for additional context. - before_text = match_symbol.group(1) - if linenum > 1: - before_text = clean_lines.elided[linenum - 1] + before_text - before_text = match_symbol.group(1) - - # Patterns that are likely to be types: - # [](type&& - # for (type&& - # sizeof(type&& - # operator=(type&& - # - if Search(r'(?:\]|\bfor|\bsizeof|\boperator\s*\S+\s*)\s*$', before_text): - return True - - # Patterns that are likely to be expressions: - # if (expression && - # while (expression && - # : initializer(expression && - # , initializer(expression && - # ( FunctionCall(expression && - # + FunctionCall(expression && - # + (expression && - # - # The last '+' represents operators such as '+' and '-'. - if Search(r'(?:\bif|\bwhile|[-+=%^(]*>)?\s*$', - match_symbol.group(1)) - if match_func: - # Check for constructors, which don't have return types. - if Search(r'\b(?:explicit|inline)$', match_func.group(1)): - return True - implicit_constructor = Match(r'\s*(\w+)\((?:const\s+)?(\w+)', prefix) - if (implicit_constructor and - implicit_constructor.group(1) == implicit_constructor.group(2)): - return True - return IsRValueType(typenames, clean_lines, nesting_state, linenum, - len(match_func.group(1))) - - # Nothing before the function name. If this is inside a block scope, - # this is probably a function call. - return not (nesting_state.previous_stack_top and - nesting_state.previous_stack_top.IsBlockInfo()) - - if match_symbol.group(2) == '>': - # Possibly a closing bracket, check that what's on the other side - # looks like the start of a template. - return IsTemplateParameterList( - clean_lines, start, len(match_symbol.group(1))) - - # Some other symbol, usually something like "a=b&&c". This is most - # likely not a type. - return False - - -def IsDeletedOrDefault(clean_lines, linenum): - """Check if current constructor or operator is deleted or default. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - Returns: - True if this is a deleted or default constructor. - """ - open_paren = clean_lines.elided[linenum].find('(') - if open_paren < 0: - return False - (close_line, _, close_paren) = CloseExpression( - clean_lines, linenum, open_paren) - if close_paren < 0: - return False - return Match(r'\s*=\s*(?:delete|default)\b', close_line[close_paren:]) - - -def IsRValueAllowed(clean_lines, linenum, typenames): - """Check if RValue reference is allowed on a particular line. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - typenames: set of type names from template-argument-list. - Returns: - True if line is within the region where RValue references are allowed. - """ - # Allow region marked by PUSH/POP macros - for i in xrange(linenum, 0, -1): - line = clean_lines.elided[i] - if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line): - if not line.endswith('PUSH'): - return False - for j in xrange(linenum, clean_lines.NumLines(), 1): - line = clean_lines.elided[j] - if Match(r'GOOGLE_ALLOW_RVALUE_REFERENCES_(?:PUSH|POP)', line): - return line.endswith('POP') - - # Allow operator= - line = clean_lines.elided[linenum] - if Search(r'\boperator\s*=\s*\(', line): - return IsDeletedOrDefault(clean_lines, linenum) - - # Allow constructors - match = Match(r'\s*(?:[\w<>]+::)*([\w<>]+)\s*::\s*([\w<>]+)\s*\(', line) - if match and match.group(1) == match.group(2): - return IsDeletedOrDefault(clean_lines, linenum) - if Search(r'\b(?:explicit|inline)\s+[\w<>]+\s*\(', line): - return IsDeletedOrDefault(clean_lines, linenum) - - if Match(r'\s*[\w<>]+\s*\(', line): - previous_line = 'ReturnType' - if linenum > 0: - previous_line = clean_lines.elided[linenum - 1] - if Match(r'^\s*$', previous_line) or Search(r'[{}:;]\s*$', previous_line): - return IsDeletedOrDefault(clean_lines, linenum) - - # Reject types not mentioned in template-argument-list - while line: - match = Match(r'^.*?(\w+)\s*&&(.*)$', line) - if not match: - break - if match.group(1) not in typenames: - return False - line = match.group(2) - - # All RValue types that were in template-argument-list should have - # been removed by now. Those were allowed, assuming that they will - # be forwarded. - # - # If there are no remaining RValue types left (i.e. types that were - # not found in template-argument-list), flag those as not allowed. - return line.find('&&') < 0 - - -def GetTemplateArgs(clean_lines, linenum): - """Find list of template arguments associated with this function declaration. - - Args: - clean_lines: A CleansedLines instance containing the file. - linenum: Line number containing the start of the function declaration, - usually one line after the end of the template-argument-list. - Returns: - Set of type names, or empty set if this does not appear to have - any template parameters. - """ - # Find start of function - func_line = linenum - while func_line > 0: - line = clean_lines.elided[func_line] - if Match(r'^\s*$', line): - return set() - if line.find('(') >= 0: - break - func_line -= 1 - if func_line == 0: - return set() - - # Collapse template-argument-list into a single string - argument_list = '' - match = Match(r'^(\s*template\s*)<', clean_lines.elided[func_line]) - if match: - # template-argument-list on the same line as function name - start_col = len(match.group(1)) - _, end_line, end_col = CloseExpression(clean_lines, func_line, start_col) - if end_col > -1 and end_line == func_line: - start_col += 1 # Skip the opening bracket - argument_list = clean_lines.elided[func_line][start_col:end_col] - - elif func_line > 1: - # template-argument-list one line before function name - match = Match(r'^(.*)>\s*$', clean_lines.elided[func_line - 1]) - if match: - end_col = len(match.group(1)) - _, start_line, start_col = ReverseCloseExpression( - clean_lines, func_line - 1, end_col) - if start_col > -1: - start_col += 1 # Skip the opening bracket - while start_line < func_line - 1: - argument_list += clean_lines.elided[start_line][start_col:] - start_col = 0 - start_line += 1 - argument_list += clean_lines.elided[func_line - 1][start_col:end_col] - - if not argument_list: - return set() - - # Extract type names - typenames = set() - while True: - match = Match(r'^[,\s]*(?:typename|class)(?:\.\.\.)?\s+(\w+)(.*)$', - argument_list) - if not match: - break - typenames.add(match.group(1)) - argument_list = match.group(2) - return typenames - - -def CheckRValueReference(filename, clean_lines, linenum, nesting_state, error): - """Check for rvalue references. - - Args: - filename: The name of the current file. - clean_lines: A CleansedLines instance containing the file. - linenum: The number of the line to check. - nesting_state: A NestingState instance which maintains information about - the current stack of nested blocks being parsed. - error: The function to call with any errors found. - """ - # Find lines missing spaces around &&. - # TODO(unknown): currently we don't check for rvalue references - # with spaces surrounding the && to avoid false positives with - # boolean expressions. - line = clean_lines.elided[linenum] - match = Match(r'^(.*\S)&&', line) - if not match: - match = Match(r'(.*)&&\S', line) - if (not match) or '(&&)' in line or Search(r'\boperator\s*$', match.group(1)): - return - - # Either poorly formed && or an rvalue reference, check the context - # to get a more accurate error message. Mostly we want to determine - # if what's to the left of "&&" is a type or not. - typenames = GetTemplateArgs(clean_lines, linenum) - and_pos = len(match.group(1)) - if IsRValueType(typenames, clean_lines, nesting_state, linenum, and_pos): - if not IsRValueAllowed(clean_lines, linenum, typenames): - error(filename, linenum, 'build/c++11', 3, - 'RValue references are an unapproved C++ feature.') - else: - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around &&') - - def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): """Checks for additional blank line issues related to sections. @@ -3973,10 +3892,13 @@ def CheckBraces(filename, clean_lines, linenum, error): # used for brace initializers inside function calls. We don't detect this # perfectly: we just don't complain if the last non-whitespace character on # the previous non-blank line is ',', ';', ':', '(', '{', or '}', or if the - # previous line starts a preprocessor block. + # previous line starts a preprocessor block. We also allow a brace on the + # following line if it is part of an array initialization and would not fit + # within the 80 character limit of the preceding line. prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0] if (not Search(r'[,;:}{(]\s*$', prevline) and - not Match(r'\s*#', prevline)): + not Match(r'\s*#', prevline) and + not (GetLineWidth(prevline) > _line_length - 2 and '[]' in prevline)): error(filename, linenum, 'whitespace/braces', 4, '{ should almost always be at the end of the previous line') @@ -4152,13 +4074,14 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # In addition to macros, we also don't want to warn on # - Compound literals # - Lambdas - # - alignas specifier with anonymous structs: + # - alignas specifier with anonymous structs + # - decltype closing_brace_pos = match.group(1).rfind(')') opening_parenthesis = ReverseCloseExpression( clean_lines, linenum, closing_brace_pos) if opening_parenthesis[2] > -1: line_prefix = opening_parenthesis[0][0:opening_parenthesis[2]] - macro = Search(r'\b([A-Z0-9_]+)\s*$', line_prefix) + macro = Search(r'\b([A-Z_][A-Z0-9_]*)\s*$', line_prefix) func = Match(r'^(.*\])\s*$', line_prefix) if ((macro and macro.group(1) not in ( @@ -4167,6 +4090,7 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): 'LOCKS_EXCLUDED', 'INTERFACE_DEF')) or (func and not Search(r'\boperator\s*\[\s*\]', func.group(1))) or Search(r'\b(?:struct|union)\s+alignas\s*$', line_prefix) or + Search(r'\bdecltype$', line_prefix) or Search(r'\s+=\s*$', line_prefix)): match = None if (match and @@ -4203,6 +4127,14 @@ def CheckTrailingSemicolon(filename, clean_lines, linenum, error): # outputting warnings for the matching closing brace, if there are # nested blocks with trailing semicolons, we will get the error # messages in reversed order. + + # We need to check the line forward for NOLINT + raw_lines = clean_lines.raw_lines + ParseNolintSuppressions(filename, raw_lines[endlinenum-1], endlinenum-1, + error) + ParseNolintSuppressions(filename, raw_lines[endlinenum], endlinenum, + error) + error(filename, endlinenum, 'readability/braces', 4, "You don't need a ; after a }") @@ -4226,7 +4158,7 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): line = clean_lines.elided[linenum] matched = Match(r'\s*(for|while|if)\s*\(', line) if matched: - # Find the end of the conditional expression + # Find the end of the conditional expression. (end_line, end_linenum, end_pos) = CloseExpression( clean_lines, linenum, line.find('(')) @@ -4241,6 +4173,75 @@ def CheckEmptyBlockBody(filename, clean_lines, linenum, error): error(filename, end_linenum, 'whitespace/empty_loop_body', 5, 'Empty loop bodies should use {} or continue') + # Check for if statements that have completely empty bodies (no comments) + # and no else clauses. + if end_pos >= 0 and matched.group(1) == 'if': + # Find the position of the opening { for the if statement. + # Return without logging an error if it has no brackets. + opening_linenum = end_linenum + opening_line_fragment = end_line[end_pos:] + # Loop until EOF or find anything that's not whitespace or opening {. + while not Search(r'^\s*\{', opening_line_fragment): + if Search(r'^(?!\s*$)', opening_line_fragment): + # Conditional has no brackets. + return + opening_linenum += 1 + if opening_linenum == len(clean_lines.elided): + # Couldn't find conditional's opening { or any code before EOF. + return + opening_line_fragment = clean_lines.elided[opening_linenum] + # Set opening_line (opening_line_fragment may not be entire opening line). + opening_line = clean_lines.elided[opening_linenum] + + # Find the position of the closing }. + opening_pos = opening_line_fragment.find('{') + if opening_linenum == end_linenum: + # We need to make opening_pos relative to the start of the entire line. + opening_pos += end_pos + (closing_line, closing_linenum, closing_pos) = CloseExpression( + clean_lines, opening_linenum, opening_pos) + if closing_pos < 0: + return + + # Now construct the body of the conditional. This consists of the portion + # of the opening line after the {, all lines until the closing line, + # and the portion of the closing line before the }. + if (clean_lines.raw_lines[opening_linenum] != + CleanseComments(clean_lines.raw_lines[opening_linenum])): + # Opening line ends with a comment, so conditional isn't empty. + return + if closing_linenum > opening_linenum: + # Opening line after the {. Ignore comments here since we checked above. + bodylist = list(opening_line[opening_pos+1:]) + # All lines until closing line, excluding closing line, with comments. + bodylist.extend(clean_lines.raw_lines[opening_linenum+1:closing_linenum]) + # Closing line before the }. Won't (and can't) have comments. + bodylist.append(clean_lines.elided[closing_linenum][:closing_pos-1]) + body = '\n'.join(bodylist) + else: + # If statement has brackets and fits on a single line. + body = opening_line[opening_pos+1:closing_pos-1] + + # Check if the body is empty + if not _EMPTY_CONDITIONAL_BODY_PATTERN.search(body): + return + # The body is empty. Now make sure there's not an else clause. + current_linenum = closing_linenum + current_line_fragment = closing_line[closing_pos:] + # Loop until EOF or find anything that's not whitespace or else clause. + while Search(r'^\s*$|^(?=\s*else)', current_line_fragment): + if Search(r'^(?=\s*else)', current_line_fragment): + # Found an else clause, so don't log an error. + return + current_linenum += 1 + if current_linenum == len(clean_lines.elided): + break + current_line_fragment = clean_lines.elided[current_linenum] + + # The body is empty and there's no else clause until EOF or other code. + error(filename, end_linenum, 'whitespace/empty_if_body', 4, + ('If statement had no body and no else clause')) + def FindCheckMacro(line): """Find a replaceable CHECK-like macro. @@ -4460,6 +4461,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # raw strings, raw_lines = clean_lines.lines_without_raw_strings line = raw_lines[linenum] + prev = raw_lines[linenum - 1] if linenum > 0 else '' if line.find('\t') != -1: error(filename, linenum, 'whitespace/tab', 1, @@ -4483,19 +4485,24 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, cleansed_line = clean_lines.elided[linenum] while initial_spaces < len(line) and line[initial_spaces] == ' ': initial_spaces += 1 - if line and line[-1].isspace(): - error(filename, linenum, 'whitespace/end_of_line', 4, - 'Line ends in whitespace. Consider deleting these extra spaces.') # There are certain situations we allow one space, notably for # section labels, and also lines containing multi-line raw strings. - elif ((initial_spaces == 1 or initial_spaces == 3) and - not Match(scope_or_label_pattern, cleansed_line) and - not (clean_lines.raw_lines[linenum] != line and - Match(r'^\s*""', line))): + # We also don't check for lines that look like continuation lines + # (of lines ending in double quotes, commas, equals, or angle brackets) + # because the rules for how to indent those are non-trivial. + if (not Search(r'[",=><] *$', prev) and + (initial_spaces == 1 or initial_spaces == 3) and + not Match(scope_or_label_pattern, cleansed_line) and + not (clean_lines.raw_lines[linenum] != line and + Match(r'^\s*""', line))): error(filename, linenum, 'whitespace/indent', 3, 'Weird number of spaces at line-start. ' 'Are you using a 2-space indent?') + if line and line[-1].isspace(): + error(filename, linenum, 'whitespace/end_of_line', 4, + 'Line ends in whitespace. Consider deleting these extra spaces.') + # Check if the line is a header guard. is_header_guard = False if file_extension in GetHeaderExtensions(): @@ -4517,15 +4524,11 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, # function declaration if (not line.startswith('#include') and not is_header_guard and not Match(r'^\s*//.*http(s?)://\S*$', line) and + not Match(r'^\s*//\s*[^\s]*$', line) and not Match(r'^// \$Id:.*#[0-9]+ \$$', line) and not Match(r'^\s*/// [@\\](copydoc|copydetails|copybrief) .*$', line)): line_width = GetLineWidth(line) - extended_length = int((_line_length * 1.25)) - if line_width > extended_length: - error(filename, linenum, 'whitespace/line_length', 4, - 'Lines should very rarely be longer than %i characters' % - extended_length) - elif line_width > _line_length: + if line_width > _line_length: error(filename, linenum, 'whitespace/line_length', 2, 'Lines should be <= %i characters long' % _line_length) @@ -4553,9 +4556,8 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckOperatorSpacing(filename, clean_lines, linenum, error) CheckParenthesisSpacing(filename, clean_lines, linenum, error) CheckCommaSpacing(filename, clean_lines, linenum, error) - CheckBracesSpacing(filename, clean_lines, linenum, error) + CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error) CheckSpacingForFunctionCall(filename, clean_lines, linenum, error) - CheckRValueReference(filename, clean_lines, linenum, nesting_state, error) CheckCheck(filename, clean_lines, linenum, error) CheckAltTokens(filename, clean_lines, linenum, error) classinfo = nesting_state.InnermostClass() @@ -4602,21 +4604,6 @@ def _DropCommonSuffixes(filename): return os.path.splitext(filename)[0] -def _IsTestFilename(filename): - """Determines if the given filename has a suffix that identifies it as a test. - - Args: - filename: The input filename. - - Returns: - True if 'filename' looks like a test, False otherwise. - """ - for test_suffix, ext in itertools.product(_test_suffixes, GetNonHeaderExtensions()): - if filename.endswith(test_suffix + '.' + ext): - return True - return False - - def _ClassifyInclude(fileinfo, include, is_system): """Figures out what kind of header 'include' is. @@ -4645,6 +4632,10 @@ def _ClassifyInclude(fileinfo, include, is_system): # those already checked for above. is_cpp_h = include in _CPP_HEADERS + # Headers with C++ extensions shouldn't be considered C system headers + if is_system and os.path.splitext(include)[1] in ['.hpp', '.hxx', '.h++']: + is_system = False + if is_system: if is_cpp_h: return _CPP_SYS_HEADER @@ -4657,9 +4648,11 @@ def _ClassifyInclude(fileinfo, include, is_system): target_dir, target_base = ( os.path.split(_DropCommonSuffixes(fileinfo.RepositoryName()))) include_dir, include_base = os.path.split(_DropCommonSuffixes(include)) + target_dir_pub = os.path.normpath(target_dir + '/../public') + target_dir_pub = target_dir_pub.replace('\\', '/') if target_base == include_base and ( include_dir == target_dir or - include_dir == os.path.normpath(target_dir + '/../public')): + include_dir == target_dir_pub): return _LIKELY_MY_HEADER # If the target and include share some initial basename @@ -4703,7 +4696,7 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): # naming convention but not the include convention. match = Match(r'#include\s*"([^/]+\.h)"', line) if match and not _THIRD_PARTY_HEADERS_PATTERN.match(match.group(1)): - error(filename, linenum, 'build/include', 4, + error(filename, linenum, 'build/include_subdir', 4, 'Include the directory when naming .h files') # we shouldn't include a file more than once. actually, there are a @@ -4757,7 +4750,7 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error): def _GetTextInside(text, start_pattern): - """Retrieves all the text between matching open and close parentheses. + r"""Retrieves all the text between matching open and close parentheses. Given a string of lines and a regular expression string, retrieve all the text following the expression and between opening punctuation symbols like @@ -4836,6 +4829,9 @@ _RE_PATTERN_REF_PARAM = re.compile( _RE_PATTERN_CONST_REF_PARAM = ( r'(?:.*\s*\bconst\s*&\s*' + _RE_PATTERN_IDENT + r'|const\s+' + _RE_PATTERN_TYPE + r'\s*&\s*' + _RE_PATTERN_IDENT + r')') +# Stream types. +_RE_PATTERN_REF_STREAM_PARAM = ( + r'(?:.*stream\s*&\s*' + _RE_PATTERN_IDENT + r')') def CheckLanguage(filename, clean_lines, linenum, file_extension, @@ -4872,6 +4868,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, if match: include_state.ResetSection(match.group(1)) + # Perform other checks now that we are sure that this is not an include line CheckCasts(filename, clean_lines, linenum, error) CheckGlobalStatic(filename, clean_lines, linenum, error) @@ -4937,10 +4934,15 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, 'Did you mean "memset(%s, 0, %s)"?' % (match.group(1), match.group(2))) - if Search(r'\busing namespace\b', line) and not Search(r'\bliterals\b', line): - error(filename, linenum, 'build/namespaces', 5, - 'Do not use namespace using-directives. ' - 'Use using-declarations instead.') + if Search(r'\busing namespace\b', line): + if Search(r'\bliterals\b', line): + error(filename, linenum, 'build/namespaces_literals', 5, + 'Do not use namespace using-directives. ' + 'Use using-declarations instead.') + else: + error(filename, linenum, 'build/namespaces', 5, + 'Do not use namespace using-directives. ' + 'Use using-declarations instead.') # Detect variable-length arrays. match = Match(r'\s*(.+::)?(\w+) [a-z]\w*\[(.+)];', line) @@ -4989,7 +4991,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, and line[-1] != '\\'): error(filename, linenum, 'build/namespaces', 4, 'Do not use unnamed namespaces in header files. See ' - 'http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces' + 'https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces' ' for more information.') @@ -5010,9 +5012,13 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # Check for people declaring static/global STL strings at the top level. # This is dangerous because the C++ language does not guarantee that - # globals with constructors are initialized before the first access. + # globals with constructors are initialized before the first access, and + # also because globals can be destroyed when some threads are still running. + # TODO(unknown): Generalize this to also find static unique_ptr instances. + # TODO(unknown): File bugs for clang-tidy to find these. match = Match( - r'((?:|static +)(?:|const +))string +([a-zA-Z0-9_:]+)\b(.*)', + r'((?:|static +)(?:|const +))(?::*std::)?string( +const)? +' + r'([a-zA-Z0-9_:]+)\b(.*)', line) # Remove false positives: @@ -5032,15 +5038,20 @@ def CheckGlobalStatic(filename, clean_lines, linenum, error): # matching identifiers. # string Class::operator*() if (match and - not Search(r'\bstring\b(\s+const)?\s*\*\s*(const\s+)?\w', line) and + not Search(r'\bstring\b(\s+const)?\s*[\*\&]\s*(const\s+)?\w', line) and not Search(r'\boperator\W', line) and - not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(3))): - error(filename, linenum, 'runtime/string', 4, - 'For a static/global string constant, use a C style string instead: ' - '"%schar %s[]".' % - (match.group(1), match.group(2))) + not Match(r'\s*(<.*>)?(::[a-zA-Z0-9_]+)*\s*\(([^"]|$)', match.group(4))): + if Search(r'\bconst\b', line): + error(filename, linenum, 'runtime/string', 4, + 'For a static/global string constant, use a C style string ' + 'instead: "%schar%s %s[]".' % + (match.group(1), match.group(2) or '', match.group(3))) + else: + error(filename, linenum, 'runtime/string', 4, + 'Static/global string variables are not permitted.') - if Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line): + if (Search(r'\b([A-Za-z0-9_]*_)\(\1\)', line) or + Search(r'\b([A-Za-z0-9_]*_)\(CHECK_NOTNULL\(\1\)\)', line)): error(filename, linenum, 'runtime/init', 4, 'You seem to be initializing a member variable with itself.') @@ -5285,7 +5296,8 @@ def CheckForNonConstReference(filename, clean_lines, linenum, decls = ReplaceAll(r'{[^}]*}', ' ', line) # exclude function body for parameter in re.findall(_RE_PATTERN_REF_PARAM, decls): - if not Match(_RE_PATTERN_CONST_REF_PARAM, parameter): + if (not Match(_RE_PATTERN_CONST_REF_PARAM, parameter) and + not Match(_RE_PATTERN_REF_STREAM_PARAM, parameter)): error(filename, linenum, 'runtime/references', 2, 'Is this a non-const reference? ' 'If so, make const or use a pointer: ' + @@ -5308,7 +5320,7 @@ def CheckCasts(filename, clean_lines, linenum, error): # Parameterless conversion functions, such as bool(), are allowed as they are # probably a member operator declaration or default constructor. match = Search( - r'(\bnew\s+|\S<\s*(?:const\s+)?)?\b' + r'(\bnew\s+(?:const\s+)?|\S<\s*(?:const\s+)?)?\b' r'(int|float|double|bool|char|int32|uint32|int64|uint64)' r'(\([^)].*)', line) expecting_function = ExpectingFunctionArgs(clean_lines, linenum) @@ -5449,63 +5461,12 @@ def CheckCStyleCast(filename, clean_lines, linenum, cast_type, pattern, error): if context.endswith(' operator++') or context.endswith(' operator--'): return False - # A single unnamed argument for a function tends to look like old - # style cast. If we see those, don't issue warnings for deprecated - # casts, instead issue warnings for unnamed arguments where - # appropriate. - # - # These are things that we want warnings for, since the style guide - # explicitly require all parameters to be named: - # Function(int); - # Function(int) { - # ConstMember(int) const; - # ConstMember(int) const { - # ExceptionMember(int) throw (...); - # ExceptionMember(int) throw (...) { - # PureVirtual(int) = 0; - # [](int) -> bool { - # - # These are functions of some sort, where the compiler would be fine - # if they had named parameters, but people often omit those - # identifiers to reduce clutter: - # (FunctionPointer)(int); - # (FunctionPointer)(int) = value; - # Function((function_pointer_arg)(int)) - # Function((function_pointer_arg)(int), int param) - # ; - # <(FunctionPointerTemplateArgument)(int)>; + # A single unnamed argument for a function tends to look like old style cast. + # If we see those, don't issue warnings for deprecated casts. remainder = line[match.end(0):] if Match(r'^\s*(?:;|const\b|throw\b|final\b|override\b|[=>{),]|->)', remainder): - # Looks like an unnamed parameter. - - # Don't warn on any kind of template arguments. - if Match(r'^\s*>', remainder): - return False - - # Don't warn on assignments to function pointers, but keep warnings for - # unnamed parameters to pure virtual functions. Note that this pattern - # will also pass on assignments of "0" to function pointers, but the - # preferred values for those would be "nullptr" or "NULL". - matched_zero = Match(r'^\s=\s*(\S+)\s*;', remainder) - if matched_zero and matched_zero.group(1) != '0': - return False - - # Don't warn on function pointer declarations. For this we need - # to check what came before the "(type)" string. - if Match(r'.*\)\s*$', line[0:match.start(0)]): - return False - - # Don't warn if the parameter is named with block comments, e.g.: - # Function(int /*unused_param*/); - raw_line = clean_lines.raw_lines[linenum] - if '/*' in raw_line: - return False - - # Passed all filters, issue warning here. - error(filename, linenum, 'readability/function', 3, - 'All parameters should be named in a function') - return True + return False # At this point, all that should be left is actual casts. error(filename, linenum, 'readability/casting', 4, @@ -5559,12 +5520,15 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('', ('numeric_limits',)), ('', ('list',)), ('', ('map', 'multimap',)), - ('', ('allocator',)), + ('', ('allocator', 'make_shared', 'make_unique', 'shared_ptr', + 'unique_ptr', 'weak_ptr')), ('', ('queue', 'priority_queue',)), ('', ('set', 'multiset',)), ('', ('stack',)), ('', ('char_traits', 'basic_string',)), ('', ('tuple',)), + ('', ('unordered_map', 'unordered_multimap')), + ('', ('unordered_set', 'unordered_multiset')), ('', ('pair',)), ('', ('vector',)), @@ -5575,18 +5539,26 @@ _HEADERS_CONTAINING_TEMPLATES = ( ('', ('slist',)), ) +_HEADERS_MAYBE_TEMPLATES = ( + ('', ('copy', 'max', 'min', 'min_element', 'sort', + 'transform', + )), + ('', ('forward', 'make_pair', 'move', 'swap')), + ) + _RE_PATTERN_STRING = re.compile(r'\bstring\b') -_re_pattern_algorithm_header = [] -for _template in ('copy', 'max', 'min', 'min_element', 'sort', 'swap', - 'transform'): - # Match max(..., ...), max(..., ...), but not foo->max, foo.max or - # type::max(). - _re_pattern_algorithm_header.append( - (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), - _template, - '')) +_re_pattern_headers_maybe_templates = [] +for _header, _templates in _HEADERS_MAYBE_TEMPLATES: + for _template in _templates: + # Match max(..., ...), max(..., ...), but not foo->max, foo.max or + # type::max(). + _re_pattern_headers_maybe_templates.append( + (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), + _template, + _header)) +# Other scripts may reach in and modify this pattern. _re_pattern_templates = [] for _header, _templates in _HEADERS_CONTAINING_TEMPLATES: for _template in _templates: @@ -5634,10 +5606,9 @@ def FilesBelongToSameModule(filename_cc, filename_h): return (False, '') filename_cc = filename_cc[:-(len(fileinfo_cc.Extension()))] - for suffix in _test_suffixes: - if filename_cc.endswith(suffix): - filename_cc = filename_cc[:-len(suffix)] - break + matched_test_suffix = Search(_TEST_FILE_SUFFIX, fileinfo_cc.BaseName()) + if matched_test_suffix: + filename_cc = filename_cc[:-len(matched_test_suffix.group(1))] filename_cc = filename_cc.replace('/public/', '/') filename_cc = filename_cc.replace('/internal/', '/') @@ -5717,7 +5688,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, if prefix.endswith('std::') or not prefix.endswith('::'): required[''] = (linenum, 'string') - for pattern, template, header in _re_pattern_algorithm_header: + for pattern, template, header in _re_pattern_headers_maybe_templates: if pattern.search(line): required[header] = (linenum, template) @@ -5726,8 +5697,13 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, continue for pattern, template, header in _re_pattern_templates: - if pattern.search(line): - required[header] = (linenum, template) + matched = pattern.search(line) + if matched: + # Don't warn about IWYU in non-STL namespaces: + # (We check only the first match per line; good enough.) + prefix = line[:matched.start()] + if prefix.endswith('std::') or not prefix.endswith('::'): + required[header] = (linenum, template) # The policy is that if you #include something in foo.h you don't need to # include it again in foo.cc. Here, we will look at possible includes. @@ -5770,7 +5746,7 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error, return # All the lines have been processed, report the errors found. - for required_header_unstripped in required: + for required_header_unstripped in sorted(required, key=required.__getitem__): template = required[required_header_unstripped][1] if required_header_unstripped.strip('<>"') not in include_dict: error(filename, required[required_header_unstripped][0], @@ -5802,31 +5778,6 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): ' OR use pair directly OR if appropriate, construct a pair directly') -def CheckDefaultLambdaCaptures(filename, clean_lines, linenum, error): - """Check that default lambda captures are not used. - - 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] - - # A lambda introducer specifies a default capture if it starts with "[=" - # or if it starts with "[&" _not_ followed by an identifier. - match = Match(r'^(.*)\[\s*(?:=|&[^\w])', line) - if match: - # Found a potential error, check what comes after the lambda-introducer. - # If it's not open parenthesis (for lambda-declarator) or open brace - # (for compound-statement), it's not a lambda. - line, _, pos = CloseExpression(clean_lines, linenum, len(match.group(1))) - if pos >= 0 and Match(r'^\s*[{(]', line[pos:]): - error(filename, linenum, 'build/c++11', - 4, # 4 = high confidence - 'Default lambda captures are an unapproved C++ feature.') - - def CheckRedundantVirtual(filename, clean_lines, linenum, error): """Check if line contains a redundant "virtual" function-specifier. @@ -5934,11 +5885,9 @@ def IsBlockInNameSpace(nesting_state, is_forward_declaration): Whether or not the new block is directly in a namespace. """ if is_forward_declaration: - if len(nesting_state.stack) >= 1 and ( - isinstance(nesting_state.stack[-1], _NamespaceInfo)): - return True - else: - return False + return len(nesting_state.stack) >= 1 and ( + isinstance(nesting_state.stack[-1], _NamespaceInfo)) + return (len(nesting_state.stack) > 1 and nesting_state.stack[-1].check_namespace_indentation and @@ -5988,7 +5937,7 @@ def CheckItemIndentationInNamespace(filename, raw_lines_no_comments, linenum, def ProcessLine(filename, file_extension, clean_lines, line, include_state, function_state, nesting_state, error, - extra_check_functions=[]): + extra_check_functions=None): """Processes a single line in the file. Args: @@ -6025,11 +5974,11 @@ def ProcessLine(filename, file_extension, clean_lines, line, CheckPosixThreading(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) CheckMakePairUsesDeduction(filename, clean_lines, line, error) - CheckDefaultLambdaCaptures(filename, clean_lines, line, error) CheckRedundantVirtual(filename, clean_lines, line, error) CheckRedundantOverrideOrFinal(filename, clean_lines, line, error) - for check_fn in extra_check_functions: - check_fn(filename, clean_lines, line, error) + if extra_check_functions: + for check_fn in extra_check_functions: + check_fn(filename, clean_lines, line, error) def FlagCxx11Features(filename, clean_lines, linenum, error): """Flag those c++11 features that we only allow in certain places. @@ -6042,8 +5991,14 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): """ line = clean_lines.elided[linenum] - # Flag unapproved C++11 headers. include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++ TR1 headers. + if include and include.group(1).startswith('tr1/'): + error(filename, linenum, 'build/c++tr1', 5, + ('C++ TR1 headers such as <%s> are unapproved.') % include.group(1)) + + # Flag unapproved C++11 headers. if include and include.group(1) in ('cfenv', 'condition_variable', 'fenv.h', @@ -6077,8 +6032,27 @@ def FlagCxx11Features(filename, clean_lines, linenum, error): 'they may let you use it.') % top_name) +def FlagCxx14Features(filename, clean_lines, linenum, error): + """Flag those C++14 features that we restrict. + + 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] + + include = Match(r'\s*#\s*include\s+[<"]([^<"]+)[">]', line) + + # Flag unapproved C++14 headers. + if include and include.group(1) in ('scoped_allocator', 'shared_mutex'): + error(filename, linenum, 'build/c++14', 5, + ('<%s> is an unapproved C++14 header.') % include.group(1)) + + def ProcessFileData(filename, file_extension, lines, error, - extra_check_functions=[]): + extra_check_functions=None): """Performs lint checks and reports any errors to the given error function. Args: @@ -6102,7 +6076,7 @@ def ProcessFileData(filename, file_extension, lines, error, ResetNolintSuppressions() CheckForCopyright(filename, lines, error) - + ProcessGlobalSuppresions(lines) RemoveMultiLineComments(filename, lines, error) clean_lines = CleansedLines(lines) @@ -6119,7 +6093,7 @@ def ProcessFileData(filename, file_extension, lines, error, CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) # Check that the .cc file has included its header if it exists. - if file_extension in GetNonHeaderExtensions(): + if _IsSourceExtension(file_extension): CheckHeaderFileIncluded(filename, include_state, error) # We check here rather than inside ProcessLine so that we see raw @@ -6175,17 +6149,16 @@ def ProcessConfigOverrides(filename): if base_name: pattern = re.compile(val) if pattern.match(base_name): - sys.stderr.write('Ignoring "%s": file excluded by "%s". ' - 'File path component "%s" matches ' - 'pattern "%s"\n' % - (filename, cfg_file, base_name, val)) + _cpplint_state.PrintInfo('Ignoring "%s": file excluded by ' + '"%s". File path component "%s" matches pattern "%s"\n' % + (filename, cfg_file, base_name, val)) return False elif name == 'linelength': global _line_length try: _line_length = int(val) except ValueError: - sys.stderr.write('Line length must be numeric.') + _cpplint_state.PrintError('Line length must be numeric.') elif name == 'extensions': global _valid_extensions try: @@ -6204,25 +6177,28 @@ def ProcessConfigOverrides(filename): sys.stderr.write('Extensions should be a comma-separated list of values;' 'for example: extensions=hpp,cpp\n' 'This could not be parsed: "%s"' % (val,)) + elif name == 'root': + global _root + _root = val else: - sys.stderr.write( + _cpplint_state.PrintError( 'Invalid configuration option (%s) in file %s\n' % (name, cfg_file)) except IOError: - sys.stderr.write( + _cpplint_state.PrintError( "Skipping config file '%s': Can't open for reading\n" % cfg_file) keep_looking = False # Apply all the accumulated filters in reverse order (top-level directory # config options having the least priority). - for filter in reversed(cfg_filters): - _AddFilters(filter) + for cfg_filter in reversed(cfg_filters): + _AddFilters(cfg_filter) return True -def ProcessFile(filename, vlevel, extra_check_functions=[]): +def ProcessFile(filename, vlevel, extra_check_functions=None): """Does google-lint on a single file. Args: @@ -6271,7 +6247,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): lf_lines.append(linenum + 1) except IOError: - sys.stderr.write( + _cpplint_state.PrintError( "Skipping input '%s': Can't open for reading\n" % filename) _RestoreFilters() return @@ -6282,7 +6258,7 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): # When reading from stdin, the extension is unknown, so no cpplint tests # should rely on the extension. if filename != '-' and file_extension not in GetAllExtensions(): - sys.stderr.write('Ignoring %s; not a valid file name ' + _cpplint_state.PrintError('Ignoring %s; not a valid file name ' '(%s)\n' % (filename, ', '.join(GetAllExtensions()))) else: ProcessFileData(filename, file_extension, lines, Error, @@ -6305,8 +6281,8 @@ def ProcessFile(filename, vlevel, extra_check_functions=[]): for linenum in crlf_lines: Error(filename, linenum, 'whitespace/newline', 1, 'Unexpected \\r (^M) found; better to use only \\n') - if vlevel > 0: - sys.stderr.write('Done processing %s\n' % filename) + + _cpplint_state.PrintInfo('Done processing %s\n' % filename) _RestoreFilters() @@ -6349,9 +6325,13 @@ def ParseArguments(args): 'counting=', 'filter=', 'root=', + 'repository=', 'linelength=', 'extensions=', - 'headers=']) + 'exclude=', + 'headers=', + 'quiet', + 'recursive']) except getopt.GetoptError: PrintUsage('Invalid arguments.') @@ -6359,13 +6339,15 @@ def ParseArguments(args): output_format = _OutputFormat() filters = '' counting_style = '' + recursive = False for (opt, val) in opts: if opt == '--help': PrintUsage(None) elif opt == '--output': - if val not in ('emacs', 'vs7', 'eclipse'): - PrintUsage('The only allowed output formats are emacs, vs7 and eclipse.') + if val not in ('emacs', 'vs7', 'eclipse', 'junit'): + PrintUsage('The only allowed output formats are emacs, vs7, eclipse ' + 'and junit.') output_format = val elif opt == '--verbose': verbosity = int(val) @@ -6380,16 +6362,24 @@ def ParseArguments(args): elif opt == '--root': global _root _root = val + elif opt == '--repository': + global _repository + _repository = val elif opt == '--linelength': global _line_length try: - _line_length = int(val) + _line_length = int(val) except ValueError: - PrintUsage('Line length must be digits.') + PrintUsage('Line length must be digits.') + elif opt == '--exclude': + global _excludes + if not _excludes: + _excludes = set() + _excludes.update(glob.glob(val)) elif opt == '--extensions': global _valid_extensions try: - _valid_extensions = set(val.split(',')) + _valid_extensions = set(val.split(',')) except ValueError: PrintUsage('Extensions must be comma seperated list.') elif opt == '--headers': @@ -6397,11 +6387,22 @@ def ParseArguments(args): try: _header_extensions = set(val.split(',')) except ValueError: - PrintUsage('Extensions must be comma seperated list.') + PrintUsage('Extensions must be comma seperated list.') + elif opt == '--recursive': + recursive = True + elif opt == '--quiet': + global _quiet + _quiet = True if not filenames: PrintUsage('No files were specified.') + if recursive: + filenames = _ExpandDirectories(filenames) + + if _excludes: + filenames = _FilterExcludedFiles(filenames) + _SetOutputFormat(output_format) _SetVerboseLevel(verbosity) _SetFilters(filters) @@ -6409,6 +6410,44 @@ def ParseArguments(args): return filenames +def _ExpandDirectories(filenames): + """Searches a list of filenames and replaces directories in the list with + all files descending from those directories. Files with extensions not in + the valid extensions list are excluded. + + Args: + filenames: A list of files or directories + + Returns: + A list of all files that are members of filenames or descended from a + directory in filenames + """ + expanded = set() + for filename in filenames: + if not os.path.isdir(filename): + expanded.add(filename) + continue + + for root, _, files in os.walk(filename): + for loopfile in files: + fullname = os.path.join(root, loopfile) + if fullname.startswith('.' + os.path.sep): + fullname = fullname[len('.' + os.path.sep):] + expanded.add(fullname) + + filtered = [] + for filename in expanded: + if os.path.splitext(filename)[1][1:] in GetAllExtensions(): + filtered.append(filename) + + return filtered + +def _FilterExcludedFiles(filenames): + """Filters out files listed in the --exclude command line switch. File paths + in the switch are evaluated relative to the current working directory + """ + exclude_paths = [os.path.abspath(f) for f in _excludes] + return [f for f in filenames if os.path.abspath(f) not in exclude_paths] def main(): filenames = ParseArguments(sys.argv[1:]) @@ -6416,12 +6455,16 @@ def main(): try: # Change stderr to write with replacement characters so we don't die # if we try to print something containing non-ASCII characters. - sys.stderr = codecs.StreamReader(sys.stderr, - 'replace') + sys.stderr = codecs.StreamReader(sys.stderr, 'replace') + _cpplint_state.ResetErrorCounts() for filename in filenames: ProcessFile(filename, _cpplint_state.verbose_level) _cpplint_state.PrintErrorCounts() + + if _cpplint_state.output_format == 'junit': + sys.stderr.write(_cpplint_state.FormatJUnitXML()) + finally: sys.stderr = backup_err From 0323edddd8c7b5df035022af75d943054cd8a32e Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Thu, 28 Jul 2016 23:09:05 +0200 Subject: [PATCH 2/6] Less verbose output of running make --- scripts/Makefile | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/Makefile b/scripts/Makefile index ada1344..39c53c3 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -21,7 +21,7 @@ cpplint-all \ check-badchars $(BUILD_DIR): - mkdir -p $(BUILD_DIR) + @mkdir -p $(BUILD_DIR) #### clean: remove all files generated by the productive rules .PHONY: clean @@ -54,28 +54,28 @@ show-diff: nodejs/node_modules/remark nodejs/remark/.remarkrc $(SOURCEPATH) $(BU .PHONY: check-references check-references: $(SOURCEPATH) $(BUILD_DIR) Makefile ## check references unique - rm -f $(BUILD_DIR)/$(SOURCEFILE).uniq - grep -oP '(?<= $(BUILD_DIR)/$(SOURCEFILE).uniq + @rm -f $(BUILD_DIR)/$(SOURCEFILE).uniq + @grep -oP '(?<= $(BUILD_DIR)/$(SOURCEFILE).uniq ## check if output has data - if [ -s "build/CppCoreGuidelines.md.uniq" ]; then echo 'Found duplicate anchors:'; cat $(BUILD_DIR)/$(SOURCEFILE).uniq; false; fi + @if [ -s "build/CppCoreGuidelines.md.uniq" ]; then echo 'Found duplicate anchors:'; cat $(BUILD_DIR)/$(SOURCEFILE).uniq; false; fi .PHONY: check-notabs check-notabs: $(SOURCEPATH) $(BUILD_DIR) Makefile # find lines with tabs # old file still might be around - rm -f $(BUILD_DIR)/CppCoreGuidelines.md.tabs + @rm -f $(BUILD_DIR)/CppCoreGuidelines.md.tabs # print file, add line numbers, remove tabs from nl tool, grep for remaining tabs, replace with stars - cat ../CppCoreGuidelines.md | nl -ba | sed -s 's/\(^[^\t]*\)\t/\1--/g' | grep -P '\t' | sed -s 's/\t/\*\*\*\*/g' > $(BUILD_DIR)/CppCoreGuidelines.md.tabs - if [ -s $(BUILD_DIR)/CppCoreGuidelines.md.tabs ]; then echo 'Warning: Tabs found:'; cat $(BUILD_DIR)/CppCoreGuidelines.md.tabs; false; fi; + @cat ../CppCoreGuidelines.md | nl -ba | sed -s 's/\(^[^\t]*\)\t/\1--/g' | grep -P '\t' | sed -s 's/\t/\*\*\*\*/g' > $(BUILD_DIR)/CppCoreGuidelines.md.tabs + @if [ -s $(BUILD_DIR)/CppCoreGuidelines.md.tabs ]; then echo 'Warning: Tabs found:'; cat $(BUILD_DIR)/CppCoreGuidelines.md.tabs; false; fi; .PHONY: check-badchars check-badchars: $(SOURCEPATH) $(BUILD_DIR) Makefile # find lines with tabs # old file still might be around - rm -f $(BUILD_DIR)/CppCoreGuidelines.md.badchars + @rm -f $(BUILD_DIR)/CppCoreGuidelines.md.badchars # print file, add line numbers, grep for bad chars - cat ../CppCoreGuidelines.md | nl -ba | grep -P '’|‘|”|“|¸|–|…|¦' > $(BUILD_DIR)/CppCoreGuidelines.md.badchars || true - if [ -s $(BUILD_DIR)/CppCoreGuidelines.md.badchars ]; then echo 'Warning: Undesired chars (–’‘“”¸…¦) found, use straight quotes instead:'; cat $(BUILD_DIR)/CppCoreGuidelines.md.badchars; false; fi; + @cat ../CppCoreGuidelines.md | nl -ba | grep -P '’|‘|”|“|¸|–|…|¦' > $(BUILD_DIR)/CppCoreGuidelines.md.badchars || true + @if [ -s $(BUILD_DIR)/CppCoreGuidelines.md.badchars ]; then echo 'Warning: Undesired chars (–’‘“”¸…¦) found, use straight quotes instead:'; cat $(BUILD_DIR)/CppCoreGuidelines.md.badchars; false; fi; @@ -88,7 +88,7 @@ cpplint-all: $(BUILD_DIR)/codeblocks $(BUILD_DIR)/Makefile python/Makefile.in #### generic makefile for sourceblocks (need to be evaluated after c++ file generation) $(BUILD_DIR)/Makefile: python/Makefile.in - cp python/Makefile.in $(BUILD_DIR)/codeblocks/Makefile + @cp python/Makefile.in $(BUILD_DIR)/codeblocks/Makefile #### split md file into plain text and code @@ -98,7 +98,7 @@ $(BUILD_DIR)/plain.txt: splitfile .PHONY: splitfile splitfile: $(SOURCEPATH) ./python/md-split.py - python ./python/md-split.py $(SOURCEPATH) $(BUILD_DIR)/plain.txt $(BUILD_DIR)/codeblocks + @python ./python/md-split.py $(SOURCEPATH) $(BUILD_DIR)/plain.txt $(BUILD_DIR)/codeblocks #### install npm modules # install/update npm dependencies defined in file package.json From db98d5f8b39e46d3eb9f9b4832b1f6ee430dcc06 Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Fri, 29 Jul 2016 00:06:10 +0200 Subject: [PATCH 3/6] Upgrade remark (markdown linter) --- scripts/nodejs/package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/nodejs/package.json b/scripts/nodejs/package.json index 2b728e8..133637b 100644 --- a/scripts/nodejs/package.json +++ b/scripts/nodejs/package.json @@ -4,9 +4,9 @@ "description": "CppCoreGuidelines Check", "private": true, "dependencies": { - "remark": "^4.2.0", - "remark-lint": "^3.2.0", + "remark": "^4.2.2", + "remark-lint": "^4.0.2", "remark-lint-sentence-newline": "^2.0.0", - "remark-validate-links": "^3.0.0" + "remark-validate-links": "^4.1.0" } } From 6e1599f6f995cd269ed3e1b5225e2da1039eb8c8 Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Thu, 11 Aug 2016 10:54:08 +0200 Subject: [PATCH 4/6] style fixes --- CppCoreGuidelines.md | 203 ++++++++++++++++++++++--------------------- 1 file changed, 105 insertions(+), 98 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 8fc4bf8..a95d734 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -592,7 +592,7 @@ This example is easily simplified ##### Example void read(int* p, int n); // read max n integers into *p - + int a[100]; read(a, 1000); // bad @@ -802,7 +802,6 @@ Excess checking can be costly. There are cases where checking early is dumb because you may not ever need the value, or may only need part of the value that is more easily checked than the whole. Similarly, don't add validity checks that change the asymptotic behavior of your interface (e.g., don't add a `O(n)` check to an interface with an average complexity of `O(1)`). class Jet { // Physics says: e * e < x * x + y * y + z * z - float x; float y; float z; @@ -823,7 +822,7 @@ There are cases where checking early is dumb because you may not ever need the v ??? }; -The physical law for a jet (`e*e < x*x + y*y + z*z`) is not an invariant because of the possibility for measurement errors. +The physical law for a jet (`e * e < x * x + y * y + z * z`) is not an invariant because of the possibility for measurement errors. ??? @@ -983,17 +982,17 @@ Messy, low-level code breads more such code. ##### Example int sz = 100; - int* p = (int*) malloc(sizeof(int)*sz); + int* p = (int*) malloc(sizeof(int) * sz); // ... - if (count==sz) - p = (int*) realloc(p,sizeof(int)*sz*2); + if (count == sz) + p = (int*) realloc(p, sizeof(int) * sz * 2); // ... This is low-level, verbose, and error-prone. Instead, we could use `vector`: vector v(100); - + v.push_back(yet_another)int); ##### Note @@ -1554,7 +1553,7 @@ A facility [structured bindings](http://www.open-std.org/jtc1/sc22/wg21/docs/pap // ... handle the error or exit ... } // ... use val ... - + ##### Note @@ -1791,7 +1790,7 @@ To really reduce the number of arguments, we need to bundle the arguments into h Grouping arguments into "bundles" is a general technique to reduce the number of arguments and to increase the opportunities for checking. Alternatively, we could use concepts (as defined by the ISO TS) to define the notion of three types that must be usable for merging: - + Mergeable{In1 In2, Out} OutputIterator merge(In1 r1, In2 r2, Out result); @@ -2101,7 +2100,6 @@ Consider: // simpleFunc: takes a value and calculates the expected ASIC output, // given the two mode flags. { - double intermediate; if (flag1 > 0) { intermediate = func1(val); @@ -2118,9 +2116,10 @@ Consider: intermediate = func2(intermediate); } switch (flag2 / 10) { - case 1: if (flag1 == -1) return finalize(intermediate, 1.171); break; + case 1: if (flag1 == -1) return finalize(intermediate, 1.171); + break; case 2: return finalize(intermediate, 13.1); - default: ; + default: break; } return finalize(intermediate, 0.); } @@ -2247,7 +2246,7 @@ Specifying `inline` encourages the compiler to do a better job. ##### Example - inline string cat(const string& s, const string& s2) { return s+s2; } + inline string cat(const string& s, const string& s2) { return s + s2; } **Exception**: Do not put an `inline` function in what is meant to be a stable interface unless you are really sure that it will not change. An inline function is part of the ABI. @@ -2344,13 +2343,13 @@ Passing a shared smart pointer (e.g., `std::shared_ptr`) implies a run-time cost void g(unique_ptr); // can only accept ints for which you are willing to share ownership - void g(shared_ptr); + void g(shared_ptr); // doesn't change ownership, but requires a particular ownership of the caller void h(const unique_ptr&); // accepts any int - void h(int&); + void h(int&); ##### Example, bad @@ -2697,7 +2696,7 @@ Explicitly passing an in-out parameter back out again as a return value is often For example: istream& operator>>(istream& is, string& s); // much like std::operator>>() - + for (string s; cin >> s; ) { // do something with line } @@ -2733,7 +2732,7 @@ However, we prefer to be explicit, rather than subtle. In many cases it may be useful to return a specific, user-defined "Value or error" type. For example: - struct + struct The overly-generic `pair` and `tuple` should be used only when the value returned represents to independent entities rather than an abstraction. @@ -2769,21 +2768,21 @@ It complicates checking and tool support. void use(int* p, int n, char* s, int* q) { - p[n-1] = 666; // Bad: we don't know if p points to n elements; - // assume it does not or use span - cout << s; // Bad: we don't know if that s points to a zero-terminated array of char; - // assume it does not or use zstring - delete q; // Bad: we don't know if *q is allocated on the free store; - // assume it does not or use owner + p[n - 1] = 666; // Bad: we don't know if p points to n elements; + // assume it does not or use span + cout << s; // Bad: we don't know if that s points to a zero-terminated array of char; + // assume it does not or use zstring + delete q; // Bad: we don't know if *q is allocated on the free store; + // assume it does not or use owner } better void use2(span p, zstring s, owner q) { - p[p.size()-1] = 666; // OK, a range error can be caught + p[p.size() - 1] = 666; // OK, a range error can be caught cout << s; // OK - delete q; // OK + delete q; // OK } ##### Note @@ -3257,7 +3256,7 @@ principle of "do as the ints do." ##### Note Historically there was some guidance to make the assignment operator return `const T&`. -This was primarily to avoid code of the form `(a=b)=c` -- such code is not common enough to warrant violating consistency with standard types. +This was primarily to avoid code of the form `(a = b) = c` -- such code is not common enough to warrant violating consistency with standard types. ##### Example @@ -3338,7 +3337,7 @@ There is not a choice when a set of functions are used to do a semantically equi ##### See also - [Default arguments for virtual functions](#Rf-virtual-default-arg} +[Default arguments for virtual functions](#Rf-virtual-default-arg} ##### Enforcement @@ -3709,7 +3708,7 @@ Flag protected data. One ideal for a class is to be a regular type. That means roughly "behaves like an `int`." A concrete type is the simplest kind of class. A value of regular type can be copied and the result of a copy is an independent object with the same value as the original. -If a concrete type has both `=` and `==`, `a=b` should result in `a == b` being `true`. +If a concrete type has both `=` and `==`, `a = b` should result in `a == b` being `true`. Concrete classes without assignment and equality can be defined, but they are (and should be) rare. The C++ built-in types are regular, and so are standard-library classes, such as `string`, `vector`, and `map`. Concrete types are also often referred to as value types to distinguish them from types uses as part of a hierarchy. @@ -3793,7 +3792,7 @@ Regular types are easier to understand and reason about than types that are not b2.name = "the other bundle"; if (b1 == b2) error("No!"); -In particular, if a concrete type has an assignment also give it an equals operator so that `a=b` implies `a == b`. +In particular, if a concrete type has an assignment also give it an equals operator so that `a = b` implies `a == b`. ##### Enforcement @@ -4247,7 +4246,7 @@ Independently of whether `Handle` owns its `Shape`, we must consider the default Handle y {*new Triangle{p1, p2, p3}}; x = y; // the default assignment will try *x.s = *y.s -That `x=y` is highly suspect. +That `x = y` is highly suspect. Assigning a `Triangle` to a `Circle`? Unless `Shape` has its [copy assignment `=deleted`](#Rc-copy-virtual), only the `Shape` part of `Triangle` is copied into the `Circle`. @@ -5092,7 +5091,7 @@ See [copy constructor vs. `clone()`](#Rc-copy-virtual). ##### Reason -That is the generally assumed semantics. After `x=y`, we should have `x == y`. +That is the generally assumed semantics. After `x = y`, we should have `x == y`. After a copy `x` and `y` can be independent objects (value semantics, the way non-pointer built-in types and the standard-library types work) or refer to a shared object (pointer semantics, the way pointers work). ##### Example @@ -5163,7 +5162,7 @@ Prefer copy semantics unless you are building a "smart pointer". Value semantics ##### Reason -If `x=x` changes the value of `x`, people will be surprised and bad errors will occur (often including leaks). +If `x = x` changes the value of `x`, people will be surprised and bad errors will occur (often including leaks). ##### Example @@ -5246,7 +5245,7 @@ Equivalent to what is done for [copy-assignment](#Rc-copy-assignment). ##### Reason That is the generally assumed semantics. -After `y=std::move(x)` the value of `y` should be the value `x` had and `x` should be in a valid state. +After `y = std::move(x)` the value of `y` should be the value `x` had and `x` should be in a valid state. ##### Example @@ -5324,7 +5323,7 @@ The one-in-a-million argument against `if (this == &a) return *this;` tests from ##### Note -There is no know general way of avoiding a `if (this == &a) return *this;` test for a move assignment and still get a correct answer (i.e., after `x=x` the value of `x` is unchanged). +There is no know general way of avoiding a `if (this == &a) return *this;` test for a move assignment and still get a correct answer (i.e., after `x = x` the value of `x` is unchanged). ##### Note @@ -5668,14 +5667,18 @@ Asymmetric treatment of operands is surprising and a source of errors where conv int number; }; - bool operator==(const X& a, const X& b) noexcept { return a.name == b.name && a.number == b.number; } + bool operator==(const X& a, const X& b) noexcept { + return a.name == b.name && a.number == b.number; + } ##### Example, bad class B { string name; int number; - bool operator==(const B& a) const { return name == a.name && number == a.number; } + bool operator==(const B& a) const { + return name == a.name && number == a.number; + } // ... }; @@ -6082,7 +6085,7 @@ by making useful operations available for implementers of related new operations A pure interface class is simply a set of pure virtual functions; see [I.25](#Ri-abstract). -In early OOP (e.g., in the 1980s and 1990s), implementation inheritance and interface inheritance were often mixed +In early OOP (e.g., in the 1980s and 1990s), implementation inheritance and interface inheritance were often mixed and bad habits die hard. Even now, mixtures are not uncommon in old code bases and in old-style teaching material. @@ -6099,7 +6102,7 @@ The importance of keeping the two kinds of inheritance increases class Shape { // BAD, mixed interface and implementation public: Shape(); - Shape(Point ce = {0,0}, Color co = none): cent{ce}, col {co} { /* ... */} + Shape(Point ce = {0, 0}, Color co = none): cent{ce}, col {co} { /* ... */} Point center() const { return cent; } Color color() const { return col; } @@ -6164,7 +6167,7 @@ Note that a pure interface rarely have constructors: there is nothing to constru class Circle : public Shape { public: Circle(Point c, int r, Color c) :cent{c}, rad{r}, col{c} { /* ... */ } - + Point center() const override { return cent; } Color color() const override { return col; } @@ -6503,7 +6506,8 @@ Capping an individual virtual function with `final` is error-prone as that `fina class Widget { /* ... */ }; - class My_widget final : public Widget { /* ... */ }; // nobody will ever want to improve My_widget (or so you thought) + // nobody will ever want to improve My_widget (or so you thought) + class My_widget final : public Widget { /* ... */ }; class My_improved_widget : public My_widget { /* ... */ }; // error: can't do that @@ -6876,7 +6880,7 @@ Minimize surprises. // ... X& operator=(const X&); // member function defining assignment friend bool operator==(const X&, const X&); // == needs access to representation - // after a=b we have a==b + // after a = b we have a == b // ... }; @@ -7409,7 +7413,7 @@ Convenience of use and avoidance of errors. Day operator++(Day& d) { - return d==sun?mon:Day{++d}; + return d == sun ? mon : Day{++d}; } Day today = sat; @@ -7435,7 +7439,7 @@ Avoid clashes with macros. // productinfo.h // The following define product subtypes based on color - + enum class Productinfo { RED, PURPLE, BLUE }; // syntax error ##### Enforcement @@ -7458,7 +7462,7 @@ Such code is not uncommon in code written before there were convenient alternati Use `constexpr` values instead. For example: - constexpr int red = 0x,FF0000; + constexpr int red = 0xFF0000; constexpr short scale = 4; constexpr bool signed = true; @@ -7479,7 +7483,7 @@ The default is the easiest to read and write. enum class Direction : char { n, s, e, w, ne, nw, se, sw }; // underlying type saves space - + enum class Webcolor : int { red = 0xFF0000, green = 0x00FF00, blue = 0x0000FF }; // underlying type is redundant @@ -7491,7 +7495,7 @@ Specifying the underlying type is necessary in forward declarations of enumerati enum Flags : char; void f(Flags); - + // .... enum flags : char { /* ... */ }; @@ -7626,8 +7630,8 @@ What is `Port`? A handy wrapper that encapsulates the resource: operator PortHandle() { return port; } // port handles can't usually be cloned, so disable copying and assignment if necessary - Port(const Port&) =delete; - Port& operator=(const Port&) =delete; + Port(const Port&) = delete; + Port& operator=(const Port&) = delete; }; ##### Note @@ -8689,7 +8693,8 @@ Here, there is a chance that the reader knows what `trim_tail` means and that th Argument names of large functions are de facto non-local and should be meaningful: void complicated_algorithm(vector& vr, const vector& vi, map& out) - // read from events in vr (marking used Records) for the indices in vi placing (name, index) pairs into out + // read from events in vr (marking used Records) for the indices in + // vi placing (name, index) pairs into out { // ... 500 lines of code using vr, vi, and out ... } @@ -9416,13 +9421,15 @@ Requires messy cast-and-macro-laden code to get working right. #include - void error(int severity ...) // "severity" followed by a zero-terminated list of char*s; write the C-style strings to cerr + // "severity" followed by a zero-terminated list of char*s; write the C-style strings to cerr + void error(int severity ...) { va_list ap; // a magic type for holding arguments va_start(ap, severity); // arg startup: "severity" is the first argument of error() for (;;) { - char* p = va_arg(ap, char*); // treat the next var as a char*; no checking: a cast in disguise + // treat the next var as a char*; no checking: a cast in disguise + char* p = va_arg(ap, char*); if (p == nullptr) break; cerr << p << ' '; } @@ -11139,14 +11146,14 @@ This is asking for deadlock: Instead, use `lock()`: // thread 1 - lock_guard lck1(m1,defer_lock); - lock_guard lck2(m2,defer_lock); - lock(lck1,lck2); + lock_guard lck1(m1, defer_lock); + lock_guard lck2(m2, defer_lock); + lock(lck1, lck2); // thread 2 - lock_guard lck2(m2,defer_lock); - lock_guard lck1(m1,defer_lock); - lock(lck2,lck1); + lock_guard lck2(m2, defer_lock); + lock_guard lck1(m1, defer_lock); + lock(lck2, lck1); Here, the writers of `thread1` and `thread2` are still not agreeing on the order of the `mutex`es, but order no longer matters. @@ -11157,7 +11164,7 @@ In real code, `mutex`es are not always conveniently acquired on consecutive line I'm really looking forward to be able to write plain - lock_guard lck1(m1,defer_lock); + lock_guard lck1(m1, defer_lock); and have the `mutex` type deduced. @@ -11203,7 +11210,7 @@ Such problem cal often be solved by using a `recursive_mutex`. For example: // ... } -If, as it is likely, `f()` invokes operations on `*this`, we must make sure that the object's invariant holds before the call. +If, as it is likely, `f()` invokes operations on `*this`, we must make sure that the object's invariant holds before the call. ##### Enforcement @@ -11231,11 +11238,11 @@ If a `thread` joins, we can safely pass pointers to objects in the scope of the void some_fct(int* p) { int x = 77; - raii_thread t0(f,&x); // OK - raii_thread t1(f,p); // OK - raii_thread t2(f,&glob); // OK + raii_thread t0(f, &x); // OK + raii_thread t1(f, p); // OK + raii_thread t2(f, &glob); // OK auto q = make_unique(99); - raii_thread t3(f,q.get()); // OK + raii_thread t3(f, q.get()); // OK // ... } @@ -11452,12 +11459,12 @@ Defining "small amount" precisely is impossible. ##### Example string modify1(string); - void modify2(shared_ptr); void fct(string& s) { - auto res = async(modify1,string); - async(modify2,&s); + auto res = async(modify1, s); + async(modify2, &s); } The call of `modify1` involves copying two `string` values; the call of `modify2` does not. @@ -11528,8 +11535,8 @@ Thread creation is expensive. void master(istream& is) { - for (Message m; is>>m; ) - run_list.push_back(new thread(worker,m);} + for (Message m; is >> m; ) + run_list.push_back(new thread(worker, m)); } This spawns a `thread` per message, and the `run_list` is presumably managed to destroy those tasks once they are finished. @@ -11546,7 +11553,7 @@ Instead, we could have a set of pre-created worker threads processing the messag void worker() { - for (Message m; m=work.get(); ) { + for (Message m; m = work.get(); ) { // process } } @@ -11698,7 +11705,7 @@ An unnamed local objects is a temporary that immediately goes out of scope. unique_lock(m1); lock_guard {m2}; - lock(m1,m2); + lock(m1, m2); This looks innocent enough, but it isn't. @@ -12086,7 +12093,7 @@ The `File_handle` constructor might defined like this: :f{fopen(name.c_str(), mode.c_str())} { if (!f) - throw runtime_error{"File_handle: could not open "S-+ name + " as " + mode"} + throw runtime_error{"File_handle: could not open " + name + " as " + mode}; } ##### Note @@ -12097,7 +12104,7 @@ Examples: * A precondition that cannot be met * A constructor that cannot construct an object (failure to establish its class's [invariant](#Rc-struct)) -* An out-of-range error (e.g., `v[v.size()] =7`) +* An out-of-range error (e.g., `v[v.size()] = 7`) * Inability to acquire a resource (e.g., the network is down) In contrast, termination of an ordinary loop is not exceptional. @@ -12132,7 +12139,7 @@ C++ implementations tend to be optimized based on the assumption that exceptions int find_index(vector& vec, const string& x) { try { - for (int i =0; i < vec.size(); ++i) + for (int i = 0; i < vec.size(); ++i) if (vec[i] == x) throw i; // found x } catch (int i) { return i; @@ -13772,10 +13779,10 @@ Otherwise they cannot be distinguished automatically by the compiler. ##### Example (using TS concepts) template - concept bool Input_iter = requires (I iter) { ++iter; }; + concept bool Input_iter = requires(I iter) { ++iter; }; template - concept bool Fwd_iter = Input_iter && requires (I iter) { iter++; } + concept bool Fwd_iter = Input_iter && requires(I iter) { iter++; } The compiler can determine refinement based on the sets of required operations (here, suffix `++`). This decreases the burden on implementers of these types since @@ -13807,7 +13814,7 @@ The programmer (in a library) must define `is_contiguous` (a trait) appropriatel Wrapping a tag class into a concept leads to a simpler expression of this idea: template concept Contiguous = is_contiguous::value; - + template concept bool Contiguous_iter = RA_iter && Contiguous; @@ -13907,10 +13914,10 @@ Obviously, it would be better and easier just to use the standard `EqualityCompa but - just as an example - if you had to define such a concept, prefer: template concept Equality = requires(T a, T b) { - bool == { a==b } - bool == { a!=b } - // axiom { !(a==b)==(a!=b) } - // axiom { a=b; => a==b } // => means "implies" + bool == { a == b } + bool == { a != b } + // axiom { !(a == b) == (a != b) } + // axiom { a = b; => a == b } // => means "implies" } as opposed to defining two meaningless concepts `has_equal` and `has_not_equal` just as helpers in the definition of `Equality`. @@ -14210,7 +14217,7 @@ That is, it is highly visible. ##### Note -This rule should not be necessary, but the committee cannot agree to exclude unconstrained templated from ADL. +This rule should not be necessary, but the committee cannot agree to exclude unconstrained templated from ADL. Unfortunately this will get many false positives; the standard library violates this widely, by putting many unconstrained templates and types into the single namespace `std`. @@ -14353,7 +14360,6 @@ This looks innocent enough, but ??? // requires Regular && Allocator class List2 { public: - using iterator = Link*; iterator first() const { return head; } @@ -14448,7 +14454,7 @@ This is a simplified version of `std::copy` (ignoring the possibility of non-con struct pod_tag {}; struct non_pod_tag; - + template struct copy_trait { using tag = non_pod_tag; }; // T is not "plain old data" template<> struct copy_trait { using tab = pod_tag; }; // int is "plain old data" @@ -15004,7 +15010,7 @@ Documentation, readability, opportunity for reuse. auto x = find_if(vr.begin(), vr.end(), [&](Rec& r) { if (r.name.size() != n.size()) return false; // name to compare to is in n - for (int i=0; i < r.name.size(); ++i) + for (int i = 0; i < r.name.size(); ++i) if (tolower(r.name[i]) != tolower(n[i])) return false; return true; } @@ -15014,20 +15020,20 @@ There is a useful function lurking here (case insensitive string comparison), as bool compare_insensitive(const string& a, const string& b) { - if (a.size()!=b.size()) return false; - for (int i=0; i 100; }); - + ##### Exception @@ -16756,7 +16762,8 @@ Code says what is done, not what is supposed to be done. Often intent can be sta ##### Example void stable_sort(Sortable& c) - // sort c in the order determined by <, keep equal elements (as defined by ==) in their original relative order + // sort c in the order determined by <, keep equal elements (as defined by ==) in + // their original relative order { // ... quite a few lines of non-trivial code ... } @@ -16799,9 +16806,9 @@ Readability. Avoidance of "silly mistakes." Always indenting the statement after `if (...)`, `for (...)`, and `while (...)` is usually a good idea: - if (i<0) error("negative argument"); + if (i < 0) error("negative argument"); - if (i<0) + if (i < 0) error("negative argument"); ##### Enforcement @@ -16828,10 +16835,10 @@ Minimize unintentional conversions. Names with types encoded are either verbose or cryptic. - printS // print a std::string + printS // print a std::string prints // print a C-style string printi // print an int - + PS. Hungarian notation is evil (at least in a strongly statically-typed language). ##### Note @@ -16992,7 +16999,7 @@ Too much space makes the text larger and distracts. #include < map > - int main (int argc, char * argv [ ]) + int main(int argc, char * argv [ ]) { // ... } @@ -17167,8 +17174,8 @@ It is really easy to overlook a statement when there is more on a line. ##### Example - int x = 7; char* p = 29; // don’t - int x = 7; f(x); ++x; // don’t + int x = 7; char* p = 29; // don't + int x = 7; f(x); ++x; // don't ##### Enforcement From 78bac72eafa0bac4293a078c30f1ec9b1f7ad83e Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Fri, 29 Jul 2016 09:15:23 +0200 Subject: [PATCH 5/6] Build: bugfix python script: detect more codeblocks as lintable --- scripts/python/md-split.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/scripts/python/md-split.py b/scripts/python/md-split.py index 7b8768f..6e0beb3 100755 --- a/scripts/python/md-split.py +++ b/scripts/python/md-split.py @@ -74,7 +74,6 @@ def main(): def process_code(read_filehandle, text_filehandle, line, linenum, sourcefile, codedir, name, index, indent_depth): - fenced = (line.strip() == '```') if fenced: try: @@ -94,18 +93,18 @@ def process_code(read_filehandle, text_filehandle, line, linenum, sourcefile, co if comment_idx >= 0: no_comment_line = line[:comment_idx] text_filehandle.write(line[comment_idx + 2:]) - if (not has_actual_code - and not line.strip().startswith('//') - and not line.strip().startswith('???') - and not line.strip() ==''): - has_actual_code = True + + if (not has_actual_code + and not line.strip().startswith('//') + and not line.strip().startswith('???') + and not line.strip() ==''): + has_actual_code = True else: # write empty line so line numbers stay stable text_filehandle.write('') if (not line.strip() == '```'): - - if ('???' in no_comment_line or '...' in no_comment_line): + if ('???' == no_comment_line or '...' == no_comment_line): has_question_marks = True linebuffer.append(dedent(line) if not fenced else line) try: @@ -115,10 +114,8 @@ def process_code(read_filehandle, text_filehandle, line, linenum, sourcefile, co line = '' break codefile = os.path.join(codedir, '%s%s.cpp' % (name, index)) - if fenced: text_filehandle.write('') - if (has_actual_code and not has_question_marks): # add commonly used headers, so that lines can compile with io.open(codefile, 'w') as code_filehandle: @@ -145,6 +142,7 @@ using namespace std; // by md-split def is_code(line, indent_depth = 4): + '''returns the indent depth, 0 means not code in markup''' if line.startswith(' ' * indent_depth): return len(line) - len(line.lstrip(' ')) return 0 @@ -171,6 +169,7 @@ def get_marker(line): namematch = NAMED_A_TAG_REGEX.match(line) if namematch: return namematch.group(1) # group 0 is full match + return None if __name__ == '__main__': From 2333364047a6def6b001461bfcfc6b658c92294b Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Tue, 16 Aug 2016 00:46:13 +0200 Subject: [PATCH 6/6] Refactor remark plugin configuration into package.json, add link validation --- scripts/Makefile | 10 +++---- scripts/nodejs/package.json | 52 ++++++++++++++++++++++++++------- scripts/nodejs/remark/.remarkrc | 31 -------------------- 3 files changed, 47 insertions(+), 46 deletions(-) delete mode 100644 scripts/nodejs/remark/.remarkrc diff --git a/scripts/Makefile b/scripts/Makefile index 39c53c3..ce30ae7 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -36,16 +36,16 @@ distclean: #### check markdown -## run remark markdown checker based on configuration in .remarkrc +## run remark markdown checker based on configuration in package.json .PHONY: check-markdown -check-markdown: nodejs/node_modules/remark nodejs/remark/.remarkrc $(SOURCEPATH) $(BUILD_DIR) Makefile +check-markdown: nodejs/node_modules/remark nodejs/package.json $(SOURCEPATH) $(BUILD_DIR) Makefile ## run remark, paste output to temporary file - cd nodejs; ./node_modules/.bin/remark ../$(SOURCEPATH) --no-color -q --config-path ./remark/.remarkrc 1> ../$(BUILD_DIR)/$(SOURCEFILE).fixed --frail + cd nodejs; ./node_modules/.bin/remark ../$(SOURCEPATH) --no-color -q 1> ../$(BUILD_DIR)/$(SOURCEFILE).fixed --frail ## show a diff with changes remark suggests .PHONY: show-diff -show-diff: nodejs/node_modules/remark nodejs/remark/.remarkrc $(SOURCEPATH) $(BUILD_DIR) Makefile - cd nodejs; ./node_modules/.bin/remark ../$(SOURCEPATH) --no-color -q --config-path ./remark/.remarkrc 1> ../$(BUILD_DIR)/$(SOURCEFILE).fixed +show-diff: nodejs/node_modules/remark nodejs/package.json $(SOURCEPATH) $(BUILD_DIR) Makefile + cd nodejs; ./node_modules/.bin/remark ../$(SOURCEPATH) --no-color -q 1> ../$(BUILD_DIR)/$(SOURCEFILE).fixed ## compare temporary file to original, error and fail with message if differences exist diff $(SOURCEPATH) $(BUILD_DIR)/$(SOURCEFILE).fixed -u3 || \ (echo "Error: remark found bad markdown syntax, see output above" && false) diff --git a/scripts/nodejs/package.json b/scripts/nodejs/package.json index 133637b..0f94c26 100644 --- a/scripts/nodejs/package.json +++ b/scripts/nodejs/package.json @@ -1,12 +1,44 @@ { - "name": "CppCoreGuidelinesCheck", - "version": "0.0.0", - "description": "CppCoreGuidelines Check", - "private": true, - "dependencies": { - "remark": "^4.2.2", - "remark-lint": "^4.0.2", - "remark-lint-sentence-newline": "^2.0.0", - "remark-validate-links": "^4.1.0" - } + "name": "CppCoreGuidelinesCheck", + "version": "0.0.0", + "description": "CppCoreGuidelines Check", + "private": true, + "dependencies": { + "remark": "^4.2.2", + "remark-lint": "^4.0.2", + "remark-lint-sentence-newline": "^2.0.0", + "remark-validate-links": "^4.1.0", + "remark-lint-are-links-valid": "^1.0.2" + }, + "remarkConfig": { + "plugins": { + "remark-lint": { + "unordered-list-marker-style": "consistent", + "list-item-bullet-indent": false, + "list-item-indent": false, + "list-item-spacing": false, + "no-html": false, + "maximum-line-length": false, + "no-file-name-mixed-case": false, + "heading-increment": false, + "no-multiple-toplevel-headings": false, + "no-consecutive-blank-lines": false, + "maximum-line-length": 9000, + "maximum-heading-length": 300, + "no-heading-punctuation": false, + "no-duplicate-headings": false, + "emphasis-marker": "*", + "no-tabs": false, + "blockquote-indentation": false, + "strong-marker": "*", + "external": ["remark-lint-are-links-valid"] + } + }, + "settings": { + "bullet": "*", + "listItemIndent": "1", + "strong": "*", + "emphasis": "*" + } + } } diff --git a/scripts/nodejs/remark/.remarkrc b/scripts/nodejs/remark/.remarkrc deleted file mode 100644 index 48e13f4..0000000 --- a/scripts/nodejs/remark/.remarkrc +++ /dev/null @@ -1,31 +0,0 @@ -{ -"plugins": { - "remark-lint": { - "unordered-list-marker-style": "consistent", - "list-item-bullet-indent": false, - "list-item-indent": false, - "list-item-spacing": false, - "no-html": false, - "maximum-line-length": false, - "no-file-name-mixed-case": false, - "heading-increment": false, - "no-multiple-toplevel-headings": false, - "no-consecutive-blank-lines": false, - "maximum-line-length": 9000, - "maximum-heading-length": 300, - "no-heading-punctuation": false, - "no-duplicate-headings": false, - "emphasis-marker": "*", - "no-tabs": false, - "blockquote-indentation": false, - "strong-marker": "*" - } - }, - "settings": { - "bullet": "*", - "listItemIndent": "1", - "strong": "*", - "emphasis": "*" - } - -}