AI Updates #4: Fix spans for IDEs, more permissive #[uses], and improve modern idioms

AGENTS.md

@@ -175,74 +175,89 @@ readability edit introduce a behavioral change.
 
 ### Scrutinize the macro codegen
 
-A CGP macro is only as correct as the code it emits, so review the implementation against the ways
-its input can be parsed and its output expanded, not just the happy path its tests exercise. Cover
-every one of these concerns, since a gap in any of them is a latent miscompilation waiting for the
-right input:
-
-- **Review every supported attribute.** Enumerate the attributes the macro accepts and confirm each
-  is parsed, validated, mutually constrained, and rejected-when-unknown exactly as documented — an
-  unrecognized attribute should fail with a spanned error, a mutually exclusive pair should error
-  when both appear, and a duplicate should not be silently accepted (or silently accepted for one
-  attribute while rejected for another).
-- **Prefer `parse_internal!`/`parse_internal` over `parse2` or `parse_quote!`.** When constructing a
-  `syn` node from quasi-quoted tokens, build it with `parse_internal!` so a malformed fragment fails
-  with an error naming the target type and the offending tokens (prelude prefix stripped) rather than
-  a bare parse error. Reserve `parse2` for re-parsing tokens already known to be valid (a span
-  override, say), and treat every `parse_quote!` as an assertion that parsing can never fail.
-- **Return `syn::Result` wherever parsing can fail.** A function that parses anything should thread
-  `syn::Result` and propagate the error, rather than `parse_quote!`-ing and risking a panic that
-  aborts the compiler with no usable diagnostic. Use the panicking `parse_quote!` only when it is
-  trivially obvious — from the surrounding, fully-controlled tokens — that the parse cannot fail.
-- **Enumerate every way the input can be parsed.** For each parser and `parse_internal!` call, think
-  through the full range of inputs a user could write — path-qualified types, generic and lifetime
-  parameters, arrays, tuples, empty lists, turbofish, associated-type bindings — and confirm none
-  reaches a parser that fails with a confusing internal error. Reject malformed or unsupported input
-  early, at the macro's own parse stage with a clear spanned message, rather than letting the failure
-  surface deep inside internal fragment parsing or in the expanded code.
-- **Enumerate every way the output can expand.** Walk the shapes the expansion can take across the
-  whole input space and confirm none can produce invalid Rust — no duplicate or conflicting `impl`
-  blocks from a cartesian expansion, no unbound or doubly-declared generic parameter, no empty
-  expansion that silently checks nothing, and no clash on a generated identifier. When you find a
-  case whose expansion fails to compile, capture it as a `trybuild` fixture in
-  `cgp-compile-fail-tests` — under `problematic/` when the macro should have rejected it or emitted
-  wrong code, or `acceptable/` when the failure is one CGP deliberately defers to the compiler — and
-  document it in the macro's implementation document, per
-  [crates/tests/AGENTS.md](crates/tests/AGENTS.md) and
-  [docs/implementation/AGENTS.md](docs/implementation/AGENTS.md).
-- **Scrutinize generics with care.** Generic parameters take many forms — lifetimes, types, consts,
-  and the distinction between *impl* generics (`impl<T>`) and *type* generics (the `<T>` in
-  `Foo<T>`) — and mixing them produces subtly wrong output. Confirm the macro keeps the kinds
-  separate, renders each in the right position, merges parameters from different sources without
-  colliding, and binds every parameter that appears in the generated header so nothing is left free.
-- **Fully qualify every CGP construct in the expansion.** Any CGP item the expansion references must
-  be emitted through the `crate::exports` markers so it resolves as `::cgp::macro_prelude::<Name>`,
-  never as a bare or hand-written path — this is what lets a user with only `cgp` in scope compile
-  the output. Grep the codegen for any CGP name that is not interpolated from an `exports` marker.
-- **Aim every generated item's span at the token the user wrote (watch for `call_site` leaks).** A
-  macro builds its output with `parse_internal!`/`quote!`, which stamp the structural tokens — the
-  `impl` keyword, the trait reference, the self type — with the macro's `call_site` span (the whole
-  invocation), while only the interpolated user fragments keep a narrower span. A compiler error that
-  reports on an item's header then underlines the *entire macro block* instead of the entry,
-  attribute, or impl the user actually wrote: a coherence conflict (`E0119`) between two generated
-  impls, an unsatisfied bound, or a name-resolution failure all read as "somewhere in this macro."
-  Re-span each generated item onto its originating token, following the
-  `delegate_components!`/`check_components!` pattern — the shared
-  [`override_span`](crates/macros/cgp-macro-core/src/functions/override_span.rs) helper re-spans an
-  item's tokens (restore the generics afterward so a per-entry generic's `E0207` keeps pointing at the
-  `<T>` the user wrote), and where the originating token is *synthesized* and has lost its span (a
-  `PathCons<..>` nest, a `Symbol`'s `Chars` encoding), carry an explicit span field through the
-  evaluated form as `EvaluatedCheckEntry.span` and `EvaluatedDelegateEntry.span` do — mirroring
-  `Symbol`, which keeps its parse-time span and stamps its output with `quote_spanned!`. The same leak
-  lurks in the provider macros (`#[cgp_impl]`, `#[cgp_provider]`, `#[cgp_new_provider]`) and every
-  other expansion, so confirm a duplicated or conflicting generated item points at the impl or
-  attribute the user wrote, not the macro name. These spans are testable: a `trybuild` `.stderr`
-  fixture records the exact line and column of each caret, so a span regression changes the snapshot
-  (see the `acceptable/delegate_components/duplicate_*` fixtures).
-
-Beyond these, weigh the concerns that recur across the macro suite: the hygiene of the reserved
-identifiers the expansion introduces (`__Component__`, `__Context__`, and the like) and the
-idempotency of the expansion when the same entry is listed more than once.
+A CGP macro is only as correct as the code it emits, so review it against the full space of inputs
+it can parse and outputs it can expand, not the happy path its tests exercise. A gap in any area
+below is a latent miscompilation — or a broken editor experience — waiting for the right input.
+**Apply every area to every macro you review**, not only the ones that already have a fixture or a
+worked example for it; these concerns recur across the whole suite, and a macro that has never been
+audited for one of them (spans especially) most likely has the bug. The areas below are the *what to
+check*; the [cross-cutting implementation notes](docs/implementation/README.md#cross-cutting-implementation-notes)
+explain *why the code behaves this way*, and most areas link out to the note or process doc that
+carries their full detail.
+
+#### Attributes
+
+Enumerate the attributes the macro accepts and confirm each is parsed, validated, mutually
+constrained, and rejected when unknown: a bad attribute should fail with a spanned error, a mutually
+exclusive pair should error when both appear, and a duplicate should not be silently accepted (nor
+accepted for one attribute while rejected for another).
+
+#### Parsing the input
+
+Build every `syn` node from quasi-quoted tokens with
+[`parse_internal!`](docs/implementation/macros/parse_internal.md) rather than `parse2`/`parse_quote!`,
+and thread `syn::Result` so a malformed fragment propagates a named error instead of panicking;
+reserve the panicking `parse_quote!` for tokens trivially guaranteed to parse and `parse2` for
+re-parsing tokens already known valid. Reject malformed or unsupported input early, at the macro's
+own parse stage with a spanned message, rather than letting it surface deep inside fragment parsing.
+Walk the full input space — path-qualified types, generics, lifetimes, arrays, tuples, turbofish,
+associated-type bindings — and confirm none reaches a parser that fails obscurely; since `syn` parses
+far more leniently than Rust accepts, validate against CGP's restricted argument types. See
+[Parsing](docs/implementation/README.md#parsing-build-with-parse_internal-and-distrust-syns-leniency).
+
+#### Expanding the output
+
+Walk the shapes the expansion can take across the whole input space and confirm none produces invalid
+Rust — no conflicting `impl` blocks from a cartesian expansion, no unbound or doubly-declared generic,
+no empty expansion that checks nothing, no clash on a generated identifier. When an expansion fails to
+compile, capture it as a `trybuild` fixture in `cgp-compile-fail-tests` — `problematic/` when the
+macro should have rejected it, `acceptable/` when CGP defers the failure to the compiler — and
+document it in the macro's implementation document, per [crates/tests/AGENTS.md](crates/tests/AGENTS.md)
+and [docs/implementation/AGENTS.md](docs/implementation/AGENTS.md).
+
+#### Generics
+
+Keep the *kinds* (lifetime, type, const) and the *roles* (impl generics `impl<T>` versus type
+generics, the `<T>` in `Foo<T>`) distinct, render each in the right position, merge parameters from
+different sources without colliding, and bind every parameter that appears in a generated header so
+nothing is left free. See
+[Generic-parameter insertion](docs/implementation/README.md#generic-parameter-insertion-and-lifetime-ordering)
+and [keeping the kinds distinct](docs/implementation/README.md#generics-keep-the-kinds-and-roles-distinct).
+
+#### Qualified paths and hygiene
+
+Emit every CGP item through a `crate::exports` marker so it resolves as `::cgp::macro_prelude::<Name>`,
+never as a bare or hand-written path — grep the codegen for any CGP name not interpolated from an
+`exports` marker. Give the reserved identifiers the expansion introduces the double-underscore form
+(`__Context__`, `__Provider__`, `__Component__`, …) so they cannot clash with a user's names, and keep
+the expansion idempotent when the same entry is listed more than once. See
+[Hygiene](docs/implementation/README.md#hygiene-exports-markers-and-reserved-identifiers).
+
+#### Spans: for the compiler *and* the IDE
+
+Span placement is a correctness concern for two tools at once, and it applies to **every macro that
+emits items** — `#[cgp_impl]`, `#[cgp_provider]`, `#[cgp_new_provider]`, `#[cgp_type]`,
+`#[cgp_getter]`, and the derives — not just `delegate_components!`/`check_components!`. By default
+`parse_internal!`/`quote!` stamp generated tokens with the macro's `call_site` span (the whole
+invocation), which misleads both tools, so confirm both for each macro:
+
+- **Compiler carets.** An error on a generated item's header — a coherence conflict (`E0119`), an
+  unsatisfied bound, a name-resolution failure — must point at the entry the user wrote, not the whole
+  macro block. Re-span each generated item onto its originating token: with
+  [`override_item_span`](crates/macros/cgp-macro-core/src/functions/override_span.rs) for a generated
+  impl, a carried span field where the origin token is synthesized (`EvaluatedCheckEntry.span`,
+  `EvaluatedDelegateEntry.span`), or reuse of the user's own `for` token.
+- **IDE go-to-definition.** rust-analyzer maps a source token to its expansion by source range alone,
+  ignoring hygiene, so never drag a resolvable reference (`IsProviderFor`, `DelegateComponent`) onto a
+  user token's range, and span a *derived name* (a component marker) on the user identifier it derives
+  from, not `Span::call_site()`. A leaked derived name makes go-to-definition on a reference offer the
+  defining macro alongside the item.
+
+Only the caret half is pinned by a test (a `trybuild` `.stderr` fixture records each caret's exact
+position, so a regression changes the snapshot). The IDE half has no fixture and must be checked by
+hand in an editor — and cross-crate, since the derived-name leak is invisible from inside the defining
+crate. See [Spans](docs/implementation/README.md#spans-aim-generated-items-at-the-token-the-user-wrote)
+for the full mechanism and the `delegate_components!`/`#[cgp_impl]` worked examples.
 
 ### Keep every view in sync and verify
 

crates/macros/cgp-macro-core/src/functions/override_span.rs

@@ -1,22 +1,18 @@
-use proc_macro2::Span;
+use proc_macro2::{Span, TokenTree};
 use quote::ToTokens;
 use syn::parse::Parse;
 use syn::parse2;
 
-/// Re-span every top-level token of `body` to `span` and re-parse it into `T`.
+/// Force every top-level token of `body` onto `span`, unconditionally, and
+/// re-parse it into `T`.
 ///
-/// A macro builds its output from quasi-quoted tokens, which carry the whole
-/// invocation's `call_site` span; a compiler error on such an item therefore
-/// highlights the entire macro block. Re-spanning the tokens onto the specific
-/// user-written token that produced the item (a component key, a wiring entry)
-/// makes the diagnostic point there instead. `check_components!` uses this to
-/// aim an unsatisfied-bound error at the checked component, and
-/// `delegate_components!` to aim a coherence conflict at the offending entry.
-///
-/// Only top-level tokens are re-spanned; tokens nested inside a delimiter group
-/// (an impl body, say) keep their spans, which is why callers that must preserve
-/// an inner span — the generic parameters of a generated impl, for instance —
-/// restore it after re-spanning rather than relying on this to skip it.
+/// This clobbers the span of *user-written* tokens too, so it is only for the
+/// case where that is the intent: `check_components!` re-spans the context type
+/// — a single user token, reused once per checked component — onto each
+/// component in turn, so an unsatisfied-bound error is reported on the component
+/// the user listed rather than on the one shared context token. To re-span a
+/// *generated item* while leaving the user's tokens navigable, use
+/// [`override_item_span`] instead.
 pub fn override_span<T>(span: Span, body: &T) -> syn::Result<T>
 where
     T: Parse + ToTokens,
@@ -31,3 +27,62 @@ where
             .collect(),
     )
 }
+
+/// Re-span a generated item's *boundary* tokens onto `span` — its leading token
+/// and its trailing token, nothing in between — and re-parse into `T`.
+///
+/// A macro builds an item from quasi-quoted tokens (`quote!`/`parse_internal!`),
+/// which stamp every token they emit — the `impl` keyword, the trait reference,
+/// the fully-qualified `exports` path, the `{ … }` body — with the macro's
+/// `call_site` span, the whole invocation. A compiler error on the item's header
+/// (a coherence conflict `E0119`, an unsatisfied bound) then underlines the
+/// entire macro block instead of the entry the user wrote.
+///
+/// The compiler derives a generated item's span — and therefore that caret —
+/// from its *first and last* tokens joined together (`first.to(last)`): with the
+/// whole item at `call_site` the caret is the whole invocation, and re-spanning
+/// just those two boundary tokens onto `span` collapses it onto the entry. The
+/// interior tokens are deliberately left alone, and that is what keeps an editor
+/// working: rust-analyzer maps a source token to an expanded one purely by source
+/// range (hygiene is ignored), so a synthesized identifier — `IsProviderFor`,
+/// `DelegateComponent` — re-spanned onto the entry would collide with the key the
+/// user wrote, and go-to-definition on that key would then offer every collided
+/// construct. Leaving the interior untouched keeps each synthesized reference at
+/// `call_site` (never coinciding with a narrower user token) and each user token
+/// — the wired provider, the target type, a per-entry generic — at its own span,
+/// so navigation resolves to the one right definition.
+///
+/// The two boundary tokens re-spanned here are structural (a keyword and a
+/// delimiter group), never references, so moving them onto the entry cannot
+/// mislead the editor. Each is re-spanned only if it is itself synthesized, so a
+/// hand-assembled item whose boundary token the user wrote is left untouched.
+pub fn override_item_span<T>(span: Span, body: &T) -> syn::Result<T>
+where
+    T: Parse + ToTokens,
+{
+    let call_site_text = Span::call_site().source_text();
+    let mut trees: Vec<TokenTree> = body.to_token_stream().into_iter().collect();
+
+    if let Some(first) = trees.first_mut()
+        && is_synthesized(first.span(), &call_site_text)
+    {
+        first.set_span(span);
+    }
+    if let Some(last) = trees.last_mut()
+        && is_synthesized(last.span(), &call_site_text)
+    {
+        last.set_span(span);
+    }
+
+    parse2(trees.into_iter().collect())
+}
+
+/// Whether `token_span` was stamped by `quote!`/`parse_internal!` rather than
+/// carried in from user input. A synthesized token carries the macro's
+/// `call_site` span, whose source text is the entire invocation; a user token's
+/// source text is just that token, so the two never coincide. Comparing the
+/// source text works because a synthesized token *is* `call_site`, so it yields
+/// the exact same result as `call_site_text` — even when that is `None`.
+fn is_synthesized(token_span: Span, call_site_text: &Option<String>) -> bool {
+    token_span.source_text() == *call_site_text
+}

crates/macros/cgp-macro-core/src/types/attributes/cgp_impl_attributes.rs

@@ -1,13 +1,12 @@
-use syn::Attribute;
 use syn::parse::Parse;
 use syn::punctuated::Punctuated;
 use syn::token::Comma;
+use syn::{Attribute, TypeParamBound};
 
 use crate::types::attributes::{
     DefaultImplAttribute, DefaultImplAttributes, UseProviderAttribute, UseProviderAttributes,
     UseTypeAttribute, UseTypeAttributes, UsesAttributes,
 };
-use crate::types::ident::PathWithTypeArgs;
 
 #[derive(Default)]
 pub struct CgpImplAttributes {
@@ -27,7 +26,7 @@ impl CgpImplAttributes {
                 match ident.to_string().as_ref() {
                     "uses" => {
                         let uses = attribute.parse_args_with(
-                            Punctuated::<PathWithTypeArgs, Comma>::parse_terminated,
+                            Punctuated::<TypeParamBound, Comma>::parse_terminated,
                         )?;
 
                         parsed_attributes.uses.imports.extend(uses);

crates/macros/cgp-macro-core/src/types/attributes/default_impl/attribute.rs

@@ -3,7 +3,7 @@ use syn::spanned::Spanned;
 use syn::token::In;
 use syn::{Generics, ItemImpl, Type};
 
-use crate::functions::override_span;
+use crate::functions::override_item_span;
 use crate::parse_internal;
 use crate::types::ident::PathWithTypeArgs;
 use crate::types::path::UniPathOrType;
@@ -41,17 +41,15 @@ impl DefaultImplAttribute {
             }
         };
 
-        // The impl is built wholly from quasi-quoted tokens, so its `impl`/`for`
-        // structural tokens carry the macro `call_site` span. Re-span it onto the
-        // user-written key token so a coherence conflict (`E0119`) between two
-        // default impls for the same key is reported on that key rather than on
-        // the whole `#[cgp_impl]` attribute. Generics keep their own spans,
-        // restored afterward, mirroring `EvaluatedDelegateEntry::respan_impl`.
-        let generics = item_impl.generics.clone();
-        let mut item_impl = override_span(key_type.span(), &item_impl)?;
-        item_impl.generics = generics;
-
-        Ok(item_impl)
+        // The impl is built from quasi-quoted tokens, so its boundary carries the
+        // macro `call_site` span. Re-span it onto the user-written key token so a
+        // coherence conflict (`E0119`) between two default impls for the same key
+        // is reported on that key rather than on the whole `#[cgp_impl]` attribute.
+        // Only the boundary moves; the interior tokens — the provider type, a
+        // per-entry generic, each synthesized reference — keep their spans, so the
+        // user's tokens stay navigable in an IDE, mirroring
+        // `EvaluatedDelegateEntry::respan_impl`.
+        override_item_span(key_type.span(), &item_impl)
     }
 }