Skip to content

Commit 46eced5

Browse files
committed
Let Rust doc comments win over stored docstrings
A /// comment on a Python-exposed item is its docstring; the rustpython-doc entry is used only when there is none. #[pymember] reads the field's /// comment instead of doc = "...". Doc comments that were shadowed by a stored docstring become // comments, so no docstring changes. AGENTS.md describes the rule. Assisted-by: Grok:4.7 Assisted-by: Claude:claude-opus-5-5
1 parent a2960bf commit 46eced5

43 files changed

Lines changed: 336 additions & 252 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ Run `./scripts/whats_left.py` to get a list of unimplemented methods, which is h
161161

162162
- Do not delete or rewrite existing comments unless they are factually wrong or directly contradict the new code.
163163
- Do not add decorative section separators (e.g. `// -----------`, `// ===`, `/* *** */`). Use `///` doc-comments or short `//` comments only when they add value.
164-
- Do not put `///` doc comments on items annotated with `#[pyattr]`, `#[pyclass]`, or `#[pyfunction]`. The derive macros pull authoritative docstrings from CPython via the `rustpython-doc` crate; a Rust doc comment overrides that source, and on `#[pyattr]` it is silently dropped.
164+
- A `///` doc comment on a Python-exposed item (`#[pyattr]`, `#[pyclass]`, `#[pyfunction]`, `#[pymethod]`, `#[pyclassmethod]`, `#[pystaticmethod]`, `#[pygetset]`, `#[pymember]`, `#[pyslot]`, `#[pyexception]`, `#[pymodule]`, `#[pystruct_sequence]`) is that item's docstring and overrides the stored docs in `rustpython-doc` (`crates/doc`). Those stored docs are used only when the item has no `///` comment. Developer notes on these items must be `//` comments.
165165

166166
#### Avoid Duplicate Code in Branches
167167

‎crates/derive-impl/src/pyclass.rs‎

