From d22320e2563e55b60bd741d2e476bb3ecd284856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Gonz=C3=A1lez?= Date: Thu, 31 Aug 2023 17:03:04 +0200 Subject: [PATCH] feat: add support for actions without parent conditions (#26) --- src/emitter.rs | 127 +++++++++++++++++++++++++++++++++++++++++++++-- src/modifiers.rs | 37 +++----------- src/semantics.rs | 40 ++++++--------- src/tokenizer.rs | 2 +- src/utils.rs | 32 ++++++++++++ 5 files changed, 179 insertions(+), 59 deletions(-) diff --git a/src/emitter.rs b/src/emitter.rs index e15ec41..e734d1b 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -158,9 +158,13 @@ impl<'a> EmitterI<'a> { // It's fine to unwrap here because we check that no action appears outside of a condition. let last_modifier = self.modifier_stack.last().unwrap(); let function_name = if is_revert { - let mut words = condition.title.split(' '); + let mut words = condition.title.split_whitespace(); // It is fine to unwrap because conditions have at least one word in them. let keyword = capitalize_first_letter(words.next().unwrap()); + + // Map an iterator over the words of a condition to the test name. + // + // Example: [when, something, happens] -> WhenSomethingHappens let test_name = words.fold( String::with_capacity(condition.title.len() - keyword.len()), |mut acc, w| { @@ -211,9 +215,40 @@ impl<'a> Visitor for EmitterI<'a> { let contract_header = self.emit_contract_header(root); emitted.push_str(&contract_header); - for condition in &root.asts { - if let Ast::Condition(condition) = condition { + for ast in &root.asts { + if let Ast::Condition(condition) = ast { emitted.push_str(&self.visit_condition(condition)?); + } else if let Ast::Action(action) = ast { + // We found a top-level action. These don't have parent conditions, + // so we emit a test without modifiers. + let fn_indentation = self.emitter.indent(); + + let words = action.title.split_whitespace(); + // Remove "it should" if it occurs. We need this step because users will usually + // write "it should", but for the test name we want to keep what's after the + // "should". + let words = words.skip_while(|&s| s == "should" || s == "it"); + + // Map an iterator over the words of an action to the test name. + // + // Example: [do, stuff] -> DoStuff + let test_name = + words.fold(String::with_capacity(action.title.len()), |mut acc, w| { + acc.reserve(w.len() + 1); + acc.push_str(&capitalize_first_letter(w)); + acc + }); + + // We need to sanitize here because and not in a previous compiler + // phase because we want to emit the action as is in a comment. + let test_name = sanitize(&test_name); + let test_name = format!("test_{}", test_name); + let fn_header = format!("{}function {}() external {{\n", fn_indentation, test_name); + + emitted.push_str(&fn_header); + emitted.push_str(&self.visit_action(action)?); + emitted.push_str(format!("{}}}\n", fn_indentation).as_str()); + emitted.push('\n'); } } @@ -380,6 +415,92 @@ contract FileTest { Ok(()) } + #[test] + fn test_actions_without_conditions() -> Result<()> { + let file_contents = + String::from("file.sol\n└── it should do st-ff\n └── it never reverts"); + + assert_eq!( + &scaffold_with_flags(&file_contents, true, 2, "0.8.0")?, + r"pragma solidity 0.8.0; + +contract FileTest { + function test_DoSt_ff() external { + // it should do st-ff + } + + function test_NeverReverts() external { + // it never reverts + } +}" + ); + + let file_contents = String::from( + "file.sol +└── it should do stuff +└── when something happens + └── it should revert", + ); + + assert_eq!( + &scaffold_with_flags(&file_contents, true, 2, "0.8.0")?, + r"pragma solidity 0.8.0; + +contract FileTest { + function test_DoStuff() external { + // it should do stuff + } + + modifier whenSomethingHappens() { + _; + } + + function test_RevertWhen_SomethingHappens() + external + whenSomethingHappens + { + // it should revert + } +}" + ); + + let file_contents = String::from( + "file.sol +└── it should do stuff +└── when something happens + └── it should revert +└── it does everything", + ); + + assert_eq!( + &scaffold_with_flags(&file_contents, true, 2, "0.8.0")?, + r"pragma solidity 0.8.0; + +contract FileTest { + function test_DoStuff() external { + // it should do stuff + } + + modifier whenSomethingHappens() { + _; + } + + function test_RevertWhen_SomethingHappens() + external + whenSomethingHappens + { + // it should revert + } + + function test_DoesEverything() external { + // it does everything + } +}" + ); + + Ok(()) + } + #[test] fn test_unsanitized_input() -> Result<()> { let file_contents = diff --git a/src/modifiers.rs b/src/modifiers.rs index 68cc7df..427994e 100644 --- a/src/modifiers.rs +++ b/src/modifiers.rs @@ -2,7 +2,7 @@ use indexmap::IndexMap; use crate::{ ast::{self, Ast}, - utils::capitalize_first_letter, + utils::{lower_first_letter, to_pascal_case}, visitor::Visitor, }; @@ -59,8 +59,10 @@ impl Visitor for ModifierDiscoverer { } fn visit_condition(&mut self, condition: &ast::Condition) -> Result { - self.modifiers - .insert(condition.title.clone(), to_modifier(&condition.title)); + self.modifiers.insert( + condition.title.clone(), + lower_first_letter(&to_pascal_case(&condition.title)), + ); for condition in &condition.asts { if let Ast::Condition(condition) = condition { @@ -77,26 +79,6 @@ impl Visitor for ModifierDiscoverer { } } -/// Converts a condition title to a modifier. -/// -/// The conversion is done by capitalizing the first letter of each word -/// in the title and removing the spaces. For example, the title -/// `when only owner` is converted to the `whenOnlyOwner` modifier. -/// Note that the `w` in `when` is not capitalized following solidity conventions. -fn to_modifier(title: &str) -> String { - title - .split_whitespace() - .enumerate() - .map(|(idx, s)| { - if idx > 0 { - capitalize_first_letter(s) - } else { - s.to_string() - } - }) - .collect::() -} - #[cfg(test)] mod tests { use indexmap::IndexMap; @@ -104,7 +86,7 @@ mod tests { use pretty_assertions::assert_eq; use crate::error::Result; - use crate::modifiers::{to_modifier, ModifierDiscoverer}; + use crate::modifiers::ModifierDiscoverer; use crate::parser::Parser; use crate::tokenizer::Tokenizer; @@ -117,13 +99,6 @@ mod tests { Ok(discoverer.modifiers) } - #[test] - fn test_to_modifier() { - assert_eq!(to_modifier("when only owner"), "whenOnlyOwner"); - assert_eq!(to_modifier("when"), "when"); - assert_eq!(to_modifier(""), ""); - } - #[test] fn test_one_child() { assert_eq!( diff --git a/src/semantics.rs b/src/semantics.rs index cc98c2c..a4cb5b6 100644 --- a/src/semantics.rs +++ b/src/semantics.rs @@ -53,8 +53,6 @@ pub enum ErrorKind { NodeUnexpected, /// Found no rules to emit. TreeEmpty, - /// Found an action with no conditions. - ActionWithoutConditions, /// Found a condition with no children. ConditionEmpty, /// This enum may grow additional variants, so this makes sure clients @@ -77,7 +75,6 @@ impl fmt::Display for ErrorKind { FileExtensionInvalid => write!(f, "output filename should have a .sol extension"), NodeUnexpected => write!(f, "unexpected child node"), TreeEmpty => write!(f, "no rules where defined"), - ActionWithoutConditions => write!(f, "found an action without conditions"), ConditionEmpty => write!(f, "found a condition with no children"), _ => unreachable!(), } @@ -154,17 +151,19 @@ impl Visitor for SemanticAnalyzer<'_> { self.error(Span::splat(root.span.end), ErrorKind::TreeEmpty); } - root.asts.iter().for_each(|ast| match ast { - Ast::Condition(condition) => { - let _ = self.visit_condition(condition); - } - Ast::Action(action) => { - self.error(action.span, ErrorKind::ActionWithoutConditions); - } - Ast::Root(root) => { - self.error(root.span, ErrorKind::NodeUnexpected); + for ast in &root.asts { + match ast { + Ast::Condition(condition) => { + self.visit_condition(condition)?; + } + Ast::Action(action) => { + self.visit_action(action)?; + } + Ast::Root(root) => { + self.error(root.span, ErrorKind::NodeUnexpected); + } } - }); + } Ok(()) } @@ -180,10 +179,10 @@ impl Visitor for SemanticAnalyzer<'_> { for ast in &condition.asts { match ast { Ast::Condition(condition) => { - let _ = self.visit_condition(condition); + self.visit_condition(condition)?; } Ast::Action(action) => { - let _ = self.visit_action(action); + self.visit_action(action)?; } Ast::Root(root) => { self.error(root.span, ErrorKind::NodeUnexpected); @@ -306,14 +305,7 @@ mod tests { } #[test] - fn test_action_without_conditions() { - assert_eq!( - analyze("file.sol\n└── it a something").unwrap_err(), - vec![semantics::Error { - kind: ActionWithoutConditions, - text: "file.sol\n└── it a something".to_string(), - span: Span::new(Position::new(9, 2, 1), Position::new(32, 2, 18)), - }] - ); + fn test_allow_action_without_conditions() { + assert!(analyze("file.sol\n└── it a something").is_ok()); } } diff --git a/src/tokenizer.rs b/src/tokenizer.rs index f568219..c3eba90 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -397,7 +397,7 @@ impl<'s, T: Borrow> TokenizerI<'s, T> { return Err(self.error(self.span(), ErrorKind::FileNameCharInvalid(self.char()))); } else if self.peek().is_none() || self.peek().is_some_and(|c| c.is_whitespace()) { lexeme.push(self.char()); - let kind = match lexeme.to_ascii_lowercase().as_str() { + let kind = match lexeme.to_lowercase().as_str() { "when" => TokenKind::When, "it" => TokenKind::It, "given" => TokenKind::Given, diff --git a/src/utils.rs b/src/utils.rs index 0dae7e1..4f7b1df 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,8 +6,40 @@ pub fn capitalize_first_letter(s: &str) -> String { } } +pub fn lower_first_letter(s: &str) -> String { + let mut c = s.chars(); + match c.next() { + None => String::new(), + Some(f) => f.to_lowercase().collect::() + c.as_str(), + } +} + /// This functions makes the appropriate changes to a string to /// make it a valid identifier. pub fn sanitize(identifier: &str) -> String { identifier.replace('-', "_").replace(['\'', '"'], "") } + +/// Converts a sentence to pascal case. +/// +/// The conversion is done by capitalizing the first letter of each word +/// in the title and removing the spaces. For example, the sentence +/// `when only owner` is converted to the `WhenOnlyOwner` string. +pub fn to_pascal_case(sentence: &str) -> String { + sentence + .split_whitespace() + .map(capitalize_first_letter) + .collect::() +} + +#[cfg(test)] +mod tests { + use super::to_pascal_case; + + #[test] + fn test_to_modifier() { + assert_eq!(to_pascal_case("when only owner"), "WhenOnlyOwner"); + assert_eq!(to_pascal_case("when"), "When"); + assert_eq!(to_pascal_case(""), ""); + } +}