From 6198bbe963c224f46e9a23f008042f80a3a27486 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 18 Dec 2017 10:22:51 -0600 Subject: [PATCH 01/13] add changed-only mode to cpplint --- script/cpplint.py | 65 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index 0b0310243839..1cd84dda1207 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -1,9 +1,11 @@ #!/usr/bin/env python +import argparse import fnmatch import os import sys +from lib.config import enable_verbose_mode from lib.util import execute IGNORE_FILES = [ @@ -40,34 +42,73 @@ SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) def main(): + + parser = argparse.ArgumentParser( + description="Run cpplint on Electron's C++ files", + formatter_class=argparse.RawTextHelpFormatter + ) + parser.add_argument( + '-c', '--only-changed', + action='store_true', + default=False, + dest='only_changed', + help='only run on changed files' + ) + parser.add_argument( + '-v', '--verbose', + action='store_true', + default=False, + dest='verbose', + help='show cpplint output' + ) + args = parser.parse_args() + if not os.path.isfile(cpplint_path()): print("[INFO] Skipping cpplint, dependencies has not been bootstrapped") return - os.chdir(SOURCE_ROOT) - atom_files = list_files('atom', - ['app', 'browser', 'common', 'renderer', 'utility'], - ['*.cc', '*.h']) - call_cpplint(list(set(atom_files) - set(IGNORE_FILES))) + if args.only_changed: + changed_files = get_changed_files() - brightray_files = list_files('brightray', ['browser', 'common'], - ['*.cc', '*.h']) - call_cpplint(list(set(brightray_files) - set(IGNORE_FILES))) + if args.verbose: + enable_verbose_mode() + + ignore = set(IGNORE_FILES) + + os.chdir(SOURCE_ROOT) + files = list_files('atom', + ['app', 'browser', 'common', 'renderer', 'utility'], + ['*.cc', '*.h']) + files -= ignore + if args.only_changed: + files &= changed_files + call_cpplint(list(files)) + + files = list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) + files -= ignore + if args.only_changed: + files &= changed_files + call_cpplint(list(files)) def list_files(parent, directories, filters): - matches = [] + matches = set() for directory in directories: for root, _, filenames, in os.walk(os.path.join(parent, directory)): for f in filters: for filename in fnmatch.filter(filenames, f): - matches.append(os.path.join(root, filename)) + matches.add(os.path.join(root, filename)) return matches +def get_changed_files(): + return set(execute(['git', 'diff', '--name-only']).splitlines()) + + def call_cpplint(files): - cpplint = cpplint_path() - execute([sys.executable, cpplint] + files) + if files: + cpplint = cpplint_path() + execute([sys.executable, cpplint] + files) def cpplint_path(): From b6c16a520ab9f2cc4ad924fa79be2abab5673879 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:07:11 -0600 Subject: [PATCH 02/13] only call cppcheck once --- script/cpplint.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index 1cd84dda1207..fb73cdf01036 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -79,12 +79,7 @@ def main(): files = list_files('atom', ['app', 'browser', 'common', 'renderer', 'utility'], ['*.cc', '*.h']) - files -= ignore - if args.only_changed: - files &= changed_files - call_cpplint(list(files)) - - files = list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) + files += list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) files -= ignore if args.only_changed: files &= changed_files From 512fb670b4139895e5799016d86fbe1a47962775 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:37:41 -0600 Subject: [PATCH 03/13] remove unnecessary changed_files variable --- script/cpplint.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index fb73cdf01036..ecc99f8ab6c3 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -67,9 +67,6 @@ def main(): print("[INFO] Skipping cpplint, dependencies has not been bootstrapped") return - if args.only_changed: - changed_files = get_changed_files() - if args.verbose: enable_verbose_mode() @@ -82,7 +79,7 @@ def main(): files += list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) files -= ignore if args.only_changed: - files &= changed_files + files &= get_changed_files() call_cpplint(list(files)) From 589c6a5b7e3764e60307c0d398299cd41687e464 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:38:23 -0600 Subject: [PATCH 04/13] remove unnecessary ignore variable --- script/cpplint.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index ecc99f8ab6c3..59b639527f78 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -70,14 +70,12 @@ def main(): if args.verbose: enable_verbose_mode() - ignore = set(IGNORE_FILES) - os.chdir(SOURCE_ROOT) files = list_files('atom', ['app', 'browser', 'common', 'renderer', 'utility'], ['*.cc', '*.h']) files += list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) - files -= ignore + files -= set(IGNORE_FILES) if args.only_changed: files &= get_changed_files() call_cpplint(list(files)) From f5f6d99cd7423f87f2f1c042ac691d968e8197d9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:40:11 -0600 Subject: [PATCH 05/13] make file list function names clearer since they return sets rather than lists, don't use 'list' in the name --- script/cpplint.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index 59b639527f78..c1f6deae2afc 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -71,17 +71,17 @@ def main(): enable_verbose_mode() os.chdir(SOURCE_ROOT) - files = list_files('atom', + files = find_files('atom', ['app', 'browser', 'common', 'renderer', 'utility'], ['*.cc', '*.h']) - files += list_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) + files += find_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) files -= set(IGNORE_FILES) if args.only_changed: - files &= get_changed_files() + files &= find_changed_files() call_cpplint(list(files)) -def list_files(parent, directories, filters): +def find_files(parent, directories, filters): matches = set() for directory in directories: for root, _, filenames, in os.walk(os.path.join(parent, directory)): @@ -91,7 +91,7 @@ def list_files(parent, directories, filters): return matches -def get_changed_files(): +def find_changed_files(): return set(execute(['git', 'diff', '--name-only']).splitlines()) From cab1b810264eae9aab302c2244179ea9843b575e Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:50:09 -0600 Subject: [PATCH 06/13] give find_files() a filename tester function arg --- script/cpplint.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index c1f6deae2afc..c0eae971cb64 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -73,24 +73,29 @@ def main(): os.chdir(SOURCE_ROOT) files = find_files('atom', ['app', 'browser', 'common', 'renderer', 'utility'], - ['*.cc', '*.h']) - files += find_files('brightray', ['browser', 'common'], ['*.cc', '*.h']) + is_cpp_file) + files |= find_files('brightray', ['browser', 'common'], is_cpp_file) files -= set(IGNORE_FILES) if args.only_changed: files &= find_changed_files() call_cpplint(list(files)) -def find_files(parent, directories, filters): +def find_files(root, directories, test): matches = set() for directory in directories: - for root, _, filenames, in os.walk(os.path.join(parent, directory)): - for f in filters: - for filename in fnmatch.filter(filenames, f): - matches.add(os.path.join(root, filename)) + for parent, _, children, in os.walk(os.path.join(root, directory)): + for child in children: + filename = os.path.join(parent,child) + if test(filename): + matches.add(filename) return matches +def is_cpp_file(filename): + return filename.endswith('.cc') or filename.endswith('.h') + + def find_changed_files(): return set(execute(['git', 'diff', '--name-only']).splitlines()) From 30f8660a20870822a6d748ffbd81393ecc12983a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 15:57:03 -0600 Subject: [PATCH 07/13] walk all files in atom/ and brightray/ --- script/cpplint.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index c0eae971cb64..9dd25276aeef 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -71,20 +71,17 @@ def main(): enable_verbose_mode() os.chdir(SOURCE_ROOT) - files = find_files('atom', - ['app', 'browser', 'common', 'renderer', 'utility'], - is_cpp_file) - files |= find_files('brightray', ['browser', 'common'], is_cpp_file) + files = find_files(['atom', 'brightray'], is_cpp_file) files -= set(IGNORE_FILES) if args.only_changed: files &= find_changed_files() call_cpplint(list(files)) -def find_files(root, directories, test): +def find_files(roots, test): matches = set() - for directory in directories: - for parent, _, children, in os.walk(os.path.join(root, directory)): + for root in roots: + for parent, _, children, in os.walk(root): for child in children: filename = os.path.join(parent,child) if test(filename): From 58edfc26edf834a5ff263c7778a89ee167232651 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 16:02:36 -0600 Subject: [PATCH 08/13] flake8 --ignore=E111,E121 script/cpplint.py --- script/cpplint.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index 9dd25276aeef..ca4cdf70f50f 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -1,7 +1,6 @@ #!/usr/bin/env python import argparse -import fnmatch import os import sys @@ -83,7 +82,7 @@ def find_files(roots, test): for root in roots: for parent, _, children, in os.walk(root): for child in children: - filename = os.path.join(parent,child) + filename = os.path.join(parent, child) if test(filename): matches.add(filename) return matches From 8373e1bf41051b8381c99e9fa05212cd70096f80 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 16:09:53 -0600 Subject: [PATCH 09/13] fix previously-hidden cppcheck warnings in osfcheck.cc --- atom/node/osfhandle.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/node/osfhandle.cc b/atom/node/osfhandle.cc index e5c09d25b53a..66c13b65fa65 100644 --- a/atom/node/osfhandle.cc +++ b/atom/node/osfhandle.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#include "osfhandle.h" +#include "atom/node/osfhandle.h" #if !defined(DEBUG) #define U_I18N_IMPLEMENTATION @@ -23,10 +23,10 @@ #include "third_party/icu/source/i18n/unicode/ucsdet.h" #include "third_party/icu/source/i18n/unicode/ulocdata.h" #include "third_party/icu/source/i18n/unicode/uregex.h" -#include "third_party/icu/source/i18n/unicode/uspoof.h" #include "third_party/icu/source/i18n/unicode/usearch.h" -#include "v8-profiler.h" -#include "v8-inspector.h" +#include "third_party/icu/source/i18n/unicode/uspoof.h" +#include "v8/include/v8-inspector.h" +#include "v8/include/v8-profiler.h" namespace node { From 2f88e69ed4d26fc52d09d2656b8a8a75117f9c58 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 16:23:27 -0600 Subject: [PATCH 10/13] fix inconsistent indentation --- script/cpplint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/cpplint.py b/script/cpplint.py index ca4cdf70f50f..674426bb663d 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -89,7 +89,7 @@ def find_files(roots, test): def is_cpp_file(filename): - return filename.endswith('.cc') or filename.endswith('.h') + return filename.endswith('.cc') or filename.endswith('.h') def find_changed_files(): From 4f533dded390edc0ef9e4e02373f1960c4d43b41 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 17:39:03 -0600 Subject: [PATCH 11/13] Revert "fix previously-hidden cppcheck warnings in osfcheck.cc" This reverts commit 8373e1bf41051b8381c99e9fa05212cd70096f80. --- atom/node/osfhandle.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/node/osfhandle.cc b/atom/node/osfhandle.cc index 66c13b65fa65..e5c09d25b53a 100644 --- a/atom/node/osfhandle.cc +++ b/atom/node/osfhandle.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#include "atom/node/osfhandle.h" +#include "osfhandle.h" #if !defined(DEBUG) #define U_I18N_IMPLEMENTATION @@ -23,10 +23,10 @@ #include "third_party/icu/source/i18n/unicode/ucsdet.h" #include "third_party/icu/source/i18n/unicode/ulocdata.h" #include "third_party/icu/source/i18n/unicode/uregex.h" -#include "third_party/icu/source/i18n/unicode/usearch.h" #include "third_party/icu/source/i18n/unicode/uspoof.h" -#include "v8/include/v8-inspector.h" -#include "v8/include/v8-profiler.h" +#include "third_party/icu/source/i18n/unicode/usearch.h" +#include "v8-profiler.h" +#include "v8-inspector.h" namespace node { From 0521302940c977cdb5313d8d68f2b4d8447df169 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 17:41:34 -0600 Subject: [PATCH 12/13] add atom/node/osfhandle.cc to the do-not-lint list --- script/cpplint.py | 1 + 1 file changed, 1 insertion(+) diff --git a/script/cpplint.py b/script/cpplint.py index 674426bb663d..02b5729e261f 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -20,6 +20,7 @@ IGNORE_FILES = [ os.path.join('atom', 'common', 'api', 'api_messages.h'), os.path.join('atom', 'common', 'common_message_generator.cc'), os.path.join('atom', 'common', 'common_message_generator.h'), + os.path.join('atom', 'node', 'osfhandle.cc'), os.path.join('brightray', 'browser', 'mac', 'bry_inspectable_web_contents_view.h'), os.path.join('brightray', 'browser', 'mac', 'event_dispatching_window.h'), From d36e451301d19c2fa77795ba62fbafc0385374df Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 19 Dec 2017 18:37:02 -0600 Subject: [PATCH 13/13] make IGNORE_FILES more readable --- script/cpplint.py | 61 ++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/script/cpplint.py b/script/cpplint.py index 02b5729e261f..49ddb877f114 100755 --- a/script/cpplint.py +++ b/script/cpplint.py @@ -7,36 +7,31 @@ import sys from lib.config import enable_verbose_mode from lib.util import execute -IGNORE_FILES = [ - os.path.join('atom', 'browser', 'mac', 'atom_application.h'), - os.path.join('atom', 'browser', 'mac', 'atom_application_delegate.h'), - os.path.join('atom', 'browser', 'resources', 'win', 'resource.h'), - os.path.join('atom', 'browser', 'ui', 'cocoa', 'atom_menu_controller.h'), - os.path.join('atom', 'browser', 'ui', 'cocoa', 'atom_touch_bar.h'), - os.path.join('atom', 'browser', 'ui', 'cocoa', - 'touch_bar_forward_declarations.h'), - os.path.join('atom', 'browser', 'ui', 'cocoa', 'NSColor+Hex.h'), - os.path.join('atom', 'browser', 'ui', 'cocoa', 'NSString+ANSI.h'), - os.path.join('atom', 'common', 'api', 'api_messages.h'), - os.path.join('atom', 'common', 'common_message_generator.cc'), - os.path.join('atom', 'common', 'common_message_generator.h'), - os.path.join('atom', 'node', 'osfhandle.cc'), - os.path.join('brightray', 'browser', 'mac', - 'bry_inspectable_web_contents_view.h'), - os.path.join('brightray', 'browser', 'mac', 'event_dispatching_window.h'), - os.path.join('brightray', 'browser', 'mac', - 'notification_center_delegate.h'), - os.path.join('brightray', 'browser', 'win', 'notification_presenter_win7.h'), - os.path.join('brightray', 'browser', 'win', 'win32_desktop_notifications', - 'common.h'), - os.path.join('brightray', 'browser', 'win', 'win32_desktop_notifications', - 'desktop_notification_controller.cc'), - os.path.join('brightray', 'browser', 'win', 'win32_desktop_notifications', - 'desktop_notification_controller.h'), - os.path.join('brightray', 'browser', 'win', 'win32_desktop_notifications', - 'toast.h'), - os.path.join('brightray', 'browser', 'win', 'win32_notification.h') -] +IGNORE_FILES = set(os.path.join(*components) for components in [ + ['atom', 'browser', 'mac', 'atom_application.h'], + ['atom', 'browser', 'mac', 'atom_application_delegate.h'], + ['atom', 'browser', 'resources', 'win', 'resource.h'], + ['atom', 'browser', 'ui', 'cocoa', 'atom_menu_controller.h'], + ['atom', 'browser', 'ui', 'cocoa', 'atom_touch_bar.h'], + ['atom', 'browser', 'ui', 'cocoa', 'touch_bar_forward_declarations.h'], + ['atom', 'browser', 'ui', 'cocoa', 'NSColor+Hex.h'], + ['atom', 'browser', 'ui', 'cocoa', 'NSString+ANSI.h'], + ['atom', 'common', 'api', 'api_messages.h'], + ['atom', 'common', 'common_message_generator.cc'], + ['atom', 'common', 'common_message_generator.h'], + ['atom', 'node', 'osfhandle.cc'], + ['brightray', 'browser', 'mac', 'bry_inspectable_web_contents_view.h'], + ['brightray', 'browser', 'mac', 'event_dispatching_window.h'], + ['brightray', 'browser', 'mac', 'notification_center_delegate.h'], + ['brightray', 'browser', 'win', 'notification_presenter_win7.h'], + ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'common.h'], + ['brightray', 'browser', 'win', 'win32_desktop_notifications', + 'desktop_notification_controller.cc'], + ['brightray', 'browser', 'win', 'win32_desktop_notifications', + 'desktop_notification_controller.h'], + ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'toast.h'], + ['brightray', 'browser', 'win', 'win32_notification.h'] +]) SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) @@ -72,10 +67,10 @@ def main(): os.chdir(SOURCE_ROOT) files = find_files(['atom', 'brightray'], is_cpp_file) - files -= set(IGNORE_FILES) + files -= IGNORE_FILES if args.only_changed: files &= find_changed_files() - call_cpplint(list(files)) + call_cpplint(files) def find_files(roots, test): @@ -100,7 +95,7 @@ def find_changed_files(): def call_cpplint(files): if files: cpplint = cpplint_path() - execute([sys.executable, cpplint] + files) + execute([sys.executable, cpplint] + list(files)) def cpplint_path():