fix(observed_macros): resolve renamed dependencies and reject mutable-reference fields - #726
Draft
Evgenii (Vaiz) wants to merge 2 commits into
Draft
fix(observed_macros): resolve renamed dependencies and reject mutable-reference fields#726Evgenii (Vaiz) wants to merge 2 commits into
Evgenii (Vaiz) wants to merge 2 commits into
Conversation
…d paths
Both generators emitted a hard-coded `::observed` into every path they
produce. `extern crate self as observed` in the runtime crate only helps
expansions inside that crate, so a consumer that renames its dependency
telemetry = { package = "observed", version = "0.24" }
had no `observed` in scope and failed to compile for both `#[event(...)]`
and `#[derive(Enrichment)]`.
Resolve the runtime crate once through `proc-macro-crate` and thread the
resolved path through event generation, enrichment generation and the
`where`-clause predicates shared by both. The resolver follows the local
pattern in `routerama_build::macro_impl::resolver`: `FoundCrate::Itself`
and an unreadable manifest both keep `::observed`, so the runtime crate's
own expansions and every crate that does not rename the dependency emit
exactly what they emitted before - the existing expansion snapshots are
unchanged.
Add hidden `*_with_runtime_path` entry points so the expansion tests can
drive the generators against a renamed path, and cover the path
derivation itself (rename, dashes, keyword alias, `Itself`, unresolvable)
with unit tests.
Fixes AB#7757619
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`is_reference_type` matched any `syn::Type::Reference` and never looked at `TypeReference::mutability`, so `#[event(...)]` accepted a `&mut T` field and generated a visit body for it. An event is visited through `&self`, so that body cannot hand out the exclusive borrow it asks for, and the input was only rejected later - by the compiler, against generated code the author never wrote. Check mutability during field parsing and report it against the offending field type. `Option<&mut T>` is checked as well, because the visit body dereferences the inner type rather than the field type, and the check recurses through nested references so `&&mut T` is caught too. Shared references are untouched. Fixes AB#7757620 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #726 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 582 583 +1
Lines 62852 62942 +90
=======================================
+ Hits 62852 62942 +90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
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
1 participant
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.
🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.
Two independent hygiene fixes in the
observedprocedural macros, both incrates/observed_macros_impl. One commit each, so they can be reviewed separately.Resolve a renamed
observeddependency in generated paths (AB#7757619)Both generators emitted a hard-coded
::observedinto every path they produce.extern crate self as observedin the runtime crate only helps expansions inside that crate, so a consumer that renames its dependency:had no
observedin scope and failed to compile for both#[event(...)]and#[derive(Enrichment)].The runtime crate is now resolved once through
proc-macro-crateand the resolved path is threaded through event generation, enrichment generation and thewhere-clause predicates the two share. The newresolvermodule follows the local pattern inrouterama_build::macro_impl::resolver, including its handling ofFoundCrate::Itself.Behaviour is unchanged for every crate that does not rename the dependency:
FoundCrate::Itself(the runtime crate expanding its own events, which reaches itself viaextern crate self as observed) and an unreadable manifest both keep::observed. All 32 pre-existing expansion snapshots pass untouched, which is the evidence for that.Coverage: a new expansion snapshot drives both entry points against a renamed path and pins every emitted path — the trait, the descriptors,
Value, the visitor function type, the__privateitems and the spelled-out bounds — so a single hard-coded::observedleft anywhere would show up in it. Unit tests cover the path derivation itself (rename, dash-containing alias, keyword alias needing a raw identifier,Itself, unresolvable). Hidden*_with_runtime_pathentry points exist only so the tests can vary a path the production entry points read from the caller's manifest.Reject mutable-reference event fields (AB#7757620)
is_reference_typematched anysyn::Type::Referenceand never inspectedTypeReference::mutability, so#[event(...)]accepted a&mut Tfield and generated a visit body for it. An event is visited through&self, so that body cannot hand out the exclusive borrow it asks for — the input was rejected only later, by the compiler, against generated code the author never wrote.Mutability is now checked during field parsing and reported against the offending field type, saying that event fields support only shared references.
Option<&mut T>is checked too, because the visit body dereferences the inner type rather than the field type, and the check recurses through nested references so&&mut Tis caught as well. A companion snapshot proves shared references (&'a T,&'a &'a T,Option<&'a T>) still expand exactly as before.Deliberately not fixed here
AB#7757617("Expose fields of signal-less events to name-based processors") lives in the same file —field_reaches_signalinsrc/event.rs— but needs a design decision about what the contract for signal-less fields should be. It is left untouched, and theexpansion__an_event_with_no_signal_at_all_visits_nothingsnapshot is unchanged.Validation
Run for
observed,observed_macros,observed_macros_implandobserved_testing:cargo +nightly-2026-05-30 fmt --checkcargo +1.96.1 clippy --all-targets(-D warnings)cargo +1.96.1 testNo pre-existing snapshot was re-blessed; the only new
.snapfiles are the three added by the new tests.