mirror of
https://github.com/openai/codex.git
synced 2026-04-30 09:26:44 +00:00
fix: address coverage gap in argument-comment-lint
This commit is contained in:
@@ -129,7 +129,8 @@ Run the lint against `codex-rs` from the repo root:
|
||||
|
||||
```bash
|
||||
just argument-comment-lint
|
||||
bazel build --config=argument-comment-lint -- //codex-rs/...
|
||||
bazel build --config=argument-comment-lint -- \
|
||||
$(./tools/argument-comment-lint/list-bazel-targets.sh)
|
||||
./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core
|
||||
just argument-comment-lint -p codex-core
|
||||
```
|
||||
@@ -138,10 +139,13 @@ If no package selection is provided, `just argument-comment-lint` now defaults
|
||||
to the Bazel aspect path over `//codex-rs/...`. The Python wrappers remain the
|
||||
package-scoped escape hatch and still default the underlying Cargo invocation
|
||||
to `--all-targets` unless you explicitly narrow the target set, so targeted
|
||||
wrapper runs cover test-only call sites by default.
|
||||
wrapper runs cover test-only call sites by default. The Bazel entrypoints use
|
||||
`tools/argument-comment-lint/list-bazel-targets.sh` to add the internal
|
||||
manual `*-unit-tests-bin` Rust targets explicitly, so inline `#[cfg(test)]`
|
||||
call sites are covered without pulling in unrelated manual release targets.
|
||||
|
||||
Repo runs also promote `uncommented_anonymous_literal_argument` to an error by
|
||||
default:
|
||||
Repo runs also promote `argument_comment_mismatch` and
|
||||
`uncommented_anonymous_literal_argument` to errors by default:
|
||||
|
||||
```bash
|
||||
./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core
|
||||
@@ -154,7 +158,7 @@ rustc incremental compilation ICE locally. To override that behavior for an ad
|
||||
hoc run:
|
||||
|
||||
```bash
|
||||
DYLINT_RUSTFLAGS="-A uncommented-anonymous-literal-argument" \
|
||||
DYLINT_RUSTFLAGS="-A argument-comment-mismatch -A uncommented-anonymous-literal-argument" \
|
||||
CARGO_INCREMENTAL=1 \
|
||||
./tools/argument-comment-lint/run.py -p codex-core
|
||||
```
|
||||
|
||||
@@ -16,6 +16,7 @@ load(
|
||||
)
|
||||
|
||||
_STRICT_LINT_FLAGS = [
|
||||
"-Dargument-comment-mismatch",
|
||||
"-Duncommented-anonymous-literal-argument",
|
||||
"-Aunknown-lints",
|
||||
]
|
||||
|
||||
13
tools/argument-comment-lint/list-bazel-targets.sh
Executable file
13
tools/argument-comment-lint/list-bazel-targets.sh
Executable file
@@ -0,0 +1,13 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
||||
cd "${repo_root}"
|
||||
|
||||
# Bazel wildcard builds skip manual targets, which misses the internal
|
||||
# `*-unit-tests-bin` rust_test targets generated by `codex_rust_crate()`.
|
||||
# Add only those manual rust_test targets explicitly so inline `#[cfg(test)]`
|
||||
# call sites are linted without pulling in unrelated manual release targets.
|
||||
printf '%s\n' "//codex-rs/..."
|
||||
bazel query 'kind("rust_test rule", attr(tags, "manual", //codex-rs/...))'
|
||||
@@ -8,6 +8,11 @@ use std::process::ExitCode;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
const STRICT_LINTS: [&str; 2] = [
|
||||
"argument-comment-mismatch",
|
||||
"uncommented-anonymous-literal-argument",
|
||||
];
|
||||
|
||||
fn main() -> ExitCode {
|
||||
match run() {
|
||||
Ok(code) => code,
|
||||
@@ -85,14 +90,13 @@ fn has_library_selection(args: &[OsString]) -> bool {
|
||||
fn set_default_env(command: &mut Command) -> Result<(), String> {
|
||||
if let Some(flags) = env::var_os("DYLINT_RUSTFLAGS") {
|
||||
let mut flags = flags.to_string_lossy().to_string();
|
||||
append_flag_if_missing(&mut flags, "-D uncommented-anonymous-literal-argument");
|
||||
for strict_lint in STRICT_LINTS {
|
||||
append_flag_if_missing(&mut flags, &format!("-D {strict_lint}"));
|
||||
}
|
||||
append_flag_if_missing(&mut flags, "-A unknown_lints");
|
||||
command.env("DYLINT_RUSTFLAGS", flags);
|
||||
} else {
|
||||
command.env(
|
||||
"DYLINT_RUSTFLAGS",
|
||||
"-D uncommented-anonymous-literal-argument -A unknown_lints",
|
||||
);
|
||||
command.env("DYLINT_RUSTFLAGS", strict_rustflags());
|
||||
}
|
||||
|
||||
if env::var_os("CARGO_INCREMENTAL").is_none() {
|
||||
@@ -108,6 +112,15 @@ fn set_default_env(command: &mut Command) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn strict_rustflags() -> String {
|
||||
let strict_flags = STRICT_LINTS
|
||||
.iter()
|
||||
.map(|lint| format!("-D {lint}"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
format!("{strict_flags} -A unknown_lints")
|
||||
}
|
||||
|
||||
fn append_flag_if_missing(flags: &mut String, flag: &str) {
|
||||
if flags.contains(flag) {
|
||||
return;
|
||||
@@ -253,6 +266,7 @@ fn exit_code_from_status(code: Option<i32>) -> ExitCode {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::normalize_nightly_library_filename;
|
||||
use super::strict_rustflags;
|
||||
use std::path::Path;
|
||||
|
||||
#[test]
|
||||
@@ -276,4 +290,12 @@ mod tests {
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_rustflags_promotes_both_enforced_lints() {
|
||||
assert_eq!(
|
||||
strict_rustflags(),
|
||||
"-D argument-comment-mismatch -D uncommented-anonymous-literal-argument -A unknown_lints"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -103,6 +103,19 @@ class WrapperCommonTest(unittest.TestCase):
|
||||
],
|
||||
)
|
||||
|
||||
def test_default_lint_env_promotes_both_strict_lints(self) -> None:
|
||||
env: dict[str, str] = {}
|
||||
|
||||
wrapper_common.set_default_lint_env(env)
|
||||
|
||||
self.assertEqual(
|
||||
env["DYLINT_RUSTFLAGS"],
|
||||
"-D argument-comment-mismatch "
|
||||
"-D uncommented-anonymous-literal-argument "
|
||||
"-A unknown_lints",
|
||||
)
|
||||
self.assertEqual(env["CARGO_INCREMENTAL"], "0")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -13,7 +13,10 @@ import sys
|
||||
import tempfile
|
||||
from typing import MutableMapping, Sequence
|
||||
|
||||
STRICT_LINT = "uncommented-anonymous-literal-argument"
|
||||
STRICT_LINTS = [
|
||||
"argument-comment-mismatch",
|
||||
"uncommented-anonymous-literal-argument",
|
||||
]
|
||||
NOISE_LINT = "unknown_lints"
|
||||
TOOLCHAIN_CHANNEL = "nightly-2025-09-18"
|
||||
|
||||
@@ -129,7 +132,8 @@ def append_env_flag(env: MutableMapping[str, str], key: str, flag: str) -> None:
|
||||
|
||||
|
||||
def set_default_lint_env(env: MutableMapping[str, str]) -> None:
|
||||
append_env_flag(env, "DYLINT_RUSTFLAGS", f"-D {STRICT_LINT}")
|
||||
for strict_lint in STRICT_LINTS:
|
||||
append_env_flag(env, "DYLINT_RUSTFLAGS", f"-D {strict_lint}")
|
||||
append_env_flag(env, "DYLINT_RUSTFLAGS", f"-A {NOISE_LINT}")
|
||||
if not env.get("CARGO_INCREMENTAL"):
|
||||
env["CARGO_INCREMENTAL"] = "0"
|
||||
|
||||
Reference in New Issue
Block a user