codex: address PR review feedback (#20179)

This commit is contained in:
Eric Traut
2026-04-29 02:18:16 -07:00
parent 8b1505b299
commit bb1f2cacec

View File

@@ -14,6 +14,9 @@ ROOT = Path(__file__).resolve().parents[2]
TUI_ROOT = ROOT / "codex-rs" / "tui"
TUI_MANIFEST = TUI_ROOT / "Cargo.toml"
FORBIDDEN_PACKAGE = "codex-core"
CODEX_PROTOCOL_PACKAGE = "codex-protocol"
CODEX_PROTOCOL_MESSAGE = "references `codex_protocol::protocol`"
IDENTIFIER = r"[A-Za-z_][A-Za-z0-9_]*"
FORBIDDEN_SOURCE_RULES = (
(
"imports `codex_core`",
@@ -23,20 +26,15 @@ FORBIDDEN_SOURCE_RULES = (
re.compile(r"\bextern\s+crate\s+codex_core\b"),
),
),
(
"references `codex_protocol::protocol`",
(
re.compile(r"\bcodex_protocol\s*::\s*protocol\b"),
re.compile(r"\bcodex_protocol\s*::\s*\{[^}]*\bprotocol\b"),
),
),
)
CODEX_PROTOCOL_ALIAS_PATTERNS = (
re.compile(r"\buse\s+codex_protocol\s+as\s+([A-Za-z_][A-Za-z0-9_]*)\s*;"),
re.compile(
r"\bextern\s+crate\s+codex_protocol\s+as\s+([A-Za-z_][A-Za-z0-9_]*)\s*;"
),
CRATE_ALIAS_PATTERNS = (
re.compile(rf"\buse\s+({IDENTIFIER})\s+as\s+({IDENTIFIER})\s*;"),
re.compile(rf"\bextern\s+crate\s+({IDENTIFIER})\s+as\s+({IDENTIFIER})\s*;"),
)
GROUPED_USE_PATTERN = re.compile(
rf"\buse\s+({IDENTIFIER})\s*::\s*\{{([^;]*)\}}\s*;"
)
GROUPED_SELF_ALIAS_PATTERN = re.compile(rf"\bself\s+as\s+({IDENTIFIER})\b")
def main() -> int:
@@ -92,6 +90,8 @@ def dependency_sections(manifest: dict) -> list[tuple[str, dict]]:
def source_failures() -> list[str]:
failures = []
manifest = tomllib.loads(TUI_MANIFEST.read_text())
codex_protocol_names = protocol_dependency_names(manifest)
for path in sorted(TUI_ROOT.glob("**/*.rs")):
text = path.read_text()
seen_locations = set()
@@ -101,33 +101,70 @@ def source_failures() -> list[str]:
failures.append(source_failure(path, text, match.start(), message))
seen_locations.add((match.start(), message))
for alias_match in alias_matches(text):
alias = re.escape(alias_match.group(1))
patterns = (
re.compile(rf"\b{alias}\s*::\s*protocol\b"),
re.compile(rf"\b{alias}\s*::\s*\{{[^}}]*\bprotocol\b"),
)
for pattern in patterns:
for match in pattern.finditer(text):
key = (match.start(), "references `codex_protocol::protocol`")
if key in seen_locations:
continue
failures.append(
source_failure(
path, text, match.start(), "references `codex_protocol::protocol`"
)
)
seen_locations.add(key)
for match in protocol_reference_matches(text, codex_protocol_names):
key = (match.start(), CODEX_PROTOCOL_MESSAGE)
if key in seen_locations:
continue
failures.append(source_failure(path, text, match.start(), CODEX_PROTOCOL_MESSAGE))
seen_locations.add(key)
return failures
def alias_matches(text: str) -> list[re.Match[str]]:
def protocol_dependency_names(manifest: dict) -> set[str]:
names = {"codex_protocol"}
for _section_name, dependencies in dependency_sections(manifest):
for dependency_name, dependency_value in dependencies.items():
package_name = dependency_name
if isinstance(dependency_value, dict):
package_name = dependency_value.get("package", dependency_name)
if package_name == CODEX_PROTOCOL_PACKAGE:
names.add(rust_crate_name(dependency_name))
return names
def rust_crate_name(package_or_dependency_name: str) -> str:
return package_or_dependency_name.replace("-", "_")
def protocol_reference_matches(
text: str, codex_protocol_names: set[str]
) -> list[re.Match[str]]:
matches = []
for pattern in CODEX_PROTOCOL_ALIAS_PATTERNS:
matches.extend(pattern.finditer(text))
for crate_name in expanded_crate_aliases(text, codex_protocol_names):
escaped_name = re.escape(crate_name)
patterns = (
re.compile(rf"\b{escaped_name}\s*::\s*protocol\b"),
re.compile(rf"\b{escaped_name}\s*::\s*\{{[^;]*\bprotocol\b"),
)
for pattern in patterns:
matches.extend(pattern.finditer(text))
return matches
def expanded_crate_aliases(text: str, crate_names: set[str]) -> set[str]:
aliases = set(crate_names)
while True:
previous_count = len(aliases)
for source, alias in crate_alias_pairs(text):
if source in aliases:
aliases.add(alias)
if len(aliases) == previous_count:
return aliases
def crate_alias_pairs(text: str) -> list[tuple[str, str]]:
pairs = []
for pattern in CRATE_ALIAS_PATTERNS:
for match in pattern.finditer(text):
pairs.append((match.group(1), match.group(2)))
for match in GROUPED_USE_PATTERN.finditer(text):
source = match.group(1)
body = match.group(2)
for alias_match in GROUPED_SELF_ALIAS_PATTERN.finditer(body):
pairs.append((source, alias_match.group(1)))
return pairs
def source_failure(path: Path, text: str, offset: int, message: str) -> str:
line_number = text.count("\n", 0, offset) + 1
return f"{relative_path(path)}:{line_number} {message}"