Re: [PATCH v4 05/11] scripts: generate_rust_analyzer.py: add type hints

From: Daniel Almeida
Date: Tue Mar 25 2025 - 09:55:49 EST


Hi Tamir,

> On 22 Mar 2025, at 10:23, Tamir Duberstein <tamird@xxxxxxxxx> wrote:
>
> Python type hints allow static analysis tools like mypy to detect type
> errors during development, improving the developer experience.
>
> Python type hints have been present in the kernel since 2019 at the
> latest; see commit 6ebf5866f2e8 ("kunit: tool: add Python wrappers for
> running KUnit tests").
>
> Run `mypy --strict scripts/generate_rust_analyzer.py --python-version
> 3.8` to verify. Note that `mypy` no longer supports python < 3.8.
>
> This removes `"is_proc_macro": false` from `rust-project.json` in
> exchange for stricter types. This field is interpreted as false if
> absent[1] so this doesn't change the behavior of rust-analyzer.

Can this be a separate patch? Not sure how this is related to Python type
hints, but it makes the current patch harder to review.

>
> Link: https://github.com/rust-lang/rust-analyzer/blob/8d01570b5e812a49daa1f08404269f6ea5dd73a1/crates/project-model/src/project_json.rs#L372-L373 [1]
> Signed-off-by: Tamir Duberstein <tamird@xxxxxxxxx>
> ---
> scripts/generate_rust_analyzer.py | 172 +++++++++++++++++++++++++-------------
> 1 file changed, 112 insertions(+), 60 deletions(-)
>
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index 03f55cce673c..0772ea309f94 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -10,8 +10,10 @@ import os
> import pathlib
> import subprocess
> import sys
> +from typing import Dict, Iterable, List, Literal, Optional, TypedDict
>
> -def args_crates_cfgs(cfgs):
> +
> +def args_crates_cfgs(cfgs: Iterable[str]) -> Dict[str, List[str]]:
> crates_cfgs = {}
> for cfg in cfgs:
> crate, vals = cfg.split("=", 1)
> @@ -19,7 +21,45 @@ def args_crates_cfgs(cfgs):
>
> return crates_cfgs
>
> -def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> +
> +class Dependency(TypedDict):
> + crate: int
> + name: str
> +
> +
> +class Source(TypedDict):
> + include_dirs: List[str]
> + exclude_dirs: List[str]
> +
> +
> +class Crate(TypedDict):
> + display_name: str
> + root_module: str
> + is_workspace_member: bool
> + deps: List[Dependency]
> + cfg: List[str]
> + edition: Literal["2021"]
> + env: Dict[str, str]
> +
> +
> +# `NotRequired` fields on `Crate` would be better but `NotRequired` was added in 3.11.
> +class ProcMacroCrate(Crate):
> + is_proc_macro: Literal[True]
> + proc_macro_dylib_path: Optional[str] # `pathlib.Path` is not JSON serializable.
> +
> +
> +# `NotRequired` fields on `Crate` would be better but `NotRequired` was added in 3.11.
> +class CrateWithGenerated(Crate):
> + source: Optional[Source]
> +
> +
> +def generate_crates(
> + srctree: pathlib.Path,
> + objtree: pathlib.Path,
> + sysroot_src: pathlib.Path,
> + external_src: pathlib.Path,
> + cfgs: List[str],
> +) -> List[Crate]:
> # Generate the configuration list.
> cfg = []
> with open(objtree / "include" / "generated" / "rustc_cfg") as fd:
> @@ -31,67 +71,75 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> # Now fill the crates list -- dependencies need to come first.
> #
> # Avoid O(n^2) iterations by keeping a map of indexes.
> - crates = []
> - crates_indexes = {}
> + crates: List[Crate] = []
> + crates_indexes: Dict[str, int] = {}
> crates_cfgs = args_crates_cfgs(cfgs)
>
> def build_crate(
> - display_name,
> - root_module,
> - deps,
> - cfg=[],
> - is_workspace_member=True,
> - is_proc_macro=False,
> - ):
> - crate = {
> + display_name: str,
> + root_module: pathlib.Path,
> + deps: List[str],
> + cfg: List[str] = [],
> + is_workspace_member: bool = True,
> + ) -> Crate:
> + return {
> "display_name": display_name,
> "root_module": str(root_module),
> "is_workspace_member": is_workspace_member,
> - "is_proc_macro": is_proc_macro,
> "deps": [{"crate": crates_indexes[dep], "name": dep} for dep in deps],
> "cfg": cfg,
> "edition": "2021",
> "env": {
> "RUST_MODFILE": "This is only for rust-analyzer"
> - }
> + },
> }
> - if is_proc_macro:
> - proc_macro_dylib_name = subprocess.check_output(
> - [os.environ["RUSTC"], "--print", "file-names", "--crate-name", display_name, "--crate-type", "proc-macro", "-"],
> - stdin=subprocess.DEVNULL,
> - ).decode('utf-8').strip()
> - crate["proc_macro_dylib_path"] = f"{objtree}/rust/{proc_macro_dylib_name}"
> - return crate
> -
> - def register_crate(crate):
> +
> + def register_crate(crate: Crate) -> None:
> crates_indexes[crate["display_name"]] = len(crates)
> crates.append(crate)
>
> def append_crate(
> - display_name,
> - root_module,
> - deps,
> - cfg=[],
> - is_workspace_member=True,
> - is_proc_macro=False,
> - ):
> + display_name: str,
> + root_module: pathlib.Path,
> + deps: List[str],
> + cfg: List[str] = [],
> + is_workspace_member: bool = True,
> + ) -> None:
> register_crate(
> - build_crate(
> - display_name, root_module, deps, cfg, is_workspace_member, is_proc_macro
> - )
> + build_crate(display_name, root_module, deps, cfg, is_workspace_member)
> )
>
> + def append_proc_macro_crate(
> + display_name: str,
> + root_module: pathlib.Path,
> + deps: List[str],
> + cfg: List[str] = [],
> + ) -> None:
> + crate = build_crate(display_name, root_module, deps, cfg)
> + proc_macro_dylib_name = subprocess.check_output(
> + [os.environ["RUSTC"], "--print", "file-names", "--crate-name", display_name, "--crate-type", "proc-macro", "-"],
> + stdin=subprocess.DEVNULL,
> + ).decode('utf-8').strip()
> + proc_macro_crate: ProcMacroCrate = {
> + **crate,
> + "is_proc_macro": True,
> + "proc_macro_dylib_path": f"{objtree}/rust/{proc_macro_dylib_name}",
> + }
> + register_crate(proc_macro_crate)
> +

Same here. Is this supposed to be related to Python type hints somehow?

> def append_sysroot_crate(
> - display_name,
> - deps,
> - cfg=[],
> - ):
> - append_crate(
> - display_name,
> - sysroot_src / display_name / "src" / "lib.rs",
> - deps,
> - cfg,
> - is_workspace_member=False,
> + display_name: str,
> + deps: List[str],
> + cfg: List[str] = [],
> + ) -> None:
> + register_crate(

Why is register_crate here now? Maybe this change belongs to the preceding patch?

> + build_crate(
> + display_name,
> + sysroot_src / display_name / "src" / "lib.rs",
> + deps,
> + cfg,
> + is_workspace_member=False,
> + )
> )
>
> # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
> @@ -108,11 +156,10 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> [],
> )
>
> - append_crate(
> + append_proc_macro_crate(
> "macros",
> srctree / "rust" / "macros" / "lib.rs",
> ["std", "proc_macro"],
> - is_proc_macro=True,
> )
>
> append_crate(
> @@ -121,12 +168,11 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> ["core", "compiler_builtins"],
> )
>
> - append_crate(
> + append_proc_macro_crate(
> "pin_init_internal",
> srctree / "rust" / "pin-init" / "internal" / "src" / "lib.rs",
> [],
> cfg=["kernel"],
> - is_proc_macro=True,
> )
>
> append_crate(
> @@ -137,9 +183,9 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> )
>
> def append_crate_with_generated(
> - display_name,
> - deps,
> - ):
> + display_name: str,
> + deps: List[str],
> + ) -> None:
> crate = build_crate(
> display_name,
> srctree / "rust" / display_name / "lib.rs",
> @@ -147,20 +193,23 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> cfg=cfg,
> )
> crate["env"]["OBJTREE"] = str(objtree.resolve(True))
> - crate["source"] = {
> - "include_dirs": [
> - str(srctree / "rust" / display_name),
> - str(objtree / "rust")
> - ],
> - "exclude_dirs": [],
> + crate_with_generated: CrateWithGenerated = {
> + **crate,
> + "source": {
> + "include_dirs": [
> + str(srctree / "rust" / display_name),
> + str(objtree / "rust")
> + ],
> + "exclude_dirs": [],
> + }
> }
> - register_crate(crate)
> + register_crate(crate_with_generated)
>
> append_crate_with_generated("bindings", ["core"])
> append_crate_with_generated("uapi", ["core"])
> append_crate_with_generated("kernel", ["core", "macros", "build_error", "bindings", "pin_init", "uapi"])
>
> - def is_root_crate(build_file, target):
> + def is_root_crate(build_file: pathlib.Path, target: str) -> bool:
> try:
> return f"{target}.o" in open(build_file).read()
> except FileNotFoundError:
> @@ -169,7 +218,9 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> # Then, the rest outside of `rust/`.
> #
> # We explicitly mention the top-level folders we want to cover.
> - extra_dirs = map(lambda dir: srctree / dir, ("samples", "drivers"))
> + extra_dirs: Iterable[pathlib.Path] = map(
> + lambda dir: srctree / dir, ("samples", "drivers")
> + )
> if external_src is not None:
> extra_dirs = [external_src]
> for folder in extra_dirs:
> @@ -192,7 +243,8 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>
> return crates
>
> -def main():
> +

What is this extra blank line for? Automatically generated by a formatter?

> +def main() -> None:
> parser = argparse.ArgumentParser()
> parser.add_argument("--verbose", "-v", action="store_true")
> parser.add_argument("--cfgs", action="append", default=[])
>
> --
> 2.48.1
>
>

Other than what I said above, the type hints themselves are fine.

— Daniel