From 00de780a6e957359ae20f0df56bf1b42a9e12c3d Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Mon, 23 May 2022 19:03:50 +0000 Subject: [PATCH] Improve cpplint defaults While enabling cpplint on base I found an extraneous error and an overly complicated invocation of CheckChangeLintsClean. This change disables the extraneous error and improves the file filtering defaults to make it easier to invoke cpplint. The warning was: base\win\win_util.cc(193): Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2] Chromium has over 10,000 TODO comments that are links to bugs instead of users so we clearly want to suppress this warning. Most calls to CheckChangeLintsClean in Chromium pass in custom file filtering in order to only analyze C++ files. Failing to pass a file filter leads to errors like this: Ignoring base\win\embedded_i18n\create_string_rc.py; not a valid file name (cc, cuh, cu, cpp, h) Ignoring c:\src\chromium\src\components\arc\PRESUBMIT.py; not a valid file name (cu, cc, cuh, cpp, h) It doesn't make sense for the canned checks version of CheckChangeLintsClean to default to passing in files that cpplint will complain about so if source_file_filter is None then I configure an appropriate filter. This change also switches to vs7 output format by default on Windows, and deletes a trailing space. Bug: 1309977 Change-Id: I342ca9e56a41502755dfc09307b441dfa6a48ec0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3655592 Reviewed-by: Will Harris Commit-Queue: Bruce Dawson --- presubmit_canned_checks.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index e1909dd8a..22619721e 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -41,11 +41,13 @@ OFF_BY_DEFAULT_LINT_FILTERS = [ # - build/c++11 : Include file and feature blocklists are # google3-specific # - build/header_guard : Checked by CheckForIncludeGuards +# - readability/todo : Chromium puts bug links, not usernames, in TODOs # - runtime/references : No longer banned by Google style guide # - whitespace/... : Most whitespace issues handled by clang-format OFF_UNLESS_MANUALLY_ENABLED_LINT_FILTERS = [ '-build/c++11', '-build/header_guard', + '-readability/todo', '-runtime/references', '-whitespace/braces', '-whitespace/comma', @@ -229,6 +231,20 @@ def CheckChangeLintsClean(input_api, output_api, source_file_filter=None, cpplint._SetFilters(','.join(GetCppLintFilters(lint_filters))) + # Use VS error format on Windows to make it easier to step through the + # results. + if input_api.platform == 'win32': + cpplint._SetOutputFormat('vs7') + + if source_file_filter == None: + # The only valid extensions for cpplint are .cc, .h, .cpp, .cu, and .ch. + # Only process those extensions which are used in Chromium. + INCLUDE_CPP_FILES_ONLY = (r'.*\.(cc|h|cpp)$', ) + source_file_filter = lambda x: input_api.FilterSourceFile( + x, + files_to_check=INCLUDE_CPP_FILES_ONLY, + files_to_skip=input_api.DEFAULT_FILES_TO_SKIP) + # We currently are more strict with normal code than unit tests; 4 and 5 are # the verbosity level that would normally be passed to cpplint.py through # --verbose=#. Hopefully, in the future, we can be more verbose. @@ -1240,7 +1256,7 @@ def CheckOwners( if input_api.gerrit and input_api.gerrit.IsCodeOwnersEnabledOnRepo(): return [] - affected_files = {f.LocalPath() for f in + affected_files = {f.LocalPath() for f in input_api.change.AffectedFiles( file_filter=source_file_filter)} owner_email, reviewers = GetCodereviewOwnerAndReviewers(