Skip to content

Commit

Permalink
Implement tests for unused variable detection
Browse files Browse the repository at this point in the history 
Signed-off-by: LucasSte <[email protected]>
  • Loading branch information
@LucasSte @seanyoung
LucasSte authored and seanyoung committed Jul 9, 2021
1 parent edfaf76 commit be73b62
Show file tree
Hide file tree
Showing 10 changed files with 1,065 additions and 128 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

4 changes: 2 additions & 2 deletions src/sema/contracts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,9 +870,9 @@ fn resolve_bodies(
{
broken = true;
} else {
for (_, variable) in &ns.functions[function_no].symtable.vars {
for variable in ns.functions[function_no].symtable.vars.values() {
if let Some(warning) = emit_warning_local_variable(&variable) {
ns.diagnostics.push(*warning);
ns.diagnostics.push(warning);
}
}
}
Expand Down
73 changes: 39 additions & 34 deletions src/sema/expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1321,17 +1321,25 @@ pub fn expression(
resolve_to: Option<&Type>,
) -> Result<Expression, ()> {
match expr {
pt::Expression::ArrayLiteral(loc, exprs) => resolve_array_literal(
loc,
exprs,
file_no,
contract_no,
ns,
symtable,
is_constant,
diagnostics,
resolve_to,
),
pt::Expression::ArrayLiteral(loc, exprs) => {
let res = resolve_array_literal(
loc,
exprs,
file_no,
contract_no,
ns,
symtable,
is_constant,
diagnostics,
resolve_to,
);

if let Ok(exp) = &res {
used_variable(ns, exp, symtable);
}

res
}
pt::Expression::BoolLiteral(loc, v) => Ok(Expression::BoolLiteral(*loc, *v)),
pt::Expression::StringLiteral(v) => {
// Concatenate the strings
Expand Down Expand Up @@ -2479,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 @@ -2759,26 +2768,18 @@ pub fn expression(
is_constant,
diagnostics,
),
pt::Expression::MemberAccess(loc, e, id) => {
let res = member_access(
loc,
e,
id,
file_no,
contract_no,
ns,
symtable,
is_constant,
diagnostics,
resolve_to,
);

if let Ok(exp) = &res {
used_variable(ns, &exp, symtable);
}

res
}
pt::Expression::MemberAccess(loc, e, id) => member_access(
loc,
e,
id,
file_no,
contract_no,
ns,
symtable,
is_constant,
diagnostics,
resolve_to,
),
pt::Expression::Or(loc, left, right) => {
let boolty = Type::Bool;
let l = cast(
Expand Down Expand Up @@ -4426,7 +4427,10 @@ fn member_access(
if id.name == "length" {
return match dim.last().unwrap() {
None => Ok(Expression::DynamicArrayLength(*loc, Box::new(expr))),
Some(d) => bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32))),
Some(d) => {
used_variable(ns, &expr, symtable);
bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32)))
}
};
}
}
Expand Down Expand Up @@ -4793,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 @@ -5408,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 @@ -6640,6 +6644,7 @@ fn resolve_array_literal(
first.ty()
};

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

for e in flattened {
Expand Down
14 changes: 8 additions & 6 deletions src/sema/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,18 @@ pub const SOLANA_BUCKET_SIZE: u64 = 251;
pub const SOLANA_FIRST_OFFSET: u64 = 16;
pub const SOLANA_SPARSE_ARRAY_SIZE: u64 = 1024;

/// Performs semantic analysis and checks for unused variables
/// Load a file file from the cache, parse and resolve it. The file must be present in
/// the cache.
pub fn sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) {
perform_sema(file, cache, ns);
sema_file(file, cache, ns);

// Checks for unused variables
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) {
/// Parse and resolve a file and its imports in a recursive manner.
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 +179,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
70 changes: 38 additions & 32 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 @@ -332,6 +330,7 @@ fn statement(
diagnostics,
Some(&var_ty),
)?;

used_variable(ns, &expr, symtable);

Some(cast(&expr.loc(), expr, &var_ty, true, ns, diagnostics)?)
Expand All @@ -343,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 @@ -717,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 @@ -1261,27 +1255,35 @@ fn destructure_values(
diagnostics: &mut Vec<Diagnostic>,
) -> Result<Expression, ()> {
let expr = match expr {
pt::Expression::FunctionCall(loc, ty, args) => function_call_expr(
loc,
ty,
args,
file_no,
contract_no,
ns,
symtable,
false,
diagnostics,
)?,
pt::Expression::NamedFunctionCall(loc, ty, args) => named_function_call_expr(
loc,
ty,
args,
file_no,
contract_no,
ns,
symtable,
diagnostics,
)?,
pt::Expression::FunctionCall(loc, ty, args) => {
let res = function_call_expr(
loc,
ty,
args,
file_no,
contract_no,
ns,
symtable,
false,
diagnostics,
)?;
check_function_call(ns, &res, symtable);
res
}
pt::Expression::NamedFunctionCall(loc, ty, args) => {
let res = named_function_call_expr(
loc,
ty,
args,
file_no,
contract_no,
ns,
symtable,
diagnostics,
)?;
check_function_call(ns, &res, symtable);
res
}
pt::Expression::Ternary(loc, cond, left, right) => {
let cond = expression(
cond,
Expand All @@ -1294,6 +1296,7 @@ fn destructure_values(
Some(&Type::Bool),
)?;

used_variable(ns, &cond, symtable);
let left = destructure_values(
&left.loc(),
left,
Expand All @@ -1305,7 +1308,7 @@ fn destructure_values(
ns,
diagnostics,
)?;

used_variable(ns, &left, symtable);
let right = destructure_values(
&right.loc(),
right,
Expand All @@ -1317,6 +1320,7 @@ fn destructure_values(
ns,
diagnostics,
)?;
used_variable(ns, &right, symtable);

return Ok(Expression::Ternary(
*loc,
Expand Down Expand Up @@ -1363,7 +1367,9 @@ fn destructure_values(
));
return Err(());
}
_ => (),
_ => {
used_variable(ns, &e, symtable);
}
}

list.push(e);
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
Loading

0 comments on commit be73b62

Please sign in to comment.