Upgrade cpplint to external upstream target

CPPLINT.cfg

@@ -12,6 +12,10 @@ set noparent
 # this warning is irrelevant.
 filter=-build/c++11
 
+# Unlike Google, drake embraces <filesystem>.
+# https://drake.mit.edu/styleguide/cppguide.html#Other_Features
+filter=-build/c++17
+
 # Drake uses `#pragma once`, not the `#ifndef FOO_H` guard.
 # https://drake.mit.edu/styleguide/cppguide.html#The__pragma_once_Guard
 filter=-build/header_guard
@@ -39,6 +43,10 @@ filter=-build/include
 # by the readability/todo rule, which we leave enabled.)
 filter=-whitespace/todo
 
+# TODO(jwnimmer-tri) This has hundreds of false positives. Seems completely
+# broken. With our growing use of clang-format, we probably don't care.
+filter=-whitespace/indent_namespace
+
 # TODO(#1805) Fix this.
 filter=-legal/copyright
 

MODULE.bazel

@@ -191,6 +191,7 @@ use_repo(
     "common_robotics_utilities_internal",
     "commons_io",  # TODO(jwnimmer-tri) Mark as internal.
     "conex_internal",
+    "cpplint_internal",
     "csdp_internal",
     "curl_internal",
     "dm_control_internal",

bindings/pydrake/common/text_logging_pybind.cc

@@ -64,7 +64,7 @@ class pylogging_sink final
       is_configured_.store(true);
     }
 
-    // NOLINTNEXTLINE(build/namespaces) This is how pybind11 wants it.
+    // NOLINTNEXTLINE(build/namespaces_literals) This is how pybind11 wants it.
     using namespace pybind11::literals;
 
     // Construct the LogRecord.

common/hwy_dynamic_impl.h

@@ -9,7 +9,7 @@
 // This file should only ever be included from `*.cc` implementation files,
 // and we always want its code to be private to that file, so we'll use an
 // anonymous namespace in a header file, for simplicity.
