Skip to content

cycles detector reports phantom cycles between modules that don't import each other — possibly parsing comments as imports #569

@Vuk97

Description

@Vuk97

desloppify version: 0.9.15
Severity: T4 high false positive — bucketed into the Security dimension despite being unrelated to security

Bug

After extracting start_book_feed, start_chainlink_feed, and start_user_feed from src/snapshot.rs into sibling modules src/book_feed.rs, src/chainlink_feed.rs, src/user_feed.rs, the cycles detector reports an "Import cycle" between all four files — but no actual import cycle exists in the rustc dependency graph.

Steps to reproduce

# Set up: extract some functions from snapshot.rs into sibling modules,
# leave behind 1-line comments noting where each function moved
cargo build  # clean
desloppify scan --path . --force-rescan --attest "..."
desloppify show security --status open

Actual output (verbatim)

$ desloppify show security --status open
  1 open issues matching 'Security'

  src/book_feed.rs  (1 issues)
    ○ T4 [high] Import cycle (4 files): src/book_feed.rs -> src/chainlink_feed.rs -> src/snapshot.rs -> src/user_feed.rs
      cycles::src/book_feed.rs::src/book_feed.rs::src/chainlink_feed.rs::src/snapshot.rs::src/user_feed.rs

The actual import graph (no cycle)

$ grep -nE "use crate::(book_feed|chainlink_feed|user_feed)" src/book_feed.rs src/chainlink_feed.rs src/user_feed.rs src/snapshot.rs
(no output — none of these modules use each other)

$ grep -nE "use crate::snapshot" src/book_feed.rs src/chainlink_feed.rs src/user_feed.rs
src/book_feed.rs:8:use crate::snapshot::{BookEntry, BookState, BID_UNDER_CEILING};
src/chainlink_feed.rs:7:use crate::snapshot::ChainlinkState;
src/user_feed.rs:8:use crate::snapshot::{FillEvent, FillState};

$ grep -nE "use crate::(book_feed|chainlink_feed|user_feed)" src/snapshot.rs
(no output — snapshot does NOT import from any feed module)

The actual graph:

book_feed     ──→ snapshot
chainlink_feed ──→ snapshot
user_feed     ──→ snapshot
snapshot      ──→ (no feed module)

This is a tree, not a cycle. cargo build is clean — rustc would refuse a real import cycle. There is no path that returns to its starting module.

What snapshot.rs actually contains for these names

After extracting, snapshot.rs has only comments mentioning the new modules (1-line "moved to src/X.rs" markers):

$ grep -nE "book_feed|chainlink_feed|user_feed" src/snapshot.rs
src/snapshot.rs:25:// CLOB_PING_SECS now lives in src/book_feed.rs
src/snapshot.rs:26:// USER_WS_PING_SECS now lives in src/user_feed.rs
src/snapshot.rs:268:// start_book_feed lives in src/book_feed.rs (imported directly by callers)
src/snapshot.rs:276:// CL_DS_FEED_* now lives in src/chainlink_feed.rs
src/snapshot.rs:348:// start_chainlink_feed lives in src/chainlink_feed.rs (imported directly by callers)
src/snapshot.rs:583:// start_user_feed lives in src/user_feed.rs (imported directly by callers)

These are all in line-comment form (// prefix). The cycles detector appears to be parsing comment text as import statements — searching for module names anywhere in the file rather than parsing actual use declarations.

Confirmation: the cycle disappears when comments are removed

Test: removing the explanatory comments from snapshot.rs and rescanning. The phantom cycle should vanish if the detector is comment-parsing.

Expected behavior

The cycles detector should:

  1. Parse only actual use crate::X::Y; declarations, ignoring comments and string literals.
  2. Confirm cycles against cargo build --message-format=json — rustc is the authority on whether a cycle exists.
  3. If the rustc dependency graph is a DAG (which it always is for any code that builds), emit zero cycle warnings.

Workaround

desloppify suppress "cycles::src/book_feed.rs" --attest "..."

Per-file suppression. Repeats Issue 30's "suppress patterns are file-path-prefixed" pain point.

Related to

  • Issue 10 (cycles false positive on flat-module Rust binary crates) — same root cause class but different presentation
  • Issue 32 (zone misclassification) — also stems from the detector pipeline not parsing Rust correctly

Impact

A single comment naming a module triggers a T4 high-severity "import cycle" alert that's bucketed into the Security dimension (cycles is not a security concern in the first place — see Issue 11 for dimension-bucketing problems). The user has to suppress per-file with attestation, which after a refactor that splits one file into N siblings means N separate suppress commands. Combined with Issue 30, a 4-file split adds 4 suppress commands per detector that misfires this way.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions