feat(validators): unused param warning

Params being unused were being incorrectly reported.
This was because params need to be initialized
at a scope above both the validator functions. This
manifested when using a multi-validator where one of
the params was not used in both validators.

The easy fix was to add a field called
`is_validator_param` to `ArgName`. Then
when infering a function we don't initialize args
that are validator params. We now handle this
in a scope that is created before in the match branch for
validator in the `infer_definition` function. In there
we call `.in_new_scope` and initialize params for usage
detection.
This commit is contained in:
rvcas 2023-03-30 16:34:22 -04:00 committed by Lucas
parent 8fad5b77c6
commit d8cbcde61d
7 changed files with 208 additions and 118 deletions

View File

@ -564,6 +564,7 @@ pub enum ArgName {
name: String, name: String,
label: String, label: String,
location: Span, location: Span,
is_validator_param: bool,
}, },
} }

View File

@ -675,6 +675,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "self".to_string(), name: "self".to_string(),
label: "self".to_string(), label: "self".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,
@ -724,6 +725,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "a".to_string(), name: "a".to_string(),
label: "a".to_string(), label: "a".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,
@ -770,6 +772,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "a".to_string(), name: "a".to_string(),
label: "a".to_string(), label: "a".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,
@ -830,6 +833,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "f".to_string(), name: "f".to_string(),
label: "f".to_string(), label: "f".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,
@ -845,6 +849,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "b".to_string(), name: "b".to_string(),
label: "b".to_string(), label: "b".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,
@ -855,6 +860,7 @@ pub fn prelude_functions(id_gen: &IdGenerator) -> IndexMap<FunctionAccessKey, Ty
name: "a".to_string(), name: "a".to_string(),
label: "a".to_string(), label: "a".to_string(),
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
location: Span::empty(), location: Span::empty(),
annotation: None, annotation: None,

View File