-namespace {  // NOLINT(build/namespaces)
+namespace {  // NOLINT(build/namespaces_headers)
 
 /* LateBoundFunction is a small wrapper class around a C-style raw function
 pointer. (It doesn't support std::function objects.)

common/overloaded.h

@@ -27,7 +27,7 @@ use the variant visit pattern, i.e.
 
 // We put this in the anonymous namespace to ensure that it will not generate
 // linkable symbols for every specialization in every library that uses it.
-namespace {  // NOLINT(build/namespaces)
+namespace {  // NOLINT(build/namespaces_headers)
 
 // Boilerplate for `std::visit`; see
 // https://en.cppreference.com/w/cpp/utility/variant/visit

doc/_pages/clion.md

@@ -244,7 +244,7 @@ following values for the fields:
 * **Name:** ``Cpplint File``
 * **Description:** ``Apply cpplint to the current file``
 * **Program:** ``bazel``
-* **Arguments:** ``run @styleguide//:cpplint -- --output=eclipse
+* **Arguments:** ``run @cpplint_internal//:cpplint -- --output=eclipse
                  $FilePath$``
 * **Working directory:** ``$Projectpath$``
 * **Advanced Options:** Confirm ``Open console for tool output`` is checked
@@ -304,7 +304,7 @@ Change the following fields in the instructions given above:
 
 Building the google styleguide lint tool:
 
-``bazel build @styleguide//:cpplint``
+``bazel build @cpplint_internal//:cpplint``
 
 **Drake style addenda**
 

examples/pendulum/pendulum_parameters_derivatives.cc

@@ -47,7 +47,7 @@ int DoMain() {
   const PendulumState<AutoDiffXd>& xdot =
       PendulumPlant<AutoDiffXd>::get_state(*derivatives);
 
-  // NOLINTNEXTLINE(build/namespaces)  Usage documented by fmt library.
+  // NOLINTNEXTLINE(build/namespaces_literals)  Usage documented by fmt library.
   using namespace fmt::literals;
   // Derivatives values
   fmt::print("Forward dynamics, ẋ = [θ̇, ω̇]:\n");

tools/lint/cpplint.bzl

@@ -73,10 +73,10 @@ def _add_linter_rules(
     # Google cpplint.
     py_test_isolated(
         name = name + "_cpplint",
-        srcs = ["@styleguide//:cpplint"],
+        srcs = ["@cpplint_internal//:cpplint"],
         data = cpplint_data + source_labels,
         args = _EXTENSIONS_ARGS + source_filenames,
-        main = "@styleguide//:cpplint/cpplint.py",
+        main = "@cpplint_internal//:cpplint.py",
         size = size,
         tags = ["cpplint", "lint"],
     )

tools/workspace/cpplint_internal/BUILD.bazel

@@ -0,0 +1,3 @@
+load("//tools/lint:lint.bzl", "add_lint_tests")
+
+add_lint_tests()

tools/workspace/cpplint_internal/package.BUILD.bazel

@@ -0,0 +1,17 @@
+# -*- bazel -*-
+
+load("@drake//tools/skylark:py.bzl", "py_binary")
+
+licenses(["notice"])  # BSD-3-Clause
+
+package(default_visibility = ["//visibility:public"])
+
+py_binary(
+    name = "cpplint",
+    srcs = [
+        "cpplint.py",
+    ],
+    main = "cpplint.py",
+    python_version = "PY3",
+    srcs_version = "PY3",
+)

tools/workspace/cpplint_internal/patches/transitive_includes.patch

@@ -0,0 +1,96 @@
+diff --git a/cpplint.py b/cpplint.py
+index 7a8b8ab..b52a4e5 100755
+--- cpplint.py
++++ cpplint.py
+@@ -6192,24 +6192,12 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
+     if not line or line[0] == '#':
+       continue
+
+-    _re_patterns = []
+-    _re_patterns.extend(_re_pattern_types_or_objs)
+-    _re_patterns.extend(_re_pattern_functions)
+-    for pattern, item, header in _re_patterns:
+-      matched = pattern.search(line)
+-      if matched:
+-        # Don't warn about strings in non-STL namespaces:
+-        # (We check only the first match per line; good enough.)
+-        prefix = line[:matched.start()]
+-        if prefix.endswith('std::') or not prefix.endswith('::'):
+-          required[header] = (linenum, item)
+-
+     for pattern, template, header in _re_pattern_headers_maybe_templates:
+       if pattern.search(line):
+         required[header] = (linenum, template)
+
+     # The following function is just a speed up, no semantics are changed.
+-    if '<' not in line:  # Reduces the cpu time usage by skipping lines.
++    if not '<' in line:  # Reduces the cpu time usage by skipping lines.
+       continue
+
+     for pattern, template, header in _re_pattern_templates:
+@@ -6221,25 +6209,57 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
+         if prefix.endswith('std::') or not prefix.endswith('::'):
+           required[header] = (linenum, template)
+
++  # The policy is that if you #include something in foo.h you don't need to
++  # include it again in foo.cc. Here, we will look at possible includes.
+   # Let's flatten the include_state include_list and copy it into a dictionary.
+   include_dict = dict([item for sublist in include_state.include_list
+                        for item in sublist])
+
++  # Did we find the header for this file (if any) and successfully load it?
++  header_found = False
++
++  # Use the absolute path so that matching works properly.
++  abs_filename = FileInfo(filename).FullName()
++
++  # For Emacs's flymake.
++  # If cpplint is invoked from Emacs's flymake, a temporary file is generated
++  # by flymake and that file name might end with '_flymake.cc'. In that case,
++  # restore original file name here so that the corresponding header file can be
++  # found.
++  # e.g. If the file name is 'foo_flymake.cc', we should search for 'foo.h'
++  # instead of 'foo_flymake.h'
++  abs_filename = re.sub(r'_flymake\.cc$', '.cc', abs_filename)
++
++  # include_dict is modified during iteration, so we iterate over a copy of
++  # the keys.
++  header_keys = list(include_dict.keys())
++  for header in header_keys:
++    (same_module, common_path) = FilesBelongToSameModule(abs_filename, header)
++    fullpath = common_path + header
++    if same_module:
++      header_found = True
++
++  # If we can't find the header file for a .cc, assume it's because we don't
++  # know where to look. In that case we'll give up as we're not sure they
++  # didn't include it in the .h file.
++  # TODO(unknown): Do a better job of finding .h files so we are confident that
++  # not having the .h file means there isn't one.
++  if filename.endswith('.cc') and not header_found:
++    return
++
+   # All the lines have been processed, report the errors found.
+-  for header in sorted(required, key=required.__getitem__):
+-    template = required[header][1]
+-    header_stripped = header.strip('<>"')
+-    if (header_stripped not in include_dict
+-            and not (header_stripped[0] == 'c'
+-                     and (header_stripped[1:] + '.h') in include_dict)):
+-      error(filename, required[header][0],
++  for required_header_unstripped in required:
++    template = required[required_header_unstripped][1]
++    if required_header_unstripped.strip('<>"') not in include_dict:
++      error(filename, required[required_header_unstripped][0],
+             'build/include_what_you_use', 4,
+-            'Add #include ' + header + ' for ' + template)
++            'Add #include ' + required_header_unstripped + ' for ' + template)
+
+
+ _RE_PATTERN_EXPLICIT_MAKEPAIR = re.compile(r'\bmake_pair\s*<')
+
+
++
+ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error):
+   """Check that make_pair's template arguments are deduced.
+

tools/workspace/cpplint_internal/patches/whitespace_newline.patch

@@ -0,0 +1,49 @@
+Address discrepancy between cpplint upstream and Google styleguide.
+
+Google styleguide allows for certain simple if statements to fit on
+one line with spaces between the curly braces. While this is technically
+a violation of the general styleguide, Google permits this exception
+"for historical reasons." Nevertheless, cpplint does not allow this.
+
+The fix basically adds a "made-up" error category and diverts the
+expressions that trigger the aforementioned cases out of the more
+general "whitespace/newline" category. The "made-up" name is then
+added to the "_DEFAULT_FILTERS" list, effectively suppressing the
+error violations in these specific cases.
+
+diff --git a/cpplint.py b/cpplint.py
+index 7a8b8ab..130fd9d 100755
+--- cpplint.py
++++ cpplint.py
+@@ -355,6 +355,7 @@ _ERROR_CATEGORIES = [
+     'whitespace/indent_namespace',
+     'whitespace/line_length',
+     'whitespace/newline',
++    'whitespace/newline_controlled_statement',
+     'whitespace/operators',
+     'whitespace/parens',
+     'whitespace/semicolon',
+@@ -414,6 +415,7 @@ _OTHER_NOLINT_CATEGORY_PREFIXES = [
+ _DEFAULT_FILTERS = [
+     '-build/include_alpha',
+     '-readability/fn_size',
++    '-whitespace/newline_controlled_statement',
+     ]
+
+ # The default list of categories suppressed for C (not C++) files.
+@@ -4390,13 +4392,13 @@ def CheckBraces(filename, clean_lines, linenum, error):
+   if keyword := re.search(
+       r'\b(else if|if|while|for|switch)'  # These have parens
+       r'\s*\(.*\)\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\};]', line):
+-    error(filename, linenum, 'whitespace/newline', 5,
++    error(filename, linenum, 'whitespace/newline_controlled_statement', 5,
+           f'Controlled statements inside brackets of {keyword.group(1)} clause'
+           ' should be on a separate line')
+   elif keyword := re.search(
+       r'\b(else|do|try)'  # These don't have parens
+       r'\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\}]', line):
+-    error(filename, linenum, 'whitespace/newline', 5,
++    error(filename, linenum, 'whitespace/newline_controlled_statement', 5,
+           f'Controlled statements inside brackets of {keyword.group(1)} clause'
+           ' should be on a separate line')
+

tools/workspace/cpplint_internal/repository.bzl

@@ -0,0 +1,17 @@
+load("//tools/workspace:github.bzl", "github_archive")
+
+def cpplint_internal_repository(
+        name,
+        mirrors = None):
+    github_archive(
+        name = name,
+        repository = "cpplint/cpplint",
+        commit = "80da3c1ef37af017715dc1366fbda8263179bdeb",
+        sha256 = "006d8ce87a2dcc8a644b78fe0c2d4d45bc52ca41414ea66d3c7dcd6ae3e61d9e",  # noqa
+        build_file = ":package.BUILD.bazel",
+        patches = [
+            ":patches/transitive_includes.patch",
+            ":patches/whitespace_newline.patch",
+        ],
+        mirrors = mirrors,
+    )

tools/workspace/default.bzl

@@ -16,6 +16,7 @@ load("//tools/workspace/com_jidesoft_jide_oss:repository.bzl", "com_jidesoft_jid
 load("//tools/workspace/common_robotics_utilities_internal:repository.bzl", "common_robotics_utilities_internal_repository", "common_robotics_utilities_repository")  # noqa
 load("//tools/workspace/commons_io:repository.bzl", "commons_io_repository")
 load("//tools/workspace/conex_internal:repository.bzl", "conex_internal_repository")  # noqa
+load("//tools/workspace/cpplint_internal:repository.bzl", "cpplint_internal_repository")  # noqa
 load("//tools/workspace/crate_universe:repository.bzl", "crate_universe_repositories")  # noqa
 load("//tools/workspace/csdp_internal:repository.bzl", "csdp_internal_repository")  # noqa
 load("//tools/workspace/curl_internal:repository.bzl", "curl_internal_repository")  # noqa
@@ -171,6 +172,8 @@ def add_default_repositories(
         commons_io_repository(name = "commons_io", mirrors = mirrors, _is_drake_self_call = True)  # noqa
     if "conex_internal" not in excludes:
         conex_internal_repository(name = "conex_internal", mirrors = mirrors)
+    if "cpplint_internal" not in excludes:
+        cpplint_internal_repository(name = "cpplint_internal", mirrors = mirrors)  # noqa
     if "crate_universe" not in excludes:
         crate_universe_repositories(mirrors = mirrors, excludes = excludes, _is_drake_self_call = True)  # noqa
     if "csdp_internal" not in excludes:

tools/workspace/styleguide/BUILD.bazel

@@ -1,13 +1,3 @@
 load("//tools/lint:lint.bzl", "add_lint_tests")
-load("//tools/skylark:py.bzl", "py_test")
-
-py_test(
-    name = "cpplint_unittest",
-    size = "small",
-    srcs = ["@styleguide//:test_files"],
-    data = ["@styleguide//:test_files"],
-    main = "@styleguide//:cpplint/cpplint_unittest.py",
-    tags = ["no_kcov"],
-)
 
 add_lint_tests()

tools/workspace/styleguide/package.BUILD.bazel

@@ -1,7 +1,5 @@
 # -*- bazel -*-
 
-load("@drake//tools/skylark:py.bzl", "py_binary")
-
 licenses(["notice"])  # BSD-3-Clause
 
 package(default_visibility = ["//visibility:public"])
@@ -11,30 +9,3 @@ exports_files(glob([
     "*",
     "include/*",
 ]))
-
-# We can't set name="cpplint" here because that's the directory name so the
-# sandbox gets confused.  We'll give it a private name with a public alias.
-py_binary(
-    name = "cpplint_binary",
-    srcs = ["cpplint/cpplint.py"],
-    imports = ["cpplint"],
-    main = "cpplint/cpplint.py",
-    python_version = "PY3",
-    srcs_version = "PY3",
-    visibility = [],
-)
-
-alias(
-    name = "cpplint",
-    actual = ":cpplint_binary",
-)
-
-filegroup(
-    name = "test_files",
-    testonly = True,
-    srcs = [
-        "cpplint/cpplint.py",
-        "cpplint/cpplint_test_header.h",
-        "cpplint/cpplint_unittest.py",
-    ],
-)