From 200ba8667e25c93115dafe879f8f2c9a67ae092f Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 00:16:12 -0500 Subject: [PATCH 01/21] wip --- crates/forge/bin/cmd/lint.rs | 10 ++++++++ crates/lint/src/lib.rs | 46 +++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 7ca9d97113f1..ed4fd0e32d87 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -87,6 +87,7 @@ impl LintArgs { bail!("No source files found in path"); } + // TODO: maybe compile and lint on the aggreagted compiler output? Linter::new(input) .with_severity(self.severity) .with_description(self.with_description) @@ -95,3 +96,12 @@ impl LintArgs { Ok(()) } } + +pub struct ProjectLinter {} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SourceLocation { + pub file: String, + pub start: i32, + pub end: i32, +} diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 3f1979b865fc..a1a73ec346f3 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -4,7 +4,12 @@ pub mod info; pub mod med; use rayon::prelude::*; -use std::{collections::HashMap, hash::Hasher, path::PathBuf}; +use std::{ + collections::{BTreeMap, HashMap}, + error::Error, + hash::Hasher, + path::PathBuf, +}; use clap::ValueEnum; use solar_ast::{ @@ -28,6 +33,45 @@ pub enum Severity { Gas, } +pub struct ProjectLinter {} + +/// The main compiler abstraction trait. +/// +/// Currently mostly represents a wrapper around compiler binary aware of the version and able to +/// compile given input into [`CompilerOutput`] including artifacts and errors. +#[auto_impl::auto_impl(&, Box, Arc)] +pub trait Linter: Send + Sync + Clone { + // TODO: keep this + /// Input type for the compiler. Contains settings and sources to be compiled. + type Input: CompilerInput; + /// Compiler settings. + type Settings: LinterSettings; + + type LinterError: Error; + /// Enum of languages supported by the linter. + type Language: Language; + /// Main entrypoint for the linter. + fn lint(&self, input: &Self::Input) -> Result; +} + +pub struct LinterOutput { + pub results: BTreeMap>, +} + +/// Linter for languages supported by the Solc compiler +pub struct SolcLinter {} + +impl Language for SolcLinter { + const FILE_EXTENSIONS: &'static [&'static str] = SOLC_EXTENSIONS; +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SourceLocation { + pub file: String, + pub start: i32, + pub end: i32, +} + pub struct Linter { pub input: Vec, pub lints: Vec, From 869d6229375b7868c6ffcc81553dda30d845aef8 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 00:32:11 -0500 Subject: [PATCH 02/21] wip --- crates/forge/bin/cmd/lint.rs | 13 +- crates/lint/src/lib.rs | 465 +++++++++++++++++++---------------- 2 files changed, 255 insertions(+), 223 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index ed4fd0e32d87..7496a9c4a705 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -52,6 +52,10 @@ impl_figment_convert_basic!(LintArgs); impl LintArgs { pub fn run(self) -> Result<()> { let config = self.try_load_config_emit_warnings()?; + + // Set up the project. + let project = config.project()?; + let root = if let Some(root) = &self.root { root } else { &config.root }; // Expand ignore globs and canonicalize paths @@ -96,12 +100,3 @@ impl LintArgs { Ok(()) } } - -pub struct ProjectLinter {} - -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct SourceLocation { - pub file: String, - pub start: i32, - pub end: i32, -} diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index a1a73ec346f3..7995888c06bf 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -7,7 +7,8 @@ use rayon::prelude::*; use std::{ collections::{BTreeMap, HashMap}, error::Error, - hash::Hasher, + hash::{Hash, Hasher}, + marker::PhantomData, path::PathBuf, }; @@ -33,12 +34,48 @@ pub enum Severity { Gas, } -pub struct ProjectLinter {} +pub struct ProjectLinter +where + L: Linter, +{ + // TODO: remove later + phantom: PhantomData, +} + +impl ProjectLinter +where + L: Linter, +{ + // pub fn new() -> Self { + // Self {} + // } + + /// Lints the project. + pub fn lint>( + mut self, + project: &Project, + ) -> Result { + if !project.paths.has_input_files() && self.files.is_empty() { + sh_println!("Nothing to compile")?; + // nothing to do here + std::process::exit(0); + } -/// The main compiler abstraction trait. -/// -/// Currently mostly represents a wrapper around compiler binary aware of the version and able to -/// compile given input into [`CompilerOutput`] including artifacts and errors. + // Taking is fine since we don't need these in `compile_with`. + let files = std::mem::take(&mut self.files); + self.compile_with(|| { + let sources = if !files.is_empty() { + Source::read_all(files)? + } else { + project.paths.read_input_files()? + }; + + todo!() + }) + } +} + +/// The main linter abstraction trait #[auto_impl::auto_impl(&, Box, Arc)] pub trait Linter: Send + Sync + Clone { // TODO: keep this @@ -46,18 +83,21 @@ pub trait Linter: Send + Sync + Clone { type Input: CompilerInput; /// Compiler settings. type Settings: LinterSettings; - type LinterError: Error; /// Enum of languages supported by the linter. type Language: Language; + /// Main entrypoint for the linter. fn lint(&self, input: &Self::Input) -> Result; } -pub struct LinterOutput { - pub results: BTreeMap>, +// TODO: make Lint a generic +pub struct LinterOutput { + pub results: BTreeMap>, } +pub trait Lint: Hash {} + /// Linter for languages supported by the Solc compiler pub struct SolcLinter {} @@ -68,210 +108,207 @@ impl Language for SolcLinter { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct SourceLocation { pub file: String, - pub start: i32, - pub end: i32, -} - -pub struct Linter { - pub input: Vec, - pub lints: Vec, - pub description: bool, -} - -impl Linter { - pub fn new(input: Vec) -> Self { - Self { input, lints: Lint::all(), description: false } - } - - pub fn with_severity(mut self, severity: Option>) -> Self { - if let Some(severity) = severity { - self.lints.retain(|lint| severity.contains(&lint.severity())); - } - self - } - - pub fn with_description(mut self, description: bool) -> Self { - self.description = description; - self - } - - pub fn lint(self) { - let all_findings = self - .input - .par_iter() - .map(|file| { - let lints = self.lints.clone(); - let mut local_findings = HashMap::new(); - - // Create a new session for this file - let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - let arena = ast::Arena::new(); - - // Enter the session context for this thread - let _ = sess.enter(|| -> solar_interface::Result<()> { - let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; - - let ast = - parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); - - // Run all lints on the parsed AST and collect findings - for mut lint in lints { - let results = lint.lint(&ast); - local_findings.entry(lint).or_insert_with(Vec::new).extend(results); - } - - Ok(()) - }); - - local_findings - }) - .collect::>>>(); - - let mut aggregated_findings = HashMap::new(); - for file_findings in all_findings { - for (lint, results) in file_findings { - aggregated_findings.entry(lint).or_insert_with(Vec::new).extend(results); - } - } - - // TODO: make the output nicer - for finding in aggregated_findings { - let (lint, results) = finding; - let _description = if self.description { lint.description() } else { "" }; - - for _result in results { - // TODO: display the finding - } - } - } -} - -macro_rules! declare_lints { - ($(($name:ident, $severity:expr, $lint_name:expr, $description:expr)),* $(,)?) => { - #[derive(Debug, Clone, PartialEq, Eq)] - pub enum Lint { - $( - $name($name), - )* - } - - impl Lint { - pub fn all() -> Vec { - vec![ - $( - Lint::$name($name::new()), - )* - ] - } - - pub fn severity(&self) -> Severity { - match self { - $( - Lint::$name(_) => $severity, - )* - } - } - - pub fn name(&self) -> &'static str { - match self { - $( - Lint::$name(_) => $lint_name, - )* - } - } - - pub fn description(&self) -> &'static str { - match self { - $( - Lint::$name(_) => $description, - )* - } - } - - - /// Lint a source unit and return the findings - pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { - match self { - $( - Lint::$name(lint) => { - lint.visit_source_unit(source_unit); - lint.items.clone() - }, - )* - } - } - } - - impl<'ast> Visit<'ast> for Lint { - fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { - match self { - $( - Lint::$name(lint) => lint.visit_source_unit(source_unit), - )* - } - } - } - - - impl std::hash::Hash for Lint { - fn hash(&self, state: &mut H) { - self.name().hash(state); - } - } - - $( - #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] - pub struct $name { - pub items: Vec, - } - - impl $name { - pub fn new() -> Self { - Self { items: Vec::new() } - } - - /// Returns the severity of the lint - pub fn severity() -> Severity { - $severity - } - - /// Returns the name of the lint - pub fn name() -> &'static str { - $lint_name - } - - /// Returns the description of the lint - pub fn description() -> &'static str { - $description - } - } - )* - }; + pub span: Span, } -declare_lints!( - //High - (IncorrectShift, Severity::High, "incorrect-shift", "TODO: description"), - (ArbitraryTransferFrom, Severity::High, "arbitrary-transfer-from", "TODO: description"), - // Med - (DivideBeforeMultiply, Severity::Med, "divide-before-multiply", "TODO: description"), - // Low - // Info - (VariableCamelCase, Severity::Info, "variable-camel-case", "TODO: description"), - (VariableCapsCase, Severity::Info, "variable-caps-case", "TODO: description"), - (StructPascalCase, Severity::Info, "struct-pascal-case", "TODO: description"), - (FunctionCamelCase, Severity::Info, "function-camel-case", "TODO: description"), - // Gas Optimizations - (AsmKeccak256, Severity::Gas, "asm-keccak256", "TODO: description"), - (PackStorageVariables, Severity::Gas, "pack-storage-variables", "TODO: description"), - (PackStructs, Severity::Gas, "pack-structs", "TODO: description"), - (UseConstantVariable, Severity::Gas, "use-constant-var", "TODO: description"), - (UseImmutableVariable, Severity::Gas, "use-immutable-var", "TODO: description"), - (UseExternalVisibility, Severity::Gas, "use-external-visibility", "TODO: description"), - ( - AvoidUsingThis, - Severity::Gas, - "avoid-using-this", - "Avoid using `this` to read public variables. This incurs an unncessary STATICCALL." - ), -); +// pub struct Linter { +// pub input: Vec, +// pub lints: Vec, +// pub description: bool, +// } + +// impl Linter { +// pub fn new(input: Vec) -> Self { +// Self { input, lints: Lint::all(), description: false } +// } + +// pub fn with_severity(mut self, severity: Option>) -> Self { +// if let Some(severity) = severity { +// self.lints.retain(|lint| severity.contains(&lint.severity())); +// } +// self +// } + +// pub fn with_description(mut self, description: bool) -> Self { +// self.description = description; +// self +// } + +// pub fn lint(self) { +// let all_findings = self +// .input +// .par_iter() +// .map(|file| { +// let lints = self.lints.clone(); +// let mut local_findings = HashMap::new(); + +// // Create a new session for this file +// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); +// let arena = ast::Arena::new(); + +// // Enter the session context for this thread +// let _ = sess.enter(|| -> solar_interface::Result<()> { +// let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; + +// let ast = +// parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); + +// // Run all lints on the parsed AST and collect findings +// for mut lint in lints { +// let results = lint.lint(&ast); +// local_findings.entry(lint).or_insert_with(Vec::new).extend(results); +// } + +// Ok(()) +// }); + +// local_findings +// }) +// .collect::>>>(); + +// let mut aggregated_findings = HashMap::new(); +// for file_findings in all_findings { +// for (lint, results) in file_findings { +// aggregated_findings.entry(lint).or_insert_with(Vec::new).extend(results); +// } +// } + +// // TODO: make the output nicer +// for finding in aggregated_findings { +// let (lint, results) = finding; +// let _description = if self.description { lint.description() } else { "" }; + +// for _result in results { +// // TODO: display the finding +// } +// } +// } +// } + +// macro_rules! declare_lints { +// ($(($name:ident, $severity:expr, $lint_name:expr, $description:expr)),* $(,)?) => { +// #[derive(Debug, Clone, PartialEq, Eq)] +// pub enum Lint { +// $( +// $name($name), +// )* +// } + +// impl Lint { +// pub fn all() -> Vec { +// vec![ +// $( +// Lint::$name($name::new()), +// )* +// ] +// } + +// pub fn severity(&self) -> Severity { +// match self { +// $( +// Lint::$name(_) => $severity, +// )* +// } +// } + +// pub fn name(&self) -> &'static str { +// match self { +// $( +// Lint::$name(_) => $lint_name, +// )* +// } +// } + +// pub fn description(&self) -> &'static str { +// match self { +// $( +// Lint::$name(_) => $description, +// )* +// } +// } + +// /// Lint a source unit and return the findings +// pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { +// match self { +// $( +// Lint::$name(lint) => { +// lint.visit_source_unit(source_unit); +// lint.items.clone() +// }, +// )* +// } +// } +// } + +// impl<'ast> Visit<'ast> for Lint { +// fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { +// match self { +// $( +// Lint::$name(lint) => lint.visit_source_unit(source_unit), +// )* +// } +// } +// } + +// impl std::hash::Hash for Lint { +// fn hash(&self, state: &mut H) { +// self.name().hash(state); +// } +// } + +// $( +// #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] +// pub struct $name { +// pub items: Vec, +// } + +// impl $name { +// pub fn new() -> Self { +// Self { items: Vec::new() } +// } + +// /// Returns the severity of the lint +// pub fn severity() -> Severity { +// $severity +// } + +// /// Returns the name of the lint +// pub fn name() -> &'static str { +// $lint_name +// } + +// /// Returns the description of the lint +// pub fn description() -> &'static str { +// $description +// } +// } +// )* +// }; +// } + +// declare_lints!( +// //High +// (IncorrectShift, Severity::High, "incorrect-shift", "TODO: description"), +// (ArbitraryTransferFrom, Severity::High, "arbitrary-transfer-from", "TODO: description"), +// // Med +// (DivideBeforeMultiply, Severity::Med, "divide-before-multiply", "TODO: description"), +// // Low +// // Info +// (VariableCamelCase, Severity::Info, "variable-camel-case", "TODO: description"), +// (VariableCapsCase, Severity::Info, "variable-caps-case", "TODO: description"), +// (StructPascalCase, Severity::Info, "struct-pascal-case", "TODO: description"), +// (FunctionCamelCase, Severity::Info, "function-camel-case", "TODO: description"), +// // Gas Optimizations +// (AsmKeccak256, Severity::Gas, "asm-keccak256", "TODO: description"), +// (PackStorageVariables, Severity::Gas, "pack-storage-variables", "TODO: description"), +// (PackStructs, Severity::Gas, "pack-structs", "TODO: description"), +// (UseConstantVariable, Severity::Gas, "use-constant-var", "TODO: description"), +// (UseImmutableVariable, Severity::Gas, "use-immutable-var", "TODO: description"), +// (UseExternalVisibility, Severity::Gas, "use-external-visibility", "TODO: description"), +// ( +// AvoidUsingThis, +// Severity::Gas, +// "avoid-using-this", +// "Avoid using `this` to read public variables. This incurs an unncessary STATICCALL." +// ), +// ); From eb249557f57bf66e68db51626ebc30b23b4b13b6 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 01:27:08 -0500 Subject: [PATCH 03/21] refactor into sol linter --- Cargo.lock | 5 ++ crates/lint/Cargo.toml | 9 +++- crates/lint/src/gas.rs | 93 ------------------------------------- crates/lint/src/high.rs | 14 ------ crates/lint/src/info.rs | 72 ---------------------------- crates/lint/src/lib.rs | 92 ++++++++++++++++++++---------------- crates/lint/src/med.rs | 68 --------------------------- crates/lint/src/sol/gas.rs | 93 +++++++++++++++++++++++++++++++++++++ crates/lint/src/sol/high.rs | 14 ++++++ crates/lint/src/sol/info.rs | 72 ++++++++++++++++++++++++++++ crates/lint/src/sol/med.rs | 68 +++++++++++++++++++++++++++ crates/lint/src/sol/mod.rs | 6 +++ 12 files changed, 317 insertions(+), 289 deletions(-) delete mode 100644 crates/lint/src/gas.rs delete mode 100644 crates/lint/src/high.rs delete mode 100644 crates/lint/src/info.rs delete mode 100644 crates/lint/src/med.rs create mode 100644 crates/lint/src/sol/gas.rs create mode 100644 crates/lint/src/sol/high.rs create mode 100644 crates/lint/src/sol/info.rs create mode 100644 crates/lint/src/sol/med.rs create mode 100644 crates/lint/src/sol/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 1a0dc19a078e..2899bc88159d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3558,10 +3558,15 @@ dependencies = [ name = "forge-lint" version = "0.3.0" dependencies = [ + "auto_impl", "clap", "eyre", + "foundry-common", + "foundry-compilers", "rayon", "regex", + "serde", + "serde_json", "solar-ast", "solar-interface", "solar-parse", diff --git a/crates/lint/Cargo.toml b/crates/lint/Cargo.toml index a5a6ab72f835..92a8de1631c4 100644 --- a/crates/lint/Cargo.toml +++ b/crates/lint/Cargo.toml @@ -14,11 +14,18 @@ repository.workspace = true workspace = true [dependencies] +# lib +foundry-common.workspace = true +foundry-compilers.workspace = true + solar-parse.workspace = true solar-ast.workspace = true solar-interface.workspace = true + eyre.workspace = true rayon.workspace = true - +serde_json.workspace = true +auto_impl.workspace = true +serde = { workspace = true, features = ["derive"] } regex = "1.11" clap = { version = "4", features = ["derive"] } diff --git a/crates/lint/src/gas.rs b/crates/lint/src/gas.rs deleted file mode 100644 index cb6540e151da..000000000000 --- a/crates/lint/src/gas.rs +++ /dev/null @@ -1,93 +0,0 @@ -use solar_ast::{ - ast::{Expr, ExprKind}, - visit::Visit, -}; - -use crate::{ - AsmKeccak256, AvoidUsingThis, PackStorageVariables, PackStructs, UseConstantVariable, - UseExternalVisibility, UseImmutableVariable, -}; - -impl<'ast> Visit<'ast> for AsmKeccak256 { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - if let ExprKind::Call(expr, _) = &expr.kind { - if let ExprKind::Ident(ident) = &expr.kind { - if ident.name.as_str() == "keccak256" { - self.items.push(expr.span); - } - } - } - self.walk_expr(expr); - } -} - -impl<'ast> Visit<'ast> for PackStorageVariables { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for PackStructs { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for UseConstantVariable { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for UseImmutableVariable { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for UseExternalVisibility { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for AvoidUsingThis { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -// TODO: avoid using `this` to read public variables - -#[cfg(test)] -mod test { - use solar_ast::{ast, visit::Visit}; - use solar_interface::{ColorChoice, Session}; - use std::path::Path; - - use crate::AsmKeccak256; - - #[test] - fn test_keccak256() -> eyre::Result<()> { - let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - - let _ = sess.enter(|| -> solar_interface::Result<()> { - let arena = ast::Arena::new(); - - let mut parser = - solar_parse::Parser::from_file(&sess, &arena, Path::new("testdata/Keccak256.sol"))?; - - // Parse the file. - let ast = parser.parse_file().map_err(|e| e.emit())?; - - let mut pattern = AsmKeccak256::default(); - pattern.visit_source_unit(&ast); - - assert_eq!(pattern.items.len(), 2); - - Ok(()) - }); - - Ok(()) - } -} diff --git a/crates/lint/src/high.rs b/crates/lint/src/high.rs deleted file mode 100644 index bc5bb186d543..000000000000 --- a/crates/lint/src/high.rs +++ /dev/null @@ -1,14 +0,0 @@ -use crate::{ArbitraryTransferFrom, IncorrectShift}; -use solar_ast::{ast::Expr, visit::Visit}; - -impl<'ast> Visit<'ast> for IncorrectShift { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} - -impl<'ast> Visit<'ast> for ArbitraryTransferFrom { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - todo!() - } -} diff --git a/crates/lint/src/info.rs b/crates/lint/src/info.rs deleted file mode 100644 index 7fc0f6239adc..000000000000 --- a/crates/lint/src/info.rs +++ /dev/null @@ -1,72 +0,0 @@ -use regex::Regex; - -use solar_ast::{ - ast::{ItemStruct, VariableDefinition}, - visit::Visit, -}; - -use crate::{FunctionCamelCase, StructPascalCase, VariableCamelCase, VariableCapsCase}; - -impl<'ast> Visit<'ast> for VariableCamelCase { - fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { - if let Some(mutability) = var.mutability { - if !mutability.is_constant() && !mutability.is_immutable() { - if let Some(name) = var.name { - if !is_camel_case(name.as_str()) { - self.items.push(var.span); - } - } - } - } - self.walk_variable_definition(var); - } -} - -impl<'ast> Visit<'ast> for VariableCapsCase { - fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { - if let Some(mutability) = var.mutability { - if mutability.is_constant() || mutability.is_immutable() { - if let Some(name) = var.name { - if !is_caps_case(name.as_str()) { - self.items.push(var.span); - } - } - } - } - self.walk_variable_definition(var); - } -} - -impl<'ast> Visit<'ast> for StructPascalCase { - fn visit_item_struct(&mut self, strukt: &'ast ItemStruct<'ast>) { - if !is_pascal_case(strukt.name.as_str()) { - self.items.push(strukt.name.span); - } - - self.walk_item_struct(strukt); - } -} - -impl Visit<'_> for FunctionCamelCase { - fn visit_function_header(&mut self, header: &'_ solar_ast::ast::FunctionHeader<'_>) { - todo!() - } -} - -// Check if a string is camelCase -pub fn is_camel_case(s: &str) -> bool { - let re = Regex::new(r"^[a-z_][a-zA-Z0-9]*$").unwrap(); - re.is_match(s) && s.chars().any(|c| c.is_uppercase()) -} - -// Check if a string is PascalCase -pub fn is_pascal_case(s: &str) -> bool { - let re = Regex::new(r"^[A-Z0-9][a-zA-Z0-9]*$").unwrap(); - re.is_match(s) -} - -// Check if a string is SCREAMING_SNAKE_CASE -pub fn is_caps_case(s: &str) -> bool { - let re = Regex::new(r"^[A-Z][A-Z0-9_]*$").unwrap(); - re.is_match(s) && s.contains('_') -} diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 7995888c06bf..5f3827532eb8 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -1,9 +1,11 @@ -pub mod gas; -pub mod high; -pub mod info; -pub mod med; +pub mod sol; +use foundry_common::sh_println; +use foundry_compilers::{ + artifacts::Contract, Compiler, CompilerContract, CompilerInput, Language, Project, +}; use rayon::prelude::*; +use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, HashMap}, error::Error, @@ -38,6 +40,10 @@ pub struct ProjectLinter where L: Linter, { + /// Extra files to include, that are not necessarily in the project's source dir. + files: Vec, + severity: Option>, + description: bool, // TODO: remove later phantom: PhantomData, } @@ -46,63 +52,73 @@ impl ProjectLinter where L: Linter, { - // pub fn new() -> Self { - // Self {} - // } - /// Lints the project. pub fn lint>( mut self, project: &Project, - ) -> Result { + ) -> eyre::Result> { if !project.paths.has_input_files() && self.files.is_empty() { sh_println!("Nothing to compile")?; // nothing to do here std::process::exit(0); } - // Taking is fine since we don't need these in `compile_with`. - let files = std::mem::take(&mut self.files); - self.compile_with(|| { - let sources = if !files.is_empty() { - Source::read_all(files)? - } else { - project.paths.read_input_files()? - }; - - todo!() - }) + // // Taking is fine since we don't need these in `compile_with`. + // let files = std::mem::take(&mut self.files); + // self.compile_with(|| { + // let sources = if !files.is_empty() { + // Source::read_all(files)? + // } else { + // project.paths.read_input_files()? + // }; + + // }) + + todo!() + } + + pub fn with_description(mut self, description: bool) -> Self { + self.description = description; + self + } + + pub fn with_severity(mut self, severity: Option>) -> Self { + self.severity = severity; + self } } +// NOTE: add some way to specify linter profiles. For example having a profile adhering to the op stack, base, etc. +// This can probably also be accomplished via the foundry.toml. Maybe have generic profile/settings + /// The main linter abstraction trait -#[auto_impl::auto_impl(&, Box, Arc)] pub trait Linter: Send + Sync + Clone { - // TODO: keep this /// Input type for the compiler. Contains settings and sources to be compiled. - type Input: CompilerInput; - /// Compiler settings. - type Settings: LinterSettings; + type Input: CompilerInput; + + // TODO: probably remove + // /// TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc.) + // type Settings: LinterSettings; + type Lint: Lint; type LinterError: Error; /// Enum of languages supported by the linter. type Language: Language; /// Main entrypoint for the linter. - fn lint(&self, input: &Self::Input) -> Result; + fn lint(&self, input: &Self::Input) -> Result, Self::LinterError>; } -// TODO: make Lint a generic -pub struct LinterOutput { - pub results: BTreeMap>, +// TODO: probably remove +pub trait LinterSettings { + fn lints() -> Vec; } -pub trait Lint: Hash {} - -/// Linter for languages supported by the Solc compiler -pub struct SolcLinter {} +pub struct LinterOutput { + pub results: BTreeMap>, +} -impl Language for SolcLinter { - const FILE_EXTENSIONS: &'static [&'static str] = SOLC_EXTENSIONS; +pub trait Lint: Hash { + fn results(&self) -> Vec; } #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -111,12 +127,6 @@ pub struct SourceLocation { pub span: Span, } -// pub struct Linter { -// pub input: Vec, -// pub lints: Vec, -// pub description: bool, -// } - // impl Linter { // pub fn new(input: Vec) -> Self { // Self { input, lints: Lint::all(), description: false } diff --git a/crates/lint/src/med.rs b/crates/lint/src/med.rs deleted file mode 100644 index e9c3808307ee..000000000000 --- a/crates/lint/src/med.rs +++ /dev/null @@ -1,68 +0,0 @@ -use solar_ast::{ - ast::{BinOp, BinOpKind, Expr, ExprKind}, - visit::Visit, -}; - -use crate::DivideBeforeMultiply; - -impl<'ast> Visit<'ast> for DivideBeforeMultiply { - fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { - if let ExprKind::Binary(left_expr, BinOp { kind: BinOpKind::Mul, .. }, _) = &expr.kind { - if contains_division(left_expr) { - self.items.push(expr.span); - } - } - - self.walk_expr(expr); - } -} - -fn contains_division<'ast>(expr: &'ast Expr<'ast>) -> bool { - match &expr.kind { - ExprKind::Binary(_, BinOp { kind: BinOpKind::Div, .. }, _) => true, - ExprKind::Tuple(inner_exprs) => inner_exprs.iter().any(|opt_expr| { - if let Some(inner_expr) = opt_expr { - contains_division(inner_expr) - } else { - false - } - }), - _ => false, - } -} - -#[cfg(test)] -mod test { - use solar_ast::{ast, visit::Visit}; - use solar_interface::{ColorChoice, Session}; - use std::path::Path; - - use crate::DivideBeforeMultiply; - - #[test] - fn test_divide_before_multiply() -> eyre::Result<()> { - let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - - let _ = sess.enter(|| -> solar_interface::Result<()> { - let arena = ast::Arena::new(); - - let mut parser = solar_parse::Parser::from_file( - &sess, - &arena, - Path::new("testdata/DivideBeforeMultiply.sol"), - )?; - - // Parse the file. - let ast = parser.parse_file().map_err(|e| e.emit())?; - - let mut pattern = DivideBeforeMultiply::default(); - pattern.visit_source_unit(&ast); - - assert_eq!(pattern.items.len(), 6); - - Ok(()) - }); - - Ok(()) - } -} diff --git a/crates/lint/src/sol/gas.rs b/crates/lint/src/sol/gas.rs new file mode 100644 index 000000000000..66d4651645a5 --- /dev/null +++ b/crates/lint/src/sol/gas.rs @@ -0,0 +1,93 @@ +// use solar_ast::{ +// ast::{Expr, ExprKind}, +// visit::Visit, +// }; + +// use crate::{ +// AsmKeccak256, AvoidUsingThis, PackStorageVariables, PackStructs, UseConstantVariable, +// UseExternalVisibility, UseImmutableVariable, +// }; + +// impl<'ast> Visit<'ast> for AsmKeccak256 { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// if let ExprKind::Call(expr, _) = &expr.kind { +// if let ExprKind::Ident(ident) = &expr.kind { +// if ident.name.as_str() == "keccak256" { +// self.items.push(expr.span); +// } +// } +// } +// self.walk_expr(expr); +// } +// } + +// impl<'ast> Visit<'ast> for PackStorageVariables { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for PackStructs { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for UseConstantVariable { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for UseImmutableVariable { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for UseExternalVisibility { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for AvoidUsingThis { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// // TODO: avoid using `this` to read public variables + +// #[cfg(test)] +// mod test { +// use solar_ast::{ast, visit::Visit}; +// use solar_interface::{ColorChoice, Session}; +// use std::path::Path; + +// use crate::AsmKeccak256; + +// #[test] +// fn test_keccak256() -> eyre::Result<()> { +// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + +// let _ = sess.enter(|| -> solar_interface::Result<()> { +// let arena = ast::Arena::new(); + +// let mut parser = +// solar_parse::Parser::from_file(&sess, &arena, Path::new("testdata/Keccak256.sol"))?; + +// // Parse the file. +// let ast = parser.parse_file().map_err(|e| e.emit())?; + +// let mut pattern = AsmKeccak256::default(); +// pattern.visit_source_unit(&ast); + +// assert_eq!(pattern.items.len(), 2); + +// Ok(()) +// }); + +// Ok(()) +// } +// } diff --git a/crates/lint/src/sol/high.rs b/crates/lint/src/sol/high.rs new file mode 100644 index 000000000000..6a43a33aff04 --- /dev/null +++ b/crates/lint/src/sol/high.rs @@ -0,0 +1,14 @@ +// use crate::{ArbitraryTransferFrom, IncorrectShift}; +// use solar_ast::{ast::Expr, visit::Visit}; + +// impl<'ast> Visit<'ast> for IncorrectShift { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } + +// impl<'ast> Visit<'ast> for ArbitraryTransferFrom { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// todo!() +// } +// } diff --git a/crates/lint/src/sol/info.rs b/crates/lint/src/sol/info.rs new file mode 100644 index 000000000000..740951d9adb1 --- /dev/null +++ b/crates/lint/src/sol/info.rs @@ -0,0 +1,72 @@ +// use regex::Regex; + +// use solar_ast::{ +// ast::{ItemStruct, VariableDefinition}, +// visit::Visit, +// }; + +// use crate::{FunctionCamelCase, StructPascalCase, VariableCamelCase, VariableCapsCase}; + +// impl<'ast> Visit<'ast> for VariableCamelCase { +// fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { +// if let Some(mutability) = var.mutability { +// if !mutability.is_constant() && !mutability.is_immutable() { +// if let Some(name) = var.name { +// if !is_camel_case(name.as_str()) { +// self.items.push(var.span); +// } +// } +// } +// } +// self.walk_variable_definition(var); +// } +// } + +// impl<'ast> Visit<'ast> for VariableCapsCase { +// fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { +// if let Some(mutability) = var.mutability { +// if mutability.is_constant() || mutability.is_immutable() { +// if let Some(name) = var.name { +// if !is_caps_case(name.as_str()) { +// self.items.push(var.span); +// } +// } +// } +// } +// self.walk_variable_definition(var); +// } +// } + +// impl<'ast> Visit<'ast> for StructPascalCase { +// fn visit_item_struct(&mut self, strukt: &'ast ItemStruct<'ast>) { +// if !is_pascal_case(strukt.name.as_str()) { +// self.items.push(strukt.name.span); +// } + +// self.walk_item_struct(strukt); +// } +// } + +// impl Visit<'_> for FunctionCamelCase { +// fn visit_function_header(&mut self, header: &'_ solar_ast::ast::FunctionHeader<'_>) { +// todo!() +// } +// } + +// // Check if a string is camelCase +// pub fn is_camel_case(s: &str) -> bool { +// let re = Regex::new(r"^[a-z_][a-zA-Z0-9]*$").unwrap(); +// re.is_match(s) && s.chars().any(|c| c.is_uppercase()) +// } + +// // Check if a string is PascalCase +// pub fn is_pascal_case(s: &str) -> bool { +// let re = Regex::new(r"^[A-Z0-9][a-zA-Z0-9]*$").unwrap(); +// re.is_match(s) +// } + +// // Check if a string is SCREAMING_SNAKE_CASE +// pub fn is_caps_case(s: &str) -> bool { +// let re = Regex::new(r"^[A-Z][A-Z0-9_]*$").unwrap(); +// re.is_match(s) && s.contains('_') +// } diff --git a/crates/lint/src/sol/med.rs b/crates/lint/src/sol/med.rs new file mode 100644 index 000000000000..bfd5902a8233 --- /dev/null +++ b/crates/lint/src/sol/med.rs @@ -0,0 +1,68 @@ +// use solar_ast::{ +// ast::{BinOp, BinOpKind, Expr, ExprKind}, +// visit::Visit, +// }; + +// use crate::DivideBeforeMultiply; + +// impl<'ast> Visit<'ast> for DivideBeforeMultiply { +// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { +// if let ExprKind::Binary(left_expr, BinOp { kind: BinOpKind::Mul, .. }, _) = &expr.kind { +// if contains_division(left_expr) { +// self.items.push(expr.span); +// } +// } + +// self.walk_expr(expr); +// } +// } + +// fn contains_division<'ast>(expr: &'ast Expr<'ast>) -> bool { +// match &expr.kind { +// ExprKind::Binary(_, BinOp { kind: BinOpKind::Div, .. }, _) => true, +// ExprKind::Tuple(inner_exprs) => inner_exprs.iter().any(|opt_expr| { +// if let Some(inner_expr) = opt_expr { +// contains_division(inner_expr) +// } else { +// false +// } +// }), +// _ => false, +// } +// } + +// #[cfg(test)] +// mod test { +// use solar_ast::{ast, visit::Visit}; +// use solar_interface::{ColorChoice, Session}; +// use std::path::Path; + +// use crate::DivideBeforeMultiply; + +// #[test] +// fn test_divide_before_multiply() -> eyre::Result<()> { +// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + +// let _ = sess.enter(|| -> solar_interface::Result<()> { +// let arena = ast::Arena::new(); + +// let mut parser = solar_parse::Parser::from_file( +// &sess, +// &arena, +// Path::new("testdata/DivideBeforeMultiply.sol"), +// )?; + +// // Parse the file. +// let ast = parser.parse_file().map_err(|e| e.emit())?; + +// let mut pattern = DivideBeforeMultiply::default(); +// pattern.visit_source_unit(&ast); + +// assert_eq!(pattern.items.len(), 6); + +// Ok(()) +// }); + +// Ok(()) +// } +// } diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs new file mode 100644 index 000000000000..91efba7fe273 --- /dev/null +++ b/crates/lint/src/sol/mod.rs @@ -0,0 +1,6 @@ +pub mod gas; +pub mod high; +pub mod info; +pub mod med; + +pub struct SolidityLinter {} From 39e6b54540d2a0a13d50ff01e02d847251a2df2b Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 01:57:57 -0500 Subject: [PATCH 04/21] impl Linter for SolidityLinter --- Cargo.lock | 1 + crates/lint/Cargo.toml | 1 + crates/lint/src/lib.rs | 34 ++++++++++++-------- crates/lint/src/sol/mod.rs | 66 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2899bc88159d..78d3460fda9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3570,6 +3570,7 @@ dependencies = [ "solar-ast", "solar-interface", "solar-parse", + "thiserror 2.0.9", ] [[package]] diff --git a/crates/lint/Cargo.toml b/crates/lint/Cargo.toml index 92a8de1631c4..2e9fcb22601d 100644 --- a/crates/lint/Cargo.toml +++ b/crates/lint/Cargo.toml @@ -24,6 +24,7 @@ solar-interface.workspace = true eyre.workspace = true rayon.workspace = true +thiserror.workspace = true serde_json.workspace = true auto_impl.workspace = true serde = { workspace = true, features = ["derive"] } diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 5f3827532eb8..2c8f7c1c764c 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -2,7 +2,8 @@ pub mod sol; use foundry_common::sh_println; use foundry_compilers::{ - artifacts::Contract, Compiler, CompilerContract, CompilerInput, Language, Project, + artifacts::{Contract, Source}, + Compiler, CompilerContract, CompilerInput, Language, Project, }; use rayon::prelude::*; use serde::{Deserialize, Serialize}; @@ -40,18 +41,21 @@ pub struct ProjectLinter where L: Linter, { + pub linter: L, /// Extra files to include, that are not necessarily in the project's source dir. - files: Vec, - severity: Option>, - description: bool, - // TODO: remove later - phantom: PhantomData, + pub files: Vec, + pub severity: Option>, + pub description: bool, } impl ProjectLinter where L: Linter, { + pub fn new(linter: L) -> Self { + Self { linter, files: Vec::new(), severity: None, description: false } + } + /// Lints the project. pub fn lint>( mut self, @@ -74,6 +78,14 @@ where // }) + let sources = if !self.files.is_empty() { + Source::read_all(self.files.clone())? + } else { + project.paths.read_input_files()? + }; + + let input = sources.into_iter().map(|(path, _)| path).collect::>(); + todo!() } @@ -89,15 +101,11 @@ where } // NOTE: add some way to specify linter profiles. For example having a profile adhering to the op stack, base, etc. -// This can probably also be accomplished via the foundry.toml. Maybe have generic profile/settings +// This can probably also be accomplished via the foundry.toml or some functions. Maybe have generic profile/settings /// The main linter abstraction trait pub trait Linter: Send + Sync + Clone { - /// Input type for the compiler. Contains settings and sources to be compiled. - type Input: CompilerInput; - - // TODO: probably remove - // /// TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc.) + // TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc. // type Settings: LinterSettings; type Lint: Lint; type LinterError: Error; @@ -105,7 +113,7 @@ pub trait Linter: Send + Sync + Clone { type Language: Language; /// Main entrypoint for the linter. - fn lint(&self, input: &Self::Input) -> Result, Self::LinterError>; + fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError>; } // TODO: probably remove diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 91efba7fe273..bb5604f0abd1 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -1,6 +1,72 @@ +use std::path::PathBuf; + +use eyre::Error; +use foundry_compilers::solc::SolcLanguage; +use solar_interface::{ + diagnostics::{DiagnosticBuilder, ErrorGuaranteed}, + ColorChoice, Session, +}; +use thiserror::Error; + +use crate::{Lint, Linter, LinterOutput, SourceLocation}; + pub mod gas; pub mod high; pub mod info; pub mod med; +#[derive(Debug, Hash)] +pub enum SolLint {} + +impl Lint for SolLint { + fn results(&self) -> Vec { + todo!() + } +} + +#[derive(Debug, Clone)] pub struct SolidityLinter {} + +#[derive(Error, Debug)] +pub enum SolLintError {} + +impl Linter for SolidityLinter { + // TODO: update this to be a solar error + type LinterError = SolLintError; + type Lint = SolLint; + type Language = SolcLanguage; + + fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { + // let all_findings = input + // .par_iter() + // .map(|file| { + // let lints = self.lints.clone(); + // let mut local_findings = HashMap::new(); + + // // Create a new session for this file + // let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + // let arena = ast::Arena::new(); + + // // Enter the session context for this thread + // let _ = sess.enter(|| -> solar_interface::Result<()> { + // let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; + + // let ast = + // parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); + + // // Run all lints on the parsed AST and collect findings + // for mut lint in lints { + // let results = lint.lint(&ast); + // local_findings.entry(lint).or_insert_with(Vec::new).extend(results); + // } + + // Ok(()) + // }); + + // local_findings + // }) + // .collect::>>>(); + + todo!() + } +} From 664bdc38692f558d7a7f26b4bff52d9cdd3d26c8 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 02:00:30 -0500 Subject: [PATCH 05/21] fmt --- crates/lint/src/sol/mod.rs | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index bb5604f0abd1..20a9d99ad465 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -1,3 +1,8 @@ +pub mod gas; +pub mod high; +pub mod info; +pub mod med; + use std::path::PathBuf; use eyre::Error; @@ -10,31 +15,13 @@ use thiserror::Error; use crate::{Lint, Linter, LinterOutput, SourceLocation}; -pub mod gas; -pub mod high; -pub mod info; -pub mod med; - -#[derive(Debug, Hash)] -pub enum SolLint {} - -impl Lint for SolLint { - fn results(&self) -> Vec { - todo!() - } -} - #[derive(Debug, Clone)] pub struct SolidityLinter {} -#[derive(Error, Debug)] -pub enum SolLintError {} - impl Linter for SolidityLinter { - // TODO: update this to be a solar error - type LinterError = SolLintError; type Lint = SolLint; type Language = SolcLanguage; + type LinterError = SolLintError; fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { // let all_findings = input @@ -70,3 +57,15 @@ impl Linter for SolidityLinter { todo!() } } + +#[derive(Debug, Hash)] +pub enum SolLint {} + +impl Lint for SolLint { + fn results(&self) -> Vec { + todo!() + } +} + +#[derive(Error, Debug)] +pub enum SolLintError {} From e9fe6dcb665cd62d81d8eea90d2678404b1d8017 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 02:05:21 -0500 Subject: [PATCH 06/21] wip --- crates/lint/src/lib.rs | 60 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 2c8f7c1c764c..ab2ff7a07a50 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -135,6 +135,66 @@ pub struct SourceLocation { pub span: Span, } +// TODO: amend to display source location +// /// Tries to mimic Solidity's own error formatting. +// /// +// /// +// impl fmt::Display for Error { +// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +// let mut short_msg = self.message.trim(); +// let fmtd_msg = self.formatted_message.as_deref().unwrap_or(""); + +// if short_msg.is_empty() { +// // if the message is empty, try to extract the first line from the formatted message +// if let Some(first_line) = fmtd_msg.lines().next() { +// // this is something like `ParserError: ` +// if let Some((_, s)) = first_line.split_once(':') { +// short_msg = s.trim_start(); +// } else { +// short_msg = first_line; +// } +// } +// } + +// // Error (XXXX): Error Message +// styled(f, self.severity.color().bold(), |f| self.fmt_severity(f))?; +// fmt_msg(f, short_msg)?; + +// let mut lines = fmtd_msg.lines(); + +// // skip the first line if it contains the same message as the one we just formatted, +// // unless it also contains a source location, in which case the entire error message is an +// // old style error message, like: +// // path/to/file:line:column: ErrorType: message +// if lines +// .clone() +// .next() +// .is_some_and(|l| l.contains(short_msg) && l.bytes().filter(|b| *b == b':').count() < 3) +// { +// let _ = lines.next(); +// } + +// // format the main source location +// fmt_source_location(f, &mut lines)?; + +// // format remaining lines as secondary locations +// while let Some(line) = lines.next() { +// f.write_str("\n")?; + +// if let Some((note, msg)) = line.split_once(':') { +// styled(f, Self::secondary_style(), |f| f.write_str(note))?; +// fmt_msg(f, msg)?; +// } else { +// f.write_str(line)?; +// } + +// fmt_source_location(f, &mut lines)?; +// } + +// Ok(()) +// } +// } + // impl Linter { // pub fn new(input: Vec) -> Self { // Self { input, lints: Lint::all(), description: false } From 18202ce9bac98cb6db60c47a6a3c407a1ec180b5 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 02:08:07 -0500 Subject: [PATCH 07/21] wip --- crates/lint/src/lib.rs | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index ab2ff7a07a50..444f628bcb4a 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -56,6 +56,16 @@ where Self { linter, files: Vec::new(), severity: None, description: false } } + pub fn with_description(mut self, description: bool) -> Self { + self.description = description; + self + } + + pub fn with_severity(mut self, severity: Option>) -> Self { + self.severity = severity; + self + } + /// Lints the project. pub fn lint>( mut self, @@ -67,17 +77,6 @@ where std::process::exit(0); } - // // Taking is fine since we don't need these in `compile_with`. - // let files = std::mem::take(&mut self.files); - // self.compile_with(|| { - // let sources = if !files.is_empty() { - // Source::read_all(files)? - // } else { - // project.paths.read_input_files()? - // }; - - // }) - let sources = if !self.files.is_empty() { Source::read_all(self.files.clone())? } else { @@ -86,17 +85,7 @@ where let input = sources.into_iter().map(|(path, _)| path).collect::>(); - todo!() - } - - pub fn with_description(mut self, description: bool) -> Self { - self.description = description; - self - } - - pub fn with_severity(mut self, severity: Option>) -> Self { - self.severity = severity; - self + Ok(self.linter.lint(&input)?) } } From c19a441c6a8f23cd95a79fac33cedeef1c5a9116 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 14:36:05 -0500 Subject: [PATCH 08/21] refactor lints into SolLint enum --- crates/lint/src/lib.rs | 132 +------------------------ crates/lint/src/sol/gas.rs | 186 ++++++++++++++++++------------------ crates/lint/src/sol/high.rs | 25 ++--- crates/lint/src/sol/info.rs | 124 ++++++++++++------------ crates/lint/src/sol/med.rs | 108 ++++++++++----------- crates/lint/src/sol/mod.rs | 141 +++++++++++++++++++++++++-- 6 files changed, 360 insertions(+), 356 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 444f628bcb4a..93f39f4ec923 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -85,7 +85,9 @@ where let input = sources.into_iter().map(|(path, _)| path).collect::>(); - Ok(self.linter.lint(&input)?) + // Ok(self.linter.lint(&input)?) + + todo!() } } @@ -251,131 +253,3 @@ pub struct SourceLocation { // } // } // } - -// macro_rules! declare_lints { -// ($(($name:ident, $severity:expr, $lint_name:expr, $description:expr)),* $(,)?) => { -// #[derive(Debug, Clone, PartialEq, Eq)] -// pub enum Lint { -// $( -// $name($name), -// )* -// } - -// impl Lint { -// pub fn all() -> Vec { -// vec![ -// $( -// Lint::$name($name::new()), -// )* -// ] -// } - -// pub fn severity(&self) -> Severity { -// match self { -// $( -// Lint::$name(_) => $severity, -// )* -// } -// } - -// pub fn name(&self) -> &'static str { -// match self { -// $( -// Lint::$name(_) => $lint_name, -// )* -// } -// } - -// pub fn description(&self) -> &'static str { -// match self { -// $( -// Lint::$name(_) => $description, -// )* -// } -// } - -// /// Lint a source unit and return the findings -// pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { -// match self { -// $( -// Lint::$name(lint) => { -// lint.visit_source_unit(source_unit); -// lint.items.clone() -// }, -// )* -// } -// } -// } - -// impl<'ast> Visit<'ast> for Lint { -// fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { -// match self { -// $( -// Lint::$name(lint) => lint.visit_source_unit(source_unit), -// )* -// } -// } -// } - -// impl std::hash::Hash for Lint { -// fn hash(&self, state: &mut H) { -// self.name().hash(state); -// } -// } - -// $( -// #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] -// pub struct $name { -// pub items: Vec, -// } - -// impl $name { -// pub fn new() -> Self { -// Self { items: Vec::new() } -// } - -// /// Returns the severity of the lint -// pub fn severity() -> Severity { -// $severity -// } - -// /// Returns the name of the lint -// pub fn name() -> &'static str { -// $lint_name -// } - -// /// Returns the description of the lint -// pub fn description() -> &'static str { -// $description -// } -// } -// )* -// }; -// } - -// declare_lints!( -// //High -// (IncorrectShift, Severity::High, "incorrect-shift", "TODO: description"), -// (ArbitraryTransferFrom, Severity::High, "arbitrary-transfer-from", "TODO: description"), -// // Med -// (DivideBeforeMultiply, Severity::Med, "divide-before-multiply", "TODO: description"), -// // Low -// // Info -// (VariableCamelCase, Severity::Info, "variable-camel-case", "TODO: description"), -// (VariableCapsCase, Severity::Info, "variable-caps-case", "TODO: description"), -// (StructPascalCase, Severity::Info, "struct-pascal-case", "TODO: description"), -// (FunctionCamelCase, Severity::Info, "function-camel-case", "TODO: description"), -// // Gas Optimizations -// (AsmKeccak256, Severity::Gas, "asm-keccak256", "TODO: description"), -// (PackStorageVariables, Severity::Gas, "pack-storage-variables", "TODO: description"), -// (PackStructs, Severity::Gas, "pack-structs", "TODO: description"), -// (UseConstantVariable, Severity::Gas, "use-constant-var", "TODO: description"), -// (UseImmutableVariable, Severity::Gas, "use-immutable-var", "TODO: description"), -// (UseExternalVisibility, Severity::Gas, "use-external-visibility", "TODO: description"), -// ( -// AvoidUsingThis, -// Severity::Gas, -// "avoid-using-this", -// "Avoid using `this` to read public variables. This incurs an unncessary STATICCALL." -// ), -// ); diff --git a/crates/lint/src/sol/gas.rs b/crates/lint/src/sol/gas.rs index 66d4651645a5..00a776aedd8a 100644 --- a/crates/lint/src/sol/gas.rs +++ b/crates/lint/src/sol/gas.rs @@ -1,93 +1,93 @@ -// use solar_ast::{ -// ast::{Expr, ExprKind}, -// visit::Visit, -// }; - -// use crate::{ -// AsmKeccak256, AvoidUsingThis, PackStorageVariables, PackStructs, UseConstantVariable, -// UseExternalVisibility, UseImmutableVariable, -// }; - -// impl<'ast> Visit<'ast> for AsmKeccak256 { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// if let ExprKind::Call(expr, _) = &expr.kind { -// if let ExprKind::Ident(ident) = &expr.kind { -// if ident.name.as_str() == "keccak256" { -// self.items.push(expr.span); -// } -// } -// } -// self.walk_expr(expr); -// } -// } - -// impl<'ast> Visit<'ast> for PackStorageVariables { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// impl<'ast> Visit<'ast> for PackStructs { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// impl<'ast> Visit<'ast> for UseConstantVariable { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// impl<'ast> Visit<'ast> for UseImmutableVariable { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// impl<'ast> Visit<'ast> for UseExternalVisibility { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// impl<'ast> Visit<'ast> for AvoidUsingThis { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } - -// // TODO: avoid using `this` to read public variables - -// #[cfg(test)] -// mod test { -// use solar_ast::{ast, visit::Visit}; -// use solar_interface::{ColorChoice, Session}; -// use std::path::Path; - -// use crate::AsmKeccak256; - -// #[test] -// fn test_keccak256() -> eyre::Result<()> { -// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - -// let _ = sess.enter(|| -> solar_interface::Result<()> { -// let arena = ast::Arena::new(); - -// let mut parser = -// solar_parse::Parser::from_file(&sess, &arena, Path::new("testdata/Keccak256.sol"))?; - -// // Parse the file. -// let ast = parser.parse_file().map_err(|e| e.emit())?; - -// let mut pattern = AsmKeccak256::default(); -// pattern.visit_source_unit(&ast); - -// assert_eq!(pattern.items.len(), 2); - -// Ok(()) -// }); - -// Ok(()) -// } -// } +use solar_ast::{ + ast::{Expr, ExprKind}, + visit::Visit, +}; + +use super::{ + AsmKeccak256, AvoidUsingThis, PackStorageVariables, PackStructs, UseConstantVariable, + UseExternalVisibility, UseImmutableVariable, +}; + +impl<'ast> Visit<'ast> for AsmKeccak256 { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + if let ExprKind::Call(expr, _) = &expr.kind { + if let ExprKind::Ident(ident) = &expr.kind { + if ident.name.as_str() == "keccak256" { + self.items.push(expr.span); + } + } + } + self.walk_expr(expr); + } +} + +impl<'ast> Visit<'ast> for PackStorageVariables { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for PackStructs { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for UseConstantVariable { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for UseImmutableVariable { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for UseExternalVisibility { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for AvoidUsingThis { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +// TODO: avoid using `this` to read public variables + +#[cfg(test)] +mod test { + use solar_ast::{ast, visit::Visit}; + use solar_interface::{ColorChoice, Session}; + use std::path::Path; + + use super::AsmKeccak256; + + #[test] + fn test_keccak256() -> eyre::Result<()> { + let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + + let _ = sess.enter(|| -> solar_interface::Result<()> { + let arena = ast::Arena::new(); + + let mut parser = + solar_parse::Parser::from_file(&sess, &arena, Path::new("testdata/Keccak256.sol"))?; + + // Parse the file. + let ast = parser.parse_file().map_err(|e| e.emit())?; + + let mut pattern = AsmKeccak256::default(); + pattern.visit_source_unit(&ast); + + assert_eq!(pattern.items.len(), 2); + + Ok(()) + }); + + Ok(()) + } +} diff --git a/crates/lint/src/sol/high.rs b/crates/lint/src/sol/high.rs index 6a43a33aff04..49e08bf56cc7 100644 --- a/crates/lint/src/sol/high.rs +++ b/crates/lint/src/sol/high.rs @@ -1,14 +1,15 @@ -// use crate::{ArbitraryTransferFrom, IncorrectShift}; -// use solar_ast::{ast::Expr, visit::Visit}; +use solar_ast::{ast::Expr, visit::Visit}; -// impl<'ast> Visit<'ast> for IncorrectShift { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } +use super::{ArbitraryTransferFrom, IncorrectShift}; -// impl<'ast> Visit<'ast> for ArbitraryTransferFrom { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// todo!() -// } -// } +impl<'ast> Visit<'ast> for IncorrectShift { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} + +impl<'ast> Visit<'ast> for ArbitraryTransferFrom { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + todo!() + } +} diff --git a/crates/lint/src/sol/info.rs b/crates/lint/src/sol/info.rs index 740951d9adb1..9235cf8e73ce 100644 --- a/crates/lint/src/sol/info.rs +++ b/crates/lint/src/sol/info.rs @@ -1,72 +1,72 @@ -// use regex::Regex; +use regex::Regex; -// use solar_ast::{ -// ast::{ItemStruct, VariableDefinition}, -// visit::Visit, -// }; +use solar_ast::{ + ast::{ItemStruct, VariableDefinition}, + visit::Visit, +}; -// use crate::{FunctionCamelCase, StructPascalCase, VariableCamelCase, VariableCapsCase}; +use super::{FunctionCamelCase, StructPascalCase, VariableCamelCase, VariableCapsCase}; -// impl<'ast> Visit<'ast> for VariableCamelCase { -// fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { -// if let Some(mutability) = var.mutability { -// if !mutability.is_constant() && !mutability.is_immutable() { -// if let Some(name) = var.name { -// if !is_camel_case(name.as_str()) { -// self.items.push(var.span); -// } -// } -// } -// } -// self.walk_variable_definition(var); -// } -// } +impl<'ast> Visit<'ast> for VariableCamelCase { + fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { + if let Some(mutability) = var.mutability { + if !mutability.is_constant() && !mutability.is_immutable() { + if let Some(name) = var.name { + if !is_camel_case(name.as_str()) { + self.items.push(var.span); + } + } + } + } + self.walk_variable_definition(var); + } +} -// impl<'ast> Visit<'ast> for VariableCapsCase { -// fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { -// if let Some(mutability) = var.mutability { -// if mutability.is_constant() || mutability.is_immutable() { -// if let Some(name) = var.name { -// if !is_caps_case(name.as_str()) { -// self.items.push(var.span); -// } -// } -// } -// } -// self.walk_variable_definition(var); -// } -// } +impl<'ast> Visit<'ast> for VariableCapsCase { + fn visit_variable_definition(&mut self, var: &'ast VariableDefinition<'ast>) { + if let Some(mutability) = var.mutability { + if mutability.is_constant() || mutability.is_immutable() { + if let Some(name) = var.name { + if !is_caps_case(name.as_str()) { + self.items.push(var.span); + } + } + } + } + self.walk_variable_definition(var); + } +} -// impl<'ast> Visit<'ast> for StructPascalCase { -// fn visit_item_struct(&mut self, strukt: &'ast ItemStruct<'ast>) { -// if !is_pascal_case(strukt.name.as_str()) { -// self.items.push(strukt.name.span); -// } +impl<'ast> Visit<'ast> for StructPascalCase { + fn visit_item_struct(&mut self, strukt: &'ast ItemStruct<'ast>) { + if !is_pascal_case(strukt.name.as_str()) { + self.items.push(strukt.name.span); + } -// self.walk_item_struct(strukt); -// } -// } + self.walk_item_struct(strukt); + } +} -// impl Visit<'_> for FunctionCamelCase { -// fn visit_function_header(&mut self, header: &'_ solar_ast::ast::FunctionHeader<'_>) { -// todo!() -// } -// } +impl Visit<'_> for FunctionCamelCase { + fn visit_function_header(&mut self, header: &'_ solar_ast::ast::FunctionHeader<'_>) { + todo!() + } +} -// // Check if a string is camelCase -// pub fn is_camel_case(s: &str) -> bool { -// let re = Regex::new(r"^[a-z_][a-zA-Z0-9]*$").unwrap(); -// re.is_match(s) && s.chars().any(|c| c.is_uppercase()) -// } +// Check if a string is camelCase +pub fn is_camel_case(s: &str) -> bool { + let re = Regex::new(r"^[a-z_][a-zA-Z0-9]*$").unwrap(); + re.is_match(s) && s.chars().any(|c| c.is_uppercase()) +} -// // Check if a string is PascalCase -// pub fn is_pascal_case(s: &str) -> bool { -// let re = Regex::new(r"^[A-Z0-9][a-zA-Z0-9]*$").unwrap(); -// re.is_match(s) -// } +// Check if a string is PascalCase +pub fn is_pascal_case(s: &str) -> bool { + let re = Regex::new(r"^[A-Z0-9][a-zA-Z0-9]*$").unwrap(); + re.is_match(s) +} -// // Check if a string is SCREAMING_SNAKE_CASE -// pub fn is_caps_case(s: &str) -> bool { -// let re = Regex::new(r"^[A-Z][A-Z0-9_]*$").unwrap(); -// re.is_match(s) && s.contains('_') -// } +// Check if a string is SCREAMING_SNAKE_CASE +pub fn is_caps_case(s: &str) -> bool { + let re = Regex::new(r"^[A-Z][A-Z0-9_]*$").unwrap(); + re.is_match(s) && s.contains('_') +} diff --git a/crates/lint/src/sol/med.rs b/crates/lint/src/sol/med.rs index bfd5902a8233..3d4c0e378eef 100644 --- a/crates/lint/src/sol/med.rs +++ b/crates/lint/src/sol/med.rs @@ -1,68 +1,68 @@ -// use solar_ast::{ -// ast::{BinOp, BinOpKind, Expr, ExprKind}, -// visit::Visit, -// }; +use solar_ast::{ + ast::{BinOp, BinOpKind, Expr, ExprKind}, + visit::Visit, +}; -// use crate::DivideBeforeMultiply; +use super::DivideBeforeMultiply; -// impl<'ast> Visit<'ast> for DivideBeforeMultiply { -// fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { -// if let ExprKind::Binary(left_expr, BinOp { kind: BinOpKind::Mul, .. }, _) = &expr.kind { -// if contains_division(left_expr) { -// self.items.push(expr.span); -// } -// } +impl<'ast> Visit<'ast> for DivideBeforeMultiply { + fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { + if let ExprKind::Binary(left_expr, BinOp { kind: BinOpKind::Mul, .. }, _) = &expr.kind { + if contains_division(left_expr) { + self.items.push(expr.span); + } + } -// self.walk_expr(expr); -// } -// } + self.walk_expr(expr); + } +} -// fn contains_division<'ast>(expr: &'ast Expr<'ast>) -> bool { -// match &expr.kind { -// ExprKind::Binary(_, BinOp { kind: BinOpKind::Div, .. }, _) => true, -// ExprKind::Tuple(inner_exprs) => inner_exprs.iter().any(|opt_expr| { -// if let Some(inner_expr) = opt_expr { -// contains_division(inner_expr) -// } else { -// false -// } -// }), -// _ => false, -// } -// } +fn contains_division<'ast>(expr: &'ast Expr<'ast>) -> bool { + match &expr.kind { + ExprKind::Binary(_, BinOp { kind: BinOpKind::Div, .. }, _) => true, + ExprKind::Tuple(inner_exprs) => inner_exprs.iter().any(|opt_expr| { + if let Some(inner_expr) = opt_expr { + contains_division(inner_expr) + } else { + false + } + }), + _ => false, + } +} -// #[cfg(test)] -// mod test { -// use solar_ast::{ast, visit::Visit}; -// use solar_interface::{ColorChoice, Session}; -// use std::path::Path; +#[cfg(test)] +mod test { + use solar_ast::{ast, visit::Visit}; + use solar_interface::{ColorChoice, Session}; + use std::path::Path; -// use crate::DivideBeforeMultiply; + use super::DivideBeforeMultiply; -// #[test] -// fn test_divide_before_multiply() -> eyre::Result<()> { -// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + #[test] + fn test_divide_before_multiply() -> eyre::Result<()> { + let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); -// let _ = sess.enter(|| -> solar_interface::Result<()> { -// let arena = ast::Arena::new(); + let _ = sess.enter(|| -> solar_interface::Result<()> { + let arena = ast::Arena::new(); -// let mut parser = solar_parse::Parser::from_file( -// &sess, -// &arena, -// Path::new("testdata/DivideBeforeMultiply.sol"), -// )?; + let mut parser = solar_parse::Parser::from_file( + &sess, + &arena, + Path::new("testdata/DivideBeforeMultiply.sol"), + )?; -// // Parse the file. -// let ast = parser.parse_file().map_err(|e| e.emit())?; + // Parse the file. + let ast = parser.parse_file().map_err(|e| e.emit())?; -// let mut pattern = DivideBeforeMultiply::default(); -// pattern.visit_source_unit(&ast); + let mut pattern = DivideBeforeMultiply::default(); + pattern.visit_source_unit(&ast); -// assert_eq!(pattern.items.len(), 6); + assert_eq!(pattern.items.len(), 6); -// Ok(()) -// }); + Ok(()) + }); -// Ok(()) -// } -// } + Ok(()) + } +} diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 20a9d99ad465..8623f11582e2 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -3,17 +3,21 @@ pub mod high; pub mod info; pub mod med; -use std::path::PathBuf; +use std::{ + hash::{Hash, Hasher}, + path::PathBuf, +}; use eyre::Error; use foundry_compilers::solc::SolcLanguage; +use solar_ast::{ast::SourceUnit, visit::Visit}; use solar_interface::{ diagnostics::{DiagnosticBuilder, ErrorGuaranteed}, - ColorChoice, Session, + ColorChoice, Session, Span, }; use thiserror::Error; -use crate::{Lint, Linter, LinterOutput, SourceLocation}; +use crate::{Lint, Linter, LinterOutput, Severity, SourceLocation}; #[derive(Debug, Clone)] pub struct SolidityLinter {} @@ -58,9 +62,6 @@ impl Linter for SolidityLinter { } } -#[derive(Debug, Hash)] -pub enum SolLint {} - impl Lint for SolLint { fn results(&self) -> Vec { todo!() @@ -69,3 +70,131 @@ impl Lint for SolLint { #[derive(Error, Debug)] pub enum SolLintError {} + +macro_rules! declare_sol_lints { + ($(($name:ident, $severity:expr, $lint_name:expr, $description:expr)),* $(,)?) => { + #[derive(Debug, Clone, PartialEq, Eq)] + pub enum SolLint { + $( + $name($name), + )* + } + + impl SolLint { + pub fn all() -> Vec { + vec![ + $( + SolLint::$name($name::new()), + )* + ] + } + + pub fn severity(&self) -> Severity { + match self { + $( + SolLint::$name(_) => $severity, + )* + } + } + + pub fn name(&self) -> &'static str { + match self { + $( + SolLint::$name(_) => $lint_name, + )* + } + } + + pub fn description(&self) -> &'static str { + match self { + $( + SolLint::$name(_) => $description, + )* + } + } + + /// Lint a source unit and return the findings + pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { + match self { + $( + SolLint::$name(lint) => { + lint.visit_source_unit(source_unit); + lint.items.clone() + }, + )* + } + } + } + + impl<'ast> Visit<'ast> for SolLint { + fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { + match self { + $( + SolLint::$name(lint) => lint.visit_source_unit(source_unit), + )* + } + } + } + + impl Hash for SolLint { + fn hash(&self, state: &mut H) { + self.name().hash(state); + } + } + + $( + #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] + pub struct $name { + pub items: Vec, + } + + impl $name { + pub fn new() -> Self { + Self { items: Vec::new() } + } + + /// Returns the severity of the lint + pub fn severity() -> Severity { + $severity + } + + /// Returns the name of the lint + pub fn name() -> &'static str { + $lint_name + } + + /// Returns the description of the lint + pub fn description() -> &'static str { + $description + } + } + )* + }; +} + +declare_sol_lints!( + //High + (IncorrectShift, Severity::High, "incorrect-shift", "TODO: description"), + (ArbitraryTransferFrom, Severity::High, "arbitrary-transfer-from", "TODO: description"), + // Med + (DivideBeforeMultiply, Severity::Med, "divide-before-multiply", "TODO: description"), + // Low + // Info + (VariableCamelCase, Severity::Info, "variable-camel-case", "TODO: description"), + (VariableCapsCase, Severity::Info, "variable-caps-case", "TODO: description"), + (StructPascalCase, Severity::Info, "struct-pascal-case", "TODO: description"), + (FunctionCamelCase, Severity::Info, "function-camel-case", "TODO: description"), + // Gas Optimizations + (AsmKeccak256, Severity::Gas, "asm-keccak256", "TODO: description"), + (PackStorageVariables, Severity::Gas, "pack-storage-variables", "TODO: description"), + (PackStructs, Severity::Gas, "pack-structs", "TODO: description"), + (UseConstantVariable, Severity::Gas, "use-constant-var", "TODO: description"), + (UseImmutableVariable, Severity::Gas, "use-immutable-var", "TODO: description"), + (UseExternalVisibility, Severity::Gas, "use-external-visibility", "TODO: description"), + ( + AvoidUsingThis, + Severity::Gas, + "avoid-using-this", + "Avoid using `this` to read public variables. This incurs an unncessary STATICCALL." + ), +); From d0f552e309e351cbfa0efb62bde9cd1428e75c96 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 14:51:19 -0500 Subject: [PATCH 09/21] update lint trait --- crates/lint/src/lib.rs | 114 +++++++----------------------------- crates/lint/src/sol/gas.rs | 4 +- crates/lint/src/sol/info.rs | 6 +- crates/lint/src/sol/med.rs | 4 +- crates/lint/src/sol/mod.rs | 55 ++++++++--------- 5 files changed, 54 insertions(+), 129 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 93f39f4ec923..52225302a071 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -16,26 +16,7 @@ use std::{ }; use clap::ValueEnum; -use solar_ast::{ - ast::{self, SourceUnit, Span}, - interface::{ColorChoice, Session}, - visit::Visit, -}; - -#[derive(Clone, Debug, ValueEnum)] -pub enum OutputFormat { - Json, - Markdown, -} - -#[derive(Clone, Debug, PartialEq, Eq, ValueEnum)] -pub enum Severity { - High, - Med, - Low, - Info, - Gas, -} +use solar_ast::ast::{self, SourceUnit, Span}; pub struct ProjectLinter where @@ -68,7 +49,7 @@ where /// Lints the project. pub fn lint>( - mut self, + self, project: &Project, ) -> eyre::Result> { if !project.paths.has_input_files() && self.files.is_empty() { @@ -85,9 +66,7 @@ where let input = sources.into_iter().map(|(path, _)| path).collect::>(); - // Ok(self.linter.lint(&input)?) - - todo!() + Ok(self.linter.lint(&input).expect("TODO: handle error")) } } @@ -117,11 +96,28 @@ pub struct LinterOutput { } pub trait Lint: Hash { - fn results(&self) -> Vec; + fn name(&self) -> &'static str; + fn description(&self) -> &'static str; + fn severity(&self) -> Severity; +} + +#[derive(Clone, Debug, ValueEnum)] +pub enum OutputFormat { + Json, + Markdown, } +#[derive(Clone, Debug, PartialEq, Eq, ValueEnum)] +pub enum Severity { + High, + Med, + Low, + Info, + Gas, +} #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct SourceLocation { + // TODO: should this be path buf? pub file: String, pub span: Span, } @@ -185,71 +181,3 @@ pub struct SourceLocation { // Ok(()) // } // } - -// impl Linter { -// pub fn new(input: Vec) -> Self { -// Self { input, lints: Lint::all(), description: false } -// } - -// pub fn with_severity(mut self, severity: Option>) -> Self { -// if let Some(severity) = severity { -// self.lints.retain(|lint| severity.contains(&lint.severity())); -// } -// self -// } - -// pub fn with_description(mut self, description: bool) -> Self { -// self.description = description; -// self -// } - -// pub fn lint(self) { -// let all_findings = self -// .input -// .par_iter() -// .map(|file| { -// let lints = self.lints.clone(); -// let mut local_findings = HashMap::new(); - -// // Create a new session for this file -// let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); -// let arena = ast::Arena::new(); - -// // Enter the session context for this thread -// let _ = sess.enter(|| -> solar_interface::Result<()> { -// let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; - -// let ast = -// parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); - -// // Run all lints on the parsed AST and collect findings -// for mut lint in lints { -// let results = lint.lint(&ast); -// local_findings.entry(lint).or_insert_with(Vec::new).extend(results); -// } - -// Ok(()) -// }); - -// local_findings -// }) -// .collect::>>>(); - -// let mut aggregated_findings = HashMap::new(); -// for file_findings in all_findings { -// for (lint, results) in file_findings { -// aggregated_findings.entry(lint).or_insert_with(Vec::new).extend(results); -// } -// } - -// // TODO: make the output nicer -// for finding in aggregated_findings { -// let (lint, results) = finding; -// let _description = if self.description { lint.description() } else { "" }; - -// for _result in results { -// // TODO: display the finding -// } -// } -// } -// } diff --git a/crates/lint/src/sol/gas.rs b/crates/lint/src/sol/gas.rs index 00a776aedd8a..5bae74a4baf5 100644 --- a/crates/lint/src/sol/gas.rs +++ b/crates/lint/src/sol/gas.rs @@ -13,7 +13,7 @@ impl<'ast> Visit<'ast> for AsmKeccak256 { if let ExprKind::Call(expr, _) = &expr.kind { if let ExprKind::Ident(ident) = &expr.kind { if ident.name.as_str() == "keccak256" { - self.items.push(expr.span); + self.results.push(expr.span); } } } @@ -83,7 +83,7 @@ mod test { let mut pattern = AsmKeccak256::default(); pattern.visit_source_unit(&ast); - assert_eq!(pattern.items.len(), 2); + assert_eq!(pattern.results.len(), 2); Ok(()) }); diff --git a/crates/lint/src/sol/info.rs b/crates/lint/src/sol/info.rs index 9235cf8e73ce..9c0bf47699ee 100644 --- a/crates/lint/src/sol/info.rs +++ b/crates/lint/src/sol/info.rs @@ -13,7 +13,7 @@ impl<'ast> Visit<'ast> for VariableCamelCase { if !mutability.is_constant() && !mutability.is_immutable() { if let Some(name) = var.name { if !is_camel_case(name.as_str()) { - self.items.push(var.span); + self.results.push(var.span); } } } @@ -28,7 +28,7 @@ impl<'ast> Visit<'ast> for VariableCapsCase { if mutability.is_constant() || mutability.is_immutable() { if let Some(name) = var.name { if !is_caps_case(name.as_str()) { - self.items.push(var.span); + self.results.push(var.span); } } } @@ -40,7 +40,7 @@ impl<'ast> Visit<'ast> for VariableCapsCase { impl<'ast> Visit<'ast> for StructPascalCase { fn visit_item_struct(&mut self, strukt: &'ast ItemStruct<'ast>) { if !is_pascal_case(strukt.name.as_str()) { - self.items.push(strukt.name.span); + self.results.push(strukt.name.span); } self.walk_item_struct(strukt); diff --git a/crates/lint/src/sol/med.rs b/crates/lint/src/sol/med.rs index 3d4c0e378eef..02314699e1c9 100644 --- a/crates/lint/src/sol/med.rs +++ b/crates/lint/src/sol/med.rs @@ -9,7 +9,7 @@ impl<'ast> Visit<'ast> for DivideBeforeMultiply { fn visit_expr(&mut self, expr: &'ast Expr<'ast>) { if let ExprKind::Binary(left_expr, BinOp { kind: BinOpKind::Mul, .. }, _) = &expr.kind { if contains_division(left_expr) { - self.items.push(expr.span); + self.results.push(expr.span); } } @@ -58,7 +58,7 @@ mod test { let mut pattern = DivideBeforeMultiply::default(); pattern.visit_source_unit(&ast); - assert_eq!(pattern.items.len(), 6); + assert_eq!(pattern.results.len(), 6); Ok(()) }); diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 8623f11582e2..b2bef347949c 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -62,12 +62,6 @@ impl Linter for SolidityLinter { } } -impl Lint for SolLint { - fn results(&self) -> Vec { - todo!() - } -} - #[derive(Error, Debug)] pub enum SolLintError {} @@ -89,68 +83,71 @@ macro_rules! declare_sol_lints { ] } - pub fn severity(&self) -> Severity { + /// Lint a source unit and return the findings + pub fn lint(&mut self, source_unit: &SourceUnit<'_>) { match self { $( - SolLint::$name(_) => $severity, + SolLint::$name(lint) => { + lint.visit_source_unit(source_unit); + }, )* } } + } - pub fn name(&self) -> &'static str { + impl<'ast> Visit<'ast> for SolLint { + fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { match self { $( - SolLint::$name(_) => $lint_name, + SolLint::$name(lint) => lint.visit_source_unit(source_unit), )* } } + } + + impl Hash for SolLint { + fn hash(&self, state: &mut H) { + self.name().hash(state); + } + } - pub fn description(&self) -> &'static str { + impl Lint for SolLint { + fn name(&self) -> &'static str { match self { $( - SolLint::$name(_) => $description, + SolLint::$name(_) => $lint_name, )* } } - /// Lint a source unit and return the findings - pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { + fn description(&self) -> &'static str { match self { $( - SolLint::$name(lint) => { - lint.visit_source_unit(source_unit); - lint.items.clone() - }, + SolLint::$name(_) => $description, )* } } - } - impl<'ast> Visit<'ast> for SolLint { - fn visit_source_unit(&mut self, source_unit: &SourceUnit<'ast>) { + fn severity(&self) -> Severity { match self { $( - SolLint::$name(lint) => lint.visit_source_unit(source_unit), + SolLint::$name(_) => $severity, )* } } } - impl Hash for SolLint { - fn hash(&self, state: &mut H) { - self.name().hash(state); - } - } $( #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] pub struct $name { - pub items: Vec, + // TODO: make source location and option + pub results: Vec, } impl $name { pub fn new() -> Self { - Self { items: Vec::new() } + Self { results: Vec::new() } } /// Returns the severity of the lint From e88bc8719cb74f93ecb637600dd2d51a55b11c91 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 14:56:20 -0500 Subject: [PATCH 10/21] wip --- crates/lint/src/sol/mod.rs | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index b2bef347949c..0cdd9157787b 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -4,13 +4,18 @@ pub mod info; pub mod med; use std::{ + collections::BTreeMap, hash::{Hash, Hasher}, path::PathBuf, }; use eyre::Error; use foundry_compilers::solc::SolcLanguage; -use solar_ast::{ast::SourceUnit, visit::Visit}; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use solar_ast::{ + ast::{Arena, SourceUnit}, + visit::Visit, +}; use solar_interface::{ diagnostics::{DiagnosticBuilder, ErrorGuaranteed}, ColorChoice, Session, Span, @@ -28,35 +33,30 @@ impl Linter for SolidityLinter { type LinterError = SolLintError; fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { - // let all_findings = input - // .par_iter() - // .map(|file| { - // let lints = self.lints.clone(); - // let mut local_findings = HashMap::new(); - - // // Create a new session for this file - // let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - // let arena = ast::Arena::new(); - - // // Enter the session context for this thread - // let _ = sess.enter(|| -> solar_interface::Result<()> { - // let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; - - // let ast = - // parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); - - // // Run all lints on the parsed AST and collect findings - // for mut lint in lints { - // let results = lint.lint(&ast); - // local_findings.entry(lint).or_insert_with(Vec::new).extend(results); - // } - - // Ok(()) - // }); - - // local_findings - // }) - // .collect::>>>(); + let all_findings = input.par_iter().map(|file| { + // NOTE: use all solidity lints for now but this should be configurable via SolidityLinter + let mut lints = SolLint::all(); + + // Initialize session and parsing environment + let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + let arena = Arena::new(); + let mut local_findings = BTreeMap::new(); + + // Enter the session context for this thread + let _ = sess.enter(|| -> solar_interface::Result<()> { + let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; + + let ast = parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); + + // Run all lints on the parsed AST and collect findings + for mut lint in lints { + let results = lint.lint(&ast); + local_findings.entry(lint).or_insert_with(Vec::new).extend(results); + } + + Ok(()) + }); + }); todo!() } From 1203d32fc1efe748a871bf1af6db2a147c2ae1f9 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 14:57:07 -0500 Subject: [PATCH 11/21] wip --- crates/lint/src/lib.rs | 4 ++-- crates/lint/src/sol/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 52225302a071..ecc9947cae70 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -75,12 +75,12 @@ where /// The main linter abstraction trait pub trait Linter: Send + Sync + Clone { + /// Enum of languages supported by the linter. + type Language: Language; // TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc. // type Settings: LinterSettings; type Lint: Lint; type LinterError: Error; - /// Enum of languages supported by the linter. - type Language: Language; /// Main entrypoint for the linter. fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError>; diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 0cdd9157787b..4f0a1ec4b876 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -28,8 +28,8 @@ use crate::{Lint, Linter, LinterOutput, Severity, SourceLocation}; pub struct SolidityLinter {} impl Linter for SolidityLinter { - type Lint = SolLint; type Language = SolcLanguage; + type Lint = SolLint; type LinterError = SolLintError; fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { From 01bb6dacc7003a01b4658bad1066039f1cc35c3c Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sat, 28 Dec 2024 14:59:03 -0500 Subject: [PATCH 12/21] wip --- crates/lint/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index ecc9947cae70..7859c1b45b42 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -107,6 +107,7 @@ pub enum OutputFormat { Markdown, } +// TODO: impl color for severity #[derive(Clone, Debug, PartialEq, Eq, ValueEnum)] pub enum Severity { High, From 9d3abf0f992203b67895d2c186a97a7f8078a7a8 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 00:45:15 -0500 Subject: [PATCH 13/21] wip --- crates/lint/src/lib.rs | 32 +++++++++++++++++++++++++++--- crates/lint/src/sol/mod.rs | 40 +++++++++++++++++++++++--------------- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 7859c1b45b42..f7a560eb20ab 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -12,6 +12,7 @@ use std::{ error::Error, hash::{Hash, Hasher}, marker::PhantomData, + ops::{Deref, DerefMut}, path::PathBuf, }; @@ -91,8 +92,27 @@ pub trait LinterSettings { fn lints() -> Vec; } -pub struct LinterOutput { - pub results: BTreeMap>, +pub struct LinterOutput(pub BTreeMap>); + +impl LinterOutput { + // Optional: You can still provide a `new` method for convenience + pub fn new() -> Self { + LinterOutput(BTreeMap::new()) + } +} + +impl Deref for LinterOutput { + type Target = BTreeMap>; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for LinterOutput { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } } pub trait Lint: Hash { @@ -119,10 +139,16 @@ pub enum Severity { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct SourceLocation { // TODO: should this be path buf? - pub file: String, + pub file: PathBuf, pub span: Span, } +impl SourceLocation { + pub fn new(file: PathBuf, span: Span) -> Self { + Self { file, span } + } +} + // TODO: amend to display source location // /// Tries to mimic Solidity's own error formatting. // /// diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 4f0a1ec4b876..ec8b4918f572 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -4,14 +4,14 @@ pub mod info; pub mod med; use std::{ - collections::BTreeMap, + collections::{BTreeMap, HashMap}, hash::{Hash, Hasher}, path::PathBuf, }; use eyre::Error; use foundry_compilers::solc::SolcLanguage; -use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use rayon::iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator}; use solar_ast::{ ast::{Arena, SourceUnit}, visit::Visit, @@ -33,28 +33,33 @@ impl Linter for SolidityLinter { type LinterError = SolLintError; fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { - let all_findings = input.par_iter().map(|file| { + let all_findings = input.into_par_iter().map(|file| { // NOTE: use all solidity lints for now but this should be configurable via SolidityLinter - let mut lints = SolLint::all(); + let lints = SolLint::all(); // Initialize session and parsing environment let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); let arena = Arena::new(); - let mut local_findings = BTreeMap::new(); // Enter the session context for this thread - let _ = sess.enter(|| -> solar_interface::Result<()> { + let _ = sess.enter(|| -> solar_interface::Result> { let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; - let ast = parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); + let mut local_findings = LinterOutput::new(); // Run all lints on the parsed AST and collect findings - for mut lint in lints { - let results = lint.lint(&ast); - local_findings.entry(lint).or_insert_with(Vec::new).extend(results); + for mut lint in lints.into_iter() { + if let Some(findings) = lint.lint(&ast) { + let findings = findings + .into_iter() + .map(|span| SourceLocation::new(file.to_owned(), span)) + .collect::>(); + + local_findings.insert(lint, findings); + } } - Ok(()) + Ok(local_findings) }); }); @@ -67,7 +72,9 @@ pub enum SolLintError {} macro_rules! declare_sol_lints { ($(($name:ident, $severity:expr, $lint_name:expr, $description:expr)),* $(,)?) => { - #[derive(Debug, Clone, PartialEq, Eq)] + + // TODO: ord based on severity + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum SolLint { $( $name($name), @@ -84,11 +91,12 @@ macro_rules! declare_sol_lints { } /// Lint a source unit and return the findings - pub fn lint(&mut self, source_unit: &SourceUnit<'_>) { + pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Option> { match self { $( SolLint::$name(lint) => { lint.visit_source_unit(source_unit); + lint.results.clone() }, )* } @@ -139,15 +147,15 @@ macro_rules! declare_sol_lints { $( - #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] + #[derive(Debug, Default, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct $name { // TODO: make source location and option - pub results: Vec, + pub results: Option>, } impl $name { pub fn new() -> Self { - Self { results: Vec::new() } + Self { results: None } } /// Returns the severity of the lint From e8b53ca6a10fb6271faedaebe95ba85d1e7d7f2f Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 00:48:42 -0500 Subject: [PATCH 14/21] wip --- crates/lint/src/sol/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index ec8b4918f572..2e51eb58209e 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -49,13 +49,14 @@ impl Linter for SolidityLinter { let mut local_findings = LinterOutput::new(); // Run all lints on the parsed AST and collect findings for mut lint in lints.into_iter() { - if let Some(findings) = lint.lint(&ast) { - let findings = findings + let findings = lint.lint(&ast); + if !findings.is_empty() { + let source_locations = findings .into_iter() .map(|span| SourceLocation::new(file.to_owned(), span)) .collect::>(); - local_findings.insert(lint, findings); + local_findings.insert(lint, source_locations); } } @@ -91,7 +92,7 @@ macro_rules! declare_sol_lints { } /// Lint a source unit and return the findings - pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Option> { + pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { match self { $( SolLint::$name(lint) => { @@ -149,13 +150,12 @@ macro_rules! declare_sol_lints { $( #[derive(Debug, Default, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct $name { - // TODO: make source location and option - pub results: Option>, + pub results: Vec, } impl $name { pub fn new() -> Self { - Self { results: None } + Self { results: Vec::new() } } /// Returns the severity of the lint From 5c5343d4d55b060826e7019e0c680776aab91752 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 01:14:43 -0500 Subject: [PATCH 15/21] update lint --- crates/lint/src/lib.rs | 10 ++++- crates/lint/src/sol/mod.rs | 76 +++++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index f7a560eb20ab..65dd0acf8d73 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -80,7 +80,7 @@ pub trait Linter: Send + Sync + Clone { type Language: Language; // TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc. // type Settings: LinterSettings; - type Lint: Lint; + type Lint: Lint + Ord; type LinterError: Error; /// Main entrypoint for the linter. @@ -115,6 +115,14 @@ impl DerefMut for LinterOutput { } } +impl Extend<(L::Lint, Vec)> for LinterOutput { + fn extend)>>(&mut self, iter: T) { + for (lint, findings) in iter { + self.0.entry(lint).or_insert_with(Vec::new).extend(findings); + } + } +} + pub trait Lint: Hash { fn name(&self) -> &'static str; fn description(&self) -> &'static str; diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 2e51eb58209e..d83f4fc18719 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -33,38 +33,48 @@ impl Linter for SolidityLinter { type LinterError = SolLintError; fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError> { - let all_findings = input.into_par_iter().map(|file| { - // NOTE: use all solidity lints for now but this should be configurable via SolidityLinter - let lints = SolLint::all(); - - // Initialize session and parsing environment - let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); - let arena = Arena::new(); - - // Enter the session context for this thread - let _ = sess.enter(|| -> solar_interface::Result> { - let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; - let ast = parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); - - let mut local_findings = LinterOutput::new(); - // Run all lints on the parsed AST and collect findings - for mut lint in lints.into_iter() { - let findings = lint.lint(&ast); - if !findings.is_empty() { - let source_locations = findings - .into_iter() - .map(|span| SourceLocation::new(file.to_owned(), span)) - .collect::>(); - - local_findings.insert(lint, source_locations); + let all_findings = input + .into_par_iter() + .map(|file| { + // NOTE: use all solidity lints for now but this should be configurable via SolidityLinter + let mut lints = SolLint::all(); + + // Initialize session and parsing environment + let sess = Session::builder().with_buffer_emitter(ColorChoice::Auto).build(); + let arena = Arena::new(); + + // Enter the session context for this thread + let _ = sess.enter(|| -> solar_interface::Result<()> { + let mut parser = solar_parse::Parser::from_file(&sess, &arena, file)?; + let ast = + parser.parse_file().map_err(|e| e.emit()).expect("Failed to parse file"); + + // Run all lints on the parsed AST and collect findings + for lint in lints.iter_mut() { + lint.lint(&ast); } - } - Ok(local_findings) - }); - }); + Ok(()) + }); + + (file.to_owned(), lints) + }) + .collect::)>>(); + + let mut output = LinterOutput::new(); + for (file, lints) in all_findings { + for lint in lints { + let source_locations = lint + .results() + .iter() + .map(|span| SourceLocation::new(file.clone(), *span)) + .collect::>(); + + output.insert(lint, source_locations); + } + } - todo!() + Ok(output) } } @@ -91,6 +101,14 @@ macro_rules! declare_sol_lints { ] } + pub fn results(&self) -> &[Span] { + match self { + $( + SolLint::$name(lint) => &lint.results, + )* + } + } + /// Lint a source unit and return the findings pub fn lint(&mut self, source_unit: &SourceUnit<'_>) -> Vec { match self { From 919f3b1c403d105a2882780240f81c801d7bae27 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 01:54:06 -0500 Subject: [PATCH 16/21] update forge lint to use ProjectLinter --- crates/forge/bin/cmd/lint.rs | 51 +++++++----------------------------- crates/lint/src/lib.rs | 34 +++++++++++++++++++++--- crates/lint/src/sol/mod.rs | 19 +++++++------- 3 files changed, 50 insertions(+), 54 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 7496a9c4a705..94b7b827f801 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -1,7 +1,9 @@ use clap::{Parser, ValueHint}; use eyre::{bail, Result}; -use forge_lint::{Linter, OutputFormat, Severity}; +use forge_lint::sol::SolidityLinter; +use forge_lint::{Linter, OutputFormat, ProjectLinter, Severity}; use foundry_cli::utils::LoadConfig; +use foundry_common::shell; use foundry_compilers::utils::{source_files_iter, SOLC_EXTENSIONS}; use foundry_config::filter::expand_globs; use foundry_config::impl_figment_convert_basic; @@ -52,50 +54,17 @@ impl_figment_convert_basic!(LintArgs); impl LintArgs { pub fn run(self) -> Result<()> { let config = self.try_load_config_emit_warnings()?; - - // Set up the project. let project = config.project()?; - let root = if let Some(root) = &self.root { root } else { &config.root }; - - // Expand ignore globs and canonicalize paths - let mut ignored = expand_globs(&root, config.fmt.ignore.iter())? - .iter() - .flat_map(foundry_common::fs::canonicalize_path) - .collect::>(); - - // Add explicitly excluded paths to the ignored set - if let Some(exclude_paths) = &self.exclude { - ignored.extend(exclude_paths.iter().flat_map(foundry_common::fs::canonicalize_path)); - } - - let entries = fs::read_dir(root).unwrap(); - println!("Files in directory: {}", root.display()); - for entry in entries { - let entry = entry.unwrap(); - let path = entry.path(); - println!("{}", path.display()); - } - - let mut input: Vec = if let Some(include_paths) = &self.include { - include_paths.iter().filter(|path| path.exists()).cloned().collect() - } else { - source_files_iter(&root, SOLC_EXTENSIONS) - .filter(|p| !(ignored.contains(p) || ignored.contains(&root.join(p)))) - .collect() - }; - - input.retain(|path| !ignored.contains(path)); + // TODO: Update this to infer the linter from the project, just hard coding to solidity for now + let linter = SolidityLinter::new(); + let output = ProjectLinter::new(linter).lint(&project)?; - if input.is_empty() { - bail!("No source files found in path"); - } + // let format_json = shell::is_json(); - // TODO: maybe compile and lint on the aggreagted compiler output? - Linter::new(input) - .with_severity(self.severity) - .with_description(self.with_description) - .lint(); + // if format_json && !self.names && !self.sizes { + // sh_println!("{}", serde_json::to_string_pretty(&output.output())?)?; + // } Ok(()) } diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 65dd0acf8d73..c618419549b3 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -7,6 +7,7 @@ use foundry_compilers::{ }; use rayon::prelude::*; use serde::{Deserialize, Serialize}; +use sol::SolidityLinter; use std::{ collections::{BTreeMap, HashMap}, error::Error, @@ -51,11 +52,37 @@ where /// Lints the project. pub fn lint>( self, - project: &Project, + mut project: &Project, ) -> eyre::Result> { + // TODO: infer linter from project + + // // Expand ignore globs and canonicalize paths + // let mut ignored = expand_globs(&root, config.fmt.ignore.iter())? + // .iter() + // .flat_map(foundry_common::fs::canonicalize_path) + // .collect::>(); + + // // Add explicitly excluded paths to the ignored set + // if let Some(exclude_paths) = &self.exclude { + // ignored.extend(exclude_paths.iter().flat_map(foundry_common::fs::canonicalize_path)); + // } + + // let mut input: Vec = if let Some(include_paths) = &self.include { + // include_paths.iter().filter(|path| path.exists()).cloned().collect() + // } else { + // source_files_iter(&root, SOLC_EXTENSIONS) + // .filter(|p| !(ignored.contains(p) || ignored.contains(&root.join(p)))) + // .collect() + // }; + + // input.retain(|path| !ignored.contains(path)); + + // if input.is_empty() { + // bail!("No source files found in path"); + // } + if !project.paths.has_input_files() && self.files.is_empty() { - sh_println!("Nothing to compile")?; - // nothing to do here + sh_println!("Nothing to lint")?; std::process::exit(0); } @@ -146,7 +173,6 @@ pub enum Severity { } #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct SourceLocation { - // TODO: should this be path buf? pub file: PathBuf, pub span: Span, } diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index d83f4fc18719..c77a54f36ec4 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -4,28 +4,29 @@ pub mod info; pub mod med; use std::{ - collections::{BTreeMap, HashMap}, hash::{Hash, Hasher}, path::PathBuf, }; -use eyre::Error; use foundry_compilers::solc::SolcLanguage; -use rayon::iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator}; +use rayon::iter::{IntoParallelIterator, ParallelIterator}; use solar_ast::{ ast::{Arena, SourceUnit}, visit::Visit, }; -use solar_interface::{ - diagnostics::{DiagnosticBuilder, ErrorGuaranteed}, - ColorChoice, Session, Span, -}; +use solar_interface::{ColorChoice, Session, Span}; use thiserror::Error; use crate::{Lint, Linter, LinterOutput, Severity, SourceLocation}; -#[derive(Debug, Clone)] -pub struct SolidityLinter {} +#[derive(Debug, Clone, Default)] +pub struct SolidityLinter; + +impl SolidityLinter { + pub fn new() -> Self { + Self::default() + } +} impl Linter for SolidityLinter { type Language = SolcLanguage; From f02c9679e2fb83f6c6b902068ef0d3477f08be3c Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 01:56:27 -0500 Subject: [PATCH 17/21] wip --- crates/forge/bin/cmd/lint.rs | 9 ++------- crates/lint/src/lib.rs | 4 ---- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 94b7b827f801..bea78f3bb2c3 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -1,14 +1,9 @@ use clap::{Parser, ValueHint}; -use eyre::{bail, Result}; +use eyre::Result; use forge_lint::sol::SolidityLinter; -use forge_lint::{Linter, OutputFormat, ProjectLinter, Severity}; +use forge_lint::{OutputFormat, ProjectLinter, Severity}; use foundry_cli::utils::LoadConfig; -use foundry_common::shell; -use foundry_compilers::utils::{source_files_iter, SOLC_EXTENSIONS}; -use foundry_config::filter::expand_globs; use foundry_config::impl_figment_convert_basic; -use std::collections::HashSet; -use std::fs; use std::path::PathBuf; /// CLI arguments for `forge lint`. diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index c618419549b3..bcc20256ec84 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -183,10 +183,6 @@ impl SourceLocation { } } -// TODO: amend to display source location -// /// Tries to mimic Solidity's own error formatting. -// /// -// /// // impl fmt::Display for Error { // fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // let mut short_msg = self.message.trim(); From e37043c3489c6198f77ad7be08f2e05a8e175b6d Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 02:09:29 -0500 Subject: [PATCH 18/21] include/exclude files from linting --- crates/forge/bin/cmd/lint.rs | 39 +++++++++++++++++++++++++--- crates/lint/src/lib.rs | 49 ++---------------------------------- 2 files changed, 38 insertions(+), 50 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index bea78f3bb2c3..2455c5fde774 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -3,7 +3,9 @@ use eyre::Result; use forge_lint::sol::SolidityLinter; use forge_lint::{OutputFormat, ProjectLinter, Severity}; use foundry_cli::utils::LoadConfig; +use foundry_compilers::artifacts::Source; use foundry_config::impl_figment_convert_basic; +use std::collections::HashSet; use std::path::PathBuf; /// CLI arguments for `forge lint`. @@ -51,10 +53,41 @@ impl LintArgs { let config = self.try_load_config_emit_warnings()?; let project = config.project()?; - // TODO: Update this to infer the linter from the project, just hard coding to solidity for now - let linter = SolidityLinter::new(); - let output = ProjectLinter::new(linter).lint(&project)?; + // Get all source files from the project + let mut sources = + project.paths.read_input_files()?.keys().cloned().collect::>(); + // Add included paths to sources + if let Some(include_paths) = &self.include { + let included = include_paths + .iter() + .filter(|path| sources.contains(path)) + .cloned() + .collect::>(); + + sources = included; + } + + // Remove excluded files from sources + if let Some(exclude_paths) = &self.exclude { + let excluded = exclude_paths.iter().cloned().collect::>(); + sources.retain(|path| !excluded.contains(path)); + } + + if sources.is_empty() { + sh_println!("Nothing to lint")?; + std::process::exit(0); + } + + let linter = if project.compiler.solc.is_some() { + SolidityLinter::new() + } else { + todo!("Linting not supported for this language"); + }; + + let output = ProjectLinter::new(linter).lint(&sources)?; + + // TODO: display output // let format_json = shell::is_json(); // if format_json && !self.names && !self.sizes { diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index bcc20256ec84..1fd2b09d8b95 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -25,8 +25,6 @@ where L: Linter, { pub linter: L, - /// Extra files to include, that are not necessarily in the project's source dir. - pub files: Vec, pub severity: Option>, pub description: bool, } @@ -36,7 +34,7 @@ where L: Linter, { pub fn new(linter: L) -> Self { - Self { linter, files: Vec::new(), severity: None, description: false } + Self { linter, severity: None, description: false } } pub fn with_description(mut self, description: bool) -> Self { @@ -50,50 +48,7 @@ where } /// Lints the project. - pub fn lint>( - self, - mut project: &Project, - ) -> eyre::Result> { - // TODO: infer linter from project - - // // Expand ignore globs and canonicalize paths - // let mut ignored = expand_globs(&root, config.fmt.ignore.iter())? - // .iter() - // .flat_map(foundry_common::fs::canonicalize_path) - // .collect::>(); - - // // Add explicitly excluded paths to the ignored set - // if let Some(exclude_paths) = &self.exclude { - // ignored.extend(exclude_paths.iter().flat_map(foundry_common::fs::canonicalize_path)); - // } - - // let mut input: Vec = if let Some(include_paths) = &self.include { - // include_paths.iter().filter(|path| path.exists()).cloned().collect() - // } else { - // source_files_iter(&root, SOLC_EXTENSIONS) - // .filter(|p| !(ignored.contains(p) || ignored.contains(&root.join(p)))) - // .collect() - // }; - - // input.retain(|path| !ignored.contains(path)); - - // if input.is_empty() { - // bail!("No source files found in path"); - // } - - if !project.paths.has_input_files() && self.files.is_empty() { - sh_println!("Nothing to lint")?; - std::process::exit(0); - } - - let sources = if !self.files.is_empty() { - Source::read_all(self.files.clone())? - } else { - project.paths.read_input_files()? - }; - - let input = sources.into_iter().map(|(path, _)| path).collect::>(); - + pub fn lint(self, input: &[PathBuf]) -> eyre::Result> { Ok(self.linter.lint(&input).expect("TODO: handle error")) } } From 8c0be7336d6f6e87fbff6cdc2bf449cd255e9e44 Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 02:11:03 -0500 Subject: [PATCH 19/21] linter output display note --- crates/forge/bin/cmd/lint.rs | 1 - crates/lint/src/lib.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 2455c5fde774..905764e87df0 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -3,7 +3,6 @@ use eyre::Result; use forge_lint::sol::SolidityLinter; use forge_lint::{OutputFormat, ProjectLinter, Severity}; use foundry_cli::utils::LoadConfig; -use foundry_compilers::artifacts::Source; use foundry_config::impl_figment_convert_basic; use std::collections::HashSet; use std::path::PathBuf; diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 1fd2b09d8b95..97f54e7cf6cb 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -18,7 +18,7 @@ use std::{ }; use clap::ValueEnum; -use solar_ast::ast::{self, SourceUnit, Span}; +use solar_ast::ast::Span; pub struct ProjectLinter where @@ -138,6 +138,7 @@ impl SourceLocation { } } +// TODO: Update to implement Display for LinterOutput, model after compiler error display // impl fmt::Display for Error { // fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // let mut short_msg = self.message.trim(); From 9fc264bba0397f75b39c59bfb4f5b3e1401702be Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 02:27:29 -0500 Subject: [PATCH 20/21] configure with severity and description --- crates/forge/bin/cmd/lint.rs | 2 ++ crates/lint/src/lib.rs | 38 +++++------------------------------- crates/lint/src/sol/mod.rs | 15 +++++++++++++- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 905764e87df0..32349c3dd9fe 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -80,6 +80,8 @@ impl LintArgs { let linter = if project.compiler.solc.is_some() { SolidityLinter::new() + .with_severity(self.severity) + .with_description(self.with_description) } else { todo!("Linting not supported for this language"); }; diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 97f54e7cf6cb..06130f7417a5 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -1,23 +1,15 @@ pub mod sol; -use foundry_common::sh_println; -use foundry_compilers::{ - artifacts::{Contract, Source}, - Compiler, CompilerContract, CompilerInput, Language, Project, -}; -use rayon::prelude::*; -use serde::{Deserialize, Serialize}; -use sol::SolidityLinter; use std::{ - collections::{BTreeMap, HashMap}, + collections::BTreeMap, error::Error, - hash::{Hash, Hasher}, - marker::PhantomData, + hash::Hash, ops::{Deref, DerefMut}, path::PathBuf, }; use clap::ValueEnum; +use foundry_compilers::Language; use solar_ast::ast::Span; pub struct ProjectLinter @@ -25,8 +17,6 @@ where L: Linter, { pub linter: L, - pub severity: Option>, - pub description: bool, } impl ProjectLinter @@ -34,20 +24,9 @@ where L: Linter, { pub fn new(linter: L) -> Self { - Self { linter, severity: None, description: false } - } - - pub fn with_description(mut self, description: bool) -> Self { - self.description = description; - self + Self { linter } } - pub fn with_severity(mut self, severity: Option>) -> Self { - self.severity = severity; - self - } - - /// Lints the project. pub fn lint(self, input: &[PathBuf]) -> eyre::Result> { Ok(self.linter.lint(&input).expect("TODO: handle error")) } @@ -56,12 +35,10 @@ where // NOTE: add some way to specify linter profiles. For example having a profile adhering to the op stack, base, etc. // This can probably also be accomplished via the foundry.toml or some functions. Maybe have generic profile/settings -/// The main linter abstraction trait +// TODO: maybe add a way to specify the linter "profile" (ex. Default, OP Stack, etc.) pub trait Linter: Send + Sync + Clone { /// Enum of languages supported by the linter. type Language: Language; - // TODO: Add docs. This represents linter settings. (ex. Default, OP Stack, etc. - // type Settings: LinterSettings; type Lint: Lint + Ord; type LinterError: Error; @@ -69,11 +46,6 @@ pub trait Linter: Send + Sync + Clone { fn lint(&self, input: &[PathBuf]) -> Result, Self::LinterError>; } -// TODO: probably remove -pub trait LinterSettings { - fn lints() -> Vec; -} - pub struct LinterOutput(pub BTreeMap>); impl LinterOutput { diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index c77a54f36ec4..79e9a53bd5f1 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -20,12 +20,25 @@ use thiserror::Error; use crate::{Lint, Linter, LinterOutput, Severity, SourceLocation}; #[derive(Debug, Clone, Default)] -pub struct SolidityLinter; +pub struct SolidityLinter { + pub severity: Option>, + pub description: bool, +} impl SolidityLinter { pub fn new() -> Self { Self::default() } + + pub fn with_description(mut self, description: bool) -> Self { + self.description = description; + self + } + + pub fn with_severity(mut self, severity: Option>) -> Self { + self.severity = severity; + self + } } impl Linter for SolidityLinter { From f8e2c4d81cc7c2c6c2717c3dda632d28608b0f1e Mon Sep 17 00:00:00 2001 From: 0xKitsune <0xkitsune@protonmail.com> Date: Sun, 29 Dec 2024 02:40:49 -0500 Subject: [PATCH 21/21] fmt --- crates/forge/bin/cmd/lint.rs | 6 ++---- crates/lint/src/lib.rs | 13 +++++++------ crates/lint/src/sol/mod.rs | 3 ++- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/forge/bin/cmd/lint.rs b/crates/forge/bin/cmd/lint.rs index 32349c3dd9fe..38072050c4aa 100644 --- a/crates/forge/bin/cmd/lint.rs +++ b/crates/forge/bin/cmd/lint.rs @@ -1,11 +1,9 @@ use clap::{Parser, ValueHint}; use eyre::Result; -use forge_lint::sol::SolidityLinter; -use forge_lint::{OutputFormat, ProjectLinter, Severity}; +use forge_lint::{sol::SolidityLinter, OutputFormat, ProjectLinter, Severity}; use foundry_cli::utils::LoadConfig; use foundry_config::impl_figment_convert_basic; -use std::collections::HashSet; -use std::path::PathBuf; +use std::{collections::HashSet, path::PathBuf}; /// CLI arguments for `forge lint`. #[derive(Clone, Debug, Parser)] diff --git a/crates/lint/src/lib.rs b/crates/lint/src/lib.rs index 06130f7417a5..dd6e2ad146f0 100644 --- a/crates/lint/src/lib.rs +++ b/crates/lint/src/lib.rs @@ -32,8 +32,9 @@ where } } -// NOTE: add some way to specify linter profiles. For example having a profile adhering to the op stack, base, etc. -// This can probably also be accomplished via the foundry.toml or some functions. Maybe have generic profile/settings +// NOTE: add some way to specify linter profiles. For example having a profile adhering to the op +// stack, base, etc. This can probably also be accomplished via the foundry.toml or some functions. +// Maybe have generic profile/settings // TODO: maybe add a way to specify the linter "profile" (ex. Default, OP Stack, etc.) pub trait Linter: Send + Sync + Clone { @@ -135,14 +136,14 @@ impl SourceLocation { // let mut lines = fmtd_msg.lines(); // // skip the first line if it contains the same message as the one we just formatted, -// // unless it also contains a source location, in which case the entire error message is an -// // old style error message, like: +// // unless it also contains a source location, in which case the entire error message is +// an // old style error message, like: // // path/to/file:line:column: ErrorType: message // if lines // .clone() // .next() -// .is_some_and(|l| l.contains(short_msg) && l.bytes().filter(|b| *b == b':').count() < 3) -// { +// .is_some_and(|l| l.contains(short_msg) && l.bytes().filter(|b| *b == b':').count() < +// 3) { // let _ = lines.next(); // } diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 79e9a53bd5f1..bb06942e52af 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -50,7 +50,8 @@ impl Linter for SolidityLinter { let all_findings = input .into_par_iter() .map(|file| { - // NOTE: use all solidity lints for now but this should be configurable via SolidityLinter + // NOTE: use all solidity lints for now but this should be configurable via + // SolidityLinter let mut lints = SolLint::all(); // Initialize session and parsing environment