feat(doc): inherit natspec implicitly from base members#15774
Conversation
An override without `@inheritdoc` rendered with empty descriptions even when the base interface documents the member. When no explicit `@inheritdoc` tag is present, resolve the natspec from the nearest documented matching member in the contract's linearized bases, the same order Solidity uses to resolve inheritance. Explicit `@inheritdoc` still takes precedence, and the overload disambiguation by parameter types applies unchanged since the implicit path reuses the same walker as the explicit resolver. Closes foundry-rs#4070
| }) | ||
| .or_else(|| { | ||
| hir_id.and_then(|cid| { | ||
| hir_ext::resolve_implicit_inheritdoc( |
There was a problem hiding this comment.
Implicit inheritance needs to be gated on the declaration having no local NatSpec. As written, a local @notice still pulls missing @dev, @param, and @return tags from the base. Solidity only auto-inherits for functions without NatSpec (and also excludes renamed parameters); partial merging should remain exclusive to explicit @inheritdoc.
|
|
||
| /// Walks `search_contracts` in order and returns the first documented state variable or | ||
| /// zero-parameter getter function matching `var_name`. | ||
| fn search_variable_doc( |
There was a problem hiding this comment.
This limits public-variable inheritance to scalar zero-argument getters. A common implementation such as mapping(address => uint256) public override balanceOf has getter signature balanceOf(address) and cannot inherit the interface getter's NatSpec. Since the PR promises public state variables generally, please match through the generated HIR getter signature and render its input/return types, or narrow the stated scope to zero-argument scalar getters.
Address the review on implicit NatSpec inheritance: - Run implicit inheritance only when the member carries no `@inheritdoc` tag and no NatSpec of its own. Solidity auto-inherits only for undocumented members, so a local `@notice` no longer pulls the base `@param`/`@return`, and an explicit `@inheritdoc` that fails to resolve no longer falls through to a different base. Partial merging stays exclusive to explicit `@inheritdoc`. - Match a function override by its exact parameter signature at every base level when the signature is known, continuing upward past a nearer base that declares a different same-name overload, instead of stopping at the first name match. - Match a public variable through its compiler-generated getter signature, so `mapping(address => uint256) public balanceOf` inherits the interface getter `balanceOf(address)`, not only scalar zero-argument getters.
0xMars42
left a comment
0xMars42
left a comment
There was a problem hiding this comment.
Thanks, all four are addressed in 1448b76.
- Fall-through and partial-merge gating: implicit inheritance now runs only when the member has no
@inheritdoctag and no NatSpec of its own. A local@noticeno longer pulls the base@param/@return, and an explicit@inheritdocthat fails to resolve no longer falls through to a different base. Partial merging stays exclusive to explicit@inheritdoc. - Overload signature: a function is now matched by its exact parameter signature at every base level when the signature is known, continuing upward past a nearer base that declares a different same-name overload instead of stopping at it.
- Public variable getter: the variable is matched through its compiler-generated getter signature, so
mapping(address => uint256) public balanceOfinherits the interface getterbalanceOf(address), instead of only scalar zero-argument getters.
Regression tests cover each: a documented override that inherits nothing, an override picking the far base's matching overload, and a mapping getter inheriting the interface docs.
Compare compiler-resolved parameter types instead of source spellings when matching inherited members, for functions and public variable getters alike. Equivalent spellings of the same type, aliases and qualified names now match, overloads with non-ABI types are disambiguated correctly, and calldata locations are normalized to memory so legal override variations still compare equal.
| if search_contracts.is_empty() { | ||
| return None; | ||
| } | ||
| search_function_doc(gcx, &search_contracts, fn_name, target) |
There was a problem hiding this comment.
Implicit inheritance must stop when more than one base function matches; Solidity requires explicit @inheritdoc for this ambiguous case. This currently picks the first documented base in linearization order.
| // documented match. | ||
| Some(want) => { | ||
| for &fid in &name_matches { | ||
| if resolved_param_types(gcx, fid) == Some(want) |
There was a problem hiding this comment.
Resolved types are not sufficient for implicit inheritance: Solidity excludes overrides whose parameter names differ. Please require matching names on the implicit path; explicit @inheritdoc can keep the existing normalization.
| if search_contracts.is_empty() { | ||
| return None; | ||
| } | ||
| let getter = variable_getter(gcx, contract_id, var_name); |
There was a problem hiding this comment.
Please require a generated getter here. As written, private/internal variables can implicitly inherit docs from same-name base variables/functions; this feature should apply only to public state-variable getters.
| }; | ||
| if f.name.map(|n| n.as_str() == var_name).unwrap_or(false) | ||
| && function_param_types(gcx, fid).map(|p| p.is_empty()).unwrap_or(false) | ||
| && signature_matches |
There was a problem hiding this comment.
Matching the getter inputs is only half the fix: rendering still drops inherited @param docs and reports the variable declaration type (for example, the mapping) instead of the getter return type. Please carry the getter signature through to rendering and snapshot @param/@return output.
| doc.natspec.iter().any(|item| { | ||
| matches!( | ||
| item.kind, | ||
| NatSpecKind::Notice |
There was a problem hiding this comment.
A local @custom:* tag is NatSpec too, so it must suppress automatic inheritance. Please treat every local NatSpec item except @inheritdoc as local documentation.
3 participants
Motivation
Closes #4070. Repositories that declare all natspec at the interface level render with empty descriptions unless every override carries an explicit
@inheritdoctag: an undocumentedfunction doThing(uint256 value) external overriderenders its parameter and return descriptions empty even whenIThing.doThingdocuments everything.Solution
When a function or public state variable has no
@inheritdoctag, resolve its natspec from the nearest documented matching member in the contract's linearized bases, following Solidity's linearization order. Explicit@inheritdockeeps taking precedence, and the overload disambiguation by parameter types applies unchanged: the implicit path reuses the same walker as the explicit resolver, extracted rather than duplicated.One design choice worth calling out: the issue suggests requiring
@inheritdocwhen a function overrides members from several bases. This implementation instead inherits from the nearest documented one in linearization order, which is the order Solidity itself resolves inheritance in, and@inheritdocstays available to pick a different base explicitly. Happy to make the ambiguous case stricter if preferred.The regression test documents
depositon the interface only and asserts the override's page renders the notice, parameter and return descriptions. The pre-existing doc tests, including the explicit@inheritdocones, pass unchanged.PR Checklist