Skip to content

Commit a969aef

Browse files
wellnanawellnana
authored
fix(forge): handle overloaded functions correctly in doc (#12040)
* handle overloaded functions correctly * change from review & add test * fix the rest of dup issue
1 parent 92d7b35 commit a969aef

File tree

5 files changed

+170
-15
lines changed

5 files changed

+170
-15
lines changed

‎crates/doc/src/helpers.rs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
1+
use itertools::Itertools;
2+
use solang_parser::pt::FunctionDefinition;
13
use toml::{Value, value::Table};
24

5+
/// Generates a function signature with parameter types (e.g., "functionName(type1,type2)").
6+
/// Returns the function name without parameters if the function has no parameters.
7+
pub fn function_signature(func: &FunctionDefinition) -> String {
8+
let func_name = func.name.as_ref().map_or(func.ty.to_string(), |n| n.name.to_owned());
9+
if func.params.is_empty() {
10+
return func_name;
11+
}
12+
13+
format!(
14+
"{}({})",
15+
func_name,
16+
func.params
17+
.iter()
18+
.map(|p| p.1.as_ref().map(|p| p.ty.to_string()).unwrap_or_default())
19+
.join(",")
20+
)
21+
}
22+
323
/// Merge original toml table with the override.
424
pub(crate) fn merge_toml_table(table: &mut Table, override_table: Table) {
525
for (key, override_value) in override_table {
@@ -26,3 +46,74 @@ pub(crate) fn merge_toml_table(table: &mut Table, override_table: Table) {
2646
};
2747
}
2848
}
49+
50+
#[cfg(test)]
51+
mod tests {
52+
use super::*;
53+
use solang_parser::{
54+
parse,
55+
pt::{ContractPart, SourceUnit, SourceUnitPart},
56+
};
57+
58+
#[test]
59+
fn test_function_signature_no_params() {
60+
let (source_unit, _) = parse(
61+
r#"
62+
contract Test {
63+
function foo() public {}
64+
}
65+
"#,
66+
0,
67+
)
68+
.unwrap();
69+
70+
let func = extract_function(&source_unit);
71+
assert_eq!(function_signature(func), "foo");
72+
}
73+
74+
#[test]
75+
fn test_function_signature_with_params() {
76+
let (source_unit, _) = parse(
77+
r#"
78+
contract Test {
79+
function transfer(address to, uint256 amount) public {}
80+
}
81+
"#,
82+
0,
83+
)
84+
.unwrap();
85+
86+
let func = extract_function(&source_unit);
87+
assert_eq!(function_signature(func), "transfer(address,uint256)");
88+
}
89+
90+
#[test]
91+
fn test_function_signature_constructor() {
92+
let (source_unit, _) = parse(
93+
r#"
94+
contract Test {
95+
constructor(address owner) {}
96+
}
97+
"#,
98+
0,
99+
)
100+
.unwrap();
101+
102+
let func = extract_function(&source_unit);
103+
assert_eq!(function_signature(func), "constructor(address)");
104+
}
105+
106+
/// Helper to extract the first function from a parsed source unit
107+
fn extract_function(source_unit: &SourceUnit) -> &FunctionDefinition {
108+
for part in &source_unit.0 {
109+
if let SourceUnitPart::ContractDefinition(contract) = part {
110+
for part in &contract.parts {
111+
if let ContractPart::FunctionDefinition(func) = part {
112+
return func;
113+
}
114+
}
115+
}
116+
}
117+
panic!("No function found in source unit");
118+
}
119+
}

‎crates/doc/src/parser/item.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{Comments, solang_ext::SafeUnwrap};
1+
use crate::{Comments, helpers::function_signature, solang_ext::SafeUnwrap};
22
use solang_parser::pt::{
33
ContractDefinition, ContractTy, EnumDefinition, ErrorDefinition, EventDefinition,
44
FunctionDefinition, StructDefinition, TypeDefinition, VariableDefinition,
@@ -169,6 +169,14 @@ impl ParseSource {
169169
}
170170
}
171171

172+
/// Get the signature of the source (for functions, includes parameter types)
173+
pub fn signature(&self) -> String {
174+
match self {
175+
Self::Function(func) => function_signature(func),
176+
_ => self.ident(),
177+
}
178+
}
179+
172180
/// Get the range of this item in the source file.
173181
pub fn range(&self) -> Range<usize> {
174182
match self {

‎crates/doc/src/preprocessor/inheritdoc.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ impl Inheritdoc {
7878
// https://docs.soliditylang.org/en/v0.8.17/natspec-format.html#tags
7979

8080
for children in &item.children {
81-
// TODO: improve matching logic
82-
if source.ident() == children.source.ident() {
83-
let key = format!("{}.{}", base, source.ident());
81+
// Match using signature for functions (includes parameter types for overloads)
82+
if source.signature() == children.source.signature() {
83+
let key = format!("{}.{}", base, source.signature());
8484
return Some((key, children.comments.clone()));
8585
}
8686
}

‎crates/doc/src/writer/as_doc.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::{
22
CONTRACT_INHERITANCE_ID, CommentTag, Comments, CommentsRef, DEPLOYMENTS_ID, Document,
33
GIT_SOURCE_ID, INHERITDOC_ID, Markdown, PreprocessorOutput,
44
document::{DocumentContent, read_context},
5+
helpers::function_signature,
56
parser::ParseSource,
67
solang_ext::SafeUnwrap,
78
writer::BufWriter,
@@ -98,16 +99,7 @@ impl AsDoc for Document {
9899

99100
for item in items {
100101
let func = item.as_function().unwrap();
101-
let mut heading = item.source.ident();
102-
if !func.params.is_empty() {
103-
heading.push_str(&format!(
104-
"({})",
105-
func.params
106-
.iter()
107-
.map(|p| p.1.as_ref().map(|p| p.ty.to_string()).unwrap_or_default())
108-
.join(", ")
109-
));
110-
}
102+
let heading = function_signature(func).replace(',', ", ");
111103
writer.write_heading(&heading)?;
112104
writer.write_section(&item.comments, &item.code)?;
113105
}
@@ -300,9 +292,10 @@ impl Document {
300292
comments: &Comments,
301293
code: &str,
302294
) -> Result<(), std::fmt::Error> {
295+
let func_sign = function_signature(func);
303296
let func_name = func.name.as_ref().map_or(func.ty.to_string(), |n| n.name.to_owned());
304297
let comments =
305-
comments.merge_inheritdoc(&func_name, read_context!(self, INHERITDOC_ID, Inheritdoc));
298+
comments.merge_inheritdoc(&func_sign, read_context!(self, INHERITDOC_ID, Inheritdoc));
306299

307300
// Write function name
308301
writer.write_heading(&func_name)?;

‎crates/forge/tests/cli/doc.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,66 @@ fn can_generate_solmate_docs() {
66
setup_forge_remote(RemoteProject::new("transmissions11/solmate").set_build(false));
77
prj.forge_command().args(["doc", "--build"]).assert_success();
88
}
9+
10+
// Test that overloaded functions in interfaces inherit the correct NatSpec comments
11+
// fixes <https://github.com/foundry-rs/foundry/issues/11823>
12+
forgetest_init!(can_generate_docs_for_overloaded_functions, |prj, cmd| {
13+
prj.add_source(
14+
"IExample.sol",
15+
r#"
16+
// SPDX-License-Identifier: MIT
17+
pragma solidity ^0.8.0;
18+
19+
interface IExample {
20+
/// @notice Process a single address
21+
/// @param addr The address to process
22+
function process(address addr) external;
23+
24+
/// @notice Process multiple addresses
25+
/// @param addrs The addresses to process
26+
function process(address[] calldata addrs) external;
27+
28+
/// @notice Process an address with a value
29+
/// @param addr The address to process
30+
/// @param value The value to use
31+
function process(address addr, uint256 value) external;
32+
}
33+
"#,
34+
);
35+
36+
prj.add_source(
37+
"Example.sol",
38+
r#"
39+
// SPDX-License-Identifier: MIT
40+
pragma solidity ^0.8.0;
41+
42+
import "./IExample.sol";
43+
44+
contract Example is IExample {
45+
/// @inheritdoc IExample
46+
function process(address addr) external {
47+
// Implementation for single address
48+
}
49+
50+
/// @inheritdoc IExample
51+
function process(address[] calldata addrs) external {
52+
// Implementation for multiple addresses
53+
}
54+
55+
/// @inheritdoc IExample
56+
function process(address addr, uint256 value) external {
57+
// Implementation for address with value
58+
}
59+
}
60+
"#,
61+
);
62+
63+
cmd.args(["doc", "--build"]).assert_success();
64+
65+
let doc_path = prj.root().join("docs/src/src/Example.sol/contract.Example.md");
66+
let content = std::fs::read_to_string(&doc_path).unwrap();
67+
68+
assert!(content.contains("Process a single address"));
69+
assert!(content.contains("Process multiple addresses"));
70+
assert!(content.contains("Process an address with a value"));
71+
});

0 commit comments

Comments
 (0)