@ -242,44 +242,9 @@ pub fn type_alias_parser() -> impl Parser<Token, ast::UntypedDefinition, Error =
} }
pub fn validator_parser() -> impl Parser<Token, ast::UntypedDefinition, Error = ParseError> { pub fn validator_parser() -> impl Parser<Token, ast::UntypedDefinition, Error = ParseError> {
let func_parser = just(Token::Fn)
.ignore_then(select! {Token::Name {name} => name})
.then(
fn_param_parser()
.separated_by(just(Token::Comma))
.allow_trailing()
.delimited_by(just(Token::LeftParen), just(Token::RightParen))
.map_with_span(|arguments, span| (arguments, span)),
)
.then(just(Token::RArrow).ignore_then(type_parser()).or_not())
.then(
expr_seq_parser()
.or_not()
.delimited_by(just(Token::LeftBrace), just(Token::RightBrace)),
)
.map_with_span(
|(((name, (arguments, args_span)), return_annotation), body), span| ast::Function {
arguments,
body: body.unwrap_or_else(|| expr::UntypedExpr::todo(span, None)),
doc: None,
location: Span {
start: span.start,
end: return_annotation
.as_ref()
.map(|l| l.location().end)
.unwrap_or_else(|| args_span.end),
},
end_position: span.end - 1,
name,
public: false,
return_annotation,
return_type: (),
},
);
just(Token::Validator) just(Token::Validator)
.ignore_then( .ignore_then(
fn_param_parser() fn_param_parser(true)
.separated_by(just(Token::Comma)) .separated_by(just(Token::Comma))
.allow_trailing() .allow_trailing()
.delimited_by(just(Token::LeftParen), just(Token::RightParen)) .delimited_by(just(Token::LeftParen), just(Token::RightParen))
@ -287,12 +252,20 @@ pub fn validator_parser() -> impl Parser<Token, ast::UntypedDefinition, Error =
.or_not(), .or_not(),
) )
.then( .then(
func_parser fn_parser()
.repeated() .repeated()
.at_least(1) .at_least(1)
.at_most(2) .at_most(2)
.delimited_by(just(Token::LeftBrace), just(Token::RightBrace)) .delimited_by(just(Token::LeftBrace), just(Token::RightBrace))
.map(IntoIterator::into_iter), .map(|defs| {
defs.into_iter().map(|def| {
let ast::UntypedDefinition::Fn(fun) = def else {
unreachable!("It should be a fn definition");
};
fun
})
}),
) )
.map_with_span(|(opt_extra_params, mut functions), span| { .map_with_span(|(opt_extra_params, mut functions), span| {
let (params, params_span) = opt_extra_params.unwrap_or(( let (params, params_span) = opt_extra_params.unwrap_or((
@ -326,7 +299,7 @@ pub fn fn_parser() -> impl Parser<Token, ast::UntypedDefinition, Error = ParseEr
.then_ignore(just(Token::Fn)) .then_ignore(just(Token::Fn))
.then(select! {Token::Name {name} => name}) .then(select! {Token::Name {name} => name})
.then( .then(
fn_param_parser() fn_param_parser(false)
.separated_by(just(Token::Comma)) .separated_by(just(Token::Comma))
.allow_trailing() .allow_trailing()
.delimited_by(just(Token::LeftParen), just(Token::RightParen)) .delimited_by(just(Token::LeftParen), just(Token::RightParen))
@ -489,7 +462,9 @@ pub fn bytearray_parser(
)) ))
} }
pub fn fn_param_parser() -> impl Parser<Token, ast::UntypedArg, Error = ParseError> { pub fn fn_param_parser(
is_validator_param: bool,
) -> impl Parser<Token, ast::UntypedArg, Error = ParseError> {
choice(( choice((
select! {Token::Name {name} => name} select! {Token::Name {name} => name}
.then(select! {Token::DiscardName {name} => name}) .then(select! {Token::DiscardName {name} => name})
@ -507,15 +482,17 @@ pub fn fn_param_parser() -> impl Parser<Token, ast::UntypedArg, Error = ParseErr
}), }),
select! {Token::Name {name} => name} select! {Token::Name {name} => name}
.then(select! {Token::Name {name} => name}) .then(select! {Token::Name {name} => name})
.map_with_span(|(label, name), span| ast::ArgName::Named { .map_with_span(move |(label, name), span| ast::ArgName::Named {
label, label,
name, name,
location: span, location: span,
is_validator_param,
}), }),
select! {Token::Name {name} => name}.map_with_span(|name, span| ast::ArgName::Named { select! {Token::Name {name} => name}.map_with_span(move |name, span| ast::ArgName::Named {
label: name.clone(), label: name.clone(),
name, name,
location: span, location: span,
is_validator_param,
}), }),
)) ))
.then(just(Token::Colon).ignore_then(type_parser()).or_not()) .then(just(Token::Colon).ignore_then(type_parser()).or_not())
@ -541,6 +518,7 @@ pub fn anon_fn_param_parser() -> impl Parser<Token, ast::UntypedArg, Error = Par
label: name.clone(), label: name.clone(),
name, name,
location: span, location: span,
is_validator_param: false,
}), }),
)) ))
.then(just(Token::Colon).ignore_then(type_parser()).or_not()) .then(just(Token::Colon).ignore_then(type_parser()).or_not())
@ -1169,6 +1147,7 @@ pub fn expr_parser(
label: name.clone(), label: name.clone(),
name, name,
location: Span::empty(), location: Span::empty(),
is_validator_param: false,
}, },
tipo: (), tipo: (),
}); });

View File

