Skip to content

Commit ece0eba

Browse files
authored
feat(lint): geiger (#11377)
1 parent 3e32767 commit ece0eba

File tree

10 files changed

+327
-187
lines changed

10 files changed

+327
-187
lines changed

crates/forge/src/args.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,7 @@ pub fn run_command(args: Forge) -> Result<()> {
130130
ForgeSubcommand::Flatten(cmd) => cmd.run(),
131131
ForgeSubcommand::Inspect(cmd) => cmd.run(),
132132
ForgeSubcommand::Tree(cmd) => cmd.run(),
133-
ForgeSubcommand::Geiger(cmd) => {
134-
let n = cmd.run()?;
135-
if n > 0 {
136-
std::process::exit(n as i32);
137-
}
138-
Ok(())
139-
}
133+
ForgeSubcommand::Geiger(cmd) => cmd.run(),
140134
ForgeSubcommand::Doc(cmd) => {
141135
if cmd.is_watch() {
142136
global.block_on(watch::watch_doc(cmd))

crates/forge/src/cmd/geiger.rs

Lines changed: 29 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -1,162 +1,59 @@
11
use clap::{Parser, ValueHint};
2-
use eyre::{Result, WrapErr};
3-
use foundry_cli::utils::LoadConfig;
4-
use foundry_compilers::{Graph, resolver::parse::SolData};
5-
use foundry_config::{Config, impl_figment_convert_basic};
6-
use itertools::Itertools;
7-
use solar_parse::{ast, ast::visit::Visit, interface::Session};
8-
use std::{
9-
ops::ControlFlow,
10-
path::{Path, PathBuf},
11-
};
2+
use eyre::Result;
3+
use foundry_cli::opts::BuildOpts;
4+
use foundry_config::impl_figment_convert;
5+
use std::path::PathBuf;
126

137
/// CLI arguments for `forge geiger`.
8+
///
9+
/// This command is an alias for `forge lint --only-lint unsafe-cheatcode`
10+
/// and detects usage of unsafe cheat codes in a project and its dependencies.
1411
#[derive(Clone, Debug, Parser)]
1512
pub struct GeigerArgs {
1613
/// Paths to files or directories to detect.
1714
#[arg(
18-
conflicts_with = "root",
1915
value_hint = ValueHint::FilePath,
2016
value_name = "PATH",
21-
num_args(1..),
17+
num_args(0..)
2218
)]
2319
paths: Vec<PathBuf>,
2420

25-
/// The project's root path.
26-
///
27-
/// By default root of the Git repository, if in one,
28-
/// or the current working directory.
29-
#[arg(long, value_hint = ValueHint::DirPath, value_name = "PATH")]
30-
root: Option<PathBuf>,
31-
32-
/// Globs to ignore.
33-
#[arg(
34-
long,
35-
value_hint = ValueHint::FilePath,
36-
value_name = "PATH",
37-
num_args(1..),
38-
)]
39-
ignore: Vec<PathBuf>,
40-
4121
#[arg(long, hide = true)]
4222
check: bool,
23+
4324
#[arg(long, hide = true)]
4425
full: bool,
26+
27+
#[command(flatten)]
28+
build: BuildOpts,
4529
}
4630

47-
impl_figment_convert_basic!(GeigerArgs);
31+
impl_figment_convert!(GeigerArgs, build);
4832

