merge: #2990

2990: feat(buck2): add yapf_check rule to check Python formatting r=fnichol a=fnichol

This change adds a new Buck2 rule called `yapf_check` which uses the
[yapf] Python formatter to enforce a consistent style. As much as
possible, the formatter follows the [PEP 8] style guide standard.

Future changes will add `check-format` targets to various `BUCK` files
which will automatically add these checks to CI in the `si/review-pr`
and `si/merge-queue` pipelines.

[PEP 8]: https://peps.python.org/pep-0008/
[yapf]: https://github.com/google/yapf

---

There is additional work in this PR, see the commit messages for more details

<img src="https://media4.giphy.com/media/26BRsI63ak8uxsU6Y/giphy.gif"/>

Co-authored-by: Fletcher Nichol <[email protected]>

.buckconfig

@@ -13,7 +13,9 @@ fbcode = none
 fbsource = none
 
 [parser]
-target_platform_detector_spec = target:root//...->prelude//platforms:default
+target_platform_detector_spec = \
+  target:root//...->prelude//platforms:default \
+  target:prelude-si//...->prelude//platforms:default
 
 [project]
 ignore = \

.ci/BUCK

@@ -0,0 +1,17 @@
+load(
+    "@prelude-si//:macros.bzl",
+    "test_suite",
+    "yapf_check",
+)
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

bxl/dependent_targets.bxl

@@ -196,8 +196,10 @@ def _filtered_promote_docker_targets(ctx: bxl.Context, targets: bxl.TargetSet) -
 
 # Computes a list of targets for all targets in the project.
 def _dependent_global_file_targets(ctx: bxl.Context) -> bxl.TargetSet:
-    query = "root//..."
-    results = ctx.uquery().eval(query)
+    results = utarget_set()
+
+    for universe in ctx.cli_args.rdeps_universe:
+      results = results + ctx.uquery().eval(universe)
 
     return results
 

dev/BUCK

