From b7da1bb6d41b871207d88905e01bfabb630f1bbb Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Tue, 1 Aug 2023 23:44:36 +0200 Subject: [PATCH 1/2] build(deps): add new dependency to the colored crate Signed-off-by: Luca Georges Francois --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index d32930b..345cbd0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,3 +17,4 @@ clap = { version = "4", features = ["derive"] } syn-solidity = "0.3.0" syn = { version = "2.0.26", features = ["full"] } proc-macro2 = { version = "1.0.65", features = ["span-locations"]} +colored = "2.0.4" From a40c6ab5eb9984a0216da5f69813c81320aae4d1 Mon Sep 17 00:00:00 2001 From: Luca Georges Francois Date: Tue, 1 Aug 2023 23:45:34 +0200 Subject: [PATCH 2/2] feat(log): replace metadata by a reporter able to log Signed-off-by: Luca Georges Francois --- src/commands/scan.rs | 9 +- .../implementations/missing_comments.rs | 117 +++++++++++------- .../implementations/mutable_functions.rs | 23 ++-- .../implementations/mutable_variables.rs | 23 ++-- .../implementations/mutation_grapher.rs | 6 +- .../implementations/struct_repacker.rs | 6 +- .../implementations/unused_imports.rs | 6 +- src/scanners/memory.rs | 1 + src/scanners/mod.rs | 19 +-- src/scanners/result.rs | 64 ++++++++++ 10 files changed, 195 insertions(+), 79 deletions(-) create mode 100644 src/scanners/result.rs diff --git a/src/commands/scan.rs b/src/commands/scan.rs index fd4c1bf..495faff 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -3,7 +3,7 @@ use clap::Parser; use std::fs; use syn_solidity::{File, Item}; -use crate::scanners::{memory::Metadata, Registry}; +use crate::scanners::{result::Reporter, Registry}; #[derive(Debug, Parser)] pub struct Command { @@ -19,13 +19,16 @@ impl Command { let ast_root: File = syn::parse_str(&code).with_context(|| "Failed to parse code into AST".to_string())?; let ast_items: &[Item] = &ast_root.items; - let metadata = Metadata::new(file_path); let scanners_registry = Registry::default(); + let mut reporter = Reporter::default(); scanners_registry .get_scanners() .iter() - .for_each(|s| s.execute(ast_items, &metadata)); + .for_each(|s| s.execute(ast_items, &mut reporter)); + + reporter.log(file_path); + Ok(()) } } diff --git a/src/scanners/implementations/missing_comments.rs b/src/scanners/implementations/missing_comments.rs index fe517d2..8f47357 100644 --- a/src/scanners/implementations/missing_comments.rs +++ b/src/scanners/implementations/missing_comments.rs @@ -2,7 +2,10 @@ use syn_solidity::{ Item, ItemContract, ItemEnum, ItemError, ItemEvent, ItemFunction, ItemStruct, ItemUdt, }; -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{ + result::{Reporter, Severity}, + Scanner, +}; /// A scanner responsible for looking at various Solidity items and reporting if documentation (comments) is missing. #[derive(Default)] @@ -10,21 +13,21 @@ pub struct MissingComments {} impl MissingComments { /// Scan every item in the provided contract object and looks for missing documentation. - fn scan_in_contract(&self, contract: &ItemContract, metadata: &Metadata) { + fn scan_in_contract(&self, contract: &ItemContract, reporter: &mut Reporter) { for item in &contract.body { match item { Item::Enum(enumeration) => { - self.check_missing_comments_for_enum(enumeration, metadata) + self.check_missing_comments_for_enum(enumeration, reporter) } - Item::Error(error) => self.check_missing_comments_for_error(error, metadata), - Item::Event(event) => self.check_missing_comments_for_event(event, metadata), + Item::Error(error) => self.check_missing_comments_for_error(error, reporter), + Item::Event(event) => self.check_missing_comments_for_event(event, reporter), Item::Function(function) => { - self.check_missing_comments_for_function(function, metadata) + self.check_missing_comments_for_function(function, reporter) } Item::Struct(structure) => { - self.check_missing_comments_for_structure(structure, metadata) + self.check_missing_comments_for_structure(structure, reporter) } - Item::Udt(udt) => self.check_missing_comments_for_udt(udt, metadata), + Item::Udt(udt) => self.check_missing_comments_for_udt(udt, reporter), /* Contracts can not be declared inside other contracts */ /* Import directives don't have attributes. */ /* Pragma directives don't have attributes. */ @@ -44,14 +47,20 @@ impl MissingComments { /// */ /// contract SimpleContract {} /// ``` - fn check_missing_comments_for_contract(&self, contract: &ItemContract, metadata: &Metadata) { + fn check_missing_comments_for_contract( + &self, + contract: &ItemContract, + reporter: &mut Reporter, + ) { if contract.attrs.is_empty() { let line = contract.span().start().line; let column = contract.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment on contract definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment on contract definition", ) } } @@ -68,14 +77,16 @@ impl MissingComments { /// Agate, /// } /// ``` - fn check_missing_comments_for_enum(&self, enumeration: &ItemEnum, metadata: &Metadata) { + fn check_missing_comments_for_enum(&self, enumeration: &ItemEnum, reporter: &mut Reporter) { if enumeration.attrs.is_empty() { let line = enumeration.span().start().line; let column = enumeration.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment on enum definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment on enum definition", ) } } @@ -87,14 +98,16 @@ impl MissingComments { /// /// @dev Emitted when a new Quartz has been mined! /// event QuartzMined(QuartzType indexed variety); /// ``` - fn check_missing_comments_for_event(&self, event: &ItemEvent, metadata: &Metadata) { + fn check_missing_comments_for_event(&self, event: &ItemEvent, reporter: &mut Reporter) { if event.attrs.is_empty() { let line = event.span().start().line; let column = event.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment on event definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment on event definition", ) } } @@ -106,14 +119,16 @@ impl MissingComments { /// /// @dev Your favorite stone was broken :( /// error BrokenQuartz(); /// ``` - fn check_missing_comments_for_error(&self, error: &ItemError, metadata: &Metadata) { + fn check_missing_comments_for_error(&self, error: &ItemError, reporter: &mut Reporter) { if error.attrs.is_empty() { let line = error.span().start().line; let column = error.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment on error definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment on error definition", ) } } @@ -127,14 +142,20 @@ impl MissingComments { /// return true; /// } /// ``` - fn check_missing_comments_for_function(&self, function: &ItemFunction, metadata: &Metadata) { + fn check_missing_comments_for_function( + &self, + function: &ItemFunction, + reporter: &mut Reporter, + ) { if function.attrs.is_empty() { let line = function.span().start().line; let column = function.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment for function definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment for function definition", ) } } @@ -148,14 +169,20 @@ impl MissingComments { /// QuartzType variety; /// } /// ``` - fn check_missing_comments_for_structure(&self, structure: &ItemStruct, metadata: &Metadata) { + fn check_missing_comments_for_structure( + &self, + structure: &ItemStruct, + reporter: &mut Reporter, + ) { if structure.attrs.is_empty() { let line = structure.span().start().line; let column = structure.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment for structure definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment for structure definition", ) } } @@ -167,14 +194,16 @@ impl MissingComments { /// /// @dev Got this one from OpenZeppelin, I ran out of Quartz references. /// type ShortString is bytes32; /// ``` - fn check_missing_comments_for_udt(&self, udt: &ItemUdt, metadata: &Metadata) { + fn check_missing_comments_for_udt(&self, udt: &ItemUdt, reporter: &mut Reporter) { if udt.attrs.is_empty() { let line = udt.span().start().line; let column = udt.span().start().column; - println!( - "{:}:{:}:{:} - Missing comment for user-defined type definition", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Warning, + "Missing comment for user-defined type definition", ) } } @@ -182,24 +211,24 @@ impl MissingComments { impl Scanner for MissingComments { /// Scans every root item (recursively if there is a contract) and reports missing documentation. - fn execute(&self, ast: &[Item], metadata: &Metadata) { + fn execute(&self, ast: &[Item], reporter: &mut Reporter) { for item in ast { match item { Item::Contract(contract) => { - self.check_missing_comments_for_contract(contract, metadata); - self.scan_in_contract(contract, metadata) + self.check_missing_comments_for_contract(contract, reporter); + self.scan_in_contract(contract, reporter) } Item::Enum(enumeration) => { - self.check_missing_comments_for_enum(enumeration, metadata) + self.check_missing_comments_for_enum(enumeration, reporter) } - Item::Error(error) => self.check_missing_comments_for_error(error, metadata), + Item::Error(error) => self.check_missing_comments_for_error(error, reporter), Item::Function(function) => { - self.check_missing_comments_for_function(function, metadata) + self.check_missing_comments_for_function(function, reporter) } Item::Struct(structure) => { - self.check_missing_comments_for_structure(structure, metadata) + self.check_missing_comments_for_structure(structure, reporter) } - Item::Udt(udt) => self.check_missing_comments_for_udt(udt, metadata), + Item::Udt(udt) => self.check_missing_comments_for_udt(udt, reporter), /* Events can not be declared at the root level. */ /* Import directives don't have attributes. */ /* Pragma directives don't have attributes. */ diff --git a/src/scanners/implementations/mutable_functions.rs b/src/scanners/implementations/mutable_functions.rs index 1fa5c5b..69ac672 100644 --- a/src/scanners/implementations/mutable_functions.rs +++ b/src/scanners/implementations/mutable_functions.rs @@ -1,4 +1,7 @@ -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{ + result::{Reporter, Severity}, + Scanner, +}; use syn_solidity::{FunctionAttribute, Item, ItemContract, ItemFunction, Mutability}; #[derive(Default)] @@ -6,16 +9,16 @@ pub struct MutableFunctions {} impl MutableFunctions { /// Scans all functions from the contract while discarding the other items. - fn scan_contract(&self, contract: &ItemContract, metadata: &Metadata) { + fn scan_contract(&self, contract: &ItemContract, reporter: &mut Reporter) { for item in &contract.body { if let Item::Function(function) = item { - self.scan_function(function, metadata) + self.scan_function(function, reporter) } } } /// Reports if a function is able to mutate the contract state. - fn scan_function(&self, function: &ItemFunction, metadata: &Metadata) { + fn scan_function(&self, function: &ItemFunction, reporter: &mut Reporter) { // TODO: There is probably a cleaner way to check this. if function.kind.as_str() == "modifier" { return; @@ -36,20 +39,22 @@ impl MutableFunctions { let line = function.span().start().line; let column = function.span().start().column; - println!( - "{:}:{:}:{:} - Function can mutate contract state", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Info, + "Function can mutate contract state", ) } } impl Scanner for MutableFunctions { /// Scans every contract and reports functions able to mutate the storage state. - fn execute(&self, ast: &[Item], metadata: &Metadata) { + fn execute(&self, ast: &[Item], reporter: &mut Reporter) { for item in ast { // Mutable functions are only located inside contracts. if let Item::Contract(contract) = item { - self.scan_contract(contract, metadata) + self.scan_contract(contract, reporter) } } } diff --git a/src/scanners/implementations/mutable_variables.rs b/src/scanners/implementations/mutable_variables.rs index 49c0f95..9fa3805 100644 --- a/src/scanners/implementations/mutable_variables.rs +++ b/src/scanners/implementations/mutable_variables.rs @@ -1,4 +1,7 @@ -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{ + result::{Reporter, Severity}, + Scanner, +}; use syn_solidity::{Item, ItemContract, VariableAttribute, VariableDefinition}; #[derive(Default)] @@ -6,16 +9,16 @@ pub struct MutableVariables {} impl MutableVariables { /// Scans all variables from the contract while discarding the other items. - fn scan_contract(&self, contract: &ItemContract, metadata: &Metadata) { + fn scan_contract(&self, contract: &ItemContract, reporter: &mut Reporter) { for item in &contract.body { if let Item::Variable(variable) = item { - self.scan_variable(variable, metadata) + self.scan_variable(variable, reporter) } } } /// Reports if a variable is likely to mutate. - fn scan_variable(&self, variable: &VariableDefinition, metadata: &Metadata) { + fn scan_variable(&self, variable: &VariableDefinition, reporter: &mut Reporter) { let immutable_variable_attributes = [ &VariableAttribute::Constant(Default::default()), &VariableAttribute::Immutable(Default::default()), @@ -30,19 +33,21 @@ impl MutableVariables { let line = variable.span().start().line; let column = variable.span().start().column; - println!( - "{:}:{:}:{:} - Variable state can be changed", - metadata.file_path, line, column + reporter.report( + line, + column, + Severity::Info, + "Variable state can be changed", ) } } impl Scanner for MutableVariables { /// Scans every contract and reports variables able to mutate. - fn execute(&self, ast: &[Item], metadata: &Metadata) { + fn execute(&self, ast: &[Item], reporter: &mut Reporter) { for item in ast { if let Item::Contract(contract) = item { - self.scan_contract(contract, metadata) + self.scan_contract(contract, reporter) } } } diff --git a/src/scanners/implementations/mutation_grapher.rs b/src/scanners/implementations/mutation_grapher.rs index e3624fd..284d56e 100644 --- a/src/scanners/implementations/mutation_grapher.rs +++ b/src/scanners/implementations/mutation_grapher.rs @@ -1,6 +1,6 @@ use syn_solidity::Item; -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{result::Reporter, Scanner}; #[derive(Default)] pub struct MutationGrapher {} @@ -8,7 +8,9 @@ pub struct MutationGrapher {} impl MutationGrapher {} impl Scanner for MutationGrapher { - fn execute(&self, _ast: &[Item], _metadata: &Metadata) { + fn execute(&self, _ast: &[Item], _reporter: &mut Reporter) { + /* println!("todo!") + */ } } diff --git a/src/scanners/implementations/struct_repacker.rs b/src/scanners/implementations/struct_repacker.rs index 260266d..f69db63 100644 --- a/src/scanners/implementations/struct_repacker.rs +++ b/src/scanners/implementations/struct_repacker.rs @@ -1,12 +1,14 @@ use syn_solidity::Item; -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{result::Reporter, Scanner}; #[derive(Default)] pub struct StructRepacker {} impl Scanner for StructRepacker { - fn execute(&self, _ast: &[Item], _metadata: &Metadata) { + fn execute(&self, _ast: &[Item], _reporter: &mut Reporter) { + /* println!("todo!") + */ } } diff --git a/src/scanners/implementations/unused_imports.rs b/src/scanners/implementations/unused_imports.rs index c6853da..d91572c 100644 --- a/src/scanners/implementations/unused_imports.rs +++ b/src/scanners/implementations/unused_imports.rs @@ -1,12 +1,14 @@ use syn_solidity::Item; -use crate::scanners::{memory::Metadata, Scanner}; +use crate::scanners::{result::Reporter, Scanner}; #[derive(Default)] pub struct UnusedImports {} impl Scanner for UnusedImports { - fn execute(&self, _ast: &[Item], _metadata: &Metadata) { + fn execute(&self, _ast: &[Item], _reporter: &mut Reporter) { + /* println!("todo!") + */ } } diff --git a/src/scanners/memory.rs b/src/scanners/memory.rs index 3bf714c..69f564d 100644 --- a/src/scanners/memory.rs +++ b/src/scanners/memory.rs @@ -3,6 +3,7 @@ pub struct Metadata<'a> { pub file_path: &'a str, } +#[allow(dead_code)] impl<'a> Metadata<'a> { pub fn new(file_path: &'a str) -> Self { Metadata { file_path } diff --git a/src/scanners/mod.rs b/src/scanners/mod.rs index 680359e..bc67c2f 100644 --- a/src/scanners/mod.rs +++ b/src/scanners/mod.rs @@ -1,16 +1,19 @@ pub mod implementations; pub mod memory; +pub mod result; -use self::implementations::{ - missing_comments::MissingComments, mutable_functions::MutableFunctions, - mutable_variables::MutableVariables, mutation_grapher::MutationGrapher, - struct_repacker::StructRepacker, unused_imports::UnusedImports, +use self::{ + implementations::{ + missing_comments::MissingComments, mutable_functions::MutableFunctions, + mutable_variables::MutableVariables, mutation_grapher::MutationGrapher, + struct_repacker::StructRepacker, unused_imports::UnusedImports, + }, + result::Reporter, }; -use crate::scanners::memory::Metadata; use syn_solidity::Item; pub trait Scanner { - fn execute(&self, ast: &[Item], metadata: &Metadata); + fn execute(&self, ast: &[Item], reporter: &mut Reporter); } pub struct Registry { @@ -45,14 +48,14 @@ impl Default for Registry { #[cfg(test)] mod tests { - use super::{Metadata, Registry, Scanner}; + use super::{result::Reporter, Registry, Scanner}; use syn_solidity::Item; #[derive(Default)] struct MockScanner {} impl Scanner for MockScanner { - fn execute(&self, _ast: &[Item], _metadata: &Metadata) { + fn execute(&self, _ast: &[Item], _reporter: &mut Reporter) { todo!() } } diff --git a/src/scanners/result.rs b/src/scanners/result.rs new file mode 100644 index 0000000..458cc15 --- /dev/null +++ b/src/scanners/result.rs @@ -0,0 +1,64 @@ +use colored::*; +use std::fmt; + +#[allow(dead_code)] +pub enum Severity { + Error, + Warning, + Info, + Boost, +} + +impl Severity { + fn to_colored_string(&self) -> ColoredString { + match self { + Severity::Error => "ERROR".bold().red(), + Severity::Warning => "WARNING".bold().yellow(), + Severity::Info => "INFO".bold().blue(), + Severity::Boost => "BOOST".bold().cyan(), + } + } +} + +struct Report { + line: usize, + column: usize, + severity: Severity, + message: String, +} + +impl fmt::Display for Report { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str( + format!( + "{:>4}:{:<4} {:10} {}", + self.line, + self.column, + self.severity.to_colored_string(), + self.message + ) + .as_str(), + ) + } +} + +#[derive(Default)] +pub struct Reporter { + reports: Vec, +} + +impl Reporter { + pub fn report(&mut self, line: usize, column: usize, severity: Severity, message: &str) { + self.reports.push(Report { + line, + column, + severity, + message: message.to_string(), + }) + } + + pub fn log(self, file_path: &str) { + println!("{}", file_path.bold().underline()); + self.reports.iter().for_each(|r| println!("{}", r)); + } +}