clippy.toml

# ─────────────────────────────────────────────────────────────────────
# LunCoSim — clippy lint configuration
# ─────────────────────────────────────────────────────────────────────
#
# Lives at the workspace root so every member crate inherits it under
# `cargo clippy --workspace`. The lint *severity* is set in `Cargo.toml`
# under `[workspace.lints.clippy]` (currently `disallowed_methods =
# "deny"`); this file only enumerates the banned symbols.
#
# TWO CONFIGS, BY DESIGN. This file is the NATIVE run. The wasm-portability
# bans (`std::fs::*`, `std::thread::spawn`, `std::time::Instant::now`) live in
# `ci/wasm-lint/clippy.toml` and are applied by the `wasm-lint` CI job with
# `--target wasm32-unknown-unknown`.
#
# Why the split — this is the whole reason clippy was unusable here. Those three
# bans exist to catch code that compiles on native and then no-ops or panics in
# the browser. But checking them ON NATIVE is checking them in the one place they
# cannot be true:
#
#   * `std::fs` / `std::thread` — the overwhelming majority of call sites are
#     already inside `#[cfg(not(target_arch = "wasm32"))]`, where using them is
#     CORRECT. Native clippy sees that code anyway and flags it.
#   * `std::time::Instant::now` — worse: `web_time` supplies its native impl via
#     `pub use std::time::*`, so on native `web_time::Instant` IS
#     `std::time::Instant` (same DefId). Clippy resolves through the re-export
#     and flags every *correct* `web_time` caller. That was 73 hits in
#     lunco-modelica alone, all false, none true.
#
# On the wasm target all three become exact: `cfg` strips the native-only code
# before clippy sees it, and `web_time::Instant` is a distinct type. The ban then
# fires only on code that will actually break in a browser — which is how
# `lunco-modelica/src/indexer.rs` was caught importing `std::time::Instant` into
# an unconditional `pub mod` that ships to the web.
#
# A lint that is wrong every time it fires is a lint people silence, and a
# silenced lint is how this hole stayed open. So: enforce each ban where it is
# true, and nowhere else.
#
# What remains BELOW is banned on native because it is genuinely wrong on native:
# the big_space re-parenting atomicity contract and deferred-entity writes.
#
# Local overrides (`#[allow(clippy::disallowed_methods)]`) still apply and still
# require review — but they should now be rare, and each one should say why.

disallowed-methods = [
    # ── Filesystem I/O and Threading — NOT BANNED HERE ────────────────
    # `std::fs::*` fails and `std::thread::spawn` panics on wasm32. Both are
    # real foot-guns, but banning them on the NATIVE target is the wrong place
    # to catch them: most call sites are already inside
    # `#[cfg(not(target_arch = "wasm32"))]` blocks, where using `std::fs` is
    # not just allowed but CORRECT — and clippy sees them anyway on native, so
    # the ban fires on code that can never reach a browser.
    #
    # They are enforced instead by the `wasm-lint` CI job
    # (`.github/workflows/lint.yml`, config `ci/wasm-lint/clippy.toml`), which
    # runs clippy with `--target wasm32-unknown-unknown`. There, `cfg` strips
    # the native-only code before clippy ever sees it, so the ban fires ONLY on
    # code that genuinely ships to the browser. Same rule, no false positives,
    # and no `#[allow]` sprinkled through correct native code.
    #
    # Everything BELOW this line is a ban that is real on native too, which is
    # why it lives here.

    # ── Time ─────────────────────────────────────────────────────────
    # `std::time::Instant::now()` panics on wasm32-unknown-unknown;
    # `web_time::Instant` is the drop-in (`performance.now()` on wasm,
    # `std::time::Instant` on native).
    #
    # DELIBERATELY NOT BANNED HERE — it is unenforceable on the native
    # target. `web_time` provides its native impl via `pub use std::time::*`,
    # so on native `web_time::Instant` IS `std::time::Instant` (same DefId)
    # and clippy resolves straight through the re-export. Listing it here
    # produced **73 hits in lunco-modelica alone, every one of them a false
    # positive on code that already does the right thing**, and zero true
    # positives. A lint that is wrong every single time is a lint people
    # silence — which is exactly how it went unnoticed that `indexer.rs` was
    # importing `std::time::Instant` into a wasm-shipped module.
    #
    # It IS enforced, precisely, by the `wasm-lint` job in
    # `.github/workflows/lint.yml`, which runs clippy with
    # `--target wasm32-unknown-unknown`. There the two types are genuinely
    # distinct, so the ban fires on real bugs only. See that job for the
    # `disallowed-methods` entry it applies.

    # ── Atomic re-parenting of big_space `GridAnchor` entities ─────────
    # Splitting `(ChildOf, CellCoord, Transform)` writes across separate
    # commands races against synchronous observers (this is the bug class
    # that silently marked rover chassis as `RigidBody::Static`).
    # Re-parenting `GridAnchor` entities must go through
    # `lunco_core::attach::migrate_to_grid` — see
    # `crates/lunco-core/src/attach.rs`. One-time scene-setup callsites in
    # `lunco-celestial::big_space_setup` and similar bootstrap code may
    # `#[allow(clippy::disallowed_methods)]` locally — they run before any
    # observer is registered.
    { path = "bevy_ecs::system::EntityCommands::add_child", reason = "Re-parenting a GridAnchor must use lunco_core::attach::migrate_to_grid for atomic (ChildOf, CellCoord, Transform) writes. Local #[allow] for non-spatial parenting (visuals, ports, wires)." },
    { path = "bevy_ecs::world::EntityWorldMut::add_child", reason = "See EntityCommands::add_child — same atomicity contract. Direct-World mode flushes synchronously, so a local #[allow] is fine for bootstrap code." },
    { path = "bevy_transform::commands::BuildChildrenTransformExt::set_parent_in_place", reason = "Use lunco_core::attach::migrate_to_grid. Local #[allow] for one-time scene construction in big_space_setup." },

    # ── Deferred writes to an entity that may already be gone ─────────
    # `Commands` are DEFERRED: they apply at a later sync point, and by then
    # another system may have despawned the entity. `insert`/`despawn` PANIC in
    # that window ("Entity despawned … its index now has generation 1");
    # `try_insert`/`try_despawn` no-op, which is the correct outcome — the entity
    # is genuinely gone, so there is nothing to render and nothing to lose.
    #
    # This is not hypothetical. It crashed the app: `rebind_changed_pbr_look`
    # queued `insert(MeshMaterial3d(..))` for a `Changed<PbrLook>` entity, and the
    # USD projector despawned + rebuilt that entity's subtree in the same frame
    # (a live edit re-instantiates the edited prim). The projector now runs a
    # schedule earlier, which closes THAT race — but `ClearScene`, scene reload
    # and the preview viewport still despawn inside `Update`, so the window is
    # real for any system holding a queued write.
    #
    # The convention already existed and was applied unevenly: `lunco-usd-avian`
    # is 100% `try_insert`, while `lunco-usd-sim` had ~25 plain `insert`s on the
    # SAME entities. An unenforced convention is one that drifts, so it is a lint
    # now — the compiler is the only reviewer that never forgets.
    #
    # NOT banned: `remove` (already a no-op on a despawned entity) and
    # `EntityWorldMut::*` (direct-World mode is synchronous — the entity provably
    # exists). Local `#[allow]` is fine where the entity was spawned by the SAME
    # command buffer and cannot have been despawned yet; say so in the comment.
    { path = "bevy_ecs::system::EntityCommands::insert", reason = "Use `try_insert`: a deferred insert can apply after another system despawned the entity (live USD re-instantiation, ClearScene, scene reload) and PANICS. `remove` is already despawn-safe; `insert` is not." },
    { path = "bevy_ecs::system::EntityCommands::despawn", reason = "Use `try_despawn`: the entity may already be gone by the time the command applies." },
]