diff --git a/.gitignore b/.gitignore index d2dc72c10..44e3f17da 100644 --- a/.gitignore +++ b/.gitignore @@ -2,5 +2,4 @@ Cargo.lock /target **/*.rs.bk -.DS_Store -.idea + diff --git a/src/sema/expression.rs b/src/sema/expression.rs index 1b9e20c69..3fa6b1436 100644 --- a/src/sema/expression.rs +++ b/src/sema/expression.rs @@ -2487,6 +2487,7 @@ pub fn expression( diagnostics, resolve_to, )?; + used_variable(ns, &cond, symtable); let cond = cast(&c.loc(), cond, &Type::Bool, true, ns, diagnostics)?; @@ -4796,7 +4797,7 @@ fn struct_literal( diagnostics, Some(&struct_def.fields[i].ty), )?; - + used_variable(ns, &expr, symtable); fields.push(cast( loc, expr, @@ -5411,7 +5412,7 @@ fn named_struct_literal( diagnostics, Some(&f.ty), )?; - + used_variable(ns, &expr, symtable); fields[i] = cast(loc, expr, &f.ty, true, ns, diagnostics)?; } None => { @@ -6643,6 +6644,7 @@ fn resolve_array_literal( first.ty() }; + used_variable(ns, &first, symtable); let mut exprs = vec![first]; for e in flattened { @@ -6656,6 +6658,7 @@ fn resolve_array_literal( diagnostics, Some(&ty), )?; + used_variable(ns, &other, symtable); if other.ty() != ty { other = cast(&e.loc(), other, &ty, true, ns, diagnostics)?; diff --git a/src/sema/mod.rs b/src/sema/mod.rs index 94be43520..97f7bdfd4 100644 --- a/src/sema/mod.rs +++ b/src/sema/mod.rs @@ -43,14 +43,14 @@ pub const SOLANA_SPARSE_ARRAY_SIZE: u64 = 1024; /// Performs semantic analysis and checks for unused variables pub fn sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) { - perform_sema(file, cache, ns); + sema_file(file, cache, ns); check_unused_namespace_variables(ns); check_unused_events(ns); } /// Load a file file from the cache, parse and resolve it. The file must be present in /// the cache. This function is recursive for imports. -pub fn perform_sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) { +pub fn sema_file(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) { let file_no = ns.files.len(); let source_code = cache.get_file_contents(&file.full_path); @@ -177,7 +177,7 @@ fn resolve_import( } Ok(file) => { if !ns.files.iter().any(|f| *f == file.full_path) { - perform_sema(file.clone(), cache, ns); + sema_file(file.clone(), cache, ns); // give up if we failed if diagnostics::any_errors(&ns.diagnostics) { diff --git a/src/sema/statements.rs b/src/sema/statements.rs index 037df29b4..cc3710a0a 100644 --- a/src/sema/statements.rs +++ b/src/sema/statements.rs @@ -319,9 +319,7 @@ fn statement( diagnostics, )?; - let mut initialized = false; let initializer = if let Some(init) = initializer { - initialized = true; let expr = expression( init, file_no, @@ -344,8 +342,8 @@ fn statement( &decl.name, var_ty.clone(), ns, - initialized, - VariableUsage::StateVariable, + initializer.is_some(), + VariableUsage::LocalVariable, ) { ns.check_shadowing(file_no, contract_no, &decl.name); @@ -718,11 +716,6 @@ fn statement( return Err(()); } - // for offset in symtable.returns.iter() { - // let elem = symtable.vars.get_mut(offset).unwrap(); - // (*elem).assigned = true; - // } - res.push(Statement::Return( *loc, symtable @@ -1302,7 +1295,9 @@ fn destructure_values( diagnostics, Some(&Type::Bool), )?; - + std::println!("Aqui!!"); + std::println!("{:?}", cond); + used_variable(ns, &cond, symtable); let left = destructure_values( &left.loc(), left, @@ -1314,7 +1309,7 @@ fn destructure_values( ns, diagnostics, )?; - + used_variable(ns, &left, symtable); let right = destructure_values( &right.loc(), right, @@ -1326,16 +1321,15 @@ fn destructure_values( ns, diagnostics, )?; + used_variable(ns, &right, symtable); - let ternary = Expression::Ternary( + return Ok(Expression::Ternary( *loc, Type::Unreachable, Box::new(cond), Box::new(left), Box::new(right), - ); - used_variable(ns, &ternary, symtable); - return Ok(ternary); + )); } _ => { let mut list = Vec::new(); diff --git a/src/sema/symtable.rs b/src/sema/symtable.rs index ad7518a83..f495b888b 100644 --- a/src/sema/symtable.rs +++ b/src/sema/symtable.rs @@ -22,7 +22,7 @@ pub enum VariableUsage { Parameter, ReturnVariable, AnonymousReturnVariable, - StateVariable, + LocalVariable, DestructureVariable, TryCatchReturns, TryCatchErrorString, diff --git a/src/sema/unused_variable.rs b/src/sema/unused_variable.rs index a694ac96f..e45f11d9f 100644 --- a/src/sema/unused_variable.rs +++ b/src/sema/unused_variable.rs @@ -5,7 +5,7 @@ use crate::sema::ast::{ use crate::sema::symtable::{Symtable, VariableUsage}; use crate::sema::{ast, symtable}; -/// Mark variables as assigned, either in the symbol table (for state variables) or in the +/// Mark variables as assigned, either in the symbol table (for local variables) or in the /// Namespace (for storage variables) pub fn assigned_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtable) { match &exp { @@ -39,7 +39,7 @@ pub fn assigned_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Sy } } -/// Mark variables as used, either in the symbol table (for state variables) or in the +/// Mark variables as used, either in the symbol table (for local variables) or in the /// Namespace (for global constants and storage variables) /// The functions handles complex expressions in a recursive fashion, such as array length call, /// assign expressions and array subscripts. @@ -73,11 +73,6 @@ pub fn used_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtab used_variable(ns, index, symtable); } - Expression::Assign(_, _, assigned, assignee) => { - assigned_variable(ns, assigned, symtable); - used_variable(ns, assignee, symtable); - } - Expression::DynamicArrayLength(_, array) | Expression::StorageArrayLength { loc: _, @@ -97,18 +92,6 @@ pub fn used_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtab used_variable(ns, expr, symtable); } - Expression::ArrayLiteral(_, _, _, args) | Expression::ConstArrayLiteral(_, _, _, args) => { - for arg in args { - used_variable(ns, arg, symtable); - } - } - - Expression::Ternary(_, _, cond, left, right) => { - used_variable(ns, cond, symtable); - used_variable(ns, left, symtable); - used_variable(ns, right, symtable); - } - _ => {} } } @@ -223,7 +206,7 @@ fn generate_unused_warning(loc: Loc, text: &str, notes: Vec) -> Diagnostic } } -/// Emit different warning types according to the state variable usage +/// Emit different warning types according to the function variable usage pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option { match &variable.usage_type { VariableUsage::Parameter => { @@ -231,7 +214,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option Option Option { + VariableUsage::LocalVariable => { if !variable.assigned && !variable.read { return Some(generate_unused_warning( variable.id.loc, &format!( - "State variable '{}' has never been read nor assigned", + "local variable '{}' has never been read nor assigned", variable.id.name ), vec![], @@ -268,7 +251,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option Option Option Option Option Option Option Option { if variable.assigned && !variable.read { return Some(generate_unused_warning( variable.loc, &format!( - "Storage variable '{}' has been assigned, but never read.", + "storage variable '{}' has been assigned, but never read", variable.name ), vec![], @@ -363,7 +346,7 @@ fn emit_warning_contract_variables(variable: &ast::Variable) -> Option ast::Names parse_and_resolve("test.sol", &mut cache, Target::Generic) } -fn count_warnings(diagnostics: &Vec) -> usize { +fn count_warnings(diagnostics: &[Diagnostic]) -> usize { diagnostics .iter() .filter(|&x| x.level == Level::Warning) .count() } -fn get_first_warning(diagnostics: &Vec) -> &Diagnostic { +fn get_first_warning(diagnostics: &[Diagnostic]) -> &Diagnostic { diagnostics .iter() .find_or_first(|&x| x.level == Level::Warning) .unwrap() } -fn get_warnings(diagnostics: &Vec) -> Vec<&Diagnostic> { +fn get_warnings(diagnostics: &[Diagnostic]) -> Vec<&Diagnostic> { let mut res = Vec::new(); for elem in diagnostics { if elem.level == Level::Warning { @@ -41,10 +41,10 @@ fn get_warnings(diagnostics: &Vec) -> Vec<&Diagnostic> { } } - return res; + res } -fn assert_message_in_warnings(diagnostics: &Vec, message: &str) -> bool { +fn assert_message_in_warnings(diagnostics: &[Diagnostic], message: &str) -> bool { let warnings = get_warnings(diagnostics); for warning in warnings { if warning.message == message { @@ -52,7 +52,7 @@ fn assert_message_in_warnings(diagnostics: &Vec, message: &str) -> b } } - return false; + false } #[test] @@ -85,7 +85,7 @@ fn emit_event() { assert_eq!(count_warnings(&ns.diagnostics), 1); assert_eq!( get_first_warning(&ns.diagnostics).message, - "Event 'Hello' has never been emitted." + "event 'Hello' has never been emitted" ); } @@ -132,20 +132,14 @@ fn constant_variable() { let ns = generic_parse_two_files(file_1, file_2); assert_eq!(count_warnings(&ns.diagnostics), 2); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'cte' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Global constant 'outside' has never been used." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'cte' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "global constant 'outside' has never been used" + )); } #[test] @@ -165,11 +159,11 @@ fn storage_variable() { assert_eq!(warnings.len(), 2); assert_eq!( warnings[0].message, - "Storage variable 'str' has been assigned, but never read." + "storage variable 'str' has been assigned, but never read" ); assert_eq!( warnings[1].message, - "Storage variable 'str2' has never been used." + "storage variable 'str2' has never been used" ); let file = r#" @@ -187,7 +181,7 @@ fn storage_variable() { assert_eq!(count_warnings(&ns.diagnostics), 1); assert_eq!( get_first_warning(&ns.diagnostics).message, - "Storage variable 'str2' has been assigned, but never read." + "storage variable 'str2' has been assigned, but never read" ); let file = r#" @@ -230,27 +224,18 @@ fn state_variable() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 3); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'b' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'a' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'c' has never been read nor assigned" - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'b' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'a' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'c' has never been read nor assigned" + )); } #[test] @@ -286,34 +271,22 @@ fn struct_usage() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 4); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 't2' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 't1' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 't5' has never been read nor assigned" - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 't6' has never been used." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 't2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 't1' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 't5' has never been read nor assigned" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 't6' has never been used" + )); } #[test] @@ -348,41 +321,26 @@ fn subscript() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 5); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'arr4' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'arr5' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'arr1' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'arr2' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'byteArr' has been assigned, but never read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr4' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr5' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'arr1' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'arr2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'byteArr' has been assigned, but never read" + )); let file = r#" contract Test { @@ -451,20 +409,14 @@ fn assign_trunc_cast() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 2); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'b32' has never been assigned a value, but has been read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'byteArr' has been assigned, but never read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'b32' has never been assigned a value, but has been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'byteArr' has been assigned, but never read" + )); } #[test] @@ -493,20 +445,14 @@ fn array_length() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 2); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'arr3' has never been assigned a value, but has been read.", - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'arr4' has never been assigned a value, but has been read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr3' has never been assigned a value, but has been read", + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'arr4' has never been assigned a value, but has been read" + )); } #[test] @@ -604,20 +550,14 @@ fn statements() { "#; let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 2); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Function parameter 'a' has never been read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'ct' has been assigned, but never read.", - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "function parameter 'a' has never been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'ct' has been assigned, but never read", + )); } #[test] @@ -715,20 +655,14 @@ fn array_push_pop() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 2); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "State variable 'vec2' has been assigned, but never read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'vec1' has been assigned, but never read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "local variable 'vec2' has been assigned, but never read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'vec1' has been assigned, but never read" + )); let file = r#" contract Test1 { @@ -772,27 +706,18 @@ fn return_variable() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 3); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Destructure variable 'a' has never been used." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Return variable 'hey' has never been assigned" - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'testing' has been assigned, but never read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "destructure variable 'a' has never been used" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "return variable 'hey' has never been assigned" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'testing' has been assigned, but never read" + )); } #[test] @@ -820,27 +745,18 @@ fn try_catch() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 3); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Try-catch error bytes 'returnData' has never been used." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Try-catch returns variable 'returnedInstance' has never been read." - ), - true - ); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Try-catch error string 'revertReason' has never been used." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch error bytes 'returnData' has never been used" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch returns variable 'returnedInstance' has never been read" + )); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "try-catch error string 'revertReason' has never been used" + )); let file = r#" contract CalledContract { @@ -870,13 +786,10 @@ fn try_catch() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 1); - assert_eq!( - assert_message_in_warnings( - &ns.diagnostics, - "Storage variable 'notOk' has been assigned, but never read." - ), - true - ); + assert!(assert_message_in_warnings( + &ns.diagnostics, + "storage variable 'notOk' has been assigned, but never read" + )); } #[test] @@ -907,3 +820,27 @@ fn destructure() { let ns = generic_target_parse(file); assert_eq!(count_warnings(&ns.diagnostics), 0); } + +#[test] +fn struct_initialization() { + let file = r#" + contract Test1{ + struct Test2{ + uint8 a; + uint8 b; + } + + function callTest() public pure returns (uint32 ret) { + uint8 tg = 1; + uint8 th = 2; + + Test2 memory t2; + t2 = Test2(tg, th); + ret = t2.a; + } + } + "#; + + let ns = generic_target_parse(file); + assert_eq!(count_warnings(&ns.diagnostics), 0); +}