From 532f1c115bc7ceb45c19e0d556b01588a07ba1f0 Mon Sep 17 00:00:00 2001 From: Ryan Northey Date: Sun, 1 Sep 2024 09:36:38 +0100 Subject: [PATCH] `envoy.code.check`: Add bazel checks Signed-off-by: Ryan Northey --- envoy.code.check/envoy/code/check/BUILD | 2 + envoy.code.check/envoy/code/check/__init__.py | 9 +- .../envoy/code/check/abstract/__init__.py | 4 + .../envoy/code/check/abstract/base.py | 12 +- .../envoy/code/check/abstract/bazel.py | 562 ++++++ .../envoy/code/check/abstract/checker.py | 25 + envoy.code.check/envoy/code/check/checker.py | 9 + .../envoy/code/check/exceptions.py | 4 + .../envoy/code/check/interface.py | 4 + envoy.code.check/envoy/code/check/shared.py | 46 + envoy.code.check/tests/test_abstract_bazel.py | 1567 +++++++++++++++++ .../tests/test_abstract_checker.py | 14 +- 12 files changed, 2249 insertions(+), 9 deletions(-) create mode 100644 envoy.code.check/envoy/code/check/abstract/bazel.py create mode 100644 envoy.code.check/envoy/code/check/shared.py create mode 100644 envoy.code.check/tests/test_abstract_bazel.py diff --git a/envoy.code.check/envoy/code/check/BUILD b/envoy.code.check/envoy/code/check/BUILD index 45f7e79b4..7f0e24fd8 100644 --- a/envoy.code.check/envoy/code/check/BUILD +++ b/envoy.code.check/envoy/code/check/BUILD @@ -15,6 +15,7 @@ toolshed_library( "__init__.py", "abstract/__init__.py", "abstract/base.py", + "abstract/bazel.py", "abstract/changelog.py", "abstract/checker.py", "abstract/extensions.py", @@ -30,6 +31,7 @@ toolshed_library( "cmd.py", "exceptions.py", "interface.py", + "shared.py", "typing.py", ], ) diff --git a/envoy.code.check/envoy/code/check/__init__.py b/envoy.code.check/envoy/code/check/__init__.py index 7c73f8b99..6b296482a 100644 --- a/envoy.code.check/envoy/code/check/__init__.py +++ b/envoy.code.check/envoy/code/check/__init__.py @@ -2,6 +2,7 @@ from . import abstract, exceptions, typing from .abstract import ( ABackticksCheck, + ABazelCheck, AChangelogChangesChecker, AChangelogCheck, AChangelogStatus, @@ -20,6 +21,7 @@ AYamllintCheck, AYapfCheck) from .checker import ( + BazelCheck, ChangelogChangesChecker, ChangelogCheck, ChangelogStatus, @@ -33,12 +35,13 @@ YamllintCheck, YapfCheck) from .cmd import run, main -from . import checker, interface +from . import checker, interface, shared __all__ = ( "abstract", "ABackticksCheck", + "ABazelCheck", "AChangelogChangesChecker", "AChangelogCheck", "AChangelogStatus", @@ -56,6 +59,7 @@ "AShellcheckCheck", "AYamllintCheck", "AYapfCheck", + "BazelCheck", "ChangelogChangesChecker", "ChangelogCheck", "ChangelogStatus", @@ -69,9 +73,8 @@ "interface", "main", "run", - "main", - "run", "RuntimeGuardsCheck", + "shared", "ShellcheckCheck", "typing", "YamllintCheck", diff --git a/envoy.code.check/envoy/code/check/abstract/__init__.py b/envoy.code.check/envoy/code/check/abstract/__init__.py index ed6764ef1..29499b42d 100644 --- a/envoy.code.check/envoy/code/check/abstract/__init__.py +++ b/envoy.code.check/envoy/code/check/abstract/__init__.py @@ -1,5 +1,6 @@ from .base import ACodeCheck, AFileCodeCheck, AProjectCodeCheck +from .bazel import ABazelCheck from .changelog import ( AChangelogCheck, AChangelogChangesChecker, @@ -19,6 +20,7 @@ from .yapf import AYapfCheck from . import ( base, + bazel, checker, extensions, flake8, @@ -32,6 +34,7 @@ __all__ = ( "ABackticksCheck", + "ABazelCheck", "AChangelogChangesChecker", "AChangelogCheck", "AChangelogStatus", @@ -50,6 +53,7 @@ "AYamllintCheck", "AYapfCheck", "base", + "bazel", "checker", "extensions", "flake8", diff --git a/envoy.code.check/envoy/code/check/abstract/base.py b/envoy.code.check/envoy/code/check/abstract/base.py index a4535baf4..4d6763461 100644 --- a/envoy.code.check/envoy/code/check/abstract/base.py +++ b/envoy.code.check/envoy/code/check/abstract/base.py @@ -1,11 +1,13 @@ import asyncio +import pathlib +import shutil from concurrent import futures from typing import Dict, List, Optional, Set import abstracts -from aio.core import event +from aio.core import event, subprocess from aio.core.directory import ADirectory from aio.core.functional import async_property @@ -35,6 +37,14 @@ def __init__( def binaries(self): return self._binaries + def command_path(self, command_name: str) -> pathlib.Path: + if command_name in self.binaries: + return pathlib.Path(self.binaries[command_name]).absolute() + if command := shutil.which(command_name): + return pathlib.Path(command) + raise subprocess.exceptions.OSCommandError( + f"Unable to find {command_name} command") + @abstracts.implementer(interface.IFileCodeCheck) class AFileCodeCheck(ACodeCheck, metaclass=abstracts.Abstraction): diff --git a/envoy.code.check/envoy/code/check/abstract/bazel.py b/envoy.code.check/envoy/code/check/abstract/bazel.py new file mode 100644 index 000000000..2fd36f15d --- /dev/null +++ b/envoy.code.check/envoy/code/check/abstract/bazel.py @@ -0,0 +1,562 @@ + +import difflib +import os +import pathlib +import re +import subprocess +import tempfile +from functools import cached_property, partial +from typing import ( + Callable, Mapping, Pattern, Sequence) + +import abstracts + +from aio.core.functional import async_property +from aio.core import subprocess as _subprocess +from aio.core import tasks +from aio.run import checker + +from envoy.code.check import abstract, exceptions, interface, shared, typing + +RE_BAZEL_MATCH = ( + "^WORKSPACE$", + r"[\w/]*/BUILD$", + r"[\w/]*.bzl$", ) + +# Match an Envoy rule, e.g. envoy_cc_library( in a BUILD file. +RE_ENVOY_RULE = r'envoy[_\w]+\(' + +# Match a load() statement for the envoy_package macros. +RE_PACKAGE_LOAD_BLOCK = r'("envoy_package".*?\)\n)' +RE_EXTENSION_PACKAGE_LOAD_BLOCK = r'("envoy_extension_package".*?\)\n)' +RE_CONTRIB_PACKAGE_LOAD_BLOCK = r'("envoy_contrib_package".*?\)\n)' +RE_MOBILE_PACKAGE_LOAD_BLOCK = r'("envoy_mobile_package".*?\)\n)' + +# Canonical Envoy license. +LICENSE_STRING = 'licenses(["notice"]) # Apache 2\n\n' + +# Match any existing licenses in a BUILD file. +RE_OLD_LICENSES = r'^licenses\(.*\n+' + +# Match Buildozer 'print' output. Example of Buildozer print output: +# cc_library json_transcoder_filter_lib +# [json_transcoder_filter.cc] (missing) (missing) +RE_BUILDOZER_PRINT = ( + r"\s*([\w_]+)\s+([\w_]+)\s+" + r"[(\[](.*?)[)\]]\s+[(\[](.*?)[)\]]\s+[(\[](.*?)[)\]]") + +# Match API header include in Envoy source file? +RE_API_INCLUDE = ( + r'#include "(contrib/envoy/.*|envoy/.*)/[^/]+\.pb\.(validate\.)?h"') + +XDS_PKG_PROTO = "@com_github_cncf_xds//udpa/annotations:pkg_cc_proto" + + +class EnvoyBuildozer(object): + + def __init__(self, buildozer_path, filepath): + self.buildozer_path = buildozer_path + self.filepath = filepath + + @cached_property + def re_api_include(self) -> Pattern[str]: + """Regex to match files to check.""" + return re.compile(RE_API_INCLUDE) + + @cached_property + def re_print(self) -> Pattern[str]: + return re.compile(RE_BUILDOZER_PRINT) + + # Run Buildozer commands on a string representing a BUILD file. + def run(self, cmds: list[tuple[str, str]], text: str) -> str: + with tempfile.NamedTemporaryFile(mode='w') as cmd_file: + # We send the BUILD contents to buildozer on stdin and receive the + # transformed BUILD on stdout. The commands are provided in a file. + cmd_path = pathlib.Path(cmd_file.name) + cmd_path.write_text( + '\n'.join( + '%s|-:%s' + % (cmd, target) for cmd, target in cmds)) + return self._run(cmd_path, text).strip() + + def _run(self, cmd_path: pathlib.Path, text: str) -> str: + response = subprocess.run( + [self.buildozer_path, '-stdout', '-f', str(cmd_path)], + input=text.encode(), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + # Buildozer uses 3 for success but no change + # (0 is success and changed). + if response.returncode not in [0, 3]: + raise exceptions.FixError( + f"buildozer execution failed: {response}") + # Sometimes buildozer feels like returning nothing when the + # transform is a nop. + return ( + response.stdout.decode() + if response.stdout + else text) + + def mutation_cmds(self, text: str) -> list[tuple[str, str]]: + buildozer_out = self.run( + [('print kind name srcs hdrs deps', '*')], + text).split('\n') + return [ + cmd + for line + in buildozer_out + if (match := self.re_print.match(line)) + for cmd + in self.mutations(match)] + + def mutations(self, match) -> list[tuple[str, str]]: + mutations = [] + kind, name, srcs, hdrs, deps = match.groups() + if kind == "envoy_pch_library" or not name: + return [] + actual_api_deps = self.api_deps_actual(srcs, hdrs) + existing_api_deps = self.api_deps(deps) + deps_to_add = actual_api_deps - existing_api_deps + deps_to_remove = existing_api_deps - actual_api_deps + if deps_to_add: + mutations.append( + (f"add deps {' '.join(deps_to_add)}", name)) + if deps_to_remove: + mutations.append( + (f"remove deps {' '.join(deps_to_remove)}", + name)) + return mutations + + def api_deps(self, deps: str) -> set[str]: + return { + dep + for dep + in (deps if deps != "missing" else "").split() + if (dep.startswith("@envoy_api") + and dep.endswith("pkg_cc_proto") + and dep != XDS_PKG_PROTO)} + + def api_deps_actual(self, srcs: str, hdrs: str) -> set[str]: + return { + dep + for path + in self.source_paths(srcs, hdrs) + for dep + in self.find_api_deps(path)} + + def dep_files( + self, + files: str, + endswith: str | tuple) -> list[str]: + return [ + self.filepath.parent.joinpath(filename) + for filename + in (files if files != "missing" else "").split() + if filename.endswith(endswith)] + + # Find all the API headers in a C++ source file. + def find_api_deps(self, path: pathlib.Path) -> set[str]: + if not path.exists(): + return set() + with path.open() as fd: + return set( + f"@envoy_api//{match.group(1)}:pkg_cc_proto" + for line in fd + if (match := self.re_api_include.match(line))) + + def source_paths(self, srcs, hdrs): + filenames = set() + for filename in self.dep_files(srcs, (".cc", ".h")): + filenames.add(filename) + yield filename + for filename in self.dep_files(hdrs, ".h"): + if filename not in filenames: + yield filename + + +@abstracts.implementer(_subprocess.ISubprocessHandler) +class EnvoyBuildifier(_subprocess.ASubprocessHandler): + api_prefix: str = "api" + _diff_output: str | None = None + + def __init__(self, *args, **kwargs) -> None: + self.config: typing.YAMLConfigDict = kwargs.pop("config") + self.filepath = kwargs.pop("filepath") + self.buildozer_path = kwargs.pop("buildozer_path") + super().__init__(*args, **kwargs) + + @cached_property + def allowed_bazel_tools(self) -> bool: + return ( + self.is_starlark + or (not self.is_mobile + and self.filepath.parts[0] == "bazel") + or (self.is_mobile + and self.filepath.parts[1] == "bazel")) + + @cached_property + def allowed_protobuf_direct(self): + return ( + self.filepath.name.endswith( + (*self.format_config.suffixes["proto"], + *self.format_config.suffixes["repositories_bzl"])) + or any( + self.filepath.is_relative_to(path) + for path + in self.format_config.paths["protobuf"]["include"])) + + @cached_property + def allowed_urls(self) -> bool: + return ( + str(self.filepath) + in self.format_config.paths["build_urls"]["include"]) + + @cached_property + def buildozer(self): + return EnvoyBuildozer(self.buildozer_path, self.filepath) + + @cached_property + def errors(self) -> list: + return [] + + @cached_property + def format_config(self) -> shared.EnvoyFormatConfig: + return shared.EnvoyFormatConfig(self.config, self.filepath) + + @cached_property + def is_api(self) -> bool: + return self.filepath.parts[0] == self.api_prefix + + @cached_property + def is_api_envoy(self) -> bool: + return ( + self.is_api + and self.filepath.parts[1] == "envoy") + + @cached_property + def is_build(self) -> bool: + return ( + not self.is_starlark + and not self.is_workspace + and not self.is_external_build) + + @property + def is_build_fixer_excluded(self) -> bool: + return any( + self.filepath.is_relative_to(path) + for path + in self.format_config.paths["build_fixer"]["exclude"]) + + @cached_property + def is_external_build(self) -> bool: + return any( + self.filepath.is_relative_to(path) + for path + in ("bazel/external", + "tools/clang_tools")) + + @cached_property + def is_mobile(self) -> bool: + return self.filepath.parts[0] == "mobile" + + @cached_property + def is_package(self) -> bool: + return ( + self.is_build + and not self.is_api + and not self.is_build_fixer_excluded) + + @cached_property + def is_starlark(self) -> bool: + return self.filepath.name.endswith(".bzl") + + @cached_property + def is_workspace(self) -> bool: + return self.filepath.name == "WORKSPACE" + + @cached_property + def re_contrib_package_load_block(self) -> Pattern[str]: + return re.compile(RE_CONTRIB_PACKAGE_LOAD_BLOCK, re.DOTALL) + + @cached_property + def re_envoy_rule(self) -> Pattern[str]: + return re.compile(RE_ENVOY_RULE) + + @cached_property + def re_extension_package_load_block(self) -> Pattern[str]: + return re.compile(RE_EXTENSION_PACKAGE_LOAD_BLOCK, re.DOTALL) + + @cached_property + def re_mobile_package_load_block(self) -> Pattern[str]: + return re.compile(RE_MOBILE_PACKAGE_LOAD_BLOCK, re.DOTALL) + + @cached_property + def re_old_license(self) -> Pattern[str]: + return re.compile(RE_OLD_LICENSES, re.MULTILINE) + + @cached_property + def re_package_load_block(self) -> Pattern[str]: + return re.compile(RE_PACKAGE_LOAD_BLOCK, re.DOTALL) + + @cached_property + def text(self) -> str: + return self.filepath.read_text() + + def bad_bazel_tools_line(self, line): + return ( + not self.allowed_bazel_tools + and "@bazel_tools" in line + and "python/runfiles" not in line) + + def bad_envoy_line(self, line): + return ( + self.is_build + and not self.is_mobile + and "@envoy//" in line) + + def bad_protobuf_line(self, line): + return ( + not self.allowed_protobuf_direct + and '"protobuf"' in line) + + def bad_url_line(self, line): + return ( + not self.allowed_urls + and (" urls = " in line + or " url = " in line)) + + def fix_build_line(self, line: str) -> str: + if self.bad_bazel_tools_line(line): + self.errors.append( + "unexpected @bazel_tools reference, " + "please indirect via a definition in //bazel") + if self.bad_protobuf_line(line): + self.errors.append( + "unexpected direct external dependency on protobuf, use " + "//source/common/protobuf instead.") + if self.bad_envoy_line(line): + self.errors.append("Superfluous '@envoy//' prefix") + line = line.replace("@envoy//", "//") + if self.bad_url_line(line): + self.errors.append( + "Only repository_locations.bzl may contains URL references") + return line + + def handle( + self, + response: subprocess.CompletedProcess) -> typing.ProblemDict: + errors = '\n'.join(self.errors) + return ( + {str(self.filepath): checker.Problems( + errors=[f"{self.filepath}\n{errors}"])} + if errors + else {}) + + def handle_error( + self, + response: subprocess.CompletedProcess) -> typing.ProblemDict: + return { + str(self.filepath): checker.Problems( + errors=[f"{self.filepath}\n{self._diff(response)}"])} + + def has_failed(self, response: subprocess.CompletedProcess) -> bool: + return bool(self._diff(response)) + + def preprocess(self, text: str) -> str: + if self.is_build: + text = self.xform_build(text) + if self.is_package: + text = self.xform_package(text) + text = self.xform_deps_api(text) + if self.is_api_envoy: + text = self.xform_api_package(text) + return text + + def subprocess_args(self, *args, **kwargs) -> tuple[Sequence[str], ...]: + if self.is_workspace: + args = (*args, "-type=workspace") + return super().subprocess_args(*args, **kwargs) + + def subprocess_kwargs(self, *args, **kwargs) -> Mapping: + return { + **self.kwargs, + "input": self.preprocess(self.text), + **kwargs} + + # Infer and adjust rule dependencies in BUILD files for @envoy_api proto + # files. This is very cheap to do purely via a grep+buildozer syntax level + # step. + # + # This could actually be done much more generally, for all symbols and + # headers if we made use of Clang libtooling semantic analysis. However, + # this requires a compilation database and full build of Envoy, + # envoy_build_fixer.py is run under check_format, which should be fast + # for developers. + def xform_deps_api(self, text: str) -> str: + return ( + self.buildozer.run(cmds, text) + if (cmds := self.buildozer.mutation_cmds(text)) + else text) + + def xform_api_package(self, text: str): + if "api_proto_package(" not in text: + self.errors.append( + "API build file does not provide api_proto_package()") + return text + + def xform_build(self, text: str) -> str: + return "\n".join( + self.fix_build_line(line) + for line + in text.split("\n")) + + # Add an Apache 2 license, envoy_package / envoy_mobile_package import + # and rule as needed. + def xform_package(self, contents): + regex_to_use = self.re_package_load_block + package_string = 'envoy_package' + path = str(self.filepath) + + if 'source/extensions' in path or 'library/common/extensions' in path: + regex_to_use = self.re_package_load_block + package_string = 'envoy_extension_package' + + if not self.is_mobile: + if 'contrib/' in path: + regex_to_use = self.re_contrib_package_load_block + package_string = 'envoy_contrib_package' + elif 'library/common/extensions' not in path: + regex_to_use = self.re_mobile_package_load_block + package_string = 'envoy_mobile_package' + + # Ensure we have an envoy_package / envoy_mobile_package import load + # if this is a real Envoy package. + # We also allow the prefix to be overridden if envoy is included + # in a larger workspace. + if self.re_envoy_rule.search(contents): + new_load = ( + "new_load {}//bazel:envoy_build_system.bzl " + f"{package_string}") + default_prefix = ("@envoy" if self.is_mobile else "") + contents = self.buildozer.run( + [(new_load.format( + os.getenv( + "ENVOY_BAZEL_PREFIX", + default_prefix)), + '__pkg__')], + contents) + # Envoy package is inserted after the load block containing the + # envoy_package / envoy_mobile_package import. + package_and_parens = f"{package_string}()" + if package_and_parens[:-1] not in contents: + contents = regex_to_use.sub( + rf"\1\n{package_and_parens}\n\n", + contents) + if package_and_parens not in contents: + raise exceptions.FixError( + "Unable to insert {package_and_parens}") + + # Delete old licenses. + if self.re_old_license.search(contents): + contents = self.re_old_license.sub('', contents) + # Add canonical Apache 2 license. + return f"{LICENSE_STRING}{contents}" + + def _diff(self, response: subprocess.CompletedProcess) -> str: + if self._diff_output: + return self._diff_output + stdout_lines = response.stdout.splitlines() + file_lines = self.text.splitlines() + self._diff_output = "\n".join( + difflib.unified_diff( + file_lines, + stdout_lines, + fromfile=str(self.filepath), + tofile=str(self.filepath))) + return self._diff_output + + +@abstracts.implementer(interface.IBazelCheck) +class ABazelCheck(abstract.AFileCodeCheck, metaclass=abstracts.Abstraction): + + @classmethod + def filter_files( + cls, + files: set[str], + match: Callable, + exclude: Callable) -> set[str]: + """Filter files for `buildifier` checking.""" + return set( + path + for path + in files + if match(path) + and not exclude(path)) + + @classmethod + def run_buildifier( + cls, + path: str, + config: typing.YAMLConfigDict, + buildozer_path: pathlib.Path, + *args) -> typing.ProblemDict: + """Run buildifier on files.""" + return EnvoyBuildifier( + path, + config=config, + filepath=pathlib.Path(args[-1]), + buildozer_path=buildozer_path).run(*args[:-1]) + + @cached_property + def buildifier_command(self) -> partial: + """Partial with buildifier command and args.""" + return partial( + self.run_buildifier, + self.directory.path, + self.config, + self.buildozer_path, + self.buildifier_path, + "-mode=fix", + "-lint=fix") + + @cached_property + def buildifier_path(self) -> pathlib.Path: + """Buildifier command, should be available in the running system.""" + return self.command_path("buildifier") + + @cached_property + def buildozer_path(self) -> pathlib.Path: + """Buildozer command, should be available in the running system.""" + return self.command_path("buildozer") + + @async_property + async def checker_files(self) -> set[str]: + return self.filter_files( + await self.directory.files, + self.re_path_match.match, + self.exclude) + + @async_property(cache=True) + async def problem_files(self) -> typing.ProblemDict: + """Discovered bazel errors.""" + if not await self.files: + return {} + errors: typing.ProblemDict = dict() + jobs = tasks.concurrent( + self.execute( + self.buildifier_command, + file) + for file + in await self.files) + async for result in jobs: + errors.update(result) + return errors + + @cached_property + def re_path_match(self) -> Pattern[str]: + """Regex to match files to check.""" + return re.compile("|".join(RE_BAZEL_MATCH)) + + def exclude(self, path: str) -> bool: + return path.startswith( + tuple(self.config["paths"]["excluded"])) diff --git a/envoy.code.check/envoy/code/check/abstract/checker.py b/envoy.code.check/envoy/code/check/abstract/checker.py index 8e92e2238..7964b828b 100644 --- a/envoy.code.check/envoy/code/check/abstract/checker.py +++ b/envoy.code.check/envoy/code/check/abstract/checker.py @@ -57,6 +57,7 @@ class ACodeChecker( """Code checker.""" checks = ( + "bazel", "changelog", "extensions_fuzzed", "extensions_metadata", @@ -74,6 +75,19 @@ class ACodeChecker( def all_files(self) -> bool: return self.args.all_files + @cached_property + def bazel(self) -> "interface.IBazelCheck": + """Bazel checker.""" + return self.bazel_class( + self.directory, + config=self.config, + **self.check_kwargs) + + @property # type:ignore + @abstracts.interfacemethod + def bazel_class(self) -> Type["interface.IBazelCheck"]: + raise NotImplementedError + @cached_property def binaries(self) -> Dict[str, str]: return dict( @@ -292,6 +306,10 @@ def add_arguments(self, parser: argparse.ArgumentParser) -> None: parser.add_argument("--extensions_build_config") parser.add_argument("--extensions_fuzzed_count") + async def check_bazel(self) -> None: + """Check for bazel issues.""" + await self._code_check(self.bazel) + async def check_changelog(self): for changelog in self.changelog: if errors := await changelog.errors: @@ -380,6 +398,13 @@ async def check_yamllint(self) -> None: """Check for yamllint issues.""" await self._code_check(self.yamllint) + @checker.preload( + when=["bazel"], + catches=[exceptions.ConfigurationError]) + async def preload_bazel(self) -> None: + await self.bazel.problem_files + self.log.debug("Preloaded bazel") + @checker.preload( when=["changelog"], catches=[utils.exceptions.ChangelogParseError]) diff --git a/envoy.code.check/envoy/code/check/checker.py b/envoy.code.check/envoy/code/check/checker.py index 1ea9cc543..55bfac80b 100644 --- a/envoy.code.check/envoy/code/check/checker.py +++ b/envoy.code.check/envoy/code/check/checker.py @@ -21,6 +21,11 @@ class Flake8Check(abstract.AFlake8Check): pass +@abstracts.implementer(interface.IBazelCheck) +class BazelCheck(abstract.ABazelCheck): + pass + + @abstracts.implementer(interface.IGlintCheck) class GlintCheck(abstract.AGlintCheck): pass @@ -98,6 +103,10 @@ def changelog_status_class(self) -> Type[interface.IChangelogStatus]: @abstracts.implementer(interface.ICodeChecker) class CodeChecker(abstract.ACodeChecker): + @property + def bazel_class(self): + return BazelCheck + @property def extensions_class(self): return ExtensionsCheck diff --git a/envoy.code.check/envoy/code/check/exceptions.py b/envoy.code.check/envoy/code/check/exceptions.py index 57b2a0010..2a2af3d47 100644 --- a/envoy.code.check/envoy/code/check/exceptions.py +++ b/envoy.code.check/envoy/code/check/exceptions.py @@ -4,6 +4,10 @@ class RunError(Exception): pass +class FixError(Exception): + pass + + class ConfigurationError(Exception): pass diff --git a/envoy.code.check/envoy/code/check/interface.py b/envoy.code.check/envoy/code/check/interface.py index fd0ace9b3..6a382b640 100644 --- a/envoy.code.check/envoy/code/check/interface.py +++ b/envoy.code.check/envoy/code/check/interface.py @@ -51,6 +51,10 @@ def __init__( raise NotImplementedError +class IBazelCheck(IFileCodeCheck, metaclass=abstracts.Interface): + pass + + class IExtensionsCheck(metaclass=abstracts.Interface): def __init__(self, directory: _directory.ADirectory, **kwargs) -> None: diff --git a/envoy.code.check/envoy/code/check/shared.py b/envoy.code.check/envoy/code/check/shared.py new file mode 100644 index 000000000..86018ab48 --- /dev/null +++ b/envoy.code.check/envoy/code/check/shared.py @@ -0,0 +1,46 @@ + +from functools import cached_property +from typing import Callable + +from envoy.code.check import typing + + +class EnvoyFormatConfig: + """Provides a format config object based on parsed YAML config.""" + + def __init__(self, config: typing.YAMLConfigDict, source_path) -> None: + self.config = config + self.source_path = source_path + + def __getitem__(self, k): + return self.config.__getitem__(k) + + @cached_property + def paths(self) -> ( + dict[str, tuple[str, ...] | dict[str, tuple[str, ...]]]): + """Mapping of named paths.""" + paths = self._normalize( + "paths", + cb=lambda paths: tuple(paths)) + return paths + + @cached_property + def suffixes(self) -> ( + dict[str, tuple[str, ...] | dict[str, tuple[str, ...]]]): + """Mapping of named file suffixes for target files.""" + return self._normalize("suffixes") + + def _normalize( + self, + config_type: str, + cb: Callable[..., tuple[str, ...]] = tuple) -> dict: + config: dict = {} + for k, v in self[config_type].items(): + if isinstance(v, dict): + config[k] = {} + for key in ("include", "exclude"): + if key in v: + config[k][key] = cb(v[key]) + else: + config[k] = cb(v) + return config diff --git a/envoy.code.check/tests/test_abstract_bazel.py b/envoy.code.check/tests/test_abstract_bazel.py new file mode 100644 index 000000000..ff3fcdb6f --- /dev/null +++ b/envoy.code.check/tests/test_abstract_bazel.py @@ -0,0 +1,1567 @@ + +import types +from unittest.mock import AsyncMock, MagicMock, PropertyMock + +import pytest + +from aio.core import subprocess + +from envoy.code import check + + +def test_envoy_buildifier_constructor(patches): + kwargs = dict( + _foo=MagicMock(), + _bar=MagicMock(), + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "_subprocess.ASubprocessHandler.__init__", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_handler, ): + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + "FOO", + "BAR", + **kwargs) + assert isinstance(buildifier, subprocess.ISubprocessHandler) + assert isinstance(buildifier, subprocess.ASubprocessHandler) + for k, v in kwargs.items(): + if not k.startswith("_"): + assert getattr(buildifier, k) == v + assert buildifier.api_prefix == "api" + assert ( + m_handler.call_args + == [("PATH", "FOO", "BAR"), + {k: v for k, v in kwargs.items() if k.startswith("_")}]) + + +@pytest.mark.parametrize("bazel", [True, False]) +@pytest.mark.parametrize("starlark", [True, False]) +@pytest.mark.parametrize("mobile", [True, False]) +def test_envoy_buildifier_allowed_bazel_tools( + patches, bazel, starlark, mobile): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_mobile", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_starlark", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + (kwargs["filepath"].parts.__getitem__.return_value + .__eq__.return_value) = bazel + + with patched as (m_mobile, m_star): + m_star.return_value = starlark + m_mobile.return_value = mobile + assert ( + buildifier.allowed_bazel_tools + == starlark or bazel) + + assert "allowed_bazel_tools" in buildifier.__dict__ + if starlark: + assert not kwargs["filepath"].parts.__getitem__.called + return + assert ( + kwargs["filepath"].parts.__getitem__.call_args + == [(0 if not mobile else 1, ), {}]) + assert ( + kwargs["filepath"].parts.__getitem__.return_value.__eq__.call_args + == [("bazel", ), {}]) + + +@pytest.mark.parametrize("suffix", [True, False]) +@pytest.mark.parametrize("included", [True, False]) +def test_envoy_buildifier_allowed_protobuf_direct( + patches, iters, suffix, included): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "any", + ("EnvoyBuildifier.format_config", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + kwargs["filepath"].name.endswith.return_value = suffix + paths = dict( + proto=iters(), + repositories_bzl=iters()) + config_paths = iters() + + with patched as (m_any, m_config): + m_any.return_value = included + (m_config.return_value.suffixes + .__getitem__.side_effect) = lambda idx: paths[idx] + (m_config.return_value.paths + .__getitem__.return_value + .__getitem__.return_value) = config_paths + assert ( + buildifier.allowed_protobuf_direct + == suffix or included) + if not suffix: + path_iter = m_any.call_args[0][0] + path_list = list(path_iter) + + assert "allowed_protobuf_direct" in buildifier.__dict__ + assert ( + kwargs["filepath"].name.endswith.call_args + == [(('I0', 'I1', 'I2', 'I3', 'I4', + 'I0', 'I1', 'I2', 'I3', 'I4'), ), {}]) + if suffix: + assert not m_any.called + return + assert type(path_iter) is types.GeneratorType + assert ( + path_list + == [ + kwargs["filepath"].is_relative_to.return_value + for x in config_paths]) + assert ( + kwargs["filepath"].is_relative_to.call_args_list + == [[(x, ), {}] + for x in config_paths]) + + +@pytest.mark.parametrize("contains", [True, False]) +def test_envoy_buildifier_allowed_urls(patches, contains): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "str", + ("EnvoyBuildifier.format_config", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_str, m_config): + (m_config.return_value.paths + .__getitem__.return_value + .__getitem__.return_value + .__contains__.return_value) = contains + assert ( + buildifier.allowed_urls + == contains) + + assert "allowed_urls" in buildifier.__dict__ + assert ( + m_str.call_args + == [(kwargs["filepath"], ), {}]) + assert ( + m_config.return_value.paths.__getitem__.call_args + == [("build_urls", ), {}]) + assert ( + (m_config.return_value.paths + .__getitem__.return_value + .__getitem__.call_args) + == [("include", ), {}]) + assert ( + (m_config.return_value.paths + .__getitem__.return_value + .__getitem__.return_value + .__contains__.call_args) + == [(m_str.return_value, ), {}]) + + +def test_envoy_buildifier_errors(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + assert buildifier.errors == [] + assert "errors" in buildifier.__dict__ + + +def test_envoy_buildifier_format_config(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "shared.EnvoyFormatConfig", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_fconfig, ): + assert ( + buildifier.format_config + == m_fconfig.return_value) + + assert "format_config" in buildifier.__dict__ + assert ( + m_fconfig.call_args + == [(kwargs["config"], kwargs["filepath"]), {}]) + + +@pytest.mark.parametrize("api", [True, False]) +def test_envoy_buildifier_is_api(patches, api): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + (kwargs["filepath"].parts.__getitem__ + .return_value.__eq__.return_value) = api + + assert buildifier.is_api == api + + assert "is_api" in buildifier.__dict__ + assert ( + kwargs["filepath"].parts.__getitem__.call_args + == [(0, ), {}]) + assert ( + kwargs["filepath"].parts.__getitem__.return_value.__eq__.call_args + == [(buildifier.api_prefix, ), {}]) + + +@pytest.mark.parametrize("starlark", [True, False]) +@pytest.mark.parametrize("workspace", [True, False]) +@pytest.mark.parametrize("external", [True, False]) +def test_envoy_buildifier_is_build(patches, starlark, workspace, external): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_starlark", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_workspace", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_external_build", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_star, m_workspace, m_external): + m_star.return_value = starlark + m_workspace.return_value = workspace + m_external.return_value = external + assert ( + buildifier.is_build + == (not starlark + and not workspace + and not external)) + + assert "is_build" in buildifier.__dict__ + + +def test_envoy_buildifier_is_build_fixer_excluded(patches, iters): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "any", + ("EnvoyBuildifier.format_config", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + paths = iters() + + with patched as (m_any, m_config): + (m_config.return_value.paths + .__getitem__.return_value + .__getitem__.return_value) = paths + assert ( + buildifier.is_build_fixer_excluded + == m_any.return_value) + any_iter = m_any.call_args[0][0] + any_list = list(any_iter) + + assert "is_build_fixer_excluded" not in buildifier.__dict__ + assert ( + type(any_iter) + is types.GeneratorType) + assert ( + any_list + == [kwargs["filepath"].is_relative_to.return_value + for x in paths]) + assert ( + kwargs["filepath"].is_relative_to.call_args_list + == [[(x, ), {}] + for x in paths]) + assert ( + m_config.return_value.paths.__getitem__.call_args + == [("build_fixer", ), {}]) + assert ( + (m_config.return_value.paths + .__getitem__.return_value.__getitem__.call_args) + == [("exclude", ), {}]) + + +@pytest.mark.parametrize("api", [True, False]) +@pytest.mark.parametrize("envoy", [True, False]) +def test_envoy_buildifier_is_api_envoy(patches, api, envoy): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_api", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + (kwargs["filepath"].parts.__getitem__ + .return_value.__eq__.return_value) = envoy + + with patched as (m_api, ): + m_api.return_value = api + assert ( + buildifier.is_api_envoy + == (api and envoy)) + + assert "is_api_envoy" in buildifier.__dict__ + if not api: + assert not kwargs["filepath"].parts.__getitem__.called + return + assert ( + kwargs["filepath"].parts.__getitem__.call_args + == [(1, ), {}]) + assert ( + (kwargs["filepath"].parts.__getitem__ + .return_value.__eq__.call_args) + == [("envoy", ), {}]) + + +def test_envoy_buildifier_is_external_build(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "any", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + (kwargs["filepath"].is_relative_to + .return_value) = False + + with patched as (m_any, ): + assert ( + buildifier.is_external_build + == m_any.return_value) + any_iters = m_any.call_args[0][0] + any_list = list(any_iters) + + assert type(any_iters) is types.GeneratorType + assert any_list == [False, False] + assert ( + kwargs["filepath"].is_relative_to.call_args_list + == [[("bazel/external", ), {}], + [("tools/clang_tools", ), {}]]) + + +@pytest.mark.parametrize("build", [True, False]) +@pytest.mark.parametrize("api", [True, False]) +@pytest.mark.parametrize("excluded", [True, False]) +def test_envoy_buildifier_is_package(patches, build, api, excluded): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_api", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_build", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_build_fixer_excluded", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_api, m_build, m_exclude): + m_api.return_value = api + m_build.return_value = build + m_exclude.return_value = excluded + assert ( + buildifier.is_package + == (build and not api and not excluded)) + + assert "is_package" in buildifier.__dict__ + + +@pytest.mark.parametrize("bzl", [True, False]) +def test_envoy_buildifier_is_starlark(patches, bzl): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + kwargs["filepath"].name.endswith.return_value = bzl + + assert ( + buildifier.is_starlark + == bzl) + + assert "is_starlark" in buildifier.__dict__ + assert ( + kwargs["filepath"].name.endswith.call_args + == [(".bzl", ), {}]) + + +@pytest.mark.parametrize("workspace", [True, False]) +def test_envoy_buildifier_is_workspace(patches, workspace): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + kwargs["filepath"].name.__eq__.return_value = workspace + + assert ( + buildifier.is_workspace + == workspace) + + assert "is_workspace" in buildifier.__dict__ + assert ( + kwargs["filepath"].name.__eq__.call_args + == [("WORKSPACE", ), {}]) + + +def __test_envoy_buildifier_re_api_include(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_api_include + == m_re.compile.return_value) + + assert "re_api_include" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_API_INCLUDE, ), {}]) + + +def __test_envoy_buildifier_re_buildozer_print(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_buildozer_print + == m_re.compile.return_value) + + assert "re_buildozer_print" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_BUILDOZER_PRINT, ), {}]) + + +def test_envoy_buildifier_re_contrib_package_load_block(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_contrib_package_load_block + == m_re.compile.return_value) + + assert "re_contrib_package_load_block" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_CONTRIB_PACKAGE_LOAD_BLOCK, + m_re.DOTALL), + {}]) + + +def test_envoy_buildifier_re_envoy_rule(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_envoy_rule + == m_re.compile.return_value) + + assert "re_envoy_rule" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_ENVOY_RULE, ), {}]) + + +def test_envoy_buildifier_re_extension_package_load_block(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_extension_package_load_block + == m_re.compile.return_value) + + assert "re_extension_package_load_block" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_EXTENSION_PACKAGE_LOAD_BLOCK, + m_re.DOTALL), + {}]) + + +def test_envoy_buildifier_re_mobile_package_load_block(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_mobile_package_load_block + == m_re.compile.return_value) + + assert "re_mobile_package_load_block" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_MOBILE_PACKAGE_LOAD_BLOCK, + m_re.DOTALL), + {}]) + + +def test_envoy_buildifier_re_old_license(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_old_license + == m_re.compile.return_value) + + assert "re_old_license" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_OLD_LICENSES, m_re.MULTILINE), {}]) + + +def test_envoy_buildifier_re_package_load_block(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + with patched as (m_re, ): + assert ( + buildifier.re_package_load_block + == m_re.compile.return_value) + + assert "re_package_load_block" in buildifier.__dict__ + assert ( + m_re.compile.call_args + == [(check.abstract.bazel.RE_PACKAGE_LOAD_BLOCK, m_re.DOTALL), {}]) + + +def test_envoy_buildifier_text(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + + assert ( + buildifier.text + == kwargs["filepath"].read_text.return_value) + + assert "text" in buildifier.__dict__ + assert ( + kwargs["filepath"].read_text.call_args + == [(), {}]) + + +@pytest.mark.parametrize("allowed", [True, False]) +@pytest.mark.parametrize("contains", [True, False]) +@pytest.mark.parametrize("excepted", [True, False]) +def test_envoy_buildifier_bad_bazel_tools_line( + patches, allowed, contains, excepted): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.allowed_bazel_tools", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + line = MagicMock() + + def has(thing): + if thing == "@bazel_tools": + return contains + if thing == "python/runfiles": + return excepted + + line.__contains__.side_effect = has + + with patched as (m_allowed, ): + m_allowed.return_value = allowed + assert ( + buildifier.bad_bazel_tools_line(line) + == (not allowed + and contains + and not excepted)) + + if allowed: + assert not line.__contains__.called + return + assert ( + line.__contains__.call_args_list[0] + == [("@bazel_tools", ), {}]) + if not contains: + assert len(line.__contains__.call_args_list) == 1 + return + assert ( + line.__contains__.call_args + == [("python/runfiles", ), {}]) + + +@pytest.mark.parametrize("build", [True, False]) +@pytest.mark.parametrize("contains", [True, False]) +def test_envoy_buildifier_bad_envoy_line(patches, build, contains): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_build", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + line = MagicMock() + line.__contains__.return_value = contains + + with patched as (m_build, ): + m_build.return_value = build + assert ( + buildifier.bad_envoy_line(line) + == (build and contains)) + + if not build: + assert not line.__contains__.called + return + assert ( + line.__contains__.call_args + == [("@envoy//", ), {}]) + + +@pytest.mark.parametrize("allowed", [True, False]) +@pytest.mark.parametrize("contains", [True, False]) +def test_envoy_buildifier_bad_protobuf_line(patches, allowed, contains): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.allowed_protobuf_direct", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + line = MagicMock() + line.__contains__.return_value = contains + + with patched as (m_allowed, ): + m_allowed.return_value = allowed + assert ( + buildifier.bad_protobuf_line(line) + == (not allowed and contains)) + + if allowed: + assert not line.__contains__.called + return + assert ( + line.__contains__.call_args + == [('"protobuf"', ), {}]) + + +@pytest.mark.parametrize("allowed", [True, False]) +@pytest.mark.parametrize("contains_plural", [True, False]) +@pytest.mark.parametrize("contains", [True, False]) +def test_envoy_buildifier_bad_url_line( + patches, allowed, contains_plural, contains): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.allowed_urls", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + line = MagicMock() + + def has(thing): + if thing == " url = ": + return contains + if thing == " urls = ": + return contains_plural + + line.__contains__.side_effect = has + + with patched as (m_allowed, ): + m_allowed.return_value = allowed + assert ( + buildifier.bad_url_line(line) + == (not allowed and (contains or contains_plural))) + + if allowed: + assert not line.__contains__.called + return + assert ( + line.__contains__.call_args_list[0] + == [(" urls = ", ), {}]) + if contains_plural: + assert len(line.__contains__.call_args_list) == 1 + return + assert ( + line.__contains__.call_args + == [(" url = ", ), {}]) + + +def __test_envoy_buildifier_find_api_deps(patches, iters): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "set", + ("EnvoyBuildifier.re_api_include", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + source_path = MagicMock() + lines = iters() + source_path.open.return_value.__enter__.return_value = lines + matchers = {} + + def _match(thing): + idx = int(thing[1]) + if idx % 2: + matchers[thing[1]] = MagicMock() + return matchers[thing[1]] + + with patched as (m_set, m_re): + m_re.return_value.match.side_effect = _match + assert ( + buildifier.find_api_deps(source_path) + == m_set.return_value) + setiter = m_set.call_args[0][0] + setlist = list(setiter) + + assert type(setiter) is types.GeneratorType + assert ( + setlist + == [f"@envoy_api//{matcher.group.return_value}:pkg_cc_proto" + for matcher in matchers.values()]) + assert ( + source_path.open.call_args + == [(), {}]) + assert ( + m_re.return_value.match.call_args_list + == [[(idx, ), {}] + for idx in lines]) + for matcher in matchers.values(): + assert ( + matcher.group.call_args + == [(1, ), {}]) + + +@pytest.mark.parametrize("tools", [True, False]) +@pytest.mark.parametrize("envoy", [True, False]) +@pytest.mark.parametrize("protobuf", [True, False]) +@pytest.mark.parametrize("url", [True, False]) +def test_envoy_buildifier_fix_build_line(patches, tools, envoy, protobuf, url): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.errors", + dict(new_callable=PropertyMock)), + "EnvoyBuildifier.bad_bazel_tools_line", + "EnvoyBuildifier.bad_envoy_line", + "EnvoyBuildifier.bad_protobuf_line", + "EnvoyBuildifier.bad_url_line", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + line = MagicMock() + all_errors = dict( + tools=( + "unexpected @bazel_tools reference, " + "please indirect via a definition in //bazel"), + protobuf=( + "unexpected direct external dependency on protobuf, use " + "//source/common/protobuf instead."), + envoy="Superfluous '@envoy//' prefix", + url="Only repository_locations.bzl may contains URL references") + local_vars = locals() + errors = [v for k, v in all_errors.items() if local_vars[k]] + + with patched as (m_errors, m_tools, m_envoy, m_proto, m_url): + m_tools.return_value = tools + m_envoy.return_value = envoy + m_proto.return_value = protobuf + m_url.return_value = url + assert ( + buildifier.fix_build_line(line) + == (line + if not envoy + else line.replace.return_value)) + + for bad in (m_tools, m_envoy, m_proto): + assert ( + bad.call_args + == [(line, ), {}]) + assert ( + m_url.call_args + == [(line if not envoy else line.replace.return_value, ), {}]) + assert ( + m_errors.return_value.append.call_args_list + == [[(error, ), {}] + for error in errors]) + if not envoy: + assert not line.replace.called + return + assert ( + line.replace.call_args + == [("@envoy//", "//"), {}]) + + +@pytest.mark.parametrize("has_errors", [True, False]) +def test_envoy_buildifier_handle(patches, iters, has_errors): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "str", + "checker.Problems", + ("EnvoyBuildifier.errors", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + errors = ( + iters() + if has_errors + else []) + response = MagicMock() + + with patched as (m_str, m_problems, m_errors): + m_errors.return_value = errors + assert ( + buildifier.handle(response) + == ({m_str.return_value: m_problems.return_value} + if has_errors + else {})) + + if not errors: + assert not m_str.called + assert not m_problems.called + return + assert ( + m_str.call_args + == [(kwargs["filepath"], ), {}]) + _errors = "\n".join(errors) + assert ( + m_problems.call_args + == [(), dict(errors=[f"{kwargs['filepath']}\n{_errors}"])]) + + +def test_envoy_buildifier_handle_error(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "str", + "checker.Problems", + "EnvoyBuildifier._diff", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + response = MagicMock() + + with patched as (m_str, m_problems, m_diff): + assert ( + buildifier.handle_error(response) + == {m_str.return_value: m_problems.return_value}) + + assert ( + m_str.call_args + == [(kwargs["filepath"], ), {}]) + assert ( + m_problems.call_args + == [(), dict(errors=[f"{kwargs['filepath']}\n{m_diff.return_value}"])]) + assert ( + m_diff.call_args + == [(response, ), {}]) + + +def test_envoy_buildifier_has_failed(patches): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "bool", + "EnvoyBuildifier._diff", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + response = MagicMock() + + with patched as (m_bool, m_diff): + assert ( + buildifier.has_failed(response) + == m_bool.return_value) + + assert ( + m_bool.call_args + == [(m_diff.return_value, ), {}]) + assert ( + m_diff.call_args + == [(response, ), {}]) + + +@pytest.mark.parametrize("build", [True, False]) +@pytest.mark.parametrize("package", [True, False]) +@pytest.mark.parametrize("api", [True, False]) +def test_envoy_buildifier_preprocess(patches, build, package, api): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.is_build", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_api_envoy", + dict(new_callable=PropertyMock)), + ("EnvoyBuildifier.is_package", + dict(new_callable=PropertyMock)), + "EnvoyBuildifier.xform_build", + "EnvoyBuildifier.xform_package", + "EnvoyBuildifier.xform_deps_api", + "EnvoyBuildifier.xform_api_package", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + text = MagicMock() + + with patched as patchy: + (m_build, m_api, m_package, m_xbuild, + m_xpackage, m_xdeps, m_xapi) = patchy + m_api.return_value = api + m_build.return_value = build + m_package.return_value = package + result = buildifier.preprocess(text) + + _result = input = text + if build: + assert ( + m_xbuild.call_args + == [(input, ), {}]) + _result = input = m_xbuild.return_value + else: + assert not m_xbuild.called + if package: + assert ( + m_xpackage.call_args + == [(input, ), {}]) + assert ( + m_xdeps.call_args + == [(m_xpackage.return_value, ), {}]) + _result = input = m_xdeps.return_value + else: + assert not m_xpackage.called + assert not m_xdeps.called + if api: + assert ( + m_xapi.call_args + == [(input, ), {}]) + _result = m_xapi.return_value + else: + assert not m_xapi.called + assert result == _result + + +def __test_envoy_buildifier_run_buildozer(patches, iters): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "pathlib", + "tempfile", + "EnvoyBuildifier._run_buildozer", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + cmds = iters(dict).items() + text = MagicMock() + + with patched as (m_plib, m_temp, m_run): + assert ( + buildifier.run_buildozer(cmds, text) + == m_run.return_value.strip.return_value) + + assert ( + m_temp.NamedTemporaryFile.call_args + == [(), dict(mode="w")]) + assert ( + m_plib.Path.call_args + == [((m_temp.NamedTemporaryFile.return_value + .__enter__.return_value.name), ), + {}]) + expected = "\n".join( + "%s|-:%s" + % (cmd, target) for cmd, target in cmds) + assert ( + m_plib.Path.return_value.write_text.call_args + == [(expected, ), {}]) + assert ( + m_run.call_args + == [(m_plib.Path.return_value, text), {}]) + + +@pytest.mark.parametrize("workspace", [True, False]) +def test_envoy_buildifier_subprocess_args(patches, iters, workspace): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "_subprocess.ASubprocessHandler.subprocess_args", + ("EnvoyBuildifier.is_workspace", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + args = iters() + kwargs = iters(dict) + + with patched as (m_super, m_workspace): + m_workspace.return_value = workspace + assert ( + buildifier.subprocess_args(*args, **kwargs) + == m_super.return_value) + + expected_args = ( + args + ["-type=workspace"] + if workspace + else args) + assert ( + m_super.call_args + == [tuple(expected_args), kwargs]) + + +def test_envoy_buildifier_subprocess_kwargs(patches, iters): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + ("EnvoyBuildifier.text", + dict(new_callable=PropertyMock)), + "EnvoyBuildifier.preprocess", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + args = iters() + kwargs = iters(dict) + + with patched as (m_text, m_preproc): + assert ( + buildifier.subprocess_kwargs(*args, **kwargs) + == {**buildifier.kwargs, + "input": m_preproc.return_value, + **kwargs}) + + assert ( + m_preproc.call_args + == [(m_text.return_value, ), {}]) + + +@pytest.mark.parametrize("cached", [None, "CACHED"]) +def test_envoy_buildifier__diff(patches, iters, cached): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "difflib", + "str", + ("EnvoyBuildifier.text", + dict(new_callable=PropertyMock)), + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + buildifier._diff_output = cached + response = MagicMock() + diff = iters() + + with patched as (m_diff, m_str, m_text): + m_diff.unified_diff.return_value = diff + assert ( + buildifier._diff(response) + == (cached if cached + else "\n".join(diff))) + + if cached: + assert not response.stdout.splitlines.called + assert not m_text.return_value.splitlines.called + assert not m_str.called + assert not m_diff.unified_diff.called + return + assert ( + response.stdout.splitlines.call_args + == [(), {}]) + assert ( + m_text.return_value.splitlines.call_args + == [(), {}]) + assert ( + m_diff.unified_diff.call_args + == [(m_text.return_value.splitlines.return_value, + response.stdout.splitlines.return_value), + dict(fromfile=m_str.return_value, + tofile=m_str.return_value)]) + assert ( + m_str.call_args_list + == [[(kwargs["filepath"], ), {}], + [(kwargs["filepath"], ), {}]]) + assert buildifier._diff_output == "\n".join(diff) + + +@pytest.mark.parametrize("responsecode", [0, 1, 3, 137]) +@pytest.mark.parametrize("stdout", [True, False]) +def __test_envoy_buildifier__run_buildozer(patches, responsecode, stdout): + kwargs = dict( + buildozer_path=MagicMock(), + config=MagicMock(), + filepath=MagicMock()) + patched = patches( + "str", + "subprocess", + prefix="envoy.code.check.abstract.bazel") + buildifier = check.abstract.bazel.EnvoyBuildifier( + "PATH", + **kwargs) + cmd_path = MagicMock() + text = MagicMock() + + with patched as (m_str, m_subproc): + m_subproc.run.return_value.returncode = responsecode + if not stdout: + m_subproc.run.return_value.stdout = None + if responsecode in [0, 3]: + assert ( + buildifier._run_buildozer(cmd_path, text) + == (text + if not stdout + else (m_subproc.run.return_value + .stdout.decode.return_value))) + else: + with pytest.raises(check.exceptions.FixError) as e: + buildifier._run_buildozer(cmd_path, text) + + assert ( + m_subproc.run.call_args + == [([kwargs["buildozer_path"], "-stdout", + "-f", m_str.return_value], ), + dict(input=text.encode.return_value, + stdout=m_subproc.PIPE, + stderr=m_subproc.PIPE)]) + assert ( + m_str.call_args + == [(cmd_path, ), {}]) + if responsecode not in [0, 3]: + assert ( + e.value.args[0] + == f"buildozer execution failed: {m_subproc.run.return_value}") + return + if stdout: + assert ( + m_subproc.run.return_value.stdout.decode.call_args + == [(), {}]) + + +def test_bazel_check_constructor(): + bazel = check.ABazelCheck("DIRECTORY") + assert bazel.directory == "DIRECTORY" + assert isinstance(bazel, check.interface.IBazelCheck) + assert isinstance(bazel, check.abstract.AFileCodeCheck) + + +def test_bazel_check_filter_files(patches, iters): + patched = patches( + "set", + prefix="envoy.code.check.abstract.bazel") + files = iters(count=10) + + def match_fun(item): + return int(item[1:]) % 3 + + def exclude_fun(item): + return int(item[1:]) % 2 + + match = MagicMock() + exclude = MagicMock() + match.side_effect = match_fun + exclude.side_effect = exclude_fun + + with patched as (m_set, ): + assert ( + check.ABazelCheck.filter_files(files, match, exclude) + == m_set.return_value) + resultiter = m_set.call_args[0][0] + result = list(resultiter) + + assert isinstance(resultiter, types.GeneratorType) + assert ( + result + == [x for x in files if int(x[1:]) % 3 and not int(x[1:]) % 2]) + assert ( + match.call_args_list + == [[(f, ), {}] for f in files]) + assert ( + exclude.call_args_list + == [[(f, ), {}] for f in files if int(f[1:]) % 3]) + + +def test_bazel_check_run_buildifier(patches, iters): + patched = patches( + "pathlib", + "EnvoyBuildifier", + prefix="envoy.code.check.abstract.bazel") + args = iters() + path = MagicMock() + config = MagicMock() + buildozer_path = MagicMock() + + with patched as (m_plib, m_build): + assert ( + check.ABazelCheck.run_buildifier( + path, config, buildozer_path, *args) + == m_build.return_value.run.return_value) + + assert ( + m_build.call_args + == [(path, ), + dict(config=config, + filepath=m_plib.Path.return_value, + buildozer_path=buildozer_path)]) + assert ( + m_plib.Path.call_args + == [(args[-1], ), {}]) + assert ( + m_build.return_value.run.call_args + == [tuple(args[:-1]), {}]) + + +def test_bazel_check_buildifier_command(patches): + directory = MagicMock() + config = MagicMock() + bazel = check.abstract.bazel.ABazelCheck( + directory, + config=config) + patched = patches( + "partial", + ("ABazelCheck.buildifier_path", + dict(new_callable=PropertyMock)), + ("ABazelCheck.buildozer_path", + dict(new_callable=PropertyMock)), + "ABazelCheck.run_buildifier", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_partial, m_ifier_path, m_ozer_path, m_run): + assert ( + bazel.buildifier_command + == m_partial.return_value) + + assert "buildifier_command" in bazel.__dict__ + assert ( + m_partial.call_args + == [(m_run, + directory.path, + config, + m_ozer_path.return_value, + m_ifier_path.return_value, + "-mode=fix", + "-lint=fix"), {}]) + + +def test_bazel_check_buildifier_path(patches): + bazel = check.abstract.bazel.ABazelCheck("DIRECTORY") + patched = patches( + "ABazelCheck.command_path", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_path, ): + assert ( + bazel.buildifier_path + == m_path.return_value) + + assert "buildifier_path" in bazel.__dict__ + assert ( + m_path.call_args + == [("buildifier", ), {}]) + + +def test_bazel_check_buildozer_path(patches): + bazel = check.abstract.bazel.ABazelCheck("DIRECTORY") + patched = patches( + "ABazelCheck.command_path", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_path, ): + assert ( + bazel.buildozer_path + == m_path.return_value) + + assert "buildozer_path" in bazel.__dict__ + assert ( + m_path.call_args + == [("buildozer", ), {}]) + + +async def test_bazel_check_checker_files(patches): + directory = MagicMock() + files = AsyncMock() + directory.files = files() + bazel = check.abstract.bazel.ABazelCheck(directory) + patched = patches( + ("ABazelCheck.re_path_match", + dict(new_callable=PropertyMock)), + "ABazelCheck.filter_files", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_re, m_filter): + assert ( + await bazel.checker_files + == m_filter.return_value) + + assert ( + m_filter.call_args + == [(files.return_value, m_re.return_value.match, bazel.exclude), {}]) + assert not ( + hasattr( + bazel, + check.ABazelCheck.checker_files.cache_name)) + + +@pytest.mark.parametrize("has_files", [True, False]) +async def test_bazel_check_problem_files(patches, iters, has_files): + _files = ( + iters() + if has_files + else None) + files = AsyncMock(return_value=_files) + bazel = check.abstract.bazel.ABazelCheck("DIRECTORY") + patched = patches( + "dict", + "tasks", + ("ABazelCheck.buildifier_command", + dict(new_callable=PropertyMock)), + ("ABazelCheck.files", + dict(new_callable=PropertyMock)), + "ABazelCheck.execute", + prefix="envoy.code.check.abstract.bazel") + errors = iters() + + async def concurrent(jobs): + for error in errors: + yield error + + with patched as (m_dict, m_tasks, m_command, m_files, m_exec): + m_files.side_effect = files + m_tasks.concurrent.side_effect = concurrent + assert ( + await bazel.problem_files + == (m_dict.return_value + if has_files + else {}) + == getattr( + bazel, + check.ABazelCheck.problem_files.cache_name)[ + "problem_files"]) + if has_files: + taskiters = m_tasks.concurrent.call_args[0][0] + tasklist = list(taskiters) + + if not has_files: + assert not m_dict.called + assert not m_tasks.concurrent.called + assert not m_exec.called + return + assert ( + m_dict.call_args + == [(), {}]) + assert type(taskiters) is types.GeneratorType + assert ( + tasklist + == [m_exec.return_value for f in _files]) + assert ( + m_exec.call_args_list + == [[(m_command.return_value, f), {}] + for f in _files]) + assert ( + m_dict.return_value.update.call_args_list + == [[(error, ), {}] + for error in errors]) + + +def test_bazel_check_re_path_match(patches): + bazel = check.abstract.bazel.ABazelCheck("DIRECTORY") + patched = patches( + "re", + prefix="envoy.code.check.abstract.bazel") + + with patched as (m_re, ): + assert ( + bazel.re_path_match + == m_re.compile.return_value) + + assert "re_path_match" in bazel.__dict__ + assert ( + m_re.compile.call_args + == [("|".join(check.abstract.bazel.RE_BAZEL_MATCH), ), {}]) + + +def test_bazel_check_exclude(patches): + config = MagicMock() + bazel = check.abstract.bazel.ABazelCheck("DIRECTORY", config=config) + patched = patches( + "tuple", + prefix="envoy.code.check.abstract.bazel") + path = MagicMock() + + with patched as (m_tuple, ): + assert ( + bazel.exclude(path) + == path.startswith.return_value) + + assert ( + path.startswith.call_args + == [(m_tuple.return_value, ), {}]) + assert ( + m_tuple.call_args + == [(config.__getitem__.return_value.__getitem__.return_value, ), {}]) + assert ( + config.__getitem__.call_args + == [("paths", ), {}]) + assert ( + config.__getitem__.return_value.__getitem__.call_args + == [("excluded", ), {}]) diff --git a/envoy.code.check/tests/test_abstract_checker.py b/envoy.code.check/tests/test_abstract_checker.py index 10fe45095..17d78e9f1 100644 --- a/envoy.code.check/tests/test_abstract_checker.py +++ b/envoy.code.check/tests/test_abstract_checker.py @@ -56,6 +56,10 @@ class DummyCodeChecker: def changelog_class(self): return super().changelog_class + @property + def bazel_class(self): + return super().bazel_class + @property def extensions_class(self): return super().extensions_class @@ -116,10 +120,10 @@ def test_abstract_checker_constructor(patches, args, kwargs): "checker.Checker.__init__", prefix="envoy.code.check.abstract.checker") iface_props = [ - "extensions_class", "fs_directory_class", "flake8_class", - "git_directory_class", "glint_class", "gofmt_class", "project_class", - "runtime_guards_class", "shellcheck_class", "yapf_class", - "changelog_class", "yamllint_class"] + "bazel_class", "extensions_class", "fs_directory_class", + "flake8_class", "git_directory_class", "glint_class", "gofmt_class", + "project_class", "runtime_guards_class", "shellcheck_class", + "yapf_class", "changelog_class", "yamllint_class"] with patched as (m_super, ): m_super.return_value = None @@ -137,7 +141,7 @@ def test_abstract_checker_constructor(patches, args, kwargs): == [tuple(args), kwargs]) assert ( checker.checks - == ("changelog", + == ("bazel", "changelog", "extensions_fuzzed", "extensions_metadata", "extensions_owners", "extensions_registered",