fix(doc): keep the :: before an associated-type segment in rendered signatures

[orizi]

Summary

Replaces the extract_and_format string-manipulation approach for formatting types in signatures with a new semantic-aware format_type function that operates directly on TypeId values. A companion format_generic_arg function handles GenericArgumentId values. Associated types are now handled specially: Self::Item is preserved as-is (rather than being reduced to just Item), and associated types accessed through named impl parameters (e.g., S::Item, U::Item) retain their impl qualifier. The extract_and_format function is made private since it is now only used internally within helpers.rs.

---

Type of change

Please check one:

• Bug fix (fixes incorrect behavior)
• New feature
• Performance improvement
• Documentation change with concrete technical impact
• Style, wording, formatting, or typo-only change

---

Why is this change needed?

The previous extract_and_format function worked by parsing the string output of TypeId::format, stripping full module paths from each path segment. This approach lost structural information about the type, making it impossible to correctly handle associated types. An associated type like Self::Item would be incorrectly reduced to just Item, which is ambiguous and does not identify a type on its own.

---

What was the behavior or documentation before?

Associated types in function signatures were rendered without their qualifier. For example, a return type of Self::Item would appear as just Item in the generated documentation signature.

---

What is the behavior or documentation after?

Associated types retain their qualifier in signatures. Self::Item renders as Self::Item, and associated types accessed through impl parameters render as S::Item or U::Item. Concrete types, snapshots, tuples, fixed-size arrays, and generic arguments are all formatted by traversing the semantic type structure directly rather than post-processing a formatted string.

---

Related issue or discussion (if any)

---

Additional context

Test cases were added covering traits with associated types and constants, impl functions that resolve associated types to concrete types (e.g., (felt252, felt252)), functions with impl parameters whose associated types appear in signatures, and snapshot parameters and return types.

[reviewable-StarkWare]

This change is 

[orizi]

• fix(doc): keep the :: before an associated-type segment in rendered signatures #10271 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cursor]

PR Summary

Low Risk
Changes affect documentation signature rendering only, not compilation or runtime; behavior is covered by expanded signature tests.

Overview
Documentation signatures no longer post-process TypeId::format strings via extract_and_format. They are built from format_type and format_generic_arg, which walk semantic types and generic arguments directly.

Associated items keep their impl qualifier (Self::Item, S::Item, O::Nested::Item, Self::SIZE in array lengths). Unnameable impls fall back to trait paths with turbofish where needed (AssocTrait::<T>::Item). Impl generic args show concrete impl names when explicit and _ when inferred; anonymous +/- bounds and type constraints are spelled from semantics. Tuples get a trailing comma for single-element types; neg impl params render as -Shape<T>.

documentable_formatter wires these helpers through write_type, generic args, implicits, and trait constants, with small cleanups (write_link takes &str, tuple comma in formatter). extract_and_format is private and only used for simple leaf types. Snapshot and signature tests are updated/expanded for these cases.

^{Reviewed by Cursor Bugbot for commit bffd769. Bugbot is set up for automated code reviews on this repo. Configure here.}

[eytan-starkware]

Orizi-review:

Approach is right — string post-processing of a {:?} dump can't preserve the ::, and no string-level patch fixes bare S::Pair. Comments below are all on the implementation, plus one real correctness gap.

helpers.rs

format_generic_arg is pub, format_type right next to it is pub(crate). Both have exactly one consumer. conform to the others.

format_impl_type's _ arm leaks full paths and debug output. ImplTypeId::format is format!("{}::{}", impl_id.name(db), ..), and ImplLongId::name is not short for every variant — ImplImpl is {impl}::{concrete_trait.full_path}, ImplVar is ImplVar({full_path}), GeneratedImpl is {:?} of the debug repr. Those now land verbatim in a rendered signature; extract_and_format used to at least strip the paths. Shorten the qualifier explicitly — don't wrap the whole string in extract_and_format, that's what eats the :: in the first place.

Same arm: a +AssocTrait<T> param has no name, so GenericParameter renders _::Item. Worth checking whether that's reachable before this lands — I couldn't construct it from source, since the assoc type isn't spellable there.

extract_and_format(&size.format(db)) in the FixedSizeArray arm — why does an array length need path stripping? If it's for a const generic param, say so; otherwise just size.format(db).

_ => in format_type — GenericParameter | Var | Coupon | Closure | Missing | NumericLiteral fall through silently, and so will the next variant anyone adds. list them.

documentable_formatter.rs

write_link: both arms now write the same string. Dropping extract_and_format from the None arm makes the match pointless. name: String too — one caller, only used by reference.

