chore(tvc): lint against inline absolute paths + AGENTS.md guidance#166
Open
daniilrrr wants to merge 1 commit into
Open
chore(tvc): lint against inline absolute paths + AGENTS.md guidance#166daniilrrr wants to merge 1 commit into
daniilrrr wants to merge 1 commit into
Conversation
Push the tvc CLI toward short `use` imports over inline fully-qualified absolute paths at call sites (e.g. prefer `use crate::commands::display::format_egress_enabled;` + `format_egress_enabled(x)` over `crate::commands::display::format_egress_enabled(x)`), as requested in the PR #158 review. Two complementary mechanisms, scoped to tvc only: - Mechanical: enable the `clippy::absolute_paths` restriction lint at warn, crate-locally via `[lints.clippy]` in tvc/Cargo.toml (no workspace plumbing). Tuning lives in a repo-root clippy.toml (absolute-paths-max-segments = 2, absolute-paths-allowed-crates = ["std", "core", "alloc"]). clippy.toml is global by nature but only takes effect where the lint is enabled (tvc). - Guidance: add AGENTS.md with an "Imports" section instructing AI agents and humans to prefer short `use` imports, with a concrete before/after example. Other workspace crates and generated code are intentionally untouched.
5b0718b to
43abeb5
Compare
This was referenced Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
In the review of #158, reviewers asked contributors to prefer a short
useimport over inline fully-qualified absolute paths at call sites — e.g. replacewith
This PR makes that convention easier to uphold, via two complementary mechanisms scoped to the
tvccrate:clippy::absolute_paths), andAGENTS.mdinstruction for AI agents / humans.What changed (3 files)
tvc/Cargo.toml— crate-local lint, no workspace wiring:clippy.toml(repo root) — tuning for the lint:clippy.tomlis global by nature (one per repo), but its settings only take effect for lints that are actually enabled — currently only intvc. So in practice these knobs scope totvctoday.max-segments = 2is Clippy's default; it still flags 3+ segment inline paths such ascrate::commands::app_status::sanitize_app_status(...)(the Add visibility for egress #158 antipattern).allowed-crates = ["std", "core", "alloc"]keeps fully-qualified std-family references (e.g.std::collections::HashMap) from being flagged.AGENTS.md(new) — guidance file (none existed in the repo). See the exact wording below.AGENTS.md instruction (the wording used)
make lint(unchanged finding, now tvc-scoped)The lint is set to
warn, notdeny. However,make lintruns:-D warningspromotes all warnings to hard errors, so undermake lint/CI this lint effectively behaves asdeny— it will FAIL while anytvcabsolute_pathsviolation remains. Verified empirically on this branch:cargo clippy -p tvc --all-targets --all-features -- -D warningsexits101(44 violations promoted to errors in the lib-test build).The Makefile was intentionally not changed. Options to decide before/at merge:
tvcviolations somake lintgoes green. Cleanest end state.tvcviolations first — convert the 47 sites to shortuseimports, then merge; CI stays green immediately.make lint— not done here, to avoid silently weakening-D warnings.The
AGENTS.mdguidance carries zero CI risk regardless of which option is chosen — it's advisory, so it can land immediately even if the lint cleanup is deferred.Blast radius (tvc-focused, pinned toolchain 1.88)
Unique
file:line:colviolation sites fromcargo clippy --all-targets --all-features, deduped across targets, with this PR's config:tvc/src/tvc/tests/Verified that no
absolute_pathswarning appears outsidetvcafter removing the workspace plumbing.For context (workspace-wide, had this been global): the lint would have produced ~1646 sites, of which 1586 are in generated code (
client/src/generated/) and only ~60 non-generated — which is exactly why scoping totvc(and skipping generated entirely) keeps this small and meaningful.max-segmentssensitivity within tvc: atmax-segments = 2(shipped) → 47 sites; bumping to3would shrink it to a handful but would stop flagging the 3-segment #158-style internal-helper calls, so2is the right threshold to actually enforce the convention.Lint vs. AGENTS.md — assessment
clippy::absolute_pathslintAGENTS.mdinstructionmake lint(-D warnings)#[allow]escape hatchesRecommendation: keep both. The lint gives a hard floor inside
tvc;AGENTS.mdgives broader, judgment-friendly guidance (and reaches code/cases the lint can't model) without any CI risk. The lint's one sharp edge is the-D warningsinteraction above — that's the only thing to decide before merge.Verification
cargo metadataparses — config is syntactically valid.cargo clippy --all-targets --all-featuresshows the lint active only in tvc (47 sites; 0 elsewhere), purely fromtvc/Cargo.toml.crate::commands::app_status::sanitize_app_status(...).cargo clippy -p tvc ... -- -D warnings→ exit101(confirms the deny-under-CI behavior).Pre-existing finding (not introduced here)
make lintalready does not pass clean onmainwith the pinned 1.88 toolchain:cargo fmt -- --checkfails on twotvcfiles (tvc/src/client.rs,tvc/src/commands/display.rs) due to edition-2024 formatting drift. Worth a separatecargo fmtcommit; flagged here for transparency. No file changed in this PR introduces a new fmt diff.