4933
impl GeigerArgs {
50-
pub fn sources(&self, config: &Config) -> Result<Vec<PathBuf>> {
51-
let cwd = std::env::current_dir()?;
52-
53-
let mut sources: Vec<PathBuf> = {
54-
if self.paths.is_empty() {
55-
let paths = config.project_paths();
56-
Graph::<SolData>::resolve(&paths)?
57-
.files()
58-
.keys()
59-
.filter(|f| !paths.has_library_ancestor(f))
60-
.cloned()
61-
.collect()
62-
} else {
63-
self.paths
64-
.iter()
65-
.flat_map(|path| foundry_common::fs::files_with_ext(path, "sol"))
66-
.unique()
67-
.collect()
68-
}
69-
};
70-
71-
sources.retain_mut(|path| {
72-
let abs_path = if path.is_absolute() { path.clone() } else { cwd.join(&path) };
73-
*path = abs_path.strip_prefix(&cwd).unwrap_or(&abs_path).to_path_buf();
74-
!self.ignore.iter().any(|ignore| {
75-
if ignore.is_absolute() {
76-
abs_path.starts_with(ignore)
77-
} else {
78-
abs_path.starts_with(cwd.join(ignore))
79-
}
80-
})
81-
});
82-
83-
Ok(sources)
84-
}
85-
86-
pub fn run(self) -> Result<usize> {
34+
pub fn run(self) -> Result<()> {
35+
// Deprecated flags warnings
8736
if self.check {
8837
sh_warn!("`--check` is deprecated as it's now the default behavior\n")?;
8938
}
9039
if self.full {
9140
sh_warn!("`--full` is deprecated as reports are not generated anymore\n")?;
9241
}
9342

94-
let config = self.load_config()?;
95-
let sources = self.sources(&config).wrap_err("Failed to resolve files")?;
96-
97-
if config.ffi {
98-
sh_warn!("FFI enabled\n")?;
99-
}
100-
101-
let mut sess = Session::builder().with_stderr_emitter().build();
102-
sess.dcx = sess.dcx.set_flags(|flags| flags.track_diagnostics = false);
103-
let unsafe_cheatcodes = &[
104-
"ffi".to_string(),
105-
"readFile".to_string(),
106-
"readLine".to_string(),
107-
"writeFile".to_string(),
108-
"writeLine".to_string(),
109-
"removeFile".to_string(),
110-
"closeFile".to_string(),
111-
"setEnv".to_string(),
112-
"deriveKey".to_string(),
113-
];
114-
Ok(sess
115-
.enter(|| sources.iter().map(|file| lint_file(&sess, unsafe_cheatcodes, file)).sum()))
116-
}
117-
}
118-
119-
fn lint_file(sess: &Session, unsafe_cheatcodes: &[String], path: &Path) -> usize {
120-
try_lint_file(sess, unsafe_cheatcodes, path).unwrap_or(0)
121-
}
122-
123-
fn try_lint_file(
124-
sess: &Session,
125-
unsafe_cheatcodes: &[String],
126-
path: &Path,
127-
) -> solar_parse::interface::Result<usize> {
128-
let arena = solar_parse::ast::Arena::new();
129-
let mut parser = solar_parse::Parser::from_file(sess, &arena, path)?;
130-
let ast = parser.parse_file().map_err(|e| e.emit())?;
131-
let mut visitor = Visitor::new(sess, unsafe_cheatcodes);
132-
let _ = visitor.visit_source_unit(&ast);
133-
Ok(visitor.count)
134-
}
135-
136-
struct Visitor<'a> {
137-
sess: &'a Session,
138-
count: usize,
139-
unsafe_cheatcodes: &'a [String],
140-
}
141-
142-
impl<'a> Visitor<'a> {
143-
fn new(sess: &'a Session, unsafe_cheatcodes: &'a [String]) -> Self {
144-
Self { sess, count: 0, unsafe_cheatcodes }
145-
}
146-
}
147-
148-
impl<'ast> Visit<'ast> for Visitor<'_> {
149-
type BreakValue = solar_parse::interface::data_structures::Never;
43+
sh_warn!(
44+
"`forge geiger` is just an alias for `forge lint --only-lint unsafe-cheatcode`\n"
45+
)?;
46+
47+
// Convert geiger command to lint command with specific lint filter
48+
let lint_args = crate::cmd::lint::LintArgs {
49+
paths: self.paths,
50+
severity: None,
51+
lint: Some(vec!["unsafe-cheatcode".to_string()]),
52+
json: false,
53+
build: self.build,
54+
};
15055

151-
fn visit_expr(&mut self, expr: &'ast ast::Expr<'ast>) -> ControlFlow<Self::BreakValue> {
152-
if let ast::ExprKind::Call(lhs, _args) = &expr.kind
153-
&& let ast::ExprKind::Member(_lhs, member) = &lhs.kind
154-
&& self.unsafe_cheatcodes.iter().any(|c| c.as_str() == member.as_str())
155-
{
156-
let msg = format!("usage of unsafe cheatcode `vm.{member}`");
157-
self.sess.dcx.err(msg).span(member.span).emit();
158-
self.count += 1;
159-
}
160-
self.walk_expr(expr)
56+
// Run the lint command with the geiger-specific configuration
57+
lint_args.run()
16158
}
16259
}

crates/forge/src/cmd/lint.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,25 @@ use std::path::PathBuf;
1717
pub struct LintArgs {
1818
/// Path to the file to be checked. Overrides the `ignore` project config.
1919
#[arg(value_hint = ValueHint::FilePath, value_name = "PATH", num_args(1..))]
20-
paths: Vec<PathBuf>,
20+
pub(crate) paths: Vec<PathBuf>,
2121

2222
/// Specifies which lints to run based on severity. Overrides the `severity` project config.
2323
///
2424
/// Supported values: `high`, `med`, `low`, `info`, `gas`.
2525
#[arg(long, value_name = "SEVERITY", num_args(1..))]
26-
severity: Option<Vec<Severity>>,
26+
pub(crate) severity: Option<Vec<Severity>>,
2727

2828
/// Specifies which lints to run based on their ID (e.g., "incorrect-shift"). Overrides the
2929
/// `exclude_lints` project config.
3030
#[arg(long = "only-lint", value_name = "LINT_ID", num_args(1..))]
31-
lint: Option<Vec<String>>,
31+
pub(crate) lint: Option<Vec<String>>,
3232

3333
/// Activates the linter's JSON formatter (rustc-compatible).
3434
#[arg(long)]
35-
json: bool,
35+
pub(crate) json: bool,
3636

3737
#[command(flatten)]
38-
build: BuildOpts,
38+
pub(crate) build: BuildOpts,
3939
}
4040

4141
foundry_config::impl_figment_convert!(LintArgs, build);

crates/forge/src/opts.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ pub enum ForgeSubcommand {
145145
Tree(tree::TreeArgs),
146146

147147
/// Detects usage of unsafe cheat codes in a project and its dependencies.
148+
///
149+
/// This is an alias for `forge lint --only-lint unsafe-cheatcode`.
148150
Geiger(geiger::GeigerArgs),
149151

150152
/// Generate documentation for the project.

crates/forge/tests/cli/geiger.rs

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
forgetest!(call, |prj, cmd| {
2-
prj.add_source(
3-
"call.sol",
1+
forgetest_init!(call, |prj, cmd| {
2+
prj.add_test(
3+
"call.t.sol",
44
r#"
5+
import {Test} from "forge-std/Test.sol";
6+
57
contract A is Test {
68
function do_ffi() public {
79
string[] memory inputs = new string[](1);
@@ -12,22 +14,25 @@ forgetest!(call, |prj, cmd| {
1214
)
1315
.unwrap();
1416

15-
cmd.arg("geiger").assert_code(1).stderr_eq(str![[r#"
16-
error: usage of unsafe cheatcode `vm.ffi`
17-
[FILE]:7:20
17+
cmd.arg("geiger").assert_success().stderr_eq(str![[r#"
18+
...
19+
note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations
20+
[FILE]:9:20
1821
|
19-
7 | vm.ffi(inputs);
20-
| ^^^
22+
9 | vm.ffi(inputs);
23+
| ---
2124
|
22-
23-
25+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode
26+
...
2427
"#]]);
2528
});
2629

27-
forgetest!(assignment, |prj, cmd| {
28-
prj.add_source(
29-
"assignment.sol",
30+
forgetest_init!(assignment, |prj, cmd| {
31+
prj.add_test(
32+
"assignment.t.sol",
3033
r#"
34+
import {Test} from "forge-std/Test.sol";
35+
3136
contract A is Test {
3237
function do_ffi() public {
3338
string[] memory inputs = new string[](1);
@@ -38,24 +43,28 @@ forgetest!(assignment, |prj, cmd| {
3843
)
3944
.unwrap();
4045

41-
cmd.arg("geiger").assert_code(1).stderr_eq(str![[r#"
42-
error: usage of unsafe cheatcode `vm.ffi`
43-
[FILE]:7:34
46+
cmd.arg("geiger").assert_success().stderr_eq(str![[r#"
47+
...
48+
note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations
49+
[FILE]:9:34
4450
|
45-
7 | bytes stuff = vm.ffi(inputs);
46-
| ^^^
51+
9 | bytes stuff = vm.ffi(inputs);
52+
| ---
4753
|
48-
49-
54+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode
55+
...
5056
"#]]);
5157
});
5258

53-
forgetest!(exit_code, |prj, cmd| {
54-
prj.add_source(
55-
"multiple.sol",
59+
forgetest_init!(exit_code, |prj, cmd| {
60+
prj.add_test(
61+
"multiple.t.sol",
5662
r#"
63+
import {Test} from "forge-std/Test.sol";
64+
5765
contract A is Test {
5866
function do_ffi() public {
67+
string[] memory inputs = new string[](1);
5968
vm.ffi(inputs);
6069
vm.ffi(inputs);
6170
vm.ffi(inputs);
@@ -65,28 +74,31 @@ forgetest!(exit_code, |prj, cmd| {
6574
)
6675
.unwrap();
6776

68-
cmd.arg("geiger").assert_code(3).stderr_eq(str![[r#"
69-
error: usage of unsafe cheatcode `vm.ffi`
70-
[FILE]:6:20
71-
|
72-
6 | vm.ffi(inputs);
73-
| ^^^
74-
|
75-
76-
error: usage of unsafe cheatcode `vm.ffi`
77-
[FILE]:7:20
78-
|
79-
7 | vm.ffi(inputs);
80-
| ^^^
81-
|
82-
83-
error: usage of unsafe cheatcode `vm.ffi`
84-
[FILE]:8:20
77+
cmd.arg("geiger").assert_success().stderr_eq(str![[r#"
78+
...
79+
note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations
80+
[FILE]:9:20
8581
|
86-
8 | vm.ffi(inputs);
87-
| ^^^
82+
9 | vm.ffi(inputs);
83+
| ---
8884
|
85+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode
8986
87+
note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations
88+
[FILE]:10:20
89+
|
90+
10 | vm.ffi(inputs);
91+
| ---
92+
|
93+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode
9094
95+
note[unsafe-cheatcode]: usage of unsafe cheatcodes that can perform dangerous operations
96+
[FILE]:11:20
97+
|
98+
11 | vm.ffi(inputs);
99+
| ---
100+
|
101+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-cheatcode
102+
...
91103
"#]]);
92104
});

0 commit comments

Comments
 (0)