fn write_link(
    &mut self,
    name: &str,
    documentable_id: Option<DocumentableItemId<'db>>,
) -> fmt::Result {
    let start_offset = self.buf.len();
    self.write_str(name)?;
    if let Some(documentable_id) = documentable_id {
        self.add_location_link(start_offset, self.buf.len(), documentable_id);
    }
    Ok(())
}

write_type: all three branches are now byte-identical. They differ only in whether a link is added — that is write_link. And formatted_element_type then survives only to feed is_the_same_root, so inline it.

} else {
    let documentable_id = is_the_same_root(full_path, &element_type.format(self.db))
        .then(|| resolve_type(self.db, element_type))
        .flatten();
    self.write_link(&format_type(self.db, element_type), documentable_id)?;
}

Tuples are now formatted in two places, and they disagree. write_type keeps its own Tuple branch (per-element links, no trailing comma), format_type adds another (trailing comma on the 1-tuple, no links). So -> (felt252,) renders (felt252) at top level and (felt252,) one level down inside Option<_>. one of the two is wrong in every 1-tuple signature — pick an owner.

syntactic_kind: String -> &str and the inline-format-args rewrite in write_function_signature — both correct, neither part of this fix.

signature.txt

PairTrait/FeltPair is a clone of the existing Shape/CircleShape. Shape already declares type ShapePair; and CircleShape already resolves it to (Circle, Circle); the only thing missing is a function returning it. add fn pair(self: T) -> Self::ShapePair; to Shape + the impl and drop the new trait and impl. yes it renumbers the golden items, that's fine.

returns_assoc is subsumed by assoc_in_param_and_return — five fns on AssocTrait for four distinct code paths. drop it.

Good catch on the @ coverage gap, that one is worth closing on its own.

@eytan-starkware made 1 comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on TomerStarkware).

[TomerStarkware]

@TomerStarkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on orizi).

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 09c4c87. Configure here.}

[orizi]

Ordering note: this review is against bc2db3d29, and the branch has been force-pushed a few times since — several of these were closed in 1a7fb1eb/bae58061a, before the current round. Marked below.

helpers.rs

format_generic_arg visibility — already pub(crate), same as format_type. Closed in 1a7fb1eb.

format_impl_type's _ arm — right on both counts, and the fix is to spell the impl rather than to shorten its name. Extracted format_impl, which walks the impl recursively, and ImplImpl is now the outer impl :: the member name:

ImplLongId::SelfImpl(_) => "Self".to_string(),
ImplLongId::Concrete(_) => impl_id.long(db).name(db),
ImplLongId::GenericParameter(param) if param.name(db).is_some() => impl_id.long(db).name(db),
ImplLongId::ImplImpl(impl_impl) => format!(
    "{}::{}",
    format_impl(db, impl_impl.impl_id()),
    impl_impl.trait_impl_id().name(db).long(db)
),
ImplLongId::GenericParameter(_) | ImplLongId::ImplVar(_) | ImplLongId::GeneratedImpl(_) => impl_id
    .concrete_trait(db)
    .map(|concrete_trait| format_concrete_trait(db, concrete_trait, true))
    .unwrap_or_else(|_| MISSING.to_string()),

extract_and_format is gone from this path. The trait is used only for the three that have no impl to name — an anonymous param has none, an inference variable and a generated impl have no declaration — and there the trait is the spelling the source itself uses.

Covered, reusing AssocTrait as the inner trait rather than adding one:

trait OuterAssoc<T> {
    impl Nested: AssocTrait<T>;
    fn impl_impl_assoc(x: Self::Nested::Item) -> Self::Nested::Item;
}
fn impl_impl_of_impl_param<T, impl O: OuterAssoc<T>>(x: O::Nested::Item) -> O::Nested::Item {}

Both render as spelled in the source. Routing ImplImpl back to the trait fallback makes both fail with AssocTrait::<T>::Item, so the golden discriminates.

Is _::Item reachable? Yes — you couldn't spell it because the qualifier is what's unspellable, not the bound. fn anon_assoc_qualified<T, +AssocTrait<T>>(x: T) -> AssocTrait::<T>::Item is in the fixture and hits that arm. That's also where the missing turbofish was, fixed in bae58061a.

extract_and_format(&size.format(db)) in the FixedSizeArray arm — agreed it can be an impl const item, so it now gets the same treatment as an associated type instead of either path stripping or a raw format:

