Skip to content

Commit e7be59e

Browse files
DaniPopesDaniPopes
authored
chore: clean up fmt entry point (#11879)
* chore: clean up fmt entry point * fix: stdin
1 parent 045f2d7 commit e7be59e

File tree

5 files changed

+103
-117
lines changed

5 files changed

+103
-117
lines changed

‎crates/cast/tests/cli/main.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use foundry_test_utils::{
1616
str,
1717
util::OutputExt,
1818
};
19-
use std::{fs, io::Write, path::Path, str::FromStr};
19+
use std::{fs, path::Path, str::FromStr};
2020

2121
#[macro_use]
2222
extern crate foundry_test_utils;
@@ -1241,11 +1241,9 @@ casttest!(rpc_raw_params, |_prj, cmd| {
12411241
casttest!(rpc_raw_params_stdin, |_prj, cmd| {
12421242
let eth_rpc_url = next_http_rpc_endpoint();
12431243

1244-
// Call `echo "\n[\n\"0x123\",\nfalse\n]\n" | cast rpc eth_getBlockByNumber --raw
1244+
// Call `echo "\n[\n\"0x123\",\nfalse\n]\n" | cast rpc eth_getBlockByNumber --raw
12451245
cmd.args(["rpc", "--rpc-url", eth_rpc_url.as_str(), "eth_getBlockByNumber", "--raw"]).stdin(
1246-
|mut stdin| {
1247-
stdin.write_all(b"\n[\n\"0x123\",\nfalse\n]\n").unwrap();
1248-
},
1246+
b"\n[\n\"0x123\",\nfalse\n]\n"
12491247
)
12501248
.assert_json_stdout(str![[r#"
12511249
{"number":"0x123","hash":"0xc5dab4e189004a1312e9db43a40abb2de91ad7dd25e75880bf36016d8e9df524","transactions":[],"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","extraData":"0x476574682f4c5649562f76312e302e302f6c696e75782f676f312e342e32","nonce":"0x29d6547c196e00e0","miner":"0xbb7b8287f3f0a933474a79eae42cbca977791171","difficulty":"0x494433b31","gasLimit":"0x1388","gasUsed":"0x0","uncles":[],"sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","size":"0x220","transactionsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","stateRoot":"0x3fe6bd17aa85376c7d566df97d9f2e536f37f7a87abb3a6f9e2891cf9442f2e4","mixHash":"0x943056aa305aa6d22a3c06110942980342d1f4d4b11c17711961436a0f963ea0","parentHash":"0x7abfd11e862ccde76d6ea8ee20978aac26f4bcb55de1188cc0335be13e817017","timestamp":"0x55ba4564"}

‎crates/forge/src/cmd/fmt.rs‎

Lines changed: 50 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use super::watch::WatchArgs;
22
use clap::{Parser, ValueHint};
3-
use eyre::{Context, Result};
3+
use eyre::Result;
44
use foundry_cli::utils::{FoundryPathExt, LoadConfig};
5-
use foundry_common::fs;
5+
use foundry_common::{errors::convert_solar_errors, fs};
66
use foundry_compilers::{compilers::solc::SolcLanguage, solc::SOLC_EXTENSIONS};
77
use foundry_config::{filter::expand_globs, impl_figment_convert_basic};
88
use rayon::prelude::*;
@@ -11,7 +11,7 @@ use solar::sema::Compiler;
1111
use std::{
1212
fmt::{self, Write},
1313
io,
14-
io::{Read, Write as _},
14+
io::Write as _,
1515
path::{Path, PathBuf},
1616
sync::Arc,
1717
};
@@ -69,11 +69,7 @@ impl FmtArgs {
6969
.collect();
7070
Input::Paths(project_paths)
7171
}
72-
[one] if one == Path::new("-") => {
73-
let mut s = String::new();
74-
io::stdin().read_to_string(&mut s).wrap_err("failed to read from stdin")?;
75-
Input::Stdin(s)
76-
}
72+
[one] if one == Path::new("-") => Input::Stdin,
7773
paths => {
7874
let mut inputs = Vec::with_capacity(paths.len());
7975
for path in paths {
@@ -99,88 +95,67 @@ impl FmtArgs {
9995
}
10096
};
10197

102-
// Handle stdin on its own
103-
if let Input::Stdin(original) = input {
104-
let formatted = forge_fmt::format(&original, config.fmt)
105-
.into_result()
106-
.map_err(|_| eyre::eyre!("failed to format stdin"))?;
107-
108-
let diff = TextDiff::from_lines(&original, &formatted);
109-
if diff.ratio() < 1.0 {
110-
if self.raw {
111-
sh_print!("{formatted}")?;
112-
} else {
113-
sh_print!("{}", format_diff_summary("stdin", &diff))?;
114-
}
115-
if self.check {
116-
std::process::exit(1);
117-
}
118-
}
119-
return Ok(());
120-
}
98+
let mut compiler = Compiler::new(
99+
solar::interface::Session::builder().with_buffer_emitter(Default::default()).build(),
100+
);
121101

122-
// Unwrap and check input paths
123-
let paths_to_fmt = match input {
124-
Input::Paths(paths) => {
125-
if paths.is_empty() {
102+
// Parse, format, and check the diffs.
103+
compiler.enter_mut(|compiler| {
104+
let mut pcx = compiler.parse();
105+
pcx.set_resolve_imports(false);
106+
match input {
107+
Input::Paths(paths) if paths.is_empty() => {
126108
sh_warn!(
127109
"Nothing to format.\n\
128-
HINT: If you are working outside of the project, \
129-
try providing paths to your source files: `forge fmt <paths>`"
110+
HINT: If you are working outside of the project, \
111+
try providing paths to your source files: `forge fmt <paths>`"
130112
)?;
131113
return Ok(());
132114
}
133-
paths
115+
Input::Paths(paths) => _ = pcx.par_load_files(paths),
116+
Input::Stdin => _ = pcx.load_stdin(),
134117
}
135-
Input::Stdin(_) => unreachable!(),
136-
};
137-
138-
let mut compiler = Compiler::new(
139-
solar::interface::Session::builder().with_buffer_emitter(Default::default()).build(),
140-
);
141-
142-
// Parse, format, and check the diffs.
143-
let res = compiler.enter_mut(|c| -> Result<()> {
144-
let mut pcx = c.parse();
145-
pcx.set_resolve_imports(false);
146-
let _ = pcx.par_load_files(paths_to_fmt);
147118
pcx.parse();
148119

149-
let gcx = c.gcx();
120+
let gcx = compiler.gcx();
150121
let fmt_config = Arc::new(config.fmt);
151122
let diffs: Vec<String> = gcx
152123
.sources
153124
.raw
154125
.par_iter()
155126
.filter_map(|source_unit| {
156-
let path = source_unit.file.name.as_real()?;
157-
let original = &source_unit.file.src;
127+
let path = source_unit.file.name.as_real();
128+
let original = source_unit.file.src.as_str();
158129
let formatted = forge_fmt::format_ast(gcx, source_unit, fmt_config.clone())?;
159130

160-
if original.as_str() == formatted {
131+
if original == formatted {
161132
return None;
162133
}
163134

164-
if self.check {
165-
let name =
166-
path.strip_prefix(&config.root).unwrap_or(path).display().to_string();
167-
let summary = format_diff_summary(
168-
&name,
169-
&TextDiff::from_lines(original.as_str(), &formatted),
170-
);
135+
if self.check || path.is_none() {
136+
let summary = if self.raw {
137+
formatted
138+
} else {
139+
let name = match path {
140+
Some(path) => path
141+
.strip_prefix(&config.root)
142+
.unwrap_or(path)
143+
.display()
144+
.to_string(),
145+
None => "stdin".to_string(),
146+
};
147+
format_diff_summary(&name, &TextDiff::from_lines(original, &formatted))
148+
};
171149
Some(Ok(summary))
172-
} else {
150+
} else if let Some(path) = path {
173151
match fs::write(path, formatted) {
174-
Ok(_) => {
175-
let _ = sh_println!("Formatted {}", path.display());
176-
None
177-
}
178-
Err(e) => Some(Err(eyre::eyre!(
179-
"Failed to write to {}: {}",
180-
path.display(),
181-
e
182-
))),
152+
Ok(()) => {}
153+
Err(e) => return Some(Err(e.into())),
183154
}
155+
let _ = sh_println!("Formatted {}", path.display());
156+
None
157+
} else {
158+
unreachable!()
184159
}
185160
})
186161
.collect::<Result<_>>()?;
@@ -194,15 +169,13 @@ impl FmtArgs {
194169
}
195170
let _ = stdout.write_all(diff.as_bytes());
196171
}
197-
std::process::exit(1);
172+
if self.check {
173+
std::process::exit(1);
174+
}
198175
}
199-
Ok(())
200-
});
201-
res?;
202176

203-
// TODO(dani): convert solar errors
204-
205-
Ok(())
177+
convert_solar_errors(compiler.dcx())
178+
})
206179
}
207180

208181
/// Returns whether `FmtArgs` was configured with `--watch`
@@ -211,14 +184,14 @@ impl FmtArgs {
211184
}
212185
}
213186

214-
struct Line(Option<usize>);
215-
216187
#[derive(Debug)]
217188
enum Input {
218-
Stdin(String),
189+
Stdin,
219190
Paths(Vec<PathBuf>),
220191
}
221192

193+
struct Line(Option<usize>);
194+
222195
impl fmt::Display for Line {
223196
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
224197
match self.0 {

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

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Integration tests for `forge fmt` command
22
33
use foundry_test_utils::{forgetest, forgetest_init};
4-
use std::{fs, io::Write};
54

65
const UNFORMATTED: &str = r#"// SPDX-License-Identifier: MIT
76
pragma solidity =0.8.30 ;
@@ -12,6 +11,7 @@ contract Test {
1211
value = _value ;
1312
}
1413
}"#;
14+
1515
const FORMATTED: &str = r#"// SPDX-License-Identifier: MIT
1616
pragma solidity =0.8.30;
1717
@@ -26,51 +26,66 @@ contract Test {
2626

2727
// Test that fmt can format a simple contract file
2828
forgetest_init!(fmt_file, |prj, cmd| {
29-
prj.add_source("FmtTest.sol", UNFORMATTED);
29+
prj.add_raw_source("FmtTest.sol", UNFORMATTED);
3030
cmd.arg("fmt").arg("src/FmtTest.sol");
31-
cmd.assert_success();
31+
cmd.assert_success().stdout_eq(str![[r#"
32+
Formatted [..]/src/FmtTest.sol
3233
33-
// Check that the file was formatted
34-
let formatted = fs::read_to_string(prj.root().join("src/FmtTest.sol")).unwrap();
35-
assert!(formatted.contains(FORMATTED));
34+
"#]]);
35+
assert_data_eq!(
36+
std::fs::read_to_string(prj.root().join("src/FmtTest.sol")).unwrap(),
37+
FORMATTED,
38+
);
3639
});
3740

3841
// Test that fmt can format from stdin
3942
forgetest!(fmt_stdin, |_prj, cmd| {
4043
cmd.args(["fmt", "-", "--raw"]);
41-
cmd.stdin(move |mut stdin| {
42-
stdin.write_all(UNFORMATTED.as_bytes()).unwrap();
43-
});
44-
45-
// Check the output contains formatted code
44+
cmd.stdin(UNFORMATTED.as_bytes());
4645
cmd.assert_success().stdout_eq(FORMATTED);
46+
47+
cmd.stdin(FORMATTED.as_bytes());
48+
cmd.assert_success().stdout_eq("");
4749
});
4850

4951
forgetest_init!(fmt_check_mode, |prj, cmd| {
5052
// Run fmt --check on a well-formatted file
51-
prj.add_source("Test.sol", FORMATTED);
53+
prj.add_raw_source("Test.sol", FORMATTED);
5254
cmd.arg("fmt").arg("--check").arg("src/Test.sol");
53-
cmd.assert_success();
55+
cmd.assert_success().stderr_eq("").stdout_eq("");
5456

5557
// Run fmt --check on a mal-formatted file
56-
prj.add_source("Test2.sol", UNFORMATTED);
57-
let mut cmd2 = prj.forge_command();
58-
cmd2.arg("fmt").arg("--check").arg("src/Test2.sol");
59-
cmd2.assert_failure();
58+
prj.add_raw_source("Test2.sol", UNFORMATTED);
59+
cmd.forge_fuse().arg("fmt").arg("--check").arg("src/Test2.sol");
60+
cmd.assert_failure();
6061
});
6162

6263
forgetest!(fmt_check_mode_stdin, |_prj, cmd| {
6364
// Run fmt --check with well-formatted stdin input
6465
cmd.arg("fmt").arg("-").arg("--check");
65-
cmd.stdin(move |mut stdin| {
66-
stdin.write_all(FORMATTED.as_bytes()).unwrap();
67-
});
68-
cmd.assert_success();
66+
cmd.stdin(FORMATTED.as_bytes());
67+
cmd.assert_success().stderr_eq("").stdout_eq("");
6968

7069
// Run fmt --check with mal-formatted stdin input
71-
cmd.arg("fmt").arg("-").arg("--check");
72-
cmd.stdin(move |mut stdin| {
73-
stdin.write_all(UNFORMATTED.as_bytes()).unwrap();
74-
});
75-
cmd.assert_failure();
70+
cmd.stdin(UNFORMATTED.as_bytes());
71+
cmd.assert_failure().stderr_eq("").stdout_eq(str![[r#"
72+
Diff in stdin:
73+
1 1 | // SPDX-License-Identifier: MIT
74+
2 |-pragma solidity =0.8.30 ;
75+
2 |+pragma solidity =0.8.30;
76+
3 3 |
77+
4 |-contract Test {
78+
5 |- uint256 public value ;
79+
6 |- function setValue ( uint256 _value ) public {
80+
7 |- value = _value ;
81+
4 |+contract Test {
82+
5 |+ uint256 public value;
83+
6 |+
84+
7 |+ function setValue(uint256 _value) public {
85+
8 |+ value = _value;
86+
8 9 | }
87+
9 |-}
88+
10 |+}
89+
90+
"#]]);
7691
});

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use foundry_test_utils::util::ExtTester;
22

33
/// Test `forge fmt` immutability.
44
/// TODO: make sure original fmt is not changed after projects format and rev available.
5-
/// TODO: enable win test after <https://github.com/foundry-rs/foundry/issues/11841> fixed.
65
macro_rules! fmt_test {
76
($name:ident, $org:expr, $repo:expr, $commit:expr) => {
87
#[test]

‎crates/test-utils/src/util.rs‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use std::{
2020
fs::{self, File},
2121
io::{BufWriter, IsTerminal, Read, Seek, Write},
2222
path::{Path, PathBuf},
23-
process::{ChildStdin, Command, Output, Stdio},
23+
process::{Command, Output, Stdio},
2424
sync::{
2525
Arc, LazyLock,
2626
atomic::{AtomicUsize, Ordering},
@@ -789,7 +789,7 @@ impl TestProject {
789789
cmd,
790790
current_dir_lock: None,
791791
saved_cwd: pretty_err("<current dir>", std::env::current_dir()),
792-
stdin_fun: None,
792+
stdin: None,
793793
redact_output: true,
794794
}
795795
}
@@ -804,7 +804,7 @@ impl TestProject {
804804
cmd,
805805
current_dir_lock: None,
806806
saved_cwd: pretty_err("<current dir>", std::env::current_dir()),
807-
stdin_fun: None,
807+
stdin: None,
808808
redact_output: true,
809809
}
810810
}
@@ -902,7 +902,7 @@ pub struct TestCommand {
902902
cmd: Command,
903903
// initial: Command,
904904
current_dir_lock: Option<parking_lot::MutexGuard<'static, ()>>,
905-
stdin_fun: Option<Box<dyn FnOnce(ChildStdin)>>,
905+
stdin: Option<Vec<u8>>,
906906
/// If true, command output is redacted.
907907
redact_output: bool,
908908
}
@@ -954,8 +954,9 @@ impl TestCommand {
954954
self
955955
}
956956

957-
pub fn stdin(&mut self, fun: impl FnOnce(ChildStdin) + 'static) -> &mut Self {
958-
self.stdin_fun = Some(Box::new(fun));
957+
/// Set the stdin bytes for the next command.
958+
pub fn stdin(&mut self, stdin: impl Into<Vec<u8>>) -> &mut Self {
959+
self.stdin = Some(stdin.into());
959960
self
960961
}
961962

@@ -1135,8 +1136,8 @@ impl TestCommand {
11351136
test_debug!("executing {:?}", self.cmd);
11361137
let mut child =
11371138
self.cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).stdin(Stdio::piped()).spawn()?;
1138-
if let Some(fun) = self.stdin_fun.take() {
1139-
fun(child.stdin.take().unwrap());
1139+
if let Some(bytes) = self.stdin.take() {
1140+
child.stdin.take().unwrap().write_all(&bytes)?;
11401141
}
11411142
child.wait_with_output()
11421143
}

0 commit comments

Comments
 (0)