Commit 00de780a authored by Bruce Dawson's avatar Bruce Dawson Committed by LUCI CQ

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/+/3655592Reviewed-by: 's avatarWill Harris <wfh@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
parent 964f8124
......@@ -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(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment