tooling: Use upstream checkers

ci/do_ci.sh

@@ -459,7 +459,7 @@ elif [[ "$CI_TARGET" == "deps" ]]; then
   "${ENVOY_SRCDIR}"/ci/check_repository_locations.sh
 
   # Run pip requirements tests
-  bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/dependency:pip_check "${ENVOY_SRCDIR}"
+  bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/dependency:pip_check
 
   exit 0
 elif [[ "$CI_TARGET" == "cve_scan" ]]; then

ci/format_pre.sh

@@ -53,7 +53,7 @@ CURRENT=configs
 bazel run "${BAZEL_BUILD_OPTIONS[@]}" //configs:example_configs_validation
 
 CURRENT=python
-bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:python_check -- --diff-file="$DIFF_OUTPUT" --fix "$(pwd)"
+bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:python_check -- --diff-file="$DIFF_OUTPUT" --fix
 
 CURRENT=extensions
 bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/extensions:extensions_check

tools/base/requirements.in

@@ -3,6 +3,8 @@ colorama
 coloredlogs
 coverage
 envoy.base.utils
+envoy.code_format.python_check>=0.0.4
+envoy.dependency.pip_check>=0.0.4
 envoy.distribution.release
 envoy.distribution.verify
 envoy.gpg.sign

tools/base/requirements.txt

@@ -2,14 +2,16 @@
 # This file is autogenerated by pip-compile
 # To update, run:
 #
-#    pip-compile --allow-unsafe --generate-hashes tools/base/requirements.in
+#    pip-compile --allow-unsafe --generate-hashes requirements.in
 #
 abstracts==0.0.12 \
     --hash=sha256:acc01ff56c8a05fb88150dff62e295f9071fc33388c42f1dfc2787a8d1c755ff
     # via
     #   aio.functional
     #   envoy.abstract.command
     #   envoy.base.utils
+    #   envoy.code-format.python-check
+    #   envoy.dependency.pip-check
     #   envoy.github.abstract
     #   envoy.github.release
 aio.functional==0.0.9 \
@@ -22,9 +24,13 @@ aio.functional==0.0.9 \
 aio.stream==0.0.2 \
     --hash=sha256:6f5baaff48f6319db134cd56c06ccf89db1f7c5f67a26382e081efc96f2f675d
     # via envoy.github.release
+aio.subprocess==0.0.4 \
+    --hash=sha256:fd504a7c02423c40fde19ad87b62932b9eaa091f5a22d26b89b452059a728750
+    # via envoy.code-format.python-check
 aio.tasks==0.0.4 \
     --hash=sha256:9abd4b0881edb292c4f91a2f63b1dea7a9829a4bd4e8440225a1a412a90461fc
     # via
+    #   envoy.code-format.python-check
     #   envoy.github.abstract
     #   envoy.github.release
 aiodocker==0.21.0 \
@@ -263,6 +269,8 @@ envoy.abstract.command==0.0.3 \
 envoy.base.checker==0.0.2 \
     --hash=sha256:2ac81efa20fd01fff644ff7dc7fadeac1c3e4dbb6210881ac7a7919ec0e048d8
     # via
+    #   envoy.code-format.python-check
+    #   envoy.dependency.pip-check
     #   envoy.distribution.distrotest
     #   envoy.distribution.verify
 envoy.base.runner==0.0.4 \
@@ -276,9 +284,17 @@ envoy.base.utils==0.0.8 \
     --hash=sha256:b82e18ab0535207b7136d6980239c9350f7113fa5da7dda781bcb6ad1e05b3ab
     # via
     #   -r requirements.in
+    #   envoy.code-format.python-check
+    #   envoy.dependency.pip-check
     #   envoy.distribution.distrotest
     #   envoy.github.release
     #   envoy.gpg.sign
+envoy.code-format.python-check==0.0.4 \
+    --hash=sha256:5e166102d1f873f0c14640bcef87b46147cbad1cb68888c977acfde7fce96e04
+    # via -r requirements.in
+envoy.dependency.pip-check==0.0.4 \
+    --hash=sha256:3213d77959f65c3c97e9b5d74cb14c02bc02dae64bac2e7c3cb829a2f4e5e40e
+    # via -r requirements.in
 envoy.distribution.distrotest==0.0.3 \
     --hash=sha256:c094adbd959eb1336f93afc00aedb7ee4e68e8252e2365be816a6f9ede8a3de7
     # via envoy.distribution.verify
@@ -305,17 +321,18 @@ envoy.gpg.identity==0.0.2 \
 envoy.gpg.sign==0.0.3 \
     --hash=sha256:31667931f5d7ff05fd809b89748f277511486311c777652af4cb8889bd641049
     # via -r requirements.in