Lines changed: 97 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -426,27 +426,26 @@ fn class_def_ty(self_ty: Option<&syn::Type>) -> Option<TokenStream> {
426426
Some(quote!(#ty))
427427
}
428428

429-
/// Const `Option<&'static str>`: table entry, else the Rust doc.
430-
/// Generic impls cannot name `Self` from a nested const, so they keep the Rust doc.
429+
/// Const `Option<&'static str>`. A non-empty Rust doc is the docstring.
430+
/// The attribute table is used only when there is no Rust doc. Generic impls
431+
/// cannot name `Self` from a nested const, so without a Rust doc they store none.
431432
fn attr_doc_expr(self_ty: Option<&syn::Type>, attr: &str, rust_doc: Option<String>) -> TokenStream {
432-
let fallback = match rust_doc {
433-
Some(doc) => quote!(Some(#doc)),
434-
None => quote!(None),
435-
};
433+
if let Some(doc) = rust_doc.filter(|doc| !doc.is_empty()) {
434+
return quote!(Some(#doc));
435+
}
436436
let Some(ty) = class_def_ty(self_ty) else {
437-
return fallback;
437+
return quote!(None);
438438
};
439439
quote! {
440440
{
441441
const FOUND: Option<&str> = ::rustpython_vm::class::attr_doc(
442442
<#ty as ::rustpython_vm::class::PyClassDef>::ATTR_DOCS,
443443
#attr,
444444
);
445-
const RUST_DOC: Option<&str> = #fallback;
446445
if let Some(doc) = FOUND {
447446
if doc.is_empty() { None } else { Some(doc) }
448447
} else {
449-
RUST_DOC
448+
None
450449
}
451450
}
452451
}
@@ -498,10 +497,11 @@ fn generate_class_def(
498497
let attrs = extras.attrs;
499498
let module_key = module_name.unwrap_or("builtins");
500499
let attr_docs = crate::class_docs::attr_docs_tokens(module_name, name);
501-
let doc = rustpython_doc::get_qualified(module_key, name, None, true)
502-
.filter(|doc| !doc.is_empty())
503-
.map(str::to_owned)
504-
.or_else(|| attrs.doc());
500+
let doc = attrs.doc().filter(|doc| !doc.is_empty()).or_else(|| {
501+
rustpython_doc::get_qualified(module_key, name, None, true)
502+
.filter(|doc| !doc.is_empty())
503+
.map(str::to_owned)
504+
});
505505
let doc = if let Some(doc) = doc {
506506
quote!(Some(#doc))
507507
} else {
@@ -1878,27 +1878,25 @@ impl MemberItemMeta {
18781878
Ok(Some(name_value.value.to_token_stream()))
18791879
}
18801880

1881-
/// `Some(Some(text))` for `doc = "text"` (used as-is), `Some(None)` for
1882-
/// `doc = false`, and `None` when `doc` is absent.
1883-
fn doc(&self) -> Result<Option<Option<String>>> {
1881+
/// `doc = false` suppresses the stored docstring. The member docstring is
1882+
/// the field's `///` comment (or the `///` lines immediately above a
1883+
/// struct-level `#[pymember]`). A string `doc` is rejected.
1884+
fn suppress_doc(&self) -> Result<bool> {
18841885
let Some((_, meta)) = self.inner().meta_map.get("doc") else {
1885-
return Ok(None);
1886+
return Ok(false);
18861887
};
18871888
match meta {
18881889
Meta::NameValue(syn::MetaNameValue {
1889-
value: syn::Expr::Lit(syn::ExprLit { lit, .. }),
1890+
value:
1891+
syn::Expr::Lit(syn::ExprLit {
1892+
lit: syn::Lit::Bool(lit),
1893+
..
1894+
}),
18901895
..
1891-
}) => match lit {
1892-
syn::Lit::Str(lit) => Ok(Some(Some(lit.value()))),
1893-
syn::Lit::Bool(lit) if !lit.value => Ok(Some(None)),
1894-
_ => Err(syn::Error::new(
1895-
lit.span(),
1896-
"#[pymember(doc = ...)] must be a string or `false`",
1897-
)),
1898-
},
1896+
}) if !lit.value => Ok(true),
18991897
_ => Err(syn::Error::new(
19001898
meta.span(),
1901-
"#[pymember(doc = ...)] must be a string or `false`",
1899+
"#[pymember] docstring is the `///` comment; only `doc = false` is accepted",
19021900
)),
19031901
}
19041902
}
@@ -1954,6 +1952,61 @@ struct BuiltMember {
19541952
span: Span,
19551953
}
19561954

1955+
fn is_doc_attr(attr: &Attribute) -> bool {
1956+
attr.path().is_ident("doc")
1957+
}
1958+
1959+
fn doc_attr_line(attr: &Attribute) -> Option<String> {
1960+
let syn::Meta::NameValue(name_value) = &attr.meta else {
1961+
return None;
1962+
};
1963+
let syn::Expr::Lit(syn::ExprLit {
1964+
lit: syn::Lit::Str(lit),
1965+
..
1966+
}) = &name_value.value
1967+
else {
1968+
return None;
1969+
};
1970+
Some(lit.value().trim().to_owned())
1971+
}
1972+
1973+
/// `#[pymember]` on a struct has no field. `///` lines placed immediately
1974+
/// above that attribute (after any other attribute) are its docstring and are
1975+
/// removed so they are not also the class docstring. Several struct-level
1976+
/// members each take the `///` lines directly above them.
1977+
fn split_struct_pymembers(attrs: &mut Vec<Attribute>) -> Vec<(Attribute, Option<String>)> {
1978+
let mut kept = Vec::with_capacity(attrs.len());
1979+
let mut members = Vec::new();
1980+
let mut pending_docs = Vec::new();
1981+
let mut pending_doc_attrs = Vec::new();
1982+
for attr in attrs.drain(..) {
1983+
if is_doc_attr(&attr)
1984+
&& let Some(text) = doc_attr_line(&attr)
1985+
{
1986+
pending_docs.push(text);
1987+
pending_doc_attrs.push(attr);
1988+
continue;
1989+
}
1990+
if attr.path().is_ident("pymember") {
1991+
let doc = if pending_docs.is_empty() {
1992+
None
1993+
} else {
1994+
Some(pending_docs.join("\n")).filter(|doc| !doc.is_empty())
1995+
};
1996+
pending_docs.clear();
1997+
pending_doc_attrs.clear();
1998+
members.push((attr, doc));
1999+
continue;
2000+
}
2001+
kept.append(&mut pending_doc_attrs);
2002+
pending_docs.clear();
2003+
kept.push(attr);
2004+
}
2005+
kept.append(&mut pending_doc_attrs);
2006+
*attrs = kept;
2007+
members
2008+
}
2009+
19572010
fn split_pymember_attrs(attrs: &mut Vec<Attribute>) -> Vec<Attribute> {
19582011
let mut kept = Vec::with_capacity(attrs.len());
19592012
let mut members = Vec::new();
@@ -2071,6 +2124,7 @@ fn build_member(
20712124
field: Option<(&syn::Field, usize)>,
20722125
cfgs: &[Attribute],
20732126
class_ty: &syn::Type,
2127+
rust_doc: Option<String>,
20742128
seen: &mut Vec<(String, Vec<Attribute>)>,
20752129
) -> Result<BuiltMember> {
20762130
let fallback_ident = field
@@ -2161,12 +2215,10 @@ fn build_member(
21612215
let kind_tokens = member_kind_tokens(kind.as_deref(), None, attr.span())?;
21622216
(offset_expr, quote!(), kind_tokens)
21632217
};
2164-
let doc = match meta.doc()? {
2165-
// An explicit string is the docstring. The attribute-doc table is not
2166-
// consulted. `doc = false` stores none. Omitted `doc` uses the table.
2167-
Some(Some(text)) => quote!(Some(#text)),
2168-
Some(None) => quote!(None),
2169-
None => attr_doc_expr(Some(class_ty), &name, None),
2218+
let doc = if meta.suppress_doc()? {
2219+
quote!(None)
2220+
} else {
2221+
attr_doc_expr(Some(class_ty), &name, rust_doc)
21702222
};
21712223
Ok(BuiltMember {
21722224
name,
@@ -2244,8 +2296,15 @@ fn extract_py_members(item: &mut Item) -> Result<(TokenStream, TokenStream, Toke
22442296
let class_ty: syn::Type = syn::parse_quote!(#struct_ident);
22452297
let mut seen = Vec::new();
22462298
let mut built = Vec::new();
2247-
for attr in split_pymember_attrs(&mut item_struct.attrs) {
2248-
built.push(build_member(&attr, None, &[], &class_ty, &mut seen)?);
2299+
for (attr, rust_doc) in split_struct_pymembers(&mut item_struct.attrs) {
2300+
built.push(build_member(
2301+
&attr,
2302+
None,
2303+
&[],
2304+
&class_ty,
2305+
rust_doc,
2306+
&mut seen,
2307+
)?);
22492308
}
22502309
let field_count = match &item_struct.fields {
22512310
syn::Fields::Named(fields) => fields.named.len(),
@@ -2259,13 +2318,16 @@ fn extract_py_members(item: &mut Item) -> Result<(TokenStream, TokenStream, Toke
22592318
syn::Fields::Unit => unreachable!(),
22602319
};
22612320
let cfgs = field_cfg_attrs(&field.attrs)?;
2321+
// Every #[pymember] on the field shares that field's `///` comment.
2322+
let rust_doc = field.attrs.doc().filter(|doc| !doc.is_empty());
22622323
let attrs = split_pymember_attrs(&mut field.attrs);
22632324
for attr in &attrs {
22642325
built.push(build_member(
22652326
attr,
22662327
Some((field, index)),
22672328
&cfgs,
22682329
&class_ty,
2330+
rust_doc.clone(),
22692331
&mut seen,
22702332
)?);
22712333
}

‎crates/derive-impl/src/pymodule.rs‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,11 @@ pub(crate) fn impl_pymodule(args: PyModuleArgs, module_item: Item) -> Result<Tok
255255
let module_name = context.name.as_str();
256256
let function_items = context.function_items.validate()?;
257257
let attribute_items = context.attribute_items.validate()?;
258-
let doc = rustpython_doc::get(module_name)
259-
.filter(|doc| !doc.is_empty())
260-
.map(str::to_owned)
261-
.or(doc);
258+
let doc = doc.filter(|doc| !doc.is_empty()).or_else(|| {
259+
rustpython_doc::get(module_name)
260+
.filter(|doc| !doc.is_empty())
261+
.map(str::to_owned)
262+
});
262263
let doc = if let Some(doc) = doc {
263264
quote!(Some(#doc))
264265
} else {
@@ -708,22 +709,22 @@ impl ModuleItem for FunctionItem {
708709
}
709710

710711
let module = args.module_name();
711-
let rust_doc = args.attrs.doc();
712-
// TODO: doc must exist at least one of code or CPython
712+
let rust_doc = args.attrs.doc().filter(|doc| !doc.is_empty());
713713
let docs = py_names
714714
.iter()
715715
.map(|py_name| {
716-
let doc = rustpython_doc::get_qualified(module, py_name, None, false)
717-
.filter(|doc| !doc.is_empty())
718-
.map(str::to_owned)
719-
.or_else(|| {
720-
if args.context.is_sub {
721-
unique_submodule_func_doc(py_name)
722-
} else {
723-
None
724-
}
725-
})
726-
.or_else(|| rust_doc.clone());
716+
let doc = rust_doc.clone().or_else(|| {
717+
rustpython_doc::get_qualified(module, py_name, None, false)
718+
.filter(|doc| !doc.is_empty())
719+
.map(str::to_owned)
720+
.or_else(|| {
721+
if args.context.is_sub {
722+
unique_submodule_func_doc(py_name)
723+
} else {
724+
None
725+
}
726+
})
727+
});
727728
let doc = match doc {
728729
Some(doc) => quote!(Some(#doc)),
729730
None => quote!(None),

‎crates/derive-impl/src/pystructseq.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::util::{ItemMeta, ItemMetaInner};
22
use proc_macro2::TokenStream;
33
use quote::{format_ident, quote};
44
use syn::{DeriveInput, Ident, Item, Result};
5-
use syn_ext::ext::{AttributeExt, GetIdent};
5+
use syn_ext::ext::{AttributeExt, AttributeIteratorExt, GetIdent};
66
use syn_ext::types::{Meta, PunctuatedNestedMeta};
77

88
// #[pystruct_sequence_data] - For Data structs
@@ -524,13 +524,18 @@ pub(crate) fn impl_pystruct_sequence(
524524
} else {
525525
class_name.clone()
526526
};
527-
let doc = rustpython_doc::get(module_class_name.as_str()).or_else(|| {
527+
let db_doc = rustpython_doc::get(module_class_name.as_str()).or_else(|| {
528528
// os re-exports the posix/nt struct sequences.
529529
let class_name = module_class_name.strip_prefix("os.")?;
530530
rustpython_doc::get(&format!("posix.{class_name}"))
531531
.or_else(|| rustpython_doc::get(&format!("nt.{class_name}")))
532532
});
533-
let doc = doc.filter(|doc| !doc.is_empty());
533+
let db_doc = db_doc.filter(|doc| !doc.is_empty()).map(str::to_owned);
534+
let doc = struct_item
535+
.attrs
536+
.doc()
537+
.filter(|doc| !doc.is_empty())
538+
.or(db_doc);
534539
let doc = match doc {
535540
Some(doc) => quote!(Some(#doc)),
536541
None => quote!(None),

‎crates/derive-impl/src/util.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -867,8 +867,9 @@ fn args_const(pieces: &[SigPiece]) -> TokenStream {
867867

868868
/// Expression of type `Option<&'static str>`: the internal doc, or the plain
869869
/// doc when the arguments cannot form a signature.
870-
/// `doc` is a const `Option<&'static str>`. A table entry wins; an empty
871-
/// string is no docstring. The text is composed into the internal doc so
870+
/// `doc` is a const `Option<&'static str>` already chosen by the caller
871+
/// (a Rust doc comment wins over the stored docstring). An empty string is
872+
/// no docstring. The text is composed into the internal doc so
872873
/// `__text_signature__` stays on the signature half.
873874
pub(crate) fn internal_doc_tokens(
874875
sig: &Signature,

‎crates/derive/src/lib.rs‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ pub fn derive_from_args(input: TokenStream) -> TokenStream {
142142
/// ...
143143
/// }
144144
/// ```
145+
/// ## Docstrings
146+
/// A `///` doc comment on the struct, or on a `#[pymethod]` / `#[pyclassmethod]` /
147+
/// `#[pystaticmethod]` / `#[pygetset]` / `#[pymember]` item, is the Python
148+
/// docstring and overrides the stored documentation. The stored documentation
149+
/// is used only when that item has no `///` comment. Use `//` for developer notes.
145150
/// ## Inner markers
146151
/// ### pymethod/pyclassmethod/pystaticmethod
147152
/// `pymethod` is used to mark a method of the Python class.
@@ -231,13 +236,16 @@ pub fn derive_from_args(input: TokenStream) -> TokenStream {
231236
/// - `audit_read`: audit `object.__getattr__` before the load.
232237
/// - `name`: Python attribute name. Defaults to the field name.
233238
/// - `path`: subfield of the annotated field (`value` with `path = "re"`).
234-
/// - `doc`: `doc = "text"` is that docstring and does not consult the stored
235-
/// attribute documentation. `doc = false` stores no docstring. When `doc` is
236-
/// omitted, the docstring is the stored attribute documentation.
239+
/// - `doc`: only `doc = false`, which stores no docstring. Otherwise the
240+
/// docstring is the field's `///` comment when present, and the stored
241+
/// attribute documentation when the field has no `///` comment. Every
242+
/// `#[pymember]` on one field shares that comment. A string `doc` is rejected.
237243
///
238244
/// A struct-level `#[pymember]` (after `#[pyclass]`) has no field. `offset` is
239-
/// required there and rejected on a field. One field may carry several
240-
/// `#[pymember]` attributes. A `#[cfg]` on the field gates that entry.
245+
/// required there and rejected on a field. `///` lines placed immediately above
246+
/// that attribute are its docstring; otherwise the stored attribute
247+
/// documentation is used. One field may carry several `#[pymember]` attributes.
248+
/// A `#[cfg]` on the field gates that entry.
241249
/// ```rust, ignore
242250
/// #[pymember(name = "fget")]
243251
/// getter: PyAtomicRef<Option<PyObject>>,
@@ -255,6 +263,8 @@ pub fn pyclass(attr: TokenStream, item: TokenStream) -> TokenStream {
255263

256264
/// Helper macro to define `Exception` types.
257265
/// More-or-less is an alias to `pyclass` macro.
266+
/// A `///` comment on the exception type is its docstring and overrides the
267+
/// stored documentation. Use `//` for developer notes.
258268
///
259269
/// This macro serves a goal of generating multiple
260270
/// `BaseException` / `Exception`
@@ -272,6 +282,10 @@ pub fn pyexception(attr: TokenStream, item: TokenStream) -> TokenStream {
272282
/// It defines a Python module in the form of a `module_def` function in the module;
273283
/// this has to be used in a `add_native_module` to properly register the module.
274284
/// Additionally, this macro defines 'MODULE_NAME' and 'DOC' in the module.
285+
/// A `///` comment on the module is its docstring and overrides the stored
286+
/// module documentation. `#[pyfunction]` works the same way: its `///` comment
287+
/// is the function docstring and overrides the stored function documentation.
288+
/// Use `//` for developer notes.
275289
/// # Arguments
276290
/// - `name`: the name of the python module,
277291
/// by default, it is the name of the module, but this can be configured.
@@ -343,7 +357,8 @@ pub fn pymodule(attr: TokenStream, item: TokenStream) -> TokenStream {
343357
/// Attribute macro for defining Python struct sequence types.
344358
///
345359
/// This macro is applied to an empty struct to create a Python type
346-
/// that wraps a Data struct.
360+
/// that wraps a Data struct. A `///` comment on that struct is the type's
361+
/// docstring and overrides the stored documentation. Use `//` for developer notes.
347362
///
348363
/// # Example
349364
/// ```ignore

‎crates/stdlib/src/_asyncio.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ pub(crate) mod _asyncio {
111111
}
112112
}
113113

114-
/// asyncio.Future implementation
114+
// asyncio.Future implementation
115115
#[pyattr]
116116
#[pyclass(name = "Future", module = "_asyncio", traverse)]
117117
#[derive(Debug, PyPayload)]

0 commit comments

Comments
 (0)