When running on macOS under python3 as part of PRESUBMIT_test.py,
a ResourceWarning is printed. This is due to the `open(..).read()`
idiom.
Use a `with` statement to ensure the file is properly closed.
Bug: none
Change-Id: I5643db0b77e6896b4b86177d2c1fe8dea04b58ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4742422
Reviewed-by: Gavin Mak <gavinmak@google.com>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
We plan on introducing a TODOC marker in v8.
(This is similar to a TODO but as a note to document something.)
At the current state the linter complains on lines like:
// TODOC: [...]
claiming that the TODO misses a username.
We'd like our new marker not to require a username.
The new regex is a merge of the existing regex with the updated
regex in the google-internal source this script is based on.
Change-Id: Ib792c82a9d62e45ef7994beab3fb3f48d7241292
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4660911
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
This CL updates cpplint.py to add some of the c++17 header files to the
list of known c++ headers. We're specifically using optional and variant
in the Tint project and would like to remove the NOLINT lines necessary
to keep the linter happy.
Bug: dawn:1379
Change-Id: I41cacd21e0742f992cdd6913b7f4def556b543c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3709241
Auto-Submit: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
A typical cpplint error report during a presubmit looks like this:
.../linux_ui.cc:64: Almost always, snprintf is better than strcpy [runtime/printf] [4]
...
Changelist failed cpplint.py check.
The problem is that the details of the error go to stderr when
CheckChangeLintsClean is run, whereas the "Changelist failed cpplint.py
check." message is printed at the end of the run. If there a lot of
errors, warnings, or messages then the association between them can be
completely lost.
This change adds the tag (cpplint) to the error messages, and adds a
comment to the "failed cpplint.py check" message to suggest looking for
the tag.
Bug: 1309977
Change-Id: I0d073b57f215e28495cbc3e009cab6ac2f23b65e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3632947
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
This snippet of code:
if constexpr (n == 1) {
return 2;
} else if constexpr (n == 2) {
int x = 2;
int y = 2;
return x + y;
}
was triggering the warning:
If/else bodies with multiple statements require braces [readability/braces]
And, in general, cpplint.py was assuming that `if (cond)` was
the only `if` construction to expect, forgetting about
`if constexpr(cond)`.
Change-Id: I4cdc8e7f134c1ebd14d00354a8baadf87c796763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3644147
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
When running git cl presubmit --all which is handy for finding
latent presubmit bugs, the 'No diff found for %s' gets printed many
thousands of times, making actual issues difficult to find. This change
suppresses that message, to make actual issues easier to find.
The message for slow presubmits prints the name of the slow presubmit,
but there are 243 different CheckChangeOnCommit functions, so the name
is actually not sufficient. Therefore this change plumbs through the
path to the script containing the presubmit and prints it.
Similarly, the cpplint message "Done processing %s" doesn't add enough
value.
These changes will make it easier to find the signal in the presubmit
noise.
Bug: 1309977
Change-Id: Iba40b5748266e3296eeb530bb00182db4814aa5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3556594
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: I4b8f8fc521386419a3121bbb07edc8ac83170a94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413679
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
This reverts commit 22bf605bb6.
Reason for revert: breaks gclient sync
Original change's description:
> Use pylint 2.7 for depot_tools
>
> This includes a few fixes for specific errors, and disables several new
> warnings introduced in this version, in order to allow for an incremental migration.
>
> Bug:1262286
> Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bug: 1262286
Change-Id: Ieb946073c7886c7bf056ce843a5a48e82becf7a5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413672
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
C++17 adds support for tuple destructuring. This allow one to write
code such as:
```
std::pair<int, int> span = getSpan();
auto [start, end] = span;
// Use start as span.first and end as span.second
```
This makes cpplint recognize and allow a space before the '[' in this
situation.
This is a purposeful divergence from the internal version where the
entire whitespace/braces category was removed. It was decided to leave
the checks in since this is sometimes used without other formatting
tools.
Test: manual
(cherry picked from commit 26470f9ccb354ff2f6d098f831271a1833701b28
from https://github.com/google/styleguide)
Bug: 1287491
Change-Id: Ib61a75853e19316b1bacf8dc56528f94c17e30a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3389431
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Victor Boivie <boivie@google.com>
__except () is the canonical layout for code that uses __try/__except,
Microsoft's structured exception handling. The majority of __except
uses in our code use a space after the keyword. However cpplint.py
thinks that __except is a function name. Apparently we have been forcing
this through by ignoring the presubmits. This just adds __except to the
list of exceptions to the whitespace/parens rule.
Change-Id: I2384cd32f26e3aa6a4b0150451a9a61147483e2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2173644
Commit-Queue: Aaron Gable <agable@chromium.org>
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
There is a cpplint include check that ensures that all includes have a
directory name preceding them. However, if all source lives in the root
of a repository, there is no point in having a directory.
Because this check lives under 'build/include', it's impossible to
filter without filtering other useful checks.
Move it to 'build/include_directory' so it can be more easily filtered.
BUG=chromium:1073191
TEST=cros lint on a file with 'filter=-build/include_directory' in
CPPLINT.cfg
Change-Id: Ie15fb2398a6d0297ef96219e67076ea262abf853
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2159690
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Fletcher Woodruff <fletcherw@chromium.org>
Cpplint.py supports checking for spaces in front of square brackets;
this is a style violation in most cases (with the typical case being
arrays), but there are already situations where this rule is skipped
for the square bracket used for lambda captures. We add another such
exclusion here for the case of c++ attributes.
Change-Id: I2d4f972be29f5427cb8d855787c90dfa5d610e86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2151125
Commit-Queue: Derek Morris <demorri@microsoft.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
In python3, dict.keys() returns a view instead of a separate
dictionary, which results in the following error while trying
to use cpplint:
"RuntimeError: dictionary changed size during iteration"
Use a list of the keyset instead.
R=ehmaldonado@chromium.org
Bug: 1027160
Change-Id: I0040b8912d7e446d35d755bb021670c9f2248f4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1966743
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Sarthak Kukreti <sarthakkukreti@chromium.org>
Ran:
vi $(git grep --name-only iteritems | grep -v third_party)
vi $(git grep --name-only itervalues | grep -v third_party)
vi $(git grep --name-only 'print ' | grep -v third_party)
and edited the files quickly with adhoc macros. Then ran in recipes/:
./recipes.py test train
There was only a small subset of files that had been updated to use
six.iteritems() and six.itervalues(). Since the dataset size that is
being used in gclient is small (pretty much always below 200 items),
it's better to just switch to .items() right away and take the temporary
performance hit, so that we don't need to come back to rewrite the code.
Recipe-Nontrivial-Roll: build
Bug: 984182
Change-Id: I5faf11486b66b0d73c9098ab0f2ce1b15a45c53e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1854900
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org>
This patch makes some changes to the cpplint.py script for integration
with the VSCode cpplint plugin, available here:
https://marketplace.visualstudio.com/items?itemName=mine.cpplint
Specifically, the "--repository" and "--headers" options currently cause
our version of cpplint to crash, when we should at worst ignore them. A
minor messaging tweak outputting the contents of the GetoptError, to
make debugging input argument bugs easier, is included
Change-Id: Id78616f5e2fdecb7da650042f0658623477ef9cb
Reviewed-on: https://chromium-review.googlesource.com/c/1481938
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Jordan Bayles <jophba@chromium.org>
In the cpplint fn_size check, there's a part where it explicitly
says to ignore macros, BUT the check for macro names didn't allow
for digits, which are present in the IPC_* macro names, which
look like "IPC_SYNC_MESSAGE_CONTROL4_1".
There are two interesting things about this:
1. This bug was filed in 2011! Yay for fixing a 5-digit crbug with
one-line fix!
2. This is a great example of how cpplint's regex-based checking is
error-prone compared to something that actually has access to an
abstract syntax tree.
Bug: 83036
Change-Id: I49acd80c265d7f7bc633f2092388525b4518ae23
Reviewed-on: https://chromium-review.googlesource.com/c/1483283
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
da90c53e3d was incorrect; the size()
functions take almost anything, not just word characters, and
std::extent() uses brackets.
BUG=837308
Change-Id: I6ee75a5da7a021758bbfb0996aeaef81f2003cd9
Reviewed-on: https://chromium-review.googlesource.com/c/1401581
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Using base/std::size and std::extent does not result in the creation
of a VLA; cpplint should not warn on their use.
BUG=837308
Change-Id: Ie133b2144b6c1c9f4f5e406144f28b333a212197
Reviewed-on: https://chromium-review.googlesource.com/c/1399961
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
This also allows digits in class decorators, to also match macros like
V8_EXPORT_PRIVATE.
Note: I uploaded a pull request for the (previous) upstream at
https://github.com/google/styleguide/pull/422, but then was told that
that repository is not maintained any more.
R=jochen@chromium.org
Bug: chromium:916550
Change-Id: I24cd1829b794b296be8e0c2829271c746751ee1c
Reviewed-on: https://chromium-review.googlesource.com/c/1383472
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
The changes include:
- root flag now can process non-subdirectories
-- e7ddd2af62
- root flag can be configured with CPPLINT.cfg
-- 2322e4ffaa
- root setting is relative to the CPPLINT.cfg file
-- 8a87a46cc7
- Cleans up header file detection for hpp and hxx
- Adds quite mode
Bug: 852898
Change-Id: Id44bbfadc913cc27192049ab9a222f22d0deab5d
Reviewed-on: https://chromium-review.googlesource.com/1108381
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
If a constructor is marked constexpr it evades the explicit constructor
check right now, since the check only knows about the inline keyword.
Teach it that constexpr can be used also.
Change-Id: Ia76ddd00b69c5a76a385d22a2b9eb18b690bd8c8
Reviewed-on: https://chromium-review.googlesource.com/445932
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Chrome on iOS downstream repository tracks Chromium via DEPS and wants
to use cpplint canned presubmit check but the cpplint.py errors due at
the include guard as it stops at the inner most git repository.
Add a new parameter --project_root to cpplint.py. If set, it overrides
the automatic detection of the project root that searches the root of
the version control repository.
The --root parameter cannot be used as it is used after the automatic
resolution is performed (and allow chopping the head of the relative
path while the need is to expend the path to include ios_internal/).
BUG=598090
Review-Url: https://codereview.chromium.org/2036773002
This is a cherry-pick of the GitHub commit, created after merging relevant
changes from Google's internal cpplint.py implementation.
456 - Tweak lint to sometimes allow { on line following array initialization.
455 - Recognize more types.
454 - Try a bit harder to detect templated types.
453 - Changed error message wording on build/storage_class to be less ambiguous.
452 - Recognize more types to silence false positives for brace warnings.
451 - <skipped>
450 - <skipped>
449 - Ignore whitespace/tab for Linux Kernel files.
448 - Remove some more false positive lint warnings for int64{n}.
447 - Just one warning message for line length is enough, don't need two.
446 - Allow braced conversions to int16, uint64, etc. per the style guide.
445 - Fixed handling of backslash escapes for checking whether a "//" is quoted.
444 - Reduced dependency on hardcoded .cc extension.
443 - Disable single-arg constructor checks by default, since ClangTidy has the
same check. We could just delete the check entirely, but it's slightly
useful in places that can't run ClangTidy.
442 - Fix matching of macro names in CheckTrailingSemicolon to include digits.
441 - Deleted checks:
- All checks for RValue references.
- Spacing check around boolean &&, because those look like RValue
references.
A huge amount of code and effort were dedicated to tell RValue
references apart from boolean expressions, and to differentiate
allowed versus banned RValue references. But we still get regular
false positives from this one check. Rather than making the check
more complex than what it already is, it seemed safer to just delete
it altogether for now, and have a different process for catching
RValue references.
440 - Add rule to cpplint to throw error on empty if statement bodies without
else clauses. If statement bodies with comments are not considered empty.
439 - Allow spaces before closing brace of namespace block so that namespaces
inside of macro definitions (where the entire macro definition is
indented) are not treated as errors.
438 - cpplint: fix a false positive on `new const int(x)`.
437 - Only check for function length when current line is inside a function.
436 - cpplint: Catch static `std::string` instances as well as those written as
`string`. Previously users would sometimes work around the lint warning by
changing their code to even worse code by adding the "std::" prefix.
435 - cpplint: Be a little smarter about warning on indentation.
434 - cpplint: Remove false positives on some functions returning string const&.
433 - cpplint: improve diagnostics of global/static string objects.
432 - Allow non-const reference parameters for iostream based types.
431 - Better handling of raw strings inside comments.
430 - <noop>
429 - <skipped>
428 - <skipped>
427 - Add support to CHECK_NOTNULL when checking if a member variable is
initialized with itself.
426 - Allow suppressing specific warnings in C headers.
425 - Allow spaces before parens for inline assembly.
424 - Remove lint checks for {EXPECT,ASSERT}_.*_M.
423 - Allow comment lines of more than 80 characters if they contain a single
"word" (without any spaces).
422 - cpplint: Warn on inclusion of C++14 headers.
421 - cpplint: recognize <shared_mutex> as a standard library header.
420 - Add <scoped_allocator> to cpplint's list of C++ standard headers.
419 - Add cpplint check for tr1/ headers. We removed the nanny guards for these
418 - <skipped>
417 - Update the styleguide and cpplint to allow unnamed parameters
416 - Remove the rule explicitly banning non-default move operations.
415 - Remove the check for explicit multi arg constructors
414 - Include clarity of lambda default captures in "pros" section, mention
of capturing `this` and lambdas which escape the current scope in "cons".
Soften the ban on default captures to a preference, with admonition against
using them for capturing `this` or when they will escape the current scope.
This is a fairly straightforward change with a brief inclusion
of 2 problematic cases for implicit capture. As this is the style
guide, I'm not sure how much more detail is appropriate.
413 - Fixed a bug by making regex in CleanseRawStrings match multiple raw strings
on a single line in left-to-right order.
412 - Fixed false positive for classes derived using decltype()
411 - <skipped>
410 - Recognize '1LL<<20' as numeric and don't flag it for spacing.
BUG=
Review-Url: https://codereview.chromium.org/1959503003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300481 0039d316-1c4b-4281-b951-d872f2087c98
Reason for revert:
This breaks WebRTC presubmit on the buildbots (due to /b/build being a Git repo, which in turn contains the checkout). See https://bugs.chromium.org/p/webrtc/issues/detail?id=5818 for more details.
It seems it also broke other projects (rakuco's comment #15).
Original issue's description:
> Convert cpplint.py to work with nested git repositories.
>
> Chrome on iOS downstream repository tracks Chromium via DEPS and wants
> to use cpplint canned presubmit check but the cpplint.py errors due at
> the include guard as it stops at the inner most git repository.
>
> Change cpplint.py to look for the outermost git repository when looking
> for the repository root.
>
> BUG=598090
>
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300085
TBR=iannucci@chromium.org,skym@chromium.org,sdefresne@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=598090
Review URL: https://codereview.chromium.org/1919483002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300119 0039d316-1c4b-4281-b951-d872f2087c98
Chrome on iOS downstream repository tracks Chromium via DEPS and wants
to use cpplint canned presubmit check but the cpplint.py errors due at
the include guard as it stops at the inner most git repository.
Change cpplint.py to look for the outermost git repository when looking
for the repository root.
BUG=598090
Review URL: https://codereview.chromium.org/1897153003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300085 0039d316-1c4b-4281-b951-d872f2087c98
In C++11 std::swap was moved from <algorithm> to <utility>. As such,
lint needed to be updated to look for/suggest <utility> when it finds
std::swap. The IWYU logic is a little bit different for <utility>
functions, including swap, because they do not typically include
template parameters, unlike most other std:: functions. Reworked the
existing algorithm pattern to be more generic and support multiple
differet headers. Did not rename/all caps _re_pattern_templates as it
is referenced/modified outside of cpplint.py.
BUG=584689
Review URL: https://codereview.chromium.org/1673543004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@298622 0039d316-1c4b-4281-b951-d872f2087c98
Added the ability to provide CPPLINT.cfg files to provide
linter message filters per sub-directory and special exclusion
rules. Each file can have instructions like:
filter=-build/include_order,+build/include_alpha
exclude_files=.*\.cc
The above disables build/include_order warning and enables
build/include_alpha as well as excludes all .cc from being
processed by linter, in the current directory (where the .cfg
file is located) and all sub-directories.
Related CL: https://chromium-review.googlesource.com/#/c/209384/
BUG=chromium:395296
TEST=ran cpplint.py on a bunch of directories in platform2/
Review URL: https://codereview.chromium.org/406373002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@285999 0039d316-1c4b-4281-b951-d872f2087c98
Sylvain Defresne
Matthias Liedtke
Peter Kasting
dan sinclair
Bruce Dawson
David Sanders
Darius M
Ryan Macnak
Aravind Vasudevan
Josip Sokcevic
Alex Light
Ayu Ishii
Fletcher Woodruff
Derek Morris
Sarthak Kukreti
Edward Lemur
Jasper Chapman-Black
Marc-Antoine Ruel
Jordan Bayles
Quinten Yearsley
Avi Drissman
Clemens Hammacher
Sergiy Byelozyorov
Amin Hassani
Alexandr Ilin
danakj
agable
lhchavez
avakulenko@chromium.org
kjellander@chromium.org
skym@chromium.org
avakulenko@google.com
avakulenko@google.com
raphael.kubo.da.costa@intel.com
mazda@chromium.org
erg@chromium.org