+flake8-polyfill==1.0.2 \
+    --hash=sha256:12be6a34ee3ab795b19ca73505e7b55826d5f6ad7230d31b18e106400169b9e9 \
+    --hash=sha256:e44b087597f6da52ec6393a709e7108b2905317d0c0b744cdca6208e670d8eda
+    # via pep8-naming
 flake8==3.9.2 \
     --hash=sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b \
     --hash=sha256:bf8fd333346d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907
     # via
     #   -r requirements.in
+    #   envoy.code-format.python-check
     #   flake8-polyfill
     #   pep8-naming
-flake8-polyfill==1.0.2 \
-    --hash=sha256:12be6a34ee3ab795b19ca73505e7b55826d5f6ad7230d31b18e106400169b9e9 \
-    --hash=sha256:e44b087597f6da52ec6393a709e7108b2905317d0c0b744cdca6208e670d8eda
-    # via pep8-naming
 frozendict==2.0.6 \
     --hash=sha256:3f00de72805cf4c9e81b334f3f04809278b967d2fed84552313a0fcce511beb1 \
     --hash=sha256:5d3f75832c35d4df041f0e19c268964cbef29c1eb34cd3517cf883f1c2d089b9
@@ -471,7 +488,9 @@ packaging==21.0 \
 pep8-naming==0.12.1 \
     --hash=sha256:4a8daeaeb33cfcde779309fc0c9c0a68a3bbe2ad8a8308b763c5068f86eb9f37 \
     --hash=sha256:bb2455947757d162aa4cad55dba4ce029005cd1692f2899a21d51d8630ca7841
-    # via -r requirements.in
+    # via
+    #   -r requirements.in
+    #   envoy.code-format.python-check
 pluggy==1.0.0 \
     --hash=sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159 \
     --hash=sha256:74134bbf457f031a36d68416e1509f34bd5ccc019f0bcc952c7b909d06b37bd3
@@ -535,14 +554,6 @@ pyparsing==2.4.7 \
 pyreadline==2.1 \
     --hash=sha256:4530592fc2e85b25b1a9f79664433da09237c1a270e4d78ea5aa3a2c7229e2d1
     # via -r requirements.in
-pytest==6.2.5 \
-    --hash=sha256:131b36680866a76e6781d13f101efb86cf674ebb9762eb70d3082b6f29889e89 \
-    --hash=sha256:7310f8d27bc79ced999e760ca304d69f6ba6c6649c0b60fb0e04a4a77cacc134
-    # via
-    #   -r requirements.in
-    #   pytest-asyncio
-    #   pytest-cov
-    #   pytest-patches
 pytest-asyncio==0.15.1 \
     --hash=sha256:2564ceb9612bbd560d19ca4b41347b54e7835c2f792c504f698e05395ed63f6f \
     --hash=sha256:3042bcdf1c5d978f6b74d96a151c4cfb9dcece65006198389ccd7e6c60eb1eea
@@ -554,6 +565,14 @@ pytest-cov==2.12.1 \
 pytest-patches==0.0.3 \
     --hash=sha256:6f8cdc8641c708c4812f58ae48d410f373a6fd16cd6cc4dc4d3fb8951df9c92a
     # via -r requirements.in
+pytest==6.2.5 \
+    --hash=sha256:131b36680866a76e6781d13f101efb86cf674ebb9762eb70d3082b6f29889e89 \
+    --hash=sha256:7310f8d27bc79ced999e760ca304d69f6ba6c6649c0b60fb0e04a4a77cacc134
+    # via
+    #   -r requirements.in
+    #   pytest-asyncio
+    #   pytest-cov
+    #   pytest-patches
 python-gnupg==0.4.7 \
     --hash=sha256:2061f56b1942c29b92727bf9aecbd3cea3893acc9cccbdc7eb4604285efe4ac7 \
     --hash=sha256:3ff5b1bf5e397de6e1fe41a7c0f403dad4e242ac92b345f440eaecfb72a7ebae
@@ -615,16 +634,6 @@ snowballstemmer==2.1.0 \
     --hash=sha256:b51b447bea85f9968c13b650126a888aabd4cb4463fca868ec596826325dedc2 \
     --hash=sha256:e997baa4f2e9139951b6f4c631bad912dfd3c792467e2f03d7239464af90e914
     # via sphinx