@ -113,6 +113,47 @@ fn validator_in_lib_warning() {
)) ))
} }
#[test]
fn multi_validator() {
let source_code = r#"
validator(foo: ByteArray, bar: Int) {
fn spend(_d, _r, _c) {
foo == #"aabb"
}
fn mint(_r, _c) {
bar == 0
}
}
"#;
let (warnings, _) = check_validator(parse(source_code)).unwrap();
assert_eq!(warnings.len(), 0)
}
#[test]
fn multi_validator_warning() {
let source_code = r#"
validator(foo: ByteArray, bar: Int) {
fn spend(_d, _r, _c) {
foo == #"aabb"
}
fn mint(_r, _c) {
True
}
}
"#;
let (warnings, _) = check_validator(parse(source_code)).unwrap();
assert!(matches!(
warnings[0],
Warning::UnusedVariable { ref name, .. } if name == "bar"
))
}
#[test] #[test]
fn if_scoping() { fn if_scoping() {
let source_code = r#" let source_code = r#"

View File

@ -60,6 +60,7 @@ fn validator() {
name: "datum".to_string(), name: "datum".to_string(),
label: "datum".to_string(), label: "datum".to_string(),
location: Span::new((), 21..26), location: Span::new((), 21..26),
is_validator_param: false,
}, },
location: Span::new((), 21..26), location: Span::new((), 21..26),
annotation: None, annotation: None,
@ -70,6 +71,7 @@ fn validator() {
name: "rdmr".to_string(), name: "rdmr".to_string(),
label: "rdmr".to_string(), label: "rdmr".to_string(),
location: Span::new((), 28..32), location: Span::new((), 28..32),
is_validator_param: false,
}, },
location: Span::new((), 28..32), location: Span::new((), 28..32),
annotation: None, annotation: None,
@ -80,6 +82,7 @@ fn validator() {
name: "ctx".to_string(), name: "ctx".to_string(),
label: "ctx".to_string(), label: "ctx".to_string(),
location: Span::new((), 34..37), location: Span::new((), 34..37),
is_validator_param: false,
}, },
location: Span::new((), 34..37), location: Span::new((), 34..37),
annotation: None, annotation: None,
@ -131,6 +134,7 @@ fn double_validator() {
name: "datum".to_string(), name: "datum".to_string(),
label: "datum".to_string(), label: "datum".to_string(),
location: Span::new((), 21..26), location: Span::new((), 21..26),
is_validator_param: false,
}, },
location: Span::new((), 21..26), location: Span::new((), 21..26),
annotation: None, annotation: None,
@ -141,6 +145,7 @@ fn double_validator() {
name: "rdmr".to_string(), name: "rdmr".to_string(),
label: "rdmr".to_string(), label: "rdmr".to_string(),
location: Span::new((), 28..32), location: Span::new((), 28..32),
is_validator_param: false,
}, },
location: Span::new((), 28..32), location: Span::new((), 28..32),
annotation: None, annotation: None,
@ -151,6 +156,7 @@ fn double_validator() {
name: "ctx".to_string(), name: "ctx".to_string(),
label: "ctx".to_string(), label: "ctx".to_string(),
location: Span::new((), 34..37), location: Span::new((), 34..37),
is_validator_param: false,
}, },
location: Span::new((), 34..37), location: Span::new((), 34..37),
annotation: None, annotation: None,
@ -176,6 +182,7 @@ fn double_validator() {
name: "rdmr".to_string(), name: "rdmr".to_string(),
label: "rdmr".to_string(), label: "rdmr".to_string(),
location: Span::new((), 64..68), location: Span::new((), 64..68),
is_validator_param: false,
}, },
location: Span::new((), 64..68), location: Span::new((), 64..68),
annotation: None, annotation: None,
@ -186,6 +193,7 @@ fn double_validator() {
name: "ctx".to_string(), name: "ctx".to_string(),
label: "ctx".to_string(), label: "ctx".to_string(),
location: Span::new((), 70..73), location: Span::new((), 70..73),
is_validator_param: false,
}, },
location: Span::new((), 70..73), location: Span::new((), 70..73),
annotation: None, annotation: None,
@ -589,6 +597,7 @@ fn plus_binop() {
label: "a".to_string(), label: "a".to_string(),
name: "a".to_string(), name: "a".to_string(),
location: Span::new((), 15..16), location: Span::new((), 15..16),
is_validator_param: false,
}, },
location: Span::new((), 15..16), location: Span::new((), 15..16),
annotation: None, annotation: None,
@ -640,6 +649,7 @@ fn pipeline() {
name: "a".to_string(), name: "a".to_string(),
label: "thing".to_string(), label: "thing".to_string(),
location: Span::new((), 13..20), location: Span::new((), 13..20),
is_validator_param: false,
}, },
location: Span::new((), 13..25), location: Span::new((), 13..25),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -808,6 +818,7 @@ fn let_bindings() {
label: "a".to_string(), label: "a".to_string(),
name: "a".to_string(), name: "a".to_string(),
location: Span::new((), 11..12), location: Span::new((), 11..12),
is_validator_param: false,
}, },
location: Span::new((), 11..17), location: Span::new((), 11..17),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -934,6 +945,7 @@ fn block() {
label: "a".to_string(), label: "a".to_string(),
name: "a".to_string(), name: "a".to_string(),
location: Span::new((), 12..13), location: Span::new((), 12..13),
is_validator_param: false,
}, },
location: Span::new((), 12..18), location: Span::new((), 12..18),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -1027,6 +1039,7 @@ fn when() {
label: "a".to_string(), label: "a".to_string(),
name: "a".to_string(), name: "a".to_string(),
location: Span::new((), 12..13), location: Span::new((), 12..13),
is_validator_param: false,
}, },
location: Span::new((), 12..18), location: Span::new((), 12..18),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -1160,6 +1173,7 @@ fn anonymous_function() {
label: "a".to_string(), label: "a".to_string(),
name: "a".to_string(), name: "a".to_string(),
location: Span::new((), 43..44), location: Span::new((), 43..44),
is_validator_param: false,
}, },
location: Span::new((), 43..49), location: Span::new((), 43..49),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -1243,6 +1257,7 @@ fn field_access() {
label: "user".to_string(), label: "user".to_string(),
name: "user".to_string(), name: "user".to_string(),
location: Span::new((), 8..12), location: Span::new((), 8..12),
is_validator_param: false,
}, },
location: Span::new((), 8..18), location: Span::new((), 8..18),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -1325,6 +1340,7 @@ fn call() {
label: "_capture__0".to_string(), label: "_capture__0".to_string(),
name: "_capture__0".to_string(), name: "_capture__0".to_string(),
location: Span::new((), 0..0), location: Span::new((), 0..0),
is_validator_param: false,
}, },
location: Span::new((), 0..0), location: Span::new((), 0..0),
annotation: None, annotation: None,
@ -1351,6 +1367,7 @@ fn call() {
label: "y".to_string(), label: "y".to_string(),
name: "y".to_string(), name: "y".to_string(),
location: Span::new((), 69..70), location: Span::new((), 69..70),
is_validator_param: false,
}, },
location: Span::new((), 69..70), location: Span::new((), 69..70),
annotation: None, annotation: None,
@ -1450,6 +1467,7 @@ fn record_update() {
label: "user".to_string(), label: "user".to_string(),
name: "user".to_string(), name: "user".to_string(),
location: Span::new((), 15..19), location: Span::new((), 15..19),
is_validator_param: false,
}, },
location: Span::new((), 15..25), location: Span::new((), 15..25),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {
@ -1465,6 +1483,7 @@ fn record_update() {
label: "name".to_string(), label: "name".to_string(),
name: "name".to_string(), name: "name".to_string(),
location: Span::new((), 27..31), location: Span::new((), 27..31),
is_validator_param: false,
}, },
location: Span::new((), 27..42), location: Span::new((), 27..42),
annotation: Some(ast::Annotation::Constructor { annotation: Some(ast::Annotation::Constructor {

View File

@ -1480,15 +1480,19 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
) -> Result<(Vec<TypedArg>, TypedExpr), Error> { ) -> Result<(Vec<TypedArg>, TypedExpr), Error> {
self.assert_no_assignment(&body)?; self.assert_no_assignment(&body)?;
for (arg, t) in args.iter().zip(args.iter().map(|arg| arg.tipo.clone())) { for arg in &args {
match &arg.arg_name { match &arg.arg_name {
ArgName::Named { name, .. } => { ArgName::Named {
name,
is_validator_param,
..
} if !is_validator_param => {
self.environment.insert_variable( self.environment.insert_variable(
name.to_string(), name.to_string(),
ValueConstructorVariant::LocalVariable { ValueConstructorVariant::LocalVariable {
location: arg.location, location: arg.location,
}, },
t, arg.tipo.clone(),
); );
self.environment.init_usage( self.environment.init_usage(
@ -1497,7 +1501,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
arg.location, arg.location,
); );
} }
ArgName::Discarded { .. } => (), ArgName::Named { .. } | ArgName::Discarded { .. } => (),
}; };
} }

View File

@ -2,9 +2,9 @@ use std::collections::HashMap;
use crate::{ use crate::{
ast::{ ast::{
DataType, Definition, Function, Layer, ModuleConstant, ModuleKind, RecordConstructor, ArgName, DataType, Definition, Function, Layer, ModuleConstant, ModuleKind,
RecordConstructorArg, Span, Tracing, TypeAlias, TypedDefinition, TypedFunction, RecordConstructor, RecordConstructorArg, Span, Tracing, TypeAlias, TypedDefinition,
TypedModule, UntypedDefinition, UntypedModule, Use, Validator, TypedFunction, TypedModule, UntypedDefinition, UntypedModule, Use, Validator,
}, },
builtins, builtins,
builtins::function, builtins::function,
@ -262,6 +262,43 @@ fn infer_definition(
let temp_params = params.iter().cloned().chain(fun.arguments); let temp_params = params.iter().cloned().chain(fun.arguments);
fun.arguments = temp_params.collect(); fun.arguments = temp_params.collect();
environment.in_new_scope(|environment| {
let preregistered_fn = environment
.get_variable(&fun.name)
.expect("Could not find preregistered type for function");
let preregistered_type = preregistered_fn.tipo.clone();
let (args_types, _return_type) = preregistered_type
.function_types()
.expect("Preregistered type for fn was not a fn");
for (arg, t) in params.iter().zip(args_types[0..params.len()].iter()) {
match &arg.arg_name {
ArgName::Named {
name,
is_validator_param,
..
} if *is_validator_param => {
environment.insert_variable(
name.to_string(),
ValueConstructorVariant::LocalVariable {
location: arg.location,
},
t.clone(),
);
environment.init_usage(
name.to_string(),
EntityKind::Variable,
arg.location,
);
}
ArgName::Named { .. } | ArgName::Discarded { .. } => (),
};
}
let Definition::Fn(mut typed_fun) = infer_definition( let Definition::Fn(mut typed_fun) = infer_definition(
Definition::Fn(fun), Definition::Fn(fun),
module_name, module_name,
@ -316,7 +353,9 @@ fn infer_definition(
other_typed_fun.arguments.drain(0..params_length); other_typed_fun.arguments.drain(0..params_length);
if other_typed_fun.arguments.len() < 2 || other_typed_fun.arguments.len() > 3 { if other_typed_fun.arguments.len() < 2
|| other_typed_fun.arguments.len() > 3
{
return Err(Error::IncorrectValidatorArity { return Err(Error::IncorrectValidatorArity {
count: other_typed_fun.arguments.len() as u32, count: other_typed_fun.arguments.len() as u32,
location: other_typed_fun.location, location: other_typed_fun.location,
@ -343,6 +382,7 @@ fn infer_definition(
location, location,
params: typed_params, params: typed_params,
})) }))
})
} }
Definition::Test(f) => { Definition::Test(f) => {