mirror of
https://github.com/openai/codex.git
synced 2026-04-30 03:12:20 +03:00
ci: sync Bazel clippy lints and fix uncovered violations (#16351)
## Why Follow-up to #16345, the Bazel clippy rollout in #15955, and the cleanup pass in #16353. `cargo clippy` was enforcing the workspace deny-list from `codex-rs/Cargo.toml` because the member crates opt into `[lints] workspace = true`, but Bazel clippy was only using `rules_rust` plus `clippy.toml`. That left the Bazel lane vulnerable to drift: `clippy.toml` can tune lint behavior, but it cannot set allow/warn/deny/forbid levels. This PR now closes both sides of the follow-up. It keeps `.bazelrc` in sync with `[workspace.lints.clippy]`, and it fixes the real clippy violations that the newly-synced Windows Bazel lane surfaced once that deny-list started matching Cargo. ## What Changed - added `.github/scripts/verify_bazel_clippy_lints.py`, a Python check that parses `codex-rs/Cargo.toml` with `tomllib`, reads the Bazel `build:clippy` `clippy_flag` entries from `.bazelrc`, and reports missing, extra, or mismatched lint levels - ran that verifier from the lightweight `ci.yml` workflow so the sync check does not depend on a Rust toolchain being installed first - expanded the `.bazelrc` comment to explain the Cargo `workspace = true` linkage and why Bazel needs the deny-list duplicated explicitly - fixed the Windows-only `codex-windows-sandbox` violations that Bazel clippy reported after the sync, using the same style as #16353: inline `format!` args, method references instead of trivial closures, removed redundant clones, and replaced SID conversion `unwrap` and `expect` calls with proper errors - cleaned up the remaining cross-platform violations the Bazel lane exposed in `codex-backend-client` and `core_test_support` ## Testing Key new test introduced by this PR: `python3 .github/scripts/verify_bazel_clippy_lints.py`
This commit is contained in:
234
.github/scripts/verify_bazel_clippy_lints.py
vendored
Normal file
234
.github/scripts/verify_bazel_clippy_lints.py
vendored
Normal file
@@ -0,0 +1,234 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import re
|
||||
import sys
|
||||
import tomllib
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
DEFAULT_CARGO_TOML = ROOT / "codex-rs" / "Cargo.toml"
|
||||
DEFAULT_BAZELRC = ROOT / ".bazelrc"
|
||||
BAZEL_CLIPPY_FLAG_PREFIX = "build:clippy --@rules_rust//rust/settings:clippy_flag="
|
||||
BAZEL_SPECIAL_FLAGS = {"-Dwarnings"}
|
||||
VALID_LEVELS = {"allow", "warn", "deny", "forbid"}
|
||||
LONG_FLAG_RE = re.compile(
|
||||
r"^--(?P<level>allow|warn|deny|forbid)=clippy::(?P<lint>[a-z0-9_]+)$"
|
||||
)
|
||||
SHORT_FLAG_RE = re.compile(r"^-(?P<level>[AWDF])clippy::(?P<lint>[a-z0-9_]+)$")
|
||||
SHORT_LEVEL_NAMES = {
|
||||
"A": "allow",
|
||||
"W": "warn",
|
||||
"D": "deny",
|
||||
"F": "forbid",
|
||||
}
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
description=(
|
||||
"Verify that Bazel clippy flags in .bazelrc stay in sync with "
|
||||
"codex-rs/Cargo.toml [workspace.lints.clippy]."
|
||||
)
|
||||
)
|
||||
parser.add_argument(
|
||||
"--cargo-toml",
|
||||
type=Path,
|
||||
default=DEFAULT_CARGO_TOML,
|
||||
help="Path to the workspace Cargo.toml to inspect.",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--bazelrc",
|
||||
type=Path,
|
||||
default=DEFAULT_BAZELRC,
|
||||
help="Path to the .bazelrc file to inspect.",
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
cargo_toml = args.cargo_toml.resolve()
|
||||
bazelrc = args.bazelrc.resolve()
|
||||
|
||||
cargo_lints = load_workspace_clippy_lints(cargo_toml)
|
||||
bazel_lints = load_bazel_clippy_lints(bazelrc)
|
||||
|
||||
missing = sorted(cargo_lints.keys() - bazel_lints.keys())
|
||||
extra = sorted(bazel_lints.keys() - cargo_lints.keys())
|
||||
mismatched = sorted(
|
||||
lint
|
||||
for lint in cargo_lints.keys() & bazel_lints.keys()
|
||||
if cargo_lints[lint] != bazel_lints[lint]
|
||||
)
|
||||
|
||||
if missing or extra or mismatched:
|
||||
print_sync_error(
|
||||
cargo_toml=cargo_toml,
|
||||
bazelrc=bazelrc,
|
||||
cargo_lints=cargo_lints,
|
||||
bazel_lints=bazel_lints,
|
||||
missing=missing,
|
||||
extra=extra,
|
||||
mismatched=mismatched,
|
||||
)
|
||||
return 1
|
||||
|
||||
print(
|
||||
"Bazel clippy flags in "
|
||||
f"{display_path(bazelrc)} match "
|
||||
f"{display_path(cargo_toml)} [workspace.lints.clippy]."
|
||||
)
|
||||
return 0
|
||||
|
||||
|
||||
def load_workspace_clippy_lints(cargo_toml: Path) -> dict[str, str]:
|
||||
workspace = tomllib.loads(cargo_toml.read_text())["workspace"]
|
||||
clippy_lints = workspace["lints"]["clippy"]
|
||||
parsed: dict[str, str] = {}
|
||||
for lint, level in clippy_lints.items():
|
||||
if not isinstance(level, str):
|
||||
raise SystemExit(
|
||||
f"expected string lint level for clippy::{lint} in {cargo_toml}, got {level!r}"
|
||||
)
|
||||
normalized = level.strip().lower()
|
||||
if normalized not in VALID_LEVELS:
|
||||
raise SystemExit(
|
||||
f"unsupported lint level {level!r} for clippy::{lint} in {cargo_toml}"
|
||||
)
|
||||
parsed[lint] = normalized
|
||||
return parsed
|
||||
|
||||
|
||||
def load_bazel_clippy_lints(bazelrc: Path) -> dict[str, str]:
|
||||
parsed: dict[str, str] = {}
|
||||
line_numbers: dict[str, int] = {}
|
||||
|
||||
for lineno, line in enumerate(bazelrc.read_text().splitlines(), start=1):
|
||||
if not line.startswith(BAZEL_CLIPPY_FLAG_PREFIX):
|
||||
continue
|
||||
|
||||
flag = line.removeprefix(BAZEL_CLIPPY_FLAG_PREFIX).strip()
|
||||
if flag in BAZEL_SPECIAL_FLAGS:
|
||||
continue
|
||||
|
||||
parsed_flag = parse_bazel_lint_flag(flag)
|
||||
if parsed_flag is None:
|
||||
continue
|
||||
|
||||
lint, level = parsed_flag
|
||||
if lint in parsed:
|
||||
raise SystemExit(
|
||||
f"duplicate Bazel clippy entry for clippy::{lint} at "
|
||||
f"{bazelrc}:{line_numbers[lint]} and {bazelrc}:{lineno}"
|
||||
)
|
||||
parsed[lint] = level
|
||||
line_numbers[lint] = lineno
|
||||
|
||||
return parsed
|
||||
|
||||
|
||||
def parse_bazel_lint_flag(flag: str) -> tuple[str, str] | None:
|
||||
long_match = LONG_FLAG_RE.match(flag)
|
||||
if long_match:
|
||||
return long_match["lint"], long_match["level"]
|
||||
|
||||
short_match = SHORT_FLAG_RE.match(flag)
|
||||
if short_match:
|
||||
return short_match["lint"], SHORT_LEVEL_NAMES[short_match["level"]]
|
||||
|
||||
return None
|
||||
|
||||
|
||||
def print_sync_error(
|
||||
*,
|
||||
cargo_toml: Path,
|
||||
bazelrc: Path,
|
||||
cargo_lints: dict[str, str],
|
||||
bazel_lints: dict[str, str],
|
||||
missing: list[str],
|
||||
extra: list[str],
|
||||
mismatched: list[str],
|
||||
) -> None:
|
||||
cargo_toml_display = display_path(cargo_toml)
|
||||
bazelrc_display = display_path(bazelrc)
|
||||
example_manifest = find_workspace_lints_example_manifest()
|
||||
|
||||
print(
|
||||
"ERROR: Bazel clippy flags are out of sync with Cargo workspace clippy lints.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(file=sys.stderr)
|
||||
print(
|
||||
f"Cargo defines the source of truth in {cargo_toml_display} "
|
||||
"[workspace.lints.clippy].",
|
||||
file=sys.stderr,
|
||||
)
|
||||
if example_manifest is not None:
|
||||
print(
|
||||
"Cargo applies those lint levels to member crates that opt into "
|
||||
f"`[lints] workspace = true`, for example {example_manifest}.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
"Bazel clippy does not ingest Cargo lint levels automatically, and "
|
||||
"`clippy.toml` can configure lint behavior but cannot set allow/warn/deny/forbid.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
f"Update {bazelrc_display} so its `build:clippy` "
|
||||
"`clippy_flag` entries match Cargo.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
|
||||
if missing:
|
||||
print(file=sys.stderr)
|
||||
print("Missing Bazel entries:", file=sys.stderr)
|
||||
for lint in missing:
|
||||
print(f" {render_bazelrc_line(lint, cargo_lints[lint])}", file=sys.stderr)
|
||||
|
||||
if mismatched:
|
||||
print(file=sys.stderr)
|
||||
print("Mismatched lint levels:", file=sys.stderr)
|
||||
for lint in mismatched:
|
||||
cargo_level = cargo_lints[lint]
|
||||
bazel_level = bazel_lints[lint]
|
||||
print(
|
||||
f" clippy::{lint}: Cargo has {cargo_level}, Bazel has {bazel_level}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
f" expected: {render_bazelrc_line(lint, cargo_level)}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
|
||||
if extra:
|
||||
print(file=sys.stderr)
|
||||
print("Extra Bazel entries with no Cargo counterpart:", file=sys.stderr)
|
||||
for lint in extra:
|
||||
print(f" {render_bazelrc_line(lint, bazel_lints[lint])}", file=sys.stderr)
|
||||
|
||||
|
||||
def render_bazelrc_line(lint: str, level: str) -> str:
|
||||
return f"{BAZEL_CLIPPY_FLAG_PREFIX}--{level}=clippy::{lint}"
|
||||
|
||||
|
||||
def display_path(path: Path) -> str:
|
||||
try:
|
||||
return str(path.relative_to(ROOT))
|
||||
except ValueError:
|
||||
return str(path)
|
||||
|
||||
|
||||
def find_workspace_lints_example_manifest() -> str | None:
|
||||
for cargo_toml in sorted((ROOT / "codex-rs").glob("**/Cargo.toml")):
|
||||
if cargo_toml == DEFAULT_CARGO_TOML:
|
||||
continue
|
||||
data = tomllib.loads(cargo_toml.read_text())
|
||||
if data.get("lints", {}).get("workspace") is True:
|
||||
return str(cargo_toml.relative_to(ROOT))
|
||||
return None
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user