-sphinx==4.1.2 \
-    --hash=sha256:3092d929cd807926d846018f2ace47ba2f3b671b309c7a89cd3306e80c826b13 \
-    --hash=sha256:46d52c6cee13fec44744b8c01ed692c18a640f6910a725cbb938bc36e8d64544
-    # via
-    #   -r requirements.in
-    #   sphinx-copybutton
-    #   sphinx-rtd-theme
-    #   sphinx-tabs
-    #   sphinxcontrib-httpdomain
-    #   sphinxext-rediraffe
 sphinx-copybutton==0.4.0 \
     --hash=sha256:4340d33c169dac6dd82dce2c83333412aa786a42dd01a81a8decac3b130dc8b0 \
     --hash=sha256:8daed13a87afd5013c3a9af3575cc4d5bec052075ccd3db243f895c07a689386
@@ -637,6 +646,16 @@ sphinx-tabs==3.2.0 \
     --hash=sha256:1e1b1846c80137bd81a78e4a69b02664b98b1e1da361beb30600b939dfc75065 \
     --hash=sha256:33137914ed9b276e6a686d7a337310ee77b1dae316fdcbce60476913a152e0a4
     # via -r requirements.in
+sphinx==4.1.2 \
+    --hash=sha256:3092d929cd807926d846018f2ace47ba2f3b671b309c7a89cd3306e80c826b13 \
+    --hash=sha256:46d52c6cee13fec44744b8c01ed692c18a640f6910a725cbb938bc36e8d64544
+    # via
+    #   -r requirements.in
+    #   sphinx-copybutton
+    #   sphinx-rtd-theme
+    #   sphinx-tabs
+    #   sphinxcontrib-httpdomain
+    #   sphinxext-rediraffe
 sphinxcontrib-applehelp==1.0.2 \
     --hash=sha256:806111e5e962be97c29ec4c1e7fe277bfd19e9652fb1a4392105b43e01af885a \
     --hash=sha256:a072735ec80e7675e3f432fcae8610ecf509c5f1869d17e2eecff44389cdbc58
@@ -710,7 +729,9 @@ wrapt==1.12.1 \
 yapf==0.31.0 \
     --hash=sha256:408fb9a2b254c302f49db83c59f9aa0b4b0fd0ec25be3a5c51181327922ff63d \
     --hash=sha256:e3a234ba8455fe201eaa649cdac872d590089a18b661e39bbac7020978dd9c2e
-    # via -r requirements.in
+    # via
+    #   -r requirements.in
+    #   envoy.code-format.python-check
 yarl==1.6.3 \
     --hash=sha256:00d7ad91b6583602eb9c1d085a2cf281ada267e9a197e8b7cae487dadbfa293e \
     --hash=sha256:0355a701b3998dcd832d0dc47cc5dedf3874f966ac7f870e0f3a6788d802d434 \

tools/code_format/BUILD

@@ -1,6 +1,6 @@
 load("@base_pip3//:requirements.bzl", "requirement")
+load("@rules_python//python:defs.bzl", "py_binary")
 load("//bazel:envoy_build_system.bzl", "envoy_package")
-load("//tools/base:envoy_python.bzl", "envoy_py_binary")
 
 licenses(["notice"])  # Apache 2
 
@@ -12,14 +12,11 @@ exports_files([
     "envoy_build_fixer.py",
 ])
 
