Skip to content

Commit

Permalink
Add more tests cases and fix reviewer's concerns
Browse files Browse the repository at this point in the history
  • Loading branch information
@LucasSte
LucasSte committed Jul 8, 2021
1 parent 8edac3e commit 2f71a12
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 271 deletions.
3 changes: 1 addition & 2 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
Cargo.lock
/target
**/*.rs.bk
.DS_Store
.idea

7 changes: 5 additions & 2 deletions src/sema/expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down Expand Up @@ -4796,7 +4797,7 @@ fn struct_literal(
diagnostics,
Some(&struct_def.fields[i].ty),
)?;

used_variable(ns, &expr, symtable);
fields.push(cast(
loc,
expr,
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -6643,6 +6644,7 @@ fn resolve_array_literal(
first.ty()
};

used_variable(ns, &first, symtable);
let mut exprs = vec![first];

for e in flattened {
Expand All @@ -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)?;
Expand Down
6 changes: 3 additions & 3 deletions src/sema/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
24 changes: 9 additions & 15 deletions src/sema/statements.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -1314,7 +1309,7 @@ fn destructure_values(
ns,
diagnostics,
)?;

used_variable(ns, &left, symtable);
let right = destructure_values(
&right.loc(),
right,
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/sema/symtable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub enum VariableUsage {
Parameter,
ReturnVariable,
AnonymousReturnVariable,
StateVariable,
LocalVariable,
DestructureVariable,
TryCatchReturns,
TryCatchErrorString,
Expand Down
53 changes: 18 additions & 35 deletions src/sema/unused_variable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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: _,
Expand All @@ -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);
}

_ => {}
}
}
Expand Down Expand Up @@ -223,15 +206,15 @@ fn generate_unused_warning(loc: Loc, text: &str, notes: Vec<Note>) -> 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<Diagnostic> {
match &variable.usage_type {
VariableUsage::Parameter => {
if !variable.read {
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Function parameter '{}' has never been read.",
"function parameter '{}' has never been read",
variable.id.name
),
vec![],
Expand All @@ -245,7 +228,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Return variable '{}' has never been assigned",
"return variable '{}' has never been assigned",
variable.id.name
),
vec![],
Expand All @@ -254,12 +237,12 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
None
}

VariableUsage::StateVariable => {
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![],
Expand All @@ -268,7 +251,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"State variable '{}' has been assigned, but never read.",
"local variable '{}' has been assigned, but never read",
variable.id.name
),
vec![],
Expand All @@ -277,7 +260,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"State variable '{}' has never been assigned a value, but has been read.",
"local variable '{}' has never been assigned a value, but has been read",
variable.id.name
),
vec![],
Expand All @@ -291,7 +274,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Destructure variable '{}' has never been used.",
"destructure variable '{}' has never been used",
variable.id.name
),
vec![],
Expand All @@ -306,7 +289,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Try-catch returns variable '{}' has never been read.",
"try-catch returns variable '{}' has never been read",
variable.id.name
),
vec![],
Expand All @@ -321,7 +304,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Try-catch error bytes '{}' has never been used.",
"try-catch error bytes '{}' has never been used",
variable.id.name
),
vec![],
Expand All @@ -336,7 +319,7 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
return Some(generate_unused_warning(
variable.id.loc,
&format!(
"Try-catch error string '{}' has never been used.",
"try-catch error string '{}' has never been used",
variable.id.name
),
vec![],
Expand All @@ -349,21 +332,21 @@ pub fn emit_warning_local_variable(variable: &symtable::Variable) -> Option<Diag
}
}

/// Emit warnings depending on the state variable usage
/// Emit warnings depending on the storage variable usage
fn emit_warning_contract_variables(variable: &ast::Variable) -> Option<Diagnostic> {
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![],
));
} else if !variable.assigned && !variable.read {
return Some(generate_unused_warning(
variable.loc,
&format!("Storage variable '{}' has never been used.", variable.name),
&format!("storage variable '{}' has never been used", variable.name),
vec![],
));
}
Expand All @@ -388,7 +371,7 @@ pub fn check_unused_namespace_variables(ns: &mut Namespace) {
if !constant.read {
ns.diagnostics.push(generate_unused_warning(
constant.loc,
&format!("Global constant '{}' has never been used.", constant.name),
&format!("global constant '{}' has never been used", constant.name),
vec![],
))
}
Expand All @@ -401,7 +384,7 @@ pub fn check_unused_events(ns: &mut Namespace) {
if !event.used {
ns.diagnostics.push(generate_unused_warning(
event.loc,
&format!("Event '{}' has never been emitted.", event.name),
&format!("event '{}' has never been emitted", event.name),
vec![],
))
}
Expand Down
Loading

0 comments on commit 2f71a12

Please sign in to comment.