Skip to content

Commit 918eb6a

Browse files
DaniPopesDaniPopes
authored
fix: inline config locations (#11883)
1 parent b25cc1b commit 918eb6a

File tree

4 files changed

+135
-103
lines changed

4 files changed

+135
-103
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/config/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ clap = { version = "4", features = ["derive"] }
5353
path-slash = "0.2"
5454

5555
[dev-dependencies]
56+
snapbox.workspace = true
5657
similar-asserts.workspace = true
5758
figment = { workspace = true, features = ["test"] }
5859
tempfile.workspace = true

‎crates/config/src/inline/natspec.rs‎

Lines changed: 132 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ use foundry_compilers::{
66
};
77
use itertools::Itertools;
88
use serde_json::Value;
9-
use solar::ast;
9+
use solar::{
10+
ast::{self, Span},
11+
interface::Session,
12+
};
1013
use std::{collections::BTreeMap, path::Path};
1114

1215
/// Convenient struct to hold in-line per-test configurations
@@ -16,7 +19,7 @@ pub struct NatSpec {
1619
pub contract: String,
1720
/// The function annotated with the natspec. None if the natspec is contract-level.
1821
pub function: Option<String>,
19-
/// The line the natspec appears, in the form `row:col:length`, i.e. `10:21:122`.
22+
/// The line the natspec begins, in the form `line:column`, i.e. `10:21`.
2023
pub line: String,
2124
/// The actual natspec comment, without slashes or block punctuation.
2225
pub docs: String,
@@ -30,7 +33,8 @@ impl NatSpec {
3033
pub fn parse(output: &ProjectCompileOutput, root: &Path) -> Vec<Self> {
3134
let mut natspecs: Vec<Self> = vec![];
3235

33-
let solar = SolarParser::new();
36+
let compiler = output.parser().solc().compiler();
37+
let solar = SolarParser::new(compiler.sess());
3438
let solc = SolcParser::new();
3539
for (id, artifact) in output.artifact_ids() {
3640
let path = id.source.as_path();
@@ -41,7 +45,6 @@ impl NatSpec {
4145
let contract = format!("{}:{}", path.display(), id.name);
4246

4347
let mut used_solar = false;
44-
let compiler = output.parser().solc().compiler();
4548
compiler.enter_sequential(|compiler| {
4649
if let Some((_, source)) = compiler.gcx().get_ast_source(abs_path)
4750
&& let Some(ast) = &source.ast
@@ -214,13 +217,13 @@ impl SolcParser {
214217
}
215218
}
216219

217-
struct SolarParser {
218-
_private: (),
220+
struct SolarParser<'a> {
221+
sess: &'a Session,
219222
}
220223

221-
impl SolarParser {
222-
fn new() -> Self {
223-
Self { _private: () }
224+
impl<'a> SolarParser<'a> {
225+
fn new(sess: &'a Session) -> Self {
226+
Self { sess }
224227
}
225228

226229
fn parse_ast(
@@ -234,6 +237,7 @@ impl SolarParser {
234237
if item.docs.is_empty() {
235238
return;
236239
}
240+
let mut span = Span::DUMMY;
237241
let lines = item
238242
.docs
239243
.iter()
@@ -242,6 +246,7 @@ impl SolarParser {
242246
if !s.contains(INLINE_CONFIG_PREFIX) {
243247
return None;
244248
}
249+
span = if span.is_dummy() { d.span } else { span.to(d.span) };
245250
match d.kind {
246251
ast::CommentKind::Line => Some(s.trim().to_string()),
247252
ast::CommentKind::Block => Some(
@@ -257,8 +262,6 @@ impl SolarParser {
257262
if lines.is_empty() {
258263
return;
259264
}
260-
let span =
261-
item.docs.iter().map(|doc| doc.span).reduce(|a, b| a.to(b)).unwrap_or_default();
262265
natspecs.push(NatSpec {
263266
contract: contract_id.to_string(),
264267
function: if let ast::ItemKind::Function(f) = &item.kind {
@@ -271,7 +274,10 @@ impl SolarParser {
271274
} else {
272275
None
273276
},
274-
line: format!("{}:{}:0", span.lo().0, span.hi().0),
277+
line: {
278+
let (_, loc) = self.sess.source_map().span_to_location_info(span);
279+
format!("{}:{}", loc.lo.line, loc.lo.col.0 + 1)
280+
},
275281
docs: lines,
276282
});
277283
};
@@ -298,12 +304,10 @@ impl SolarParser {
298304
mod tests {
299305
use super::*;
300306
use serde_json::json;
307+
use snapbox::{assert_data_eq, str};
301308
use solar::parse::{
302309
Parser,
303-
ast::{
304-
Arena,
305-
interface::{self, Session},
306-
},
310+
ast::{Arena, interface},
307311
};
308312

309313
fn parse(natspecs: &mut Vec<NatSpec>, src: &str, contract_id: &str, contract_name: &str) {
@@ -315,6 +319,7 @@ mod tests {
315319
let sess = Session::builder()
316320
.with_silent_emitter(Some("Inline config parsing failed".to_string()))
317321
.build();
322+
let solar = SolarParser::new(&sess);
318323
let _ = sess.enter(|| -> interface::Result<()> {
319324
let arena = Arena::new();
320325

@@ -327,7 +332,7 @@ mod tests {
327332

328333
let source_unit = parser.parse_file().map_err(|e| e.emit())?;
329334

330-
SolarParser::new().parse_ast(natspecs, &source_unit, contract_id, contract_name);
335+
solar.parse_ast(natspecs, &source_unit, contract_id, contract_name);
331336

332337
Ok(())
333338
});
@@ -388,40 +393,45 @@ function f2() {} /** forge-config: default.fuzz.runs = 800 */ function f3() {}
388393
}
389394
";
390395
let mut natspecs = vec![];
391-
let id = || "path.sol:C".to_string();
392-
parse(&mut natspecs, src, &id(), "C");
393-
assert_eq!(
394-
natspecs,
395-
[
396-
// f1
397-
NatSpec {
398-
contract: id(),
399-
function: Some("f1".to_string()),
400-
line: "14:134:0".to_string(),
401-
docs: "forge-config: default.fuzz.runs = 600\nforge-config: default.fuzz.runs = 601".to_string(),
402-
},
403-
// f2
404-
NatSpec {
405-
contract: id(),
406-
function: Some("f2".to_string()),
407-
line: "164:208:0".to_string(),
408-
docs: "forge-config: default.fuzz.runs = 700".to_string(),
409-
},
410-
// f3
411-
NatSpec {
412-
contract: id(),
413-
function: Some("f3".to_string()),
414-
line: "226:270:0".to_string(),
415-
docs: "forge-config: default.fuzz.runs = 800".to_string(),
416-
},
417-
// f4
418-
NatSpec {
419-
contract: id(),
420-
function: Some("f4".to_string()),
421-
line: "289:391:0".to_string(),
422-
docs: "forge-config: default.fuzz.runs = 1024\nforge-config: default.fuzz.max-test-rejects = 500".to_string(),
423-
},
424-
]
396+
parse(&mut natspecs, src, "path.sol:C", "C");
397+
assert_data_eq!(
398+
format!("{natspecs:#?}"),
399+
str![[r#"
400+
[
401+
NatSpec {
402+
contract: "path.sol:C",
403+
function: Some(
404+
"f1",
405+
),
406+
line: "2:14",
407+
docs: "forge-config: default.fuzz.runs = 600/nforge-config: default.fuzz.runs = 601",
408+
},
409+
NatSpec {
410+
contract: "path.sol:C",
411+
function: Some(
412+
"f2",
413+
),
414+
line: "7:8",
415+
docs: "forge-config: default.fuzz.runs = 700",
416+
},
417+
NatSpec {
418+
contract: "path.sol:C",
419+
function: Some(
420+
"f3",
421+
),
422+
line: "8:18",
423+
docs: "forge-config: default.fuzz.runs = 800",
424+
},
425+
NatSpec {
426+
contract: "path.sol:C",
427+
function: Some(
428+
"f4",
429+
),
430+
line: "10:1",
431+
docs: "forge-config: default.fuzz.runs = 1024/nforge-config: default.fuzz.max-test-rejects = 500",
432+
},
433+
]
434+
"#]]
425435
);
426436
}
427437

@@ -444,18 +454,21 @@ contract FuzzInlineConf is DSTest {
444454
}
445455
"#;
446456
let mut natspecs = vec![];
447-
let id = || "inline/FuzzInlineConf.t.sol:FuzzInlineConf".to_string();
448-
parse(&mut natspecs, src, &id(), "FuzzInlineConf");
449-
assert_eq!(
450-
natspecs,
451-
[
452-
NatSpec {
453-
contract: id(),
454-
function: Some("testInlineConfFuzz".to_string()),
455-
line: "141:255:0".to_string(),
456-
docs: "forge-config: default.fuzz.runs = 1024\nforge-config: default.fuzz.max-test-rejects = 500".to_string(),
457-
},
458-
]
457+
parse(&mut natspecs, src, "inline/FuzzInlineConf.t.sol:FuzzInlineConf", "FuzzInlineConf");
458+
assert_data_eq!(
459+
format!("{natspecs:#?}"),
460+
str![[r#"
461+
[
462+
NatSpec {
463+
contract: "inline/FuzzInlineConf.t.sol:FuzzInlineConf",
464+
function: Some(
465+
"testInlineConfFuzz",
466+
),
467+
line: "8:5",
468+
docs: "forge-config: default.fuzz.runs = 1024/nforge-config: default.fuzz.max-test-rejects = 500",
469+
},
470+
]
471+
"#]]
459472
);
460473
}
461474

@@ -532,30 +545,44 @@ contract FuzzInlineConf2 is DSTest {
532545
}
533546
"#;
534547
let mut natspecs = vec![];
535-
let id = || "inline/FuzzInlineConf.t.sol:FuzzInlineConf".to_string();
536-
parse(&mut natspecs, src, &id(), "FuzzInlineConf");
537-
assert_eq!(
538-
natspecs,
539-
[NatSpec {
540-
contract: id(),
541-
function: Some("testInlineConfFuzz1".to_string()),
542-
line: "142:181:0".to_string(),
543-
docs: "forge-config: default.fuzz.runs = 1".to_string(),
544-
},]
548+
parse(&mut natspecs, src, "inline/FuzzInlineConf.t.sol:FuzzInlineConf", "FuzzInlineConf");
549+
assert_data_eq!(
550+
format!("{natspecs:#?}"),
551+
str![[r#"
552+
[
553+
NatSpec {
554+
contract: "inline/FuzzInlineConf.t.sol:FuzzInlineConf",
555+
function: Some(
556+
"testInlineConfFuzz1",
557+
),
558+
line: "8:6",
559+
docs: "forge-config: default.fuzz.runs = 1",
560+
},
561+
]
562+
"#]]
545563
);
546564

547565
let mut natspecs = vec![];
548-
let id = || "inline/FuzzInlineConf2.t.sol:FuzzInlineConf2".to_string();
549-
parse(&mut natspecs, src, &id(), "FuzzInlineConf2");
550-
assert_eq!(
551-
natspecs,
552-
[NatSpec {
553-
contract: id(),
554-
function: Some("testInlineConfFuzz2".to_string()),
555-
line: "264:303:0".to_string(),
556-
// should not get config from previous contract
557-
docs: "forge-config: default.fuzz.runs = 2".to_string(),
558-
},]
566+
parse(
567+
&mut natspecs,
568+
src,
569+
"inline/FuzzInlineConf2.t.sol:FuzzInlineConf2",
570+
"FuzzInlineConf2",
571+
);
572+
assert_data_eq!(
573+
format!("{natspecs:#?}"),
574+
str![[r#"
575+
[
576+
NatSpec {
577+
contract: "inline/FuzzInlineConf2.t.sol:FuzzInlineConf2",
578+
function: Some(
579+
"testInlineConfFuzz2",
580+
),
581+
line: "13:5",
582+
docs: "forge-config: default.fuzz.runs = 2",
583+
},
584+
]
585+
"#]]
559586
);
560587
}
561588

@@ -575,24 +602,27 @@ contract FuzzInlineConf is DSTest {
575602
function testInlineConfFuzz2() {}
576603
}"#;
577604
let mut natspecs = vec![];
578-
let id = || "inline/FuzzInlineConf.t.sol:FuzzInlineConf".to_string();
579-
parse(&mut natspecs, src, &id(), "FuzzInlineConf");
580-
assert_eq!(
581-
natspecs,
582-
[
583-
NatSpec {
584-
contract: id(),
585-
function: None,
586-
line: "101:140:0".to_string(),
587-
docs: "forge-config: default.fuzz.runs = 1".to_string(),
588-
},
589-
NatSpec {
590-
contract: id(),
591-
function: Some("testInlineConfFuzz1".to_string()),
592-
line: "181:220:0".to_string(),
593-
docs: "forge-config: default.fuzz.runs = 3".to_string(),
594-
}
595-
]
605+
parse(&mut natspecs, src, "inline/FuzzInlineConf.t.sol:FuzzInlineConf", "FuzzInlineConf");
606+
assert_data_eq!(
607+
format!("{natspecs:#?}"),
608+
str![[r#"
609+
[
610+
NatSpec {
611+
contract: "inline/FuzzInlineConf.t.sol:FuzzInlineConf",
612+
function: None,
613+
line: "7:1",
614+
docs: "forge-config: default.fuzz.runs = 1",
615+
},
616+
NatSpec {
617+
contract: "inline/FuzzInlineConf.t.sol:FuzzInlineConf",
618+
function: Some(
619+
"testInlineConfFuzz1",
620+
),
621+
line: "9:5",
622+
docs: "forge-config: default.fuzz.runs = 3",
623+
},
624+
]
625+
"#]]
596626
);
597627
}
598628
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ forgetest!(invalid_profile, |prj, cmd| {
6363
);
6464

6565
cmd.arg("test").assert_failure().stderr_eq(str![[r#"
66-
Error: Inline config error at test/inline.sol:80:123:0: invalid profile `unknown.fuzz.runs = 2`; valid profiles: default
66+
Error: Inline config error at test/inline.sol:4:9: invalid profile `unknown.fuzz.runs = 2`; valid profiles: default
6767
6868
"#]]);
6969
});

0 commit comments

Comments
 (0)