AI refactoring

Author: soareschen

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

@@ -35,8 +35,11 @@ impl Parse for UseTypeAttribute {
             let _: At = input.parse()?;
 
             // The context type is followed by a `::`-separated trait path, so it
-            // must parse only a single identifier head; a full path parser would
-            // greedily consume the trailing `::Trait::Type`.
+            // must parse only a single identifier head. This is the one place
+            // that deliberately keeps `IdentWithTypeArgs` rather than the
+            // otherwise-dominant `PathWithTypeArgs`: a path parser is greedy
+            // across `::` and would silently consume the trailing `::Trait::Type`
+            // here, with no parse error. Do NOT swap this for `PathWithTypeArgs`.
             let context_type: Type = input.parse::<IdentWithTypeArgs>()?.into();
 
             let _: Colon = input.parse()?;

crates/macros/cgp-macro-core/src/types/generics/type_generics.rs

@@ -7,6 +7,18 @@ use syn::{Error, Generics};
 
 use crate::functions::parse_internal;
 
+/// A validated newtype around [`syn::Generics`] restricted to a definition-site
+/// generic list (no bounds). Because it `Deref`s to [`syn::Generics`], the full
+/// `syn` API (`split_for_impl`, mutating `params`, …) is available, and its
+/// `TryFrom<&Generics>` adapts a generic list already parsed off an item.
+///
+/// Prefer this when adapting or manipulating an existing `syn::Generics`. When
+/// instead *parsing tokens* and you want strict, kind-classified parameters,
+/// prefer [`TypeGenericParams`]. The two are intentionally kept separate; see
+/// [`TypeGenericParams`] for the full rationale (notably, `TryFrom` here
+/// normalizes a `const N: T` parameter down to a bare `N`).
+///
+/// [`TypeGenericParams`]: crate::types::ident::TypeGenericParams
 #[derive(Debug, Clone, Default)]
 pub struct TypeGenerics {
     pub generics: Generics,

crates/macros/cgp-macro-core/src/types/ident/path_with_type_args.rs

@@ -1,11 +1,10 @@
 use proc_macro2::TokenStream;
 use quote::ToTokens;
 use syn::parse::{Parse, ParseStream};
-use syn::punctuated::Punctuated;
-use syn::{Error, Ident, Path, PathArguments, Type, parse_quote};
+use syn::{Error, Ident, Path, PathArguments, Type, parse_quote, parse2};
 
 use crate::traits::ToType;
-use crate::types::ident::{IdentWithTypeArgs, TypeArg, TypeArgs};
+use crate::types::ident::{IdentWithTypeArgs, TypeArgs};
 
 /// A full Rust path followed by an optional type-expression argument list, e.g.
 /// `Foo`, `Foo<A, B>`, `path::to::Foo`, or `path::to::Bar<(A, B), B>`.
@@ -15,8 +14,8 @@ use crate::types::ident::{IdentWithTypeArgs, TypeArg, TypeArgs};
 /// generic arguments buried inside the last [`syn::PathSegment`], which is
 /// awkward to read and rewrite. This type lifts those arguments out into a
 /// separate [`TypeArgs`] field while keeping the remaining path in `path`,
-/// applying the same restrictions as [`TypeArg`] (no associated bindings or
-/// bounds).
+/// applying the same restrictions as [`TypeArg`](crate::types::ident::TypeArg)
+/// (no associated bindings or bounds).
 ///
 /// Only the final segment may carry generic arguments. Intermediate generics
 /// (e.g. `path::to<X>::Foo`) and parenthesized arguments (e.g. `Fn(A) -> B`)
@@ -34,8 +33,12 @@ impl PathWithTypeArgs {
     /// The identifier of the final path segment, e.g. `Foo` in
     /// `path::to::Foo<A, B>`.
     pub fn ident(&self) -> &Ident {
-        // A parsed `syn::Path` always has at least one segment.
-        &self.path.segments.last().unwrap().ident
+        &self
+            .path
+            .segments
+            .last()
+            .expect("PathWithTypeArgs always wraps a non-empty syn::Path")
+            .ident
     }
 }
 
@@ -59,9 +62,7 @@ impl Parse for PathWithTypeArgs {
         let last_segment = path.segments.last_mut().unwrap();
 
         let type_args = match &last_segment.arguments {
-            PathArguments::None => TypeArgs {
-                args: Punctuated::new(),
-            },
+            PathArguments::None => TypeArgs::default(),
             PathArguments::AngleBracketed(arguments) => {
                 // Reject turbofish (`Foo::<A>`); only the type-position form
                 // `Foo<A>` is accepted, matching `IdentWithTypeArgs`.
@@ -72,17 +73,13 @@ impl Parse for PathWithTypeArgs {
                     ));
                 }
 
-                let mut args = Punctuated::new();
-
-                for pair in arguments.args.pairs() {
-                    let (arg, punct) = pair.into_tuple();
-                    args.push_value(TypeArg::from_generic_argument(arg)?);
-                    if let Some(comma) = punct {
-                        args.push_punct(*comma);
-                    }
-                }
-
-                TypeArgs { args }
+                // Re-parse the already-parsed `<...>` through `TypeArgs` so the
+                // argument-form restrictions (no associated bindings or bounds)
+                // live in exactly one place — `TypeArg`'s own parser — rather
+                // than being duplicated here against `syn::GenericArgument`.
+                // With the turbofish ruled out above, `arguments` re-emits as a
+                // plain `< .. >`, which is exactly what `TypeArgs` expects.
+                parse2::<TypeArgs>(arguments.to_token_stream())?
             }
             PathArguments::Parenthesized(arguments) => {
                 return Err(Error::new_spanned(

crates/macros/cgp-macro-core/src/types/ident/type_arg.rs

@@ -3,7 +3,7 @@ use quote::ToTokens;
 use syn::parse::{Parse, ParseStream};
 use syn::punctuated::Punctuated;
 use syn::token::{Brace, Comma};
-use syn::{Error, Expr, ExprBlock, ExprLit, GenericArgument, Lifetime, Lit, Token, Type};
+use syn::{Error, Expr, ExprBlock, ExprLit, Lifetime, Lit, Token, Type};
 
 use crate::types::ident::{parse_angle_bracketed, to_tokens_angle_bracketed};
 
@@ -32,35 +32,6 @@ pub enum TypeArg {
     Const(Expr),
 }
 
-impl TypeArg {
-    /// Convert a [`syn::GenericArgument`] into a [`TypeArg`], rejecting the
-    /// associated-binding and bound forms that are not valid in type-argument
-    /// positions.
-    ///
-    /// This is useful when post-processing a value that `syn` has already
-    /// parsed into a [`syn::Path`] (see [`PathWithTypeArgs`]).
-    ///
-    /// [`PathWithTypeArgs`]: crate::types::ident::PathWithTypeArgs
-    pub fn from_generic_argument(arg: &GenericArgument) -> syn::Result<Self> {
-        match arg {
-            GenericArgument::Lifetime(life) => Ok(Self::Lifetime(life.clone())),
-            GenericArgument::Type(ty) => Ok(Self::Type(ty.clone())),
-            GenericArgument::Const(expr) => Ok(Self::Const(expr.clone())),
-            GenericArgument::AssocType(_) | GenericArgument::AssocConst(_) => {
-                Err(Error::new_spanned(
-                    arg,
-                    "associated bindings (`Name = ...`) are not allowed in type arguments",
-                ))
-            }
-            GenericArgument::Constraint(_) => Err(Error::new_spanned(
-                arg,
-                "associated type bounds (`Name: ...`) are not allowed in type arguments",
-            )),
-            _ => Err(Error::new_spanned(arg, "unsupported generic argument")),
-        }
-    }
-}
-
 impl Parse for TypeArg {
     fn parse(input: ParseStream) -> syn::Result<Self> {
         if input.peek(Lifetime) {

crates/macros/cgp-macro-core/src/types/ident/type_generic_param.rs

@@ -19,10 +19,13 @@ use crate::types::ident::{parse_angle_bracketed, to_tokens_angle_bracketed};
 /// - defaults, e.g. `A = B` or `const N: usize = 0`,
 /// - composite forms, e.g. `(A, B)`.
 ///
-/// The existing `TypeGenerics` type approximates this by parsing a full
-/// `syn::Generics` and then round-tripping it through `split_for_impl` to
-/// detect bounds. Modelling the valid forms directly is both clearer and
-/// catches more invalid inputs (such as defaults) up front.
+/// This complements (rather than replaces) [`TypeGenerics`], which detects
+/// bounds by round-tripping a full `syn::Generics` through `split_for_impl`.
+/// Modelling the valid forms directly here is clearer and catches more invalid
+/// inputs (such as defaults) up front when *parsing tokens*; see
+/// [`TypeGenericParams`] for guidance on which of the two to use.
+///
+/// [`TypeGenerics`]: crate::types::generics::TypeGenerics
 #[derive(Debug, Clone)]
 pub enum TypeGenericParam {
     /// A lifetime parameter, e.g. the `'a` in `Bar<'a>`.
@@ -117,6 +120,27 @@ impl ToTokens for TypeGenericParam {
 /// The angle-bracketed parameter list at a type definition site, e.g. the
 /// `<'a, C>` in `Bar<'a, C>`.
 ///
+/// # `TypeGenericParams` vs [`TypeGenerics`]
+///
+/// Both model a definition-site generic list, but they are different tools:
+///
+/// - Reach for `TypeGenericParams` when **parsing tokens** where you want the
+///   restrictions enforced strictly and the parameters classified by kind. It
+///   is a hand-written parser that rejects bounds and defaults up front and
+///   exposes each parameter as a [`TypeGenericParam`] variant.
+/// - Reach for [`TypeGenerics`] when adapting an **already-parsed
+///   [`syn::Generics`]** (e.g. off an `ItemTrait`/`ItemStruct`). It is a thin
+///   newtype that `Deref`s to `syn::Generics`, so `split_for_impl()` and the
+///   usual `syn` manipulation are available, and its `TryFrom<&Generics>`
+///   normalizes through `split_for_impl` (which, notably, collapses a
+///   `const N: T` parameter down to a bare type-like `N`).
+///
+/// They are intentionally not merged: the normalization behavior above is
+/// load-bearing for some callers, so a faithful conversion into the strict
+/// `TypeGenericParam` model would change behavior around const generics.
+///
+/// [`TypeGenerics`]: crate::types::generics::TypeGenerics
+///
 /// Both the absence of angle brackets and an explicit empty `<>` are
 /// represented as an empty [`Punctuated`]. An empty list renders as nothing,
 /// so a parsed `<>` round-trips back to no angle brackets.