fix: macro-token blindness across analyzers + configurable prelude-gl…#35
Merged
Conversation
…ob exemption (v1.5.1)
syn's visitor treats a macro body as an opaque TokenStream, so calls, `self`
uses, and component renders inside vec![…], format!(…), and DSL macros like
dioxus rsx!{…} were invisible to every AST-walking analyzer — producing false
DEAD_CODE, TQ_NO_SUT, TQ_UNTESTED and SRP-cohesion findings, and missed
forbidden calls in the architecture matchers. Seven sites had each reinvented a
weak Punctuated<Expr, Comma> parse blind to the `;`-repeat and block-bodied
forms.
- Add adapters/shared/macro_tokens.rs as the single macro-body recovery point:
recover_exprs (comma-list → braced-block → single-expr; lifts a trailing
Stmt::Macro back to Expr::Macro) plus the raw fallbacks
idents_in_call_position / tokens_reference_ident.
- Route all 7 call-graph/cohesion sites + call_parity through it. Reachability
consumers additionally harvest call/construction-position idents (a call
f(..) of any case, or an UpperCamelCase render Component {..}; lowercase
element tags like div {..} and prop keys are excluded so an unused fn div()
isn't masked), so rsx! component renders count as references — clearing false
DEAD_CODE / NO_SUT on component-heavy UI crates. Safe direction: an extra
recovered reference only ever suppresses a finding, never raises a false one.
- SLM: detect macro-embedded `self` via tokens_reference_ident (was
matches!-only), so format!("{}", self.x) no longer false-fires.
- IOSP own-call counting stays deliberately macro-blind (characterization
test) — descending would newly flag macro-driven render code as Violations.
- Add [[architecture.pattern]] allow_prelude_glob (default true):
forbid_glob_import stays a "find all globs" matcher; the *::prelude::*
exemption is a policy decision at the analyzer layer, so projects can opt out
with allow_prelude_glob = false.
Each of the 9 macro-blind sites + the prelude policy has its own regression
test. Self-analysis: 0 findings; cargo fmt / clippy -Dwarnings / nextest (1977
tests) all green.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a cross-cutting correctness gap where syn::visit treats macro bodies as opaque token streams, causing multiple analyzers (dead code, TQ, SRP cohesion, architecture matchers, SLM) to miss calls / idents / self usage embedded inside macros. It centralizes macro-token recovery into a single shared helper and adds a new architecture policy knob to optionally exempt *::prelude::* glob imports.
Changes:
- Introduce
adapters/shared/macro_tokens.rsand route macro-aware analyzers/matchers throughrecover_exprs(+ reachability-onlyidents_in_call_position, SLM-onlytokens_reference_ident). - Add
[[architecture.pattern]] allow_prelude_glob(defaulttrue) and enforce it at the analyzer layer while keeping the glob matcher “dumb”. - Add targeted regression tests across affected analyzers and bump version/docs/changelog to
1.5.1.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/adapters/shared/tests/mod.rs | Registers the new macro_tokens shared test module. |
| src/adapters/shared/tests/macro_tokens.rs | Unit tests for macro-token recovery helpers and edge cases (; repeat, DSL, self scan). |
| src/adapters/shared/test_references.rs | Uses shared macro-token recovery when collecting SUT references inside test macros. |
| src/adapters/shared/mod.rs | Exposes the new macro_tokens shared module. |
| src/adapters/shared/macro_tokens.rs | New shared implementation for macro token recovery (recover_exprs, idents_in_call_position, tokens_reference_ident). |
| src/adapters/config/tests/architecture.rs | Tests defaulting + opt-out behavior for allow_prelude_glob; pins default_true. |
| src/adapters/config/architecture.rs | Adds allow_prelude_glob to SymbolPattern with serde default helper default_true. |
| src/adapters/analyzers/tq/tests/untested.rs | Regression test ensuring call-graph edges are recovered from vec![f(); n] repeat macros. |
| src/adapters/analyzers/tq/tests/sut.rs | Regression tests for NO_SUT suppression via macro-embedded calls and DSL component renders. |
| src/adapters/analyzers/tq/tests/coverage.rs | Regression test for cfg-excluded functions absent from LCOV not being flagged as uncovered/untested logic. |
| src/adapters/analyzers/tq/mod.rs | Full call-graph collector now descends into macro tokens via shared recovery + positional ident harvest. |
| src/adapters/analyzers/structural/tests/slm.rs | Regression test: self referenced only inside macro args should not trigger SLM. |
| src/adapters/analyzers/structural/slm.rs | SLM macro handling switched to a raw token scan for self (tokens_reference_ident). |
| src/adapters/analyzers/srp/tests/cohesion/lcom4_and_collector.rs | Regression test for SRP cohesion seeing self.method() inside repeat-form macros. |
| src/adapters/analyzers/srp/mod.rs | SRP method-body macro traversal switched to shared recover_exprs. |
| src/adapters/analyzers/iosp/tests/root/own_calls_b.rs | Characterization test pinning IOSP’s deliberate macro-blindness for own-call counting. |
| src/adapters/analyzers/dry/tests/wildcards.rs | Adds tests ensuring external-crate *::prelude::* globs are excluded by default wildcard detection. |
| src/adapters/analyzers/dry/tests/dead_code.rs | Regression tests ensuring macro-embedded calls and DSL component renders prevent false DEAD_CODE. |
| src/adapters/analyzers/dry/call_targets.rs | DRY call-target collector uses shared macro-token recovery + positional ident harvest. |
| src/adapters/analyzers/architecture/tests/rendering.rs | Updates SymbolPattern construction to include allow_prelude_glob. |
| src/adapters/analyzers/architecture/tests/analyzer.rs | Tests config-gated prelude-glob exemption policy at analyzer level. |
| src/adapters/analyzers/architecture/matcher/tests/method_call.rs | Regression test for method matcher inside repeat-form macro bodies. |
| src/adapters/analyzers/architecture/matcher/tests/macro_call.rs | Regression test for nested macro detection inside repeat-form macro bodies. |
| src/adapters/analyzers/architecture/matcher/tests/glob_import.rs | Verifies matcher reports prelude globs (policy handled elsewhere). |
| src/adapters/analyzers/architecture/matcher/tests/function_call.rs | Regression test for function-call matcher inside repeat-form macros. |
| src/adapters/analyzers/architecture/matcher/method_call.rs | Uses shared recover_exprs to descend into macro tokens (structured-only). |
| src/adapters/analyzers/architecture/matcher/macro_call.rs | Uses shared recover_exprs to descend into macro tokens (structured-only). |
| src/adapters/analyzers/architecture/matcher/glob_import.rs | Clarifies matcher reports all globs; policy decides exemptions. |
| src/adapters/analyzers/architecture/matcher/function_call.rs | Uses shared recover_exprs to descend into macro tokens (structured-only). |
| src/adapters/analyzers/architecture/call_parity_rule/calls/mod.rs | Deduplicates macro-token parsing by delegating to shared recover_exprs. |
| src/adapters/analyzers/architecture/analyzer.rs | Implements allow_prelude_glob policy filter and makes run_pattern_matchers visible for tests. |
| CHANGELOG.md | Documents the macro-token blindness fix set and the new allow_prelude_glob option for v1.5.1. |
| Cargo.toml | Bumps crate version to 1.5.1. |
| Cargo.lock | Updates workspace package version to 1.5.1. |
| book/reference-configuration.md | Documents allow_prelude_glob in config reference table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
recover_exprs' block fallback dropped the `else { … }` branch of a let-else
statement — `stmt_expr` returned only the bound initialiser — so a call
reachable only through the diverging branch of a block-bodied macro body was
missed. `stmt_exprs` now yields both the initialiser and the diverging else
block.
Narrow gap (block-bodied macro + let-else + a call only in the else), and
already mitigated for the reachability consumers by their positional ident
harvest, but this closes the structured-recovery path for the forbid_* matchers
and SRP cohesion too. Pinned by a regression test.
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.
…ob exemption (v1.5.1)
syn's visitor treats a macro body as an opaque TokenStream, so calls,
selfuses, and component renders inside vec![…], format!(…), and DSL macros like dioxus rsx!{…} were invisible to every AST-walking analyzer — producing false DEAD_CODE, TQ_NO_SUT, TQ_UNTESTED and SRP-cohesion findings, and missed forbidden calls in the architecture matchers. Seven sites had each reinvented a weak Punctuated<Expr, Comma> parse blind to the;-repeat and block-bodied forms.selfvia tokens_reference_ident (was matches!-only), so format!("{}", self.x) no longer false-fires.Each of the 9 macro-blind sites + the prelude policy has its own regression test. Self-analysis: 0 findings; cargo fmt / clippy -Dwarnings / nextest (1977 tests) all green.