diff options
author | Mark Benvenuto <mark.benvenuto@mongodb.com> | 2019-10-25 19:51:50 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-10-25 19:51:50 +0000 |
commit | a54df9e45d85a670bc4dc40339d76347865fab69 (patch) | |
tree | d430ebf24bba4c591cb6dfda6286ea4e327e30b2 | |
parent | 8fc28f0773ca1efb0a43cc5590b9ca8b9e50559e (diff) | |
download | mongo-a54df9e45d85a670bc4dc40339d76347865fab69.tar.gz |
SERVER-43936 Implement simpler, quicker python based C++ linter
-rw-r--r-- | SConstruct | 4 | ||||
-rw-r--r-- | buildscripts/linter/simplecpplint.py | 311 | ||||
-rwxr-xr-x | buildscripts/quickcpplint.py | 107 | ||||
-rw-r--r-- | src/mongo/db/mongod_options_general.h | 1 | ||||
-rw-r--r-- | src/mongo/db/mongod_options_replication.h | 1 | ||||
-rw-r--r-- | src/mongo/db/mongod_options_sharding.h | 1 | ||||
-rw-r--r-- | src/mongo/db/query/hint_parser_test.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_impl.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/server_options_base.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/server_options_base.h | 1 | ||||
-rw-r--r-- | src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block_bm.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/dbtests/jstests.cpp | 2 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/implscope.cpp | 1 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/implscope.h | 4 |
15 files changed, 426 insertions, 14 deletions
diff --git a/SConstruct b/SConstruct index 9abda8b6597..67f2c24d326 100644 --- a/SConstruct +++ b/SConstruct @@ -4026,8 +4026,8 @@ else: lint_py = env.Command( target="#lint-lint.py", - source=["buildscripts/lint.py", "src/mongo"], - action="$PYTHON ${SOURCES[0]} ${SOURCES[1]}", + source=["buildscripts/quickcpplint.py"], + action="$PYTHON ${SOURCES[0]} lint", ) env.Alias( "lint" , [ lint_py, eslint, clang_format, pylinters ] ) diff --git a/buildscripts/linter/simplecpplint.py b/buildscripts/linter/simplecpplint.py new file mode 100644 index 00000000000..e1aef8e1e7c --- /dev/null +++ b/buildscripts/linter/simplecpplint.py @@ -0,0 +1,311 @@ +#!/usr/bin/env python3 +"""Simple C++ Linter.""" + +import argparse +import io +import logging +import re +import sys + + +def _make_polyfill_regex(): + polyfill_required_names = [ + '_', + 'adopt_lock', + 'async', + 'chrono', + 'condition_variable', + 'condition_variable_any', + 'cv_status', + 'defer_lock', + 'future', + 'future_status', + 'get_terminate', + 'launch', + 'lock_guard', + 'mutex', + 'notify_all_at_thread_exit', + 'packaged_task', + 'promise', + 'recursive_mutex', + 'set_terminate', + 'shared_lock', + 'shared_mutex', + 'shared_timed_mutex', + 'this_thread(?!::at_thread_exit)', + 'thread', + 'timed_mutex', + 'try_to_lock', + 'unique_lock', + 'unordered_map', + 'unordered_multimap', + 'unordered_multiset', + 'unordered_set', + ] + + qualified_names = ['boost::' + name + "\\b" for name in polyfill_required_names] + qualified_names.extend('std::' + name + "\\b" for name in polyfill_required_names) + qualified_names_regex = '|'.join(qualified_names) + return re.compile('(' + qualified_names_regex + ')') + + +_RE_LINT = re.compile("//.*NOLINT") +_RE_COMMENT_STRIP = re.compile("//.*") + +_RE_PATTERN_MONGO_POLYFILL = _make_polyfill_regex() +_RE_VOLATILE = re.compile('[^_]volatile') +_RE_MUTEX = re.compile('[ ({,]stdx?::mutex[ ({]') +_RE_ASSERT = re.compile(r'\bassert\s*\(') + + +class Linter: + """Simple C++ Linter.""" + + def __init__(self, file_name, raw_lines): + """Create new linter.""" + self.file_name = file_name + self.raw_lines = raw_lines + self.clean_lines = [] + self.nolint_supression = [] + self._error_count = 0 + + self.found_config_header = False + + def lint(self): + """Run linter.""" + # 3 steps: + # 1. Check for header + # 2. Check for NOLINT and Strip multi line comments + # 3. Run per line checks + + # The license header is 28 lines and we are 0 indexed unless the file does not have the + # correct header. + start_line = 0 + + # We expect the SSPL license in all files but it is not expected in the Enterprise code + expect_sspl_license = "enterprise" not in self.file_name + has_sspl_license = self._check_for_server_side_public_license(3) + + if has_sspl_license: + start_line = 27 + + if not expect_sspl_license: + self._error( + 0, 'legal/enterprise_license', 'Incorrect license header found. ' + 'Expected Enterprise license. ' + 'See https://github.com/mongodb/mongo/wiki/Server-Code-Style') + + self._check_and_strip_comments() + + for linenum in range(start_line, len(self.clean_lines)): + if not self.clean_lines[linenum]: + continue + + self._check_for_mongo_volatile(linenum) + self._check_for_mongo_polyfill(linenum) + self._check_for_mongo_atomic(linenum) + self._check_for_mongo_mutex(linenum) + self._check_for_nonmongo_assert(linenum) + self._check_for_mongo_config_header(linenum) + + return self._error_count + + def _check_and_strip_comments(self): + in_multi_line_comment = False + + for linenum in range(len(self.raw_lines)): + clean_line = self.raw_lines[linenum] + + # Users can write NOLINT different ways + # // NOLINT + # // Some explanation NOLINT + # so we need a regular expression + if _RE_LINT.search(clean_line): + self.nolint_supression.append(linenum) + + if not in_multi_line_comment: + if "/*" in clean_line and not "*/" in clean_line: + in_multi_line_comment = True + clean_line = "" + + # Trim comments - approximately + # Note, this does not understand if // is in a string + # i.e. it will think URLs are also comments but this should be good enough to find + # violators of the coding convention + if "//" in clean_line: + clean_line = _RE_COMMENT_STRIP.sub("", clean_line) + else: + if "*/" in clean_line: + in_multi_line_comment = False + + clean_line = "" + + self.clean_lines.append(clean_line) + + def _check_for_mongo_volatile(self, linenum): + line = self.clean_lines[linenum] + if _RE_VOLATILE.search(line) and not "__asm__" in line: + self._error( + linenum, 'mongodb/volatile', + 'Illegal use of the volatile storage keyword, use AtomicWord instead ' + 'from "mongo/platform/atomic_word.h"') + + def _check_for_mongo_polyfill(self, linenum): + line = self.clean_lines[linenum] + match = _RE_PATTERN_MONGO_POLYFILL.search(line) + if match: + self._error( + linenum, 'mongodb/polyfill', + 'Illegal use of banned name from std::/boost:: for "%s", use mongo::stdx:: variant instead' + % (match.group(0))) + + def _check_for_mongo_atomic(self, linenum): + line = self.clean_lines[linenum] + if 'std::atomic' in line: + self._error( + linenum, 'mongodb/stdatomic', + 'Illegal use of prohibited std::atomic<T>, use AtomicWord<T> or other types ' + 'from "mongo/platform/atomic_word.h"') + + def _check_for_mongo_mutex(self, linenum): + line = self.clean_lines[linenum] + if _RE_MUTEX.search(line): + self._error( + linenum, 'mongodb/stdxmutex', 'Illegal use of prohibited stdx::mutex, ' + 'use mongo::Mutex from mongo/platform/mutex.h instead.') + + def _check_for_nonmongo_assert(self, linenum): + line = self.clean_lines[linenum] + if _RE_ASSERT.search(line): + self._error( + linenum, 'mongodb/assert', + 'Illegal use of the bare assert function, use a function from assert_utils.h instead.' + ) + + def _check_for_server_side_public_license(self, copyright_offset): + license_header = '''\ + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */'''.splitlines() + + # We expect the first line of the license header to follow shortly after the + # "Copyright" message. + if r'This program is free software' in self.raw_lines[copyright_offset]: + license_header_start_line = copyright_offset + for i in range(len(license_header)): # pylint: disable=consider-using-enumerate + line = i + license_header_start_line + # We don't trim the lines so we check length and then do strncmp + if line >= len(self.raw_lines) or \ + len(self.raw_lines[line]) != len(license_header[i]) + 1 or \ + self.raw_lines[line][:len(license_header[i])] != license_header[i]: + self._error( + 0, 'legal/license', 'Incorrect license header found. ' + 'Expected "' + license_header[i] + '". ' + 'See https://github.com/mongodb/mongo/wiki/Server-Code-Style') + # We break here to stop reporting legal/license errors for this file. + return False + else: + self._error( + 0, 'legal/license', 'No license header found. ' + 'See https://github.com/mongodb/mongo/wiki/Server-Code-Style') + return False + + return True + + def _check_for_mongo_config_header(self, linenum): + """Check for a config file.""" + if self.found_config_header: + return + + line = self.clean_lines[linenum] + self.found_config_header = line.startswith('#include "mongo/config.h"') + + if not self.found_config_header and "MONGO_CONFIG_" in line: + self._error(linenum, 'build/config_h_include', + 'MONGO_CONFIG define used without prior inclusion of config.h.') + + def _error(self, linenum, category, message): + if linenum in self.nolint_supression: + return + + if category == "legal/license": + # Enterprise module does not have the SSPL license + if "enterprise" in self.file_name: + return + + # The following files are in the src/mongo/ directory but technically belong + # in src/third_party/ because their copyright does not belong to MongoDB. + files_to_ignore = set([ + 'src/mongo/scripting/mozjs/PosixNSPR.cpp', + 'src/mongo/shell/linenoise.cpp', + 'src/mongo/shell/linenoise.h', + 'src/mongo/shell/mk_wcwidth.cpp', + 'src/mongo/shell/mk_wcwidth.h', + 'src/mongo/util/md5.cpp', + 'src/mongo/util/md5.h', + 'src/mongo/util/md5main.cpp', + 'src/mongo/util/net/ssl_stream.cpp', + 'src/mongo/util/scopeguard.h', + ]) + + norm_file_name = self.file_name.replace('\\', '/') + for file_to_ignore in files_to_ignore: + if file_to_ignore in norm_file_name: + return + + # We count internally from 0 but users count from 1 for line numbers + print("Error: %s:%d - %s - %s" % (self.file_name, linenum + 1, category, message)) + self._error_count += 1 + + +def lint_file(file_name): + """Lint file and print errors to console.""" + with io.open(file_name, encoding='utf-8') as file_stream: + raw_lines = file_stream.readlines() + + linter = Linter(file_name, raw_lines) + return linter.lint() + + +def main(): + # type: () -> None + """Execute Main Entry point.""" + parser = argparse.ArgumentParser(description='MongoDB Simple C++ Linter.') + + parser.add_argument('file', type=str, help="C++ input file") + + parser.add_argument('-v', '--verbose', action='count', help="Enable verbose tracing") + + args = parser.parse_args() + + if args.verbose: + logging.basicConfig(level=logging.DEBUG) + + if not lint_file(args.file): + sys.exit(1) + + +if __name__ == '__main__': + main() diff --git a/buildscripts/quickcpplint.py b/buildscripts/quickcpplint.py new file mode 100755 index 00000000000..d008475a8a3 --- /dev/null +++ b/buildscripts/quickcpplint.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python3 +"""Extensible script to run one or more simple C++ Linters across a subset of files in parallel.""" + +import argparse +import logging +import os +import re +import sys +import threading +from typing import List + +# Get relative imports to work when the package is not installed on the PYTHONPATH. +if __name__ == "__main__" and __package__ is None: + sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(os.path.realpath(__file__))))) + +from buildscripts.linter import git # pylint: disable=wrong-import-position +from buildscripts.linter import parallel # pylint: disable=wrong-import-position +from buildscripts.linter import simplecpplint # pylint: disable=wrong-import-position + +FILES_RE = re.compile('\\.(h|cpp)$') + + +def is_interesting_file(file_name: str) -> bool: + """Return true if this file should be checked.""" + return (file_name.startswith("jstests") + or file_name.startswith("src") and not file_name.startswith("src/third_party/") + and not file_name.startswith("src/mongo/gotools/")) and FILES_RE.search(file_name) + + +def _lint_files(file_names: List[str]) -> None: + """Lint a list of files with clang-format.""" + run_lint1 = lambda param1: simplecpplint.lint_file(param1) == 0 + if not parallel.parallel_process([os.path.abspath(f) for f in file_names], run_lint1): + print("ERROR: Code Style does not match coding style") + sys.exit(1) + + +def lint_patch(file_name: str) -> None: + """Lint patch command entry point.""" + file_names = git.get_files_to_check_from_patch(file_name, is_interesting_file) + + # Patch may have files that we do not want to check which is fine + if file_names: + _lint_files(file_names) + + +def lint(file_names: List[str]) -> None: + # type: (str, Dict[str, str], List[str]) -> None + """Lint files command entry point.""" + all_file_names = git.get_files_to_check(file_names, is_interesting_file) + + _lint_files(all_file_names) + + +def lint_all(file_names: List[str]) -> None: + # pylint: disable=unused-argument + """Lint files command entry point based on working tree.""" + all_file_names = git.get_files_to_check_working_tree(is_interesting_file) + + _lint_files(all_file_names) + + +def lint_my(origin_branch: List[str]) -> None: + # pylint: disable=unused-argument + """Lint files command based on local changes.""" + files = git.get_my_files_to_check(is_interesting_file, origin_branch) + files = [f for f in files if os.path.exists(f)] + + _lint_files(files) + + +def main() -> None: + """Execute Main entry point.""" + + parser = argparse.ArgumentParser(description='Quick C++ Lint frontend.') + + parser.add_argument('-v', "--verbose", action='store_true', help="Enable verbose logging") + + sub = parser.add_subparsers(title="Linter subcommands", help="sub-command help") + + parser_lint = sub.add_parser('lint', help='Lint only Git files') + parser_lint.add_argument("file_names", nargs="*", help="Globs of files to check") + parser_lint.set_defaults(func=lint) + + parser_lint_all = sub.add_parser('lint-all', help='Lint All files') + parser_lint_all.add_argument("file_names", nargs="*", help="Globs of files to check") + parser_lint_all.set_defaults(func=lint_all) + + parser_lint_patch = sub.add_parser('lint-patch', help='Lint the files in a patch') + parser_lint_patch.add_argument("file_names", nargs="*", help="Globs of files to check") + parser_lint_patch.set_defaults(func=lint_patch) + + parser_lint_my = sub.add_parser('lint-my', help='Lint my files') + parser_lint_my.add_argument("--branch", dest="file_names", default="origin/master", + help="Branch to compare against") + parser_lint_my.set_defaults(func=lint_my) + + args = parser.parse_args() + + if args.verbose: + logging.basicConfig(level=logging.DEBUG) + + args.func(args.file_names) + + +if __name__ == "__main__": + main() diff --git a/src/mongo/db/mongod_options_general.h b/src/mongo/db/mongod_options_general.h index 2959904f5c8..9460dc6fee1 100644 --- a/src/mongo/db/mongod_options_general.h +++ b/src/mongo/db/mongod_options_general.h @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/mongod_options_replication.h b/src/mongo/db/mongod_options_replication.h index f39ab8d9fdc..3a962f4864b 100644 --- a/src/mongo/db/mongod_options_replication.h +++ b/src/mongo/db/mongod_options_replication.h @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/mongod_options_sharding.h b/src/mongo/db/mongod_options_sharding.h index 10d7183a629..49e5eaf0d3d 100644 --- a/src/mongo/db/mongod_options_sharding.h +++ b/src/mongo/db/mongod_options_sharding.h @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/query/hint_parser_test.cpp b/src/mongo/db/query/hint_parser_test.cpp index af4d11d1691..8441eb913f1 100644 --- a/src/mongo/db/query/hint_parser_test.cpp +++ b/src/mongo/db/query/hint_parser_test.cpp @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp index 12a5bc38b03..ca25a04134c 100644 --- a/src/mongo/db/repl/rollback_impl.cpp +++ b/src/mongo/db/repl/rollback_impl.cpp @@ -1,4 +1,3 @@ - /** * Copyright (C) 2018-present MongoDB, Inc. * diff --git a/src/mongo/db/server_options_base.cpp b/src/mongo/db/server_options_base.cpp index 98e3eebaa9c..d6770d1ee7b 100644 --- a/src/mongo/db/server_options_base.cpp +++ b/src/mongo/db/server_options_base.cpp @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/server_options_base.h b/src/mongo/db/server_options_base.h index 42ab2a9e8fe..bffd542324e 100644 --- a/src/mongo/db/server_options_base.h +++ b/src/mongo/db/server_options_base.h @@ -1,4 +1,3 @@ - /** * Copyright (C) 2019-present MongoDB, Inc. * diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block_bm.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block_bm.cpp index 2a33a5ca8f4..933b5aa2990 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block_bm.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_begin_transaction_block_bm.cpp @@ -1,4 +1,3 @@ - /** * Copyright (C) 2018-present MongoDB, Inc. * diff --git a/src/mongo/db/transaction_participant_test.cpp b/src/mongo/db/transaction_participant_test.cpp index 3dca2998ae5..32ddd12ea71 100644 --- a/src/mongo/db/transaction_participant_test.cpp +++ b/src/mongo/db/transaction_participant_test.cpp @@ -1,4 +1,5 @@ -/** * Copyright (C) 2018-present MongoDB, Inc. +/** + * Copyright (C) 2018-present MongoDB, Inc. * * This program is free software: you can redistribute it and/or modify * it under the terms of the Server Side Public License, version 1, diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp index 7ab5d285e28..635fbd3088b 100644 --- a/src/mongo/dbtests/jstests.cpp +++ b/src/mongo/dbtests/jstests.cpp @@ -503,7 +503,7 @@ public: " } catch(e) {" " threw = true;" " }" - " assert(threw);" + " assert(threw);" // NOLINT "}"; ASSERT_EQUALS(s->invoke(code, &invalidRegex, nullptr), 0); } diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index 0e03c5573af..e78d462d7e7 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -40,6 +40,7 @@ #include <jsfriendapi.h> #include "mongo/base/error_codes.h" +#include "mongo/config.h" #include "mongo/db/operation_context.h" #include "mongo/platform/decimal128.h" #include "mongo/platform/mutex.h" diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h index d3b5d590fc2..c763640e5b9 100644 --- a/src/mongo/scripting/mozjs/implscope.h +++ b/src/mongo/scripting/mozjs/implscope.h @@ -379,7 +379,7 @@ private: public: MozRuntime(const MozJSScriptEngine* engine); - std::thread _thread; + std::thread _thread; // NOLINT std::unique_ptr<JSRuntime, std::function<void(JSRuntime*)>> _runtime; std::unique_ptr<JSContext, std::function<void(JSContext*)>> _context; }; @@ -420,7 +420,7 @@ private: unsigned int _opId; // op id for this scope OperationContext* _opCtx; // Op context for DbEval std::size_t _inOp; - std::atomic<bool> _pendingGC; + std::atomic<bool> _pendingGC; // NOLINT ConnectState _connectState; Status _status; std::string _parentStack; |