fn format_const_value<'db>(db: &'db dyn Database, value: ConstValueId<'db>) -> String {
    match value.long(db) {
        ConstValue::ImplConstant(impl_constant) => format!(
            "{}::{}",
            format_impl(db, impl_constant.impl_id()),
            impl_constant.trait_constant_id().name(db).long(db)
        ),
        _ => value.format(db),
    }
}

fn assoc_const_array_size(self: T) -> [bool; Self::SIZE] renders [bool; Self::SIZE]. It rendered [bool; SIZE] before — ConstValueId::format is {:?} of ConstValue's DebugWithDb, and for ImplConstant that is {impl}::{CONST}, so extract_and_format was eating the :: exactly as it did for types. Going through format_impl also fixes the qualifier itself: ImplConstantId::format spells SelfImpl as the trait's name, so the raw text is AssocTrait::SIZE where the source says Self::SIZE.

A length is a usize, so the other spellings reachable there are a literal, a const generic param, an inference variable and a missing value — all already short, hence the fallback.

_ => in format_type — all six variants listed. Closed in 1a7fb1eb.

documentable_formatter.rs

write_link — taken verbatim, including name: &str. Closed in 1a7fb1eb.

write_type — taken verbatim, formatted_element_type inlined. Closed in 1a7fb1eb.

Tuples formatted in two places — the disagreement is fixed: write_type emits the 1-tuple comma too, so -> (felt252,) and Option<(felt252,)> agree. Both are covered.

On picking an owner, though — I don't think either site can absorb the other. write_type walks the elements so each gets its own LocationLink; resolve_type returns None for Tuple, so the tuple never carries a link itself and dropping that arm makes the elements unlinkable. format_type is the only one that reaches a tuple nested in a generic argument, a snapshot or an array element, where there is no HirFormatter at all. So it stays two sites for one spelling rule.

Worth flagging: the goldens print signature text only and never assert location links, which is why the 1-tuple disagreement got through.

Related, and it caught me out twice while testing: write_function_signature renders a parameter type through write_type only if param.ty.is_fully_concrete(f.db), and otherwise echoes the syntactic type clause. So a parameter mentioning a generic param proves nothing about the formatter - it is echoed - while a fully concrete one does exercise it. Return types always go through write_type.

syntactic_kind: String -> &str and the inline-format-args rewrite — agreed, out of scope. Both reverted here and queued as their own PR.

signature.txt

PairTrait/FeltPair — dropped; Shape gained fn pair(self: T) -> Self::ShapePair and CircleShape the matching impl, reusing the type ShapePair / (Circle, Circle) already there. Closed in 1a7fb1eb.

returns_assoc — dropped. Closed in 1a7fb1eb.

Items were added this round, each for a code path none of the others reach: SIZE + assoc_const_array_size for an associated const in a length, OuterAssoc + impl_impl_of_impl_param for the impl impl qualifier, and — from the two bugbot findings on this revision — assoc_in_anon_bound for a qualifier inside an anonymous bound and ConstGenericStruct + assoc_const_generic_arg for one in a const generic argument. Both of those were extract_and_format eating a ::, same as the rest; the replies on those discussions have the detail. After them extract_and_format survives only for type and const generic params in get_generic_params, which are bare names anyway.

One spelling change worth looking at directly, since it came out of those two and touches an existing golden: impl params and impl arguments are now spelled differently. A param is a bound, so an anonymous one keeps the source's + or - (+Shape<S::Item>, -Shape<T>); an argument is never written in surface syntax, so it is _. impl BCopy<..> of Copy<B<T, G>> therefore renders Copy<B<T, G, _>> where it used to render Copy<B<T, G, + ATrait>> — that + ATrait, space included, was extract_and_format over a {:?} dump. A negative param also used to render <missing> from get_generic_params, which had no NegImpl arm; it now goes through the same formatter, covered by neg_impl_param and NegImplStruct.

_ is only for an argument with no name to write, though - an anonymous impl param, an inference variable, a generated impl. An impl argument that is nameable is written, since the source can pass one explicitly (bar::<MyImpl>()), so format_impl_name returns the name and format_generic_arg falls back to _ only on None. fn nameable_impl_arg(b: B<felt252, felt252>) -> B<felt252, felt252> renders B<felt252, felt252, ATraitImpl> - inferred in the source, named in the docs - against Copy<B<T, G, _>> for the anonymous one, so the two sides of the distinction are pinned by adjacent goldens. The qualifier position keeps its own fallback: an unnameable impl there becomes its trait, since an associated item's name alone would not identify it.

@orizi+AGNT made 3 comments.
Reviewable status: 1 of 4 files reviewed, all discussions resolved (waiting on TomerStarkware).