-envoy_py_binary(
-    name = "tools.code_format.python_check",
+py_binary(
+    name = "python_check",
+    srcs = ["python_check.py"],
     deps = [
-        "//tools/base:aio",
-        "//tools/base:checker",
-        "//tools/base:utils",
-        requirement("flake8"),
-        requirement("pep8-naming"),
-        requirement("yapf"),
+        "@envoy_repo",
+        requirement("envoy.code_format.python_check"),
     ],
 )

tools/code_format/python_check.py

@@ -4,135 +4,51 @@
 #
 # with bazel:
 #
-#  bazel run //tools/code_format:python_check -- -h
+#  $ bazel run //tools/code_format:python_check -- -h
 #
-# alternatively, if you have the necessary python deps available
+#  $ bazel run //tools/code_format:python_check
 #
-#  PYTHONPATH=. ./tools/code_format/python_check.py -h
+# with pip:
 #
-# python requires: flake8, yapf
+#  $ pip install envoy.code_format.python_check
+#  $ envoy.code_format.python_check -h
+#
+# usage with pip requires a path, eg
+#
+#  $ envoy.code_format.python_check .
+#
+# The upstream lib is maintained here:
+#
+#    https://github.com/envoyproxy/pytooling/tree/main/envoy.code_format.python_check
+#
+# Please submit issues/PRs to the pytooling repo:
+#
+#    https://github.com/envoyproxy/pytooling
 #
 
-import argparse
 import pathlib
 import sys
 from functools import cached_property
-from typing import Iterable, List, Optional, Tuple
 
-from flake8.main.application import Application as Flake8Application  # type:ignore
+import abstracts
 
-import yapf  # type:ignore
+from envoy.code_format import python_check
 
-from tools.base import aio, checker, utils
+import envoy_repo
 
-FLAKE8_CONFIG = '.flake8'
-YAPF_CONFIG = '.style.yapf'
 
-# TODO(phlax): add checks for:
-#      - isort
-
-
-class PythonChecker(checker.AsyncChecker):
-    checks = ("flake8", "yapf")
-
-    @property
-    def diff_file_path(self) -> Optional[pathlib.Path]:
-        return pathlib.Path(self.args.diff_file) if self.args.diff_file else None
+@abstracts.implementer(python_check.APythonChecker)
+class EnvoyPythonChecker:
 
     @cached_property
-    def flake8_app(self) -> Flake8Application:
-        flake8_app = Flake8Application()
-        flake8_app.initialize(self.flake8_args)
-        return flake8_app
-
-    @property
-    def flake8_args(self) -> Tuple[str, ...]:
-        return ("--config", str(self.flake8_config_path), str(self.path))
-
-    @property
-    def flake8_config_path(self) -> pathlib.Path:
-        return self.path.joinpath(FLAKE8_CONFIG)
-
-    @property
-    def recurse(self) -> bool:
-        """Flag to determine whether to apply checks recursively"""
-        return self.args.recurse
-
-    @property
-    def yapf_config_path(self) -> pathlib.Path:
-        return self.path.joinpath(YAPF_CONFIG)
-
-    @property
-    def yapf_files(self) -> List[str]:
-        return yapf.file_resources.GetCommandLineFiles(
-            self.args.paths,
-            recursive=self.recurse,
-            exclude=yapf.file_resources.GetExcludePatternsForDir(str(self.path)))
-
-    def add_arguments(self, parser: argparse.ArgumentParser) -> None:
-        super().add_arguments(parser)
-        parser.add_argument(
-            "--recurse",
-            "-r",
-            choices=["yes", "no"],
-            default="yes",
-            help="Recurse path or paths directories")
-        parser.add_argument(
-            "--diff-file", default=None, help="Specify the path to a diff file with fixes")
-
-    async def check_flake8(self) -> None:
-        """Run flake8 on files and/or repo"""
-        errors: List[str] = []
-        with utils.buffered(stdout=errors, mangle=self._strip_lines):
-            self.flake8_app.run_checks()
-            self.flake8_app.report()
-        if errors:
-            self.error("flake8", errors)
-
-    async def check_yapf(self) -> None:
-        """Run flake8 on files and/or repo"""
-        futures = aio.concurrent(self.yapf_format(python_file) for python_file in self.yapf_files)
-
-        async for (python_file, (reformatted, encoding, changed)) in futures:
-            self.yapf_result(python_file, reformatted, changed)
-
-    async def on_check_run(self, check: str) -> None:
-        if check not in self.failed and check not in self.warned:
-            self.succeed(check, [check])
-
-    async def on_checks_complete(self) -> int:
-        if self.diff_file_path and self.has_failed:
-            result = await aio.async_subprocess.run(["git", "diff", "HEAD"],
-                                                    cwd=self.path,
-                                                    capture_output=True)
-            self.diff_file_path.write_bytes(result.stdout)
-        return await super().on_checks_complete()
-
-    async def yapf_format(self, python_file: str) -> tuple:
-        return python_file, yapf.yapf_api.FormatFile(
-            python_file,
-            style_config=str(self.yapf_config_path),
-            in_place=self.fix,
-            print_diff=not self.fix)
-
-    def yapf_result(self, python_file: str, reformatted: str, changed: bool) -> None:
-        if not changed:
-            return self.succeed("yapf", [python_file])
-        if self.fix:
-            return self.warn("yapf", [f"{python_file}: reformatted"])
-        if reformatted:
-            return self.warn("yapf", [f"{python_file}: diff\n{reformatted}"])
-        self.error("yapf", [python_file])
-
-    def _strip_line(self, line: str) -> str:
-        return line[len(str(self.path)) + 1:] if line.startswith(f"{self.path}/") else line
-
-    def _strip_lines(self, lines: Iterable[str]) -> List[str]:
-        return [self._strip_line(line) for line in lines if line]
+    def path(self) -> pathlib.Path:
+        if self.args.paths:
+            return pathlib.Path(self.args.paths[0])
+        return pathlib.Path(envoy_repo.PATH)
 
 
-def main(*args: str) -> Optional[int]:
-    return PythonChecker(*args).run()
+def main(*args) -> int:
+    return EnvoyPythonChecker(*args).run()
 
 
 if __name__ == "__main__":