From d88c4e18221c7857a7bfe2fbb0f5f6bef1da8d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Blot?= Date: Sun, 1 Apr 2018 23:57:55 +0200 Subject: [PATCH] LINT: add clang-tidy step (#6295) * Implement new travis clang-tidy build step * This step enable some rules and enforce one rule as error * This permits to have some C++ quality rules based on clang & clang contributor guidelines * Fix clang-tidy reported problems on push_back -> emplace_back --- .clang-tidy | 4 + .travis.yml | 9 + src/gui/guiEditBoxWithScrollbar.cpp | 2 +- src/script/cpp_api/s_env.cpp | 8 +- util/travis/before_install.sh | 4 +- util/travis/clangtidy.sh | 27 +++ util/travis/run-clang-tidy.py | 271 ++++++++++++++++++++++++++++ util/travis/script.sh | 2 +- 8 files changed, 319 insertions(+), 8 deletions(-) create mode 100644 .clang-tidy create mode 100755 util/travis/clangtidy.sh create mode 100755 util/travis/run-clang-tidy.py diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..80c1949f9 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,4 @@ +Checks: '-*,modernize-use-emplace,modernize-use-default-member-init,modernize-use-equals-delete,modernize-use-equals-default,modernize-return-braced-init-list,modernize-loop-convert,modernize-avoid-bind,misc-throw-by-value-catch-by-reference,misc-string-compare,misc-inefficient-algorithm,misc-inaccurate-erase,misc-incorrect-roundings,misc-unconventional-assign-operator,bugprone-suspicious-memset-usage,performance-*' +CheckOptions: + - key: modernize-use-default-member-init.UseAssignment + value: True diff --git a/.travis.yml b/.travis.yml index 0fb29fc7d..c506d21e7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -96,6 +96,15 @@ matrix: packages: ['clang-format-5.0'] sources: &sources - llvm-toolchain-trusty-5.0 + - env: CLANG_TIDY=1 + compiler: clang + os: linux + script: ./util/travis/clangtidy.sh + addons: + apt: + packages: ['clang-tidy-5.0'] + sources: &sources + - llvm-toolchain-trusty-5.0 diff --git a/src/gui/guiEditBoxWithScrollbar.cpp b/src/gui/guiEditBoxWithScrollbar.cpp index cf278a58e..4723437bd 100644 --- a/src/gui/guiEditBoxWithScrollbar.cpp +++ b/src/gui/guiEditBoxWithScrollbar.cpp @@ -743,7 +743,7 @@ void GUIEditBoxWithScrollBar::draw() if (m_passwordbox) { if (m_broken_text.size() != 1) { m_broken_text.clear(); - m_broken_text.push_back(core::stringw()); + m_broken_text.emplace_back(); } if (m_broken_text[0].size() != Text.size()){ m_broken_text[0] = Text; diff --git a/src/script/cpp_api/s_env.cpp b/src/script/cpp_api/s_env.cpp index 170640ce0..c9b652096 100644 --- a/src/script/cpp_api/s_env.cpp +++ b/src/script/cpp_api/s_env.cpp @@ -116,12 +116,12 @@ void ScriptApiEnv::initializeEnvironment(ServerEnvironment *env) while (lua_next(L, table)) { // key at index -2 and value at index -1 luaL_checktype(L, -1, LUA_TSTRING); - trigger_contents.push_back(lua_tostring(L, -1)); + trigger_contents.emplace_back(lua_tostring(L, -1)); // removes value, keeps key for next iteration lua_pop(L, 1); } } else if (lua_isstring(L, -1)) { - trigger_contents.push_back(lua_tostring(L, -1)); + trigger_contents.emplace_back(lua_tostring(L, -1)); } lua_pop(L, 1); @@ -133,12 +133,12 @@ void ScriptApiEnv::initializeEnvironment(ServerEnvironment *env) while (lua_next(L, table)) { // key at index -2 and value at index -1 luaL_checktype(L, -1, LUA_TSTRING); - required_neighbors.push_back(lua_tostring(L, -1)); + required_neighbors.emplace_back(lua_tostring(L, -1)); // removes value, keeps key for next iteration lua_pop(L, 1); } } else if (lua_isstring(L, -1)) { - required_neighbors.push_back(lua_tostring(L, -1)); + required_neighbors.emplace_back(lua_tostring(L, -1)); } lua_pop(L, 1); diff --git a/util/travis/before_install.sh b/util/travis/before_install.sh index afc16c332..19c40ef90 100755 --- a/util/travis/before_install.sh +++ b/util/travis/before_install.sh @@ -10,8 +10,8 @@ fi needs_compile || exit 0 -if [[ $PLATFORM == "Unix" ]]; then - if [[ $TRAVIS_OS_NAME == "linux" ]]; then +if [[ $PLATFORM == "Unix" ]] || [[ $CLANG_TIDY == "1" ]]; then + if [[ $TRAVIS_OS_NAME == "linux" ]] || [[ $CLANG_TIDY == "1" ]]; then install_linux_deps else install_macosx_deps diff --git a/util/travis/clangtidy.sh b/util/travis/clangtidy.sh new file mode 100755 index 000000000..2d44afe32 --- /dev/null +++ b/util/travis/clangtidy.sh @@ -0,0 +1,27 @@ +if hash clang-tidy-5.0 2>/dev/null; then + CLANG_TIDY=clang-tidy-5.0 +else + CLANG_TIDY=clang-tidy +fi + +files_to_analyze="$(find src/ -name '*.cpp' -or -name '*.h')" + +mkdir -p cmakebuild && cd cmakebuild +cmake -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ + -DRUN_IN_PLACE=TRUE \ + -DENABLE_GETTEXT=TRUE \ + -DENABLE_SOUND=FALSE \ + -DBUILD_SERVER=TRUE .. +make GenerateVersion +cd .. + +echo "Performing clang-tidy checks..." +./util/travis/run-clang-tidy.py -clang-tidy-binary=${CLANG_TIDY} -p cmakebuild \ + -checks='-*,modernize-use-emplace,modernize-avoid-bind,performance-*' \ + -warningsaserrors='-*,modernize-use-emplace' \ + -no-command-on-stdout -quiet \ + files 'src/.*' +RET=$? +echo "Clang tidy returned $RET" +exit $RET diff --git a/util/travis/run-clang-tidy.py b/util/travis/run-clang-tidy.py new file mode 100755 index 000000000..779cf7cc6 --- /dev/null +++ b/util/travis/run-clang-tidy.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python2 +# +# ===- run-clang-tidy.py - Parallel clang-tidy runner ---------*- python -*--===# +# +# The LLVM Compiler Infrastructure +# +# This file is distributed under the University of Illinois Open Source +# License. See LICENSE.TXT for details. +# +# ===------------------------------------------------------------------------===# +# FIXME: Integrate with clang-tidy-diff.py + +""" +Parallel clang-tidy runner +========================== + +Runs clang-tidy over all files in a compilation database. Requires clang-tidy +and clang-apply-replacements in $PATH. + +Example invocations. +- Run clang-tidy on all files in the current working directory with a default + set of checks and show warnings in the cpp files and all project headers. + run-clang-tidy.py $PWD + +- Fix all header guards. + run-clang-tidy.py -fix -checks=-*,llvm-header-guard + +- Fix all header guards included from clang-tidy and header guards + for clang-tidy headers. + run-clang-tidy.py -fix -checks=-*,llvm-header-guard extra/clang-tidy \ + -header-filter=extra/clang-tidy + +Compilation database setup: +http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html +""" + +from __future__ import print_function +import argparse +import json +import multiprocessing +import os +import Queue +import re +import shutil +import subprocess +import sys +import tempfile +import threading +import traceback + + +class TidyQueue(Queue.Queue): + def __init__(self, max_task): + Queue.Queue.__init__(self, max_task) + self.has_error = False + + +def find_compilation_database(path): + """Adjusts the directory until a compilation database is found.""" + result = './' + while not os.path.isfile(os.path.join(result, path)): + if os.path.realpath(result) == '/': + print('Error: could not find compilation database.') + sys.exit(1) + result += '../' + return os.path.realpath(result) + + +def get_tidy_invocation(f, clang_tidy_binary, checks, warningsaserrors, + tmpdir, build_path, + header_filter, extra_arg, extra_arg_before, quiet): + """Gets a command line for clang-tidy.""" + start = [clang_tidy_binary] + if header_filter is not None: + start.append('-header-filter=' + header_filter) + else: + # Show warnings in all in-project headers by default. + start.append('-header-filter=^' + build_path + '/.*') + if checks: + start.append('-checks=' + checks) + if warningsaserrors: + start.append('-warnings-as-errors=' + warningsaserrors) + if tmpdir is not None: + start.append('-export-fixes') + # Get a temporary file. We immediately close the handle so clang-tidy can + # overwrite it. + (handle, name) = tempfile.mkstemp(suffix='.yaml', dir=tmpdir) + os.close(handle) + start.append(name) + for arg in extra_arg: + start.append('-extra-arg=%s' % arg) + for arg in extra_arg_before: + start.append('-extra-arg-before=%s' % arg) + start.append('-p=' + build_path) + if quiet: + start.append('-quiet') + start.append(f) + return start + + +def check_clang_apply_replacements_binary(args): + """Checks if invoking supplied clang-apply-replacements binary works.""" + try: + subprocess.check_call([args.clang_apply_replacements_binary, '--version']) + except: + print('Unable to run clang-apply-replacements. Is clang-apply-replacements ' + 'binary correctly specified?', file=sys.stderr) + traceback.print_exc() + sys.exit(1) + + +def apply_fixes(args, tmpdir): + """Calls clang-apply-fixes on a given directory. Deletes the dir when done.""" + invocation = [args.clang_apply_replacements_binary] + if args.format: + invocation.append('-format') + if args.style: + invocation.append('-style=' + args.style) + invocation.append(tmpdir) + subprocess.call(invocation) + + +def run_tidy(args, tmpdir, build_path, queue): + """Takes filenames out of queue and runs clang-tidy on them.""" + while True: + name = queue.get() + invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks, + args.warningsaserrors, tmpdir, build_path, + args.header_filter, args.extra_arg, + args.extra_arg_before, args.quiet) + if not args.no_command_on_stdout: + sys.stdout.write(' '.join(invocation) + '\n') + try: + subprocess.check_call(invocation) + except subprocess.CalledProcessError: + queue.has_error = True + queue.task_done() + + +def main(): + parser = argparse.ArgumentParser(description='Runs clang-tidy over all files ' + 'in a compilation database. Requires ' + 'clang-tidy and clang-apply-replacements in ' + '$PATH.') + parser.add_argument('-clang-tidy-binary', metavar='PATH', + default='clang-tidy', + help='path to clang-tidy binary') + parser.add_argument('-clang-apply-replacements-binary', metavar='PATH', + default='clang-apply-replacements', + help='path to clang-apply-replacements binary') + parser.add_argument('-checks', default=None, + help='checks filter, when not specified, use clang-tidy ' + 'default') + parser.add_argument('-warningsaserrors', default=None, + help='warnings-as-errors filter, when not specified, ' + 'use clang-tidy default') + parser.add_argument('-header-filter', default=None, + help='regular expression matching the names of the ' + 'headers to output diagnostics from. Diagnostics from ' + 'the main file of each translation unit are always ' + 'displayed.') + parser.add_argument('-j', type=int, default=0, + help='number of tidy instances to be run in parallel.') + parser.add_argument('files', nargs='*', default=['.*'], + help='files to be processed (regex on path)') + parser.add_argument('-fix', action='store_true', help='apply fix-its') + parser.add_argument('-format', action='store_true', help='Reformat code ' + 'after applying fixes') + parser.add_argument('-style', default='file', help='The style of reformat ' + 'code after applying fixes') + parser.add_argument('-p', dest='build_path', + help='Path used to read a compile command database.') + parser.add_argument('-extra-arg', dest='extra_arg', + action='append', default=[], + help='Additional argument to append to the compiler ' + 'command line.') + parser.add_argument('-extra-arg-before', dest='extra_arg_before', + action='append', default=[], + help='Additional argument to prepend to the compiler ' + 'command line.') + parser.add_argument('-quiet', action='store_true', + help='Run clang-tidy in quiet mode') + parser.add_argument('-no-command-on-stdout', action='store_true', + help='Run clang-tidy without printing invocation on ' + 'stdout') + args = parser.parse_args() + + db_path = 'compile_commands.json' + + if args.build_path is not None: + build_path = args.build_path + else: + # Find our database + build_path = find_compilation_database(db_path) + + try: + invocation = [args.clang_tidy_binary, '-list-checks', '-p=' + build_path] + if args.checks: + invocation.append('-checks=' + args.checks) + if args.warningsaserrors: + invocation.append('-warnings-as-errors=' + args.warningsaserrors) + invocation.append('-') + print(subprocess.check_output(invocation)) + except: + print("Unable to run clang-tidy.", file=sys.stderr) + sys.exit(1) + + # Load the database and extract all files. + database = json.load(open(os.path.join(build_path, db_path))) + files = [entry['file'] for entry in database] + + max_task = args.j + if max_task == 0: + max_task = multiprocessing.cpu_count() + + tmpdir = None + if args.fix: + check_clang_apply_replacements_binary(args) + tmpdir = tempfile.mkdtemp() + + # Build up a big regexy filter from all command line arguments. + file_name_re = re.compile('|'.join(args.files)) + + try: + # Spin up a bunch of tidy-launching threads. + queue = TidyQueue(max_task) + for _ in range(max_task): + t = threading.Thread(target=run_tidy, + args=(args, tmpdir, build_path, queue)) + t.daemon = True + t.start() + + # Fill the queue with files. + for name in files: + if file_name_re.search(name): + queue.put(name) + + # Wait for all threads to be done. + queue.join() + + # If one clang-tidy process found and error, exit with non-zero + # status + if queue.has_error: + sys.exit(2) + + except KeyboardInterrupt: + # This is a sad hack. Unfortunately subprocess goes + # bonkers with ctrl-c and we start forking merrily. + print('\nCtrl-C detected, goodbye.') + if args.fix: + shutil.rmtree(tmpdir) + os.kill(0, 9) + + if args.fix: + print('Applying fixes ...') + successfully_applied = False + + try: + apply_fixes(args, tmpdir) + successfully_applied = True + except: + print('Error applying fixes.\n', file=sys.stderr) + traceback.print_exc() + + shutil.rmtree(tmpdir) + if not successfully_applied: + sys.exit(1) + + +if __name__ == '__main__': + main() diff --git a/util/travis/script.sh b/util/travis/script.sh index c68638db4..32e8d2e36 100755 --- a/util/travis/script.sh +++ b/util/travis/script.sh @@ -5,7 +5,7 @@ needs_compile || exit 0 if [[ "$LINT" == "1" ]]; then - # Lint with exit CI + # Lint and exit CI perform_lint exit 0 fi