@@ -1,9 +1,11 @@
 load(
     "@prelude-si//:macros.bzl",
     "alias",
+    "test_suite",
     "tilt_docker_compose_stop",
     "tilt_down",
     "tilt_up",
+    "yapf_check",
 )
 
 python_bootstrap_binary(
@@ -40,3 +42,15 @@ tilt_docker_compose_stop(
 tilt_down(
     name = "down",
 )
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

flake.nix

@@ -332,6 +332,7 @@
               shfmt
               tilt
               typos
+              yapf
             ]
             # Directly add the build dependencies for the packages rather than
             # use: `inputsFrom = lib.attrValues packages;`.

prelude-si/build_context/BUCK

@@ -1,6 +1,8 @@
 load(
     "@prelude-si//:macros.bzl",
     "export_file",
+    "test_suite",
+    "yapf_check",
 )
 
 export_file(
@@ -10,3 +12,15 @@ export_file(
 export_file(
     name = "build_context_srcs_from_deps.bxl",
 )
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

prelude-si/build_context/build_context.py

@@ -84,6 +84,8 @@ def main() -> int:
 
         for arg in srcs or []:
             src, dst = arg.split("=")
+            if not dst:
+                dst = os.path.dirname(src) or "."
 
             os.makedirs(os.path.join(root_dir, dst), exist_ok=True)
 

prelude-si/docker/BUCK

@@ -1,6 +1,8 @@
 load(
     "@prelude-si//:macros.bzl",
     "export_file",
+    "test_suite",
+    "yapf_check",
 )
 
 export_file(
@@ -22,3 +24,15 @@ export_file(
 export_file(
     name = "docker_image_promote.py",
 )
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

prelude-si/docker/docker_image_push.py

@@ -104,6 +104,5 @@ def report_metadata(label_metadata_file: str, tags: List[str]):
         ))
 
 
-
 if __name__ == "__main__":
     sys.exit(main())

prelude-si/git/BUCK

@@ -1,8 +1,22 @@
 load(
     "@prelude-si//:macros.bzl",
     "export_file",
+    "test_suite",
+    "yapf_check",
 )
 
 export_file(
     name = "git_info.py",
 )
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

prelude-si/macros.bzl

@@ -62,6 +62,12 @@ typescript_runnable_dist_bin = _typescript_runnable_dist_bin
 vite_app = _vite_app
 workspace_node_modules = _workspace_node_modules
 
+load(
+    "@prelude-si//macros:python.bzl",
+    _yapf_check = "yapf_check",
+)
+yapf_check = _yapf_check
+
 load(
     "@prelude-si//macros:rust.bzl",
     _rust_binary = "rust_binary",

prelude-si/macros/python.bzl

@@ -0,0 +1,14 @@
+load(
+    "@prelude-si//:python.bzl",
+    _yapf_check = "yapf_check",
+)
+
+def yapf_check(
+        name,
+        visibility = ["PUBLIC"],
+        **kwargs):
+    _yapf_check(
+        name = name,
+        visibility = visibility,
+        **kwargs
+    )

prelude-si/pnpm/BUCK

@@ -1,6 +1,8 @@
 load(
     "@prelude-si//:macros.bzl",
     "export_file",
+    "test_suite",
+    "yapf_check",
 )
 
 export_file(
@@ -38,3 +40,15 @@ export_file(
 export_file(
     name = "run_pnpm_script.py",
 )
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

prelude-si/python.bzl

@@ -0,0 +1,107 @@
+load(
+    "@prelude//python:toolchain.bzl",
+    "PythonToolchainInfo",
+)
+load(
+    "//build_context:toolchain.bzl",
+    "BuildContextToolchainInfo",
+)
+load(
+    "//python:toolchain.bzl",
+    "SiPythonToolchainInfo",
+)
+load(
+    "@prelude//decls/re_test_common.bzl",
+    "re_test_common",
+)
+load(
+    "@prelude//test/inject_test_run_info.bzl",
+    "inject_test_run_info",
+)
+load(
+    "@prelude//tests:re_utils.bzl",
+    "get_re_executor_from_props",
+)
+load(
+    "@prelude-si//:test.bzl",
+    "inject_test_env",
+)
+load(
+    "@prelude//:paths.bzl",
+    "paths",
+)
+load(
+    "//build_context.bzl",
+    "BuildContext",
+    _build_context = "build_context",
+)
+
+def yapf_check_impl(ctx: AnalysisContext) -> list[[
+    DefaultInfo,
+    RunInfo,
+    ExternalRunnerTestInfo,
+]]:
+    srcs = {}
+    for src in ctx.attrs.srcs:
+        # An empty string triggers build_context.py to map `src` into the
+        # `dirname(src)` directory in the build context
+        srcs[src] = ""
+    build_context = _build_context(ctx, [], srcs)
+
+    si_python_toolchain = ctx.attrs._si_python_toolchain[SiPythonToolchainInfo]
+
+    run_cmd_args = cmd_args(
+        ctx.attrs._python_toolchain[PythonToolchainInfo].interpreter,
+        si_python_toolchain.yapf_check[DefaultInfo].default_outputs,
+    )
+    run_cmd_args.add(build_context.root)
+
+    args_file = ctx.actions.write("args.txt", run_cmd_args)
+
+    # Setup a RE executor based on the `remote_execution` param.
+    re_executor = get_re_executor_from_props(ctx)
+
+    # We implicitly make the target run from the project root if remote
+    # excution options were specified
+    run_from_project_root = "buck2_run_from_project_root" in (
+        ctx.attrs.labels or []
+    ) or re_executor != None
+
+    return inject_test_run_info(
+        ctx,
+        ExternalRunnerTestInfo(
+            type = "shfmt",
+            command = [run_cmd_args],
+            env = ctx.attrs.env,
+            labels = ctx.attrs.labels,
+            contacts = ctx.attrs.contacts,
+            default_executor = re_executor,
+            run_from_project_root = run_from_project_root,
+            use_project_relative_paths = run_from_project_root,
+        ),
+    ) + [
+        DefaultInfo(default_output = args_file),
+    ]
+
+yapf_check = rule(
+    impl = yapf_check_impl,
+    attrs = {
+        "srcs": attrs.list(
+            attrs.source(),
+            default = [],
+            doc = """The set of source files to consider.""",
+        ),
+        "_python_toolchain": attrs.toolchain_dep(
+            default = "toolchains//:python",
+            providers = [PythonToolchainInfo],
+        ),
+        "_build_context_toolchain": attrs.toolchain_dep(
+            default = "toolchains//:build_context",
+            providers = [BuildContextToolchainInfo],
+        ),
+        "_si_python_toolchain": attrs.toolchain_dep(
+            default = "toolchains//:si_python",
+            providers = [SiPythonToolchainInfo],
+        ),
+    } | re_test_common.test_args() | inject_test_env.args(),
+)

prelude-si/python/BUCK

@@ -0,0 +1,22 @@
+load(
+    "@prelude-si//:macros.bzl",
+    "export_file",
+    "test_suite",
+    "yapf_check",
+)
+
+export_file(
+    name = "yapf_check.py",
+)
+
+yapf_check(
+    name = "check-format-python",
+    srcs = glob(["**/*.py"]),
+)
+
+test_suite(
+    name = "check-format",
+    tests = [
+        ":check-format-python",
+    ],
+)

prelude-si/python/toolchain.bzl

@@ -0,0 +1,27 @@
+SiPythonToolchainInfo = provider(
+    fields = {
+        "yapf_check": typing.Any,
+    },
+)
+
+def si_python_toolchain_impl(ctx) -> list[[DefaultInfo, SiPythonToolchainInfo]]:
+    """
+    A extended Python toolchain.
+    """
+
+    return [
+        DefaultInfo(),
+        SiPythonToolchainInfo(
+            yapf_check = ctx.attrs._yapf_check,
+        ),
+    ]
+
+si_python_toolchain = rule(
+    impl = si_python_toolchain_impl,
+    attrs = {
+        "_yapf_check": attrs.dep(
+            default = "prelude-si//python:yapf_check.py",
+        ),
+    },
+    is_toolchain_rule = True,
+)