Split up check_python into pylint and pytype aspects

.bazelrc

@@ -27,8 +27,14 @@ build --host_features=external_include_paths
 build:windows --copt='/external:W2' --host_copt='/external:W2'
 
 # Run Pylint by default.
-build --aspects='//dev:check_python.bzl%check_python'
-build --output_groups='+check_python'
+build --aspects='//dev:pylint.bzl%pylint'
+build --output_groups='+pylint'
+
+# Run Pytype on platforms where it’s supported.
+build:linux --aspects='//dev:pytype.bzl%pytype'
+build:linux --output_groups='+pytype'
+build:macos --aspects='//dev:pytype.bzl%pytype'
+build:macos --output_groups='+pytype'
 
 # Suppress unactionable warnings about duplicate libraries.
 # TODO: File bug against rules_cc about this.

dev/BUILD

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
 load("@buildifier//:rules.bzl", "buildifier_test")
 load("@gazelle//:def.bzl", "DEFAULT_LANGUAGES", "gazelle", "gazelle_binary", "gazelle_test")
@@ -63,7 +64,8 @@ compile_pip_requirements(
     tags = [
         # Don’t try to run Pylint or Pytype.  This target doesn’t contain any of
         # our source files.
-        "no-python-check",
+        "no-pylint",
+        "no-pytype",
     ],
 )
 
@@ -137,3 +139,27 @@ gazelle_binary(
         "@gazelle//language/bazel/visibility",
     ],
 )
+
+bzl_library(
+    name = "pylint",
+    srcs = ["pylint.bzl"],
+    deps = [
+        ":check_python_bzl",
+        "@rules_python//python:py_info_bzl",
+    ],
+)
+
+bzl_library(
+    name = "pytype",
+    srcs = ["pytype.bzl"],
+    deps = [
+        ":check_python_bzl",
+        "@rules_python//python:py_info_bzl",
+    ],
+)
+
+# keep
+bzl_library(
+    name = "check_python_bzl",
+    srcs = ["check_python.bzl"],
+)

dev/check_python.bzl

@@ -12,20 +12,37 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Defines the `check_python` aspect."""
-
-load("@rules_python//python:py_info.bzl", "PyInfo")
+"""Defines the `check_python` helper function."""
 
 visibility("private")
 
-def _check_python_impl(target, ctx):
-    tags = ctx.rule.attr.tags
+def check_python(
+        ctx,
+        *,
+        info,
+        stem,
+        program,
+        program_args = None,
+        additional_inputs = [],
+        mnemonic,
+        progress_message):
+    """Run Pylint or Pytype on Python source files.
+
+    Args:
+      ctx: the rule context
+      info: a PyInfo object
+      stem: base name of files that will be generated
+      program: name of the program to run, either "pylint" or "pytype"
+      program_args: additional arguments for the program, either an Args object
+          or None
+      additional_inputs: list of additional input files for the program,
+          e.g. .pylintrc
+      mnemonic: mnemonic for the action, as for ctx.actions.run
+      progress_message: progress message for the action, as for ctx.actions.run
 
-    # TODO: Require PyInfo provider using required_providers, see below.
-    if "no-python-check" in tags or PyInfo not in target:
-        return []
-    info = target[PyInfo]
-    stem = "_{}.python-check".format(target.label.name)
+    Returns:
+      a dummy file that should be added to an optional output group
+    """
     params_file = ctx.actions.declare_file(stem + ".json")
     output_file = ctx.actions.declare_file(stem + ".stamp")
     params = struct(
@@ -39,10 +56,8 @@ def _check_python_impl(target, ctx):
         ],
     )
     ctx.actions.write(params_file, json.encode(params))
-    pylintrc = ctx.file._pylintrc
     args = ctx.actions.args()
     args.add(output_file, format = "--out=%s")
-    args.add(pylintrc, format = "--pylintrc=%s")
     args.add(params_file, format = "--params=%s")
     args.add_all(
         info.imports,
@@ -59,42 +74,19 @@ def _check_python_impl(target, ctx):
     )
     args.add(ctx.workspace_name, format = "--import=%s")
     args.add(ctx.workspace_name, format = "--workspace-name=%s")
-    if "no-pytype" not in tags:
-        args.add("--pytype")
     ctx.actions.run(
         outputs = [output_file],
         inputs = depset(
-            direct = [params_file, pylintrc],
+            direct = [params_file] + additional_inputs,
             transitive = [info.transitive_sources],
         ),
         executable = ctx.executable._check,
-        arguments = [args],
-        mnemonic = "PythonCheck",
-        progress_message = "Performing static analysis of target %{label}",
+        arguments = [args, program, program_args or ctx.actions.args()],
+        mnemonic = mnemonic,
+        progress_message = progress_message,
         toolchain = None,
     )
-    return [
-        OutputGroupInfo(check_python = depset([output_file])),
-    ]
-
-check_python = aspect(
-    implementation = _check_python_impl,
-    attrs = {
-        "_check": attr.label(
-            default = Label("//dev:check_python"),
-            executable = True,
-            cfg = "exec",
-        ),
-        "_pylintrc": attr.label(
-            default = Label("//:.pylintrc"),
-            allow_single_file = True,
-        ),
-    },
-    # The Python rules don’t advertise the PyInfo provider, so we can’t use
-    # required_providers here.
-    # TODO: File bug against rules_python.
-    # required_providers = [PyInfo],
-)
+    return output_file
 
 def _repository_name(file):
     # Skip empty string for main repository.

dev/check_python.py

@@ -1,4 +1,4 @@
-# Copyright 2021, 2022, 2023, 2024 Google LLC
+# Copyright 2021, 2022, 2023, 2024, 2025 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -34,8 +34,10 @@ def main() -> None:
     parser.add_argument('--import', type=pathlib.PurePosixPath, action='append',
                         default=[], dest='path')
     parser.add_argument('--workspace-name', type=str, required=True)
-    parser.add_argument('--pylintrc', type=pathlib.Path, required=True)
-    parser.add_argument('--pytype', action='store_true', default=False)
+    subparsers = parser.add_subparsers(required=True, dest='program')
+    pylint = subparsers.add_parser('pylint', allow_abbrev=False)
+    pylint.add_argument('--pylintrc', type=pathlib.Path, required=True)
+    subparsers.add_parser('pytype', allow_abbrev=False)
     args = parser.parse_args()
     workspace_name = args.workspace_name
     dirs = [d for d in sys.path if os.path.basename(d) == workspace_name]
@@ -87,19 +89,20 @@ def main() -> None:
     env = dict(os.environ,
                PATH=os.pathsep.join([str(bindir)] + os.get_exec_path()),
                PYTHONPATH=os.pathsep.join(orig_path + repository_path))
-    result = subprocess.run(
-        [sys.executable, '-m', 'pylint',
-         # We’d like to add “--” after the options, but that’s not possible due
-         # to https://github.com/PyCQA/pylint/issues/7003.
-         '--persistent=no', '--rcfile=' + str(args.pylintrc.resolve())]
-        + [str(file.relative_to(cwd)) for file in sorted(srcs)],
-        check=False, cwd=cwd, env=env,
-        stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-        encoding='utf-8', errors='backslashreplace')
-    if result.returncode:
-        print(result.stdout)
-        sys.exit(result.returncode)
-    if platform.system() != 'Windows' and args.pytype:
+    if args.program == 'pylint':
+        result = subprocess.run(
+            [sys.executable, '-m', 'pylint',
+             # We’d like to add “--” after the options, but that’s not possible
+             # due to https://github.com/PyCQA/pylint/issues/7003.
+             '--persistent=no', '--rcfile=' + str(args.pylintrc.resolve())]
+            + [str(file.relative_to(cwd)) for file in sorted(srcs)],
+            check=False, cwd=cwd, env=env,
+            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
+            encoding='utf-8', errors='backslashreplace')
+        if result.returncode:
+            print(result.stdout)
+            sys.exit(result.returncode)
+    if platform.system() != 'Windows' and args.program == 'pytype':
         result = subprocess.run(
             [sys.executable, '-m', 'pytype',
              '--pythonpath=' + os.pathsep.join(repository_path),

dev/pylint.bzl

@@ -0,0 +1,64 @@
+# Copyright 2021, 2022, 2023, 2024, 2025 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Defines the `pylint` aspect."""
+
+load("@rules_python//python:py_info.bzl", "PyInfo")
+load(":check_python.bzl", "check_python")
+
+visibility("private")
+
+def _pylint_impl(target, ctx):
+    tags = ctx.rule.attr.tags
+
+    # TODO: Require PyInfo provider using required_providers, see below.
+    if "no-pylint" in tags or PyInfo not in target:
+        return []
+    info = target[PyInfo]
+    stem = "_{}.pylint".format(target.label.name)
+    pylintrc = ctx.file._pylintrc
+    args = ctx.actions.args()
+    args.add(pylintrc, format = "--pylintrc=%s")
+    output_file = check_python(
+        ctx,
+        info = info,
+        stem = stem,
+        program = "pylint",
+        program_args = args,
+        additional_inputs = [pylintrc],
+        mnemonic = "Pylint",
+        progress_message = "Linting Python target %{label}",
+    )
+    return [
+        OutputGroupInfo(pylint = depset([output_file])),
+    ]
+
+pylint = aspect(
+    implementation = _pylint_impl,
+    attrs = {
+        "_check": attr.label(
+            default = Label("//dev:check_python"),
+            executable = True,
+            cfg = "exec",
+        ),
+        "_pylintrc": attr.label(
+            default = Label("//:.pylintrc"),
+            allow_single_file = True,
+        ),
+    },
+    # The Python rules don’t advertise the PyInfo provider, so we can’t use
+    # required_providers here.
+    # TODO: File bug against rules_python.
+    # required_providers = [PyInfo],
+)

dev/pytype.bzl

@@ -0,0 +1,55 @@
+# Copyright 2021, 2022, 2023, 2024, 2025 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Defines the `pytype` aspect."""
+
+load("@rules_python//python:py_info.bzl", "PyInfo")
+load(":check_python.bzl", "check_python")
+
+visibility("private")
+
+def _pytype_impl(target, ctx):
+    tags = ctx.rule.attr.tags
+
+    # TODO: Require PyInfo provider using required_providers, see below.
+    if "no-pytype" in tags or PyInfo not in target:
+        return []
+    info = target[PyInfo]
+    stem = "_{}.pytype".format(target.label.name)
+    output_file = check_python(
+        ctx,
+        info = info,
+        stem = stem,
+        program = "pytype",
+        mnemonic = "Pytype",
+        progress_message = "Checking Python types in %{label}",
+    )
+    return [
+        OutputGroupInfo(pytype = depset([output_file])),
+    ]
+
+pytype = aspect(
+    implementation = _pytype_impl,
+    attrs = {
+        "_check": attr.label(
+            default = Label("//dev:check_python"),
+            executable = True,
+            cfg = "exec",
+        ),
+    },
+    # The Python rules don’t advertise the PyInfo provider, so we can’t use
+    # required_providers here.
+    # TODO: File bug against rules_python.
+    # required_providers = [PyInfo],
+)

docs/BUILD

@@ -119,7 +119,10 @@ py_binary(
 
 py_proto_library(
     name = "stardoc_output_py_proto",
-    tags = ["no-python-check"],
+    tags = [
+        "no-pylint",
+        "no-pytype",
+    ],
     deps = ["@stardoc//stardoc/proto:stardoc_output_proto"],
 )