From ddd83f1fc68f9597bda75b8447e88b3dec05ab83 Mon Sep 17 00:00:00 2001 From: joe-conigliaro Date: Sat, 20 Jun 2020 12:42:08 +1000 Subject: [PATCH] checker: error if variable used before declaration --- vlib/v/checker/checker.v | 42 +++++++++++++------ ...xpr_unresolved_variables_err_chain.run.out | 18 +------- .../tests/var_used_before_declaration.out | 6 +++ .../tests/var_used_before_declaration.vv | 4 ++ vlib/v/parser/comptime.v | 12 +++--- vlib/v/parser/parser.v | 23 ---------- 6 files changed, 47 insertions(+), 58 deletions(-) create mode 100644 vlib/v/checker/tests/var_used_before_declaration.out create mode 100644 vlib/v/checker/tests/var_used_before_declaration.vv diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index abe4d7c28c..04115ededb 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -62,20 +62,33 @@ pub fn (mut c Checker) check(ast_file ast.File) { c.expr_level = 0 c.stmt(stmt) } - // Check scopes - // TODO - /* - for i, obj in c.file.global_scope.objects { + c.check_scope_vars(c.file.scope) +} + +pub fn (mut c Checker) check_scope_vars(sc &ast.Scope) { + for _, obj in sc.objects { match obj { ast.Var { - if it.is_mut && !it.is_changed { - c.warn('`$it.name` is declared as mutable, but it was never changed', it.pos) + if !c.pref.is_repl { + if !obj.is_used && obj.name[0] != `_` { + if c.pref.is_prod { + c.error('unused variable: `$obj.name`', obj.pos) + } else { + c.warn('unused variable: `$obj.name`', obj.pos) + } + } } + // TODO: fix all of these warnings + // if obj.is_mut && !obj.is_changed { + // c.warn('`$obj.name` is declared as mutable, but it was never changed', obj.pos) + // } } else {} } } - */ + for _, child in sc.children { + c.check_scope_vars(child) + } } // not used right now @@ -1917,12 +1930,10 @@ pub fn (mut c Checker) expr(node ast.Expr) table.Type { ast.ComptimeCall { node.sym = c.table.get_type_symbol(c.unwrap_generic(c.expr(node.left))) if node.is_vweb { - x := *c.pref - xx := { - x | - is_vweb: true - } // TODO assoc parser bug - mut c2 := new_checker(c.table, xx) + // TODO assoc parser bug + pref := *c.pref + pref2 := {pref|is_vweb: true} + mut c2 := new_checker(c.table, pref2) c2.check(node.vweb_tmpl) c.warnings << c2.warnings c.errors << c2.errors @@ -2088,6 +2099,11 @@ pub fn (mut c Checker) ident(mut ident ast.Ident) table.Type { return obj.typ } ast.Var { + // incase var was not marked as used yet (vweb tmpl) + obj.is_used = true + if ident.pos.pos < obj.pos.pos { + c.error('undefined variable `$ident.name` (used before declaration)', ident.pos) + } mut typ := obj.typ if typ == 0 { if obj.expr is ast.Ident { diff --git a/vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.run.out b/vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.run.out index adf2eeab2a..7e6cb6ba61 100644 --- a/vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.run.out +++ b/vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.run.out @@ -1,27 +1,13 @@ -vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:3:2: warning: unused variable: `b` - 1 | fn main() { - 2 | a := b - 3 | b := c - | ^ - 4 | c := a - 5 | } -vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:4:2: warning: unused variable: `c` - 2 | a := b - 3 | b := c - 4 | c := a - | ^ - 5 | } -vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:2:7: error: unresolved variable: `b` +vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:2:7: error: undefined variable `b` (used before declaration) 1 | fn main() { 2 | a := b | ^ 3 | b := c 4 | c := a -vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:3:7: error: unresolved variable: `c` +vlib/v/checker/tests/run/assign_expr_unresolved_variables_err_chain.v:3:7: error: undefined variable `c` (used before declaration) 1 | fn main() { 2 | a := b 3 | b := c | ^ 4 | c := a 5 | } - diff --git a/vlib/v/checker/tests/var_used_before_declaration.out b/vlib/v/checker/tests/var_used_before_declaration.out new file mode 100644 index 0000000000..47ff91fa99 --- /dev/null +++ b/vlib/v/checker/tests/var_used_before_declaration.out @@ -0,0 +1,6 @@ +vlib/v/checker/tests/var_used_before_declaration.v:2:13: error: undefined variable `x` (used before declaration) + 1 | fn main() { + 2 | println(x) + | ^ + 3 | x := 'hello v' + 4 | } diff --git a/vlib/v/checker/tests/var_used_before_declaration.vv b/vlib/v/checker/tests/var_used_before_declaration.vv new file mode 100644 index 0000000000..5f2d8e3939 --- /dev/null +++ b/vlib/v/checker/tests/var_used_before_declaration.vv @@ -0,0 +1,4 @@ +fn main() { + println(x) + x := 'hello v' +} \ No newline at end of file diff --git a/vlib/v/parser/comptime.v b/vlib/v/parser/comptime.v index 37c08be860..fe494923fc 100644 --- a/vlib/v/parser/comptime.v +++ b/vlib/v/parser/comptime.v @@ -107,7 +107,7 @@ fn (mut p Parser) vweb() ast.ComptimeCall { start_pos: 0 parent: p.global_scope } - file := parse_text(v_code, p.table, p.pref, scope, p.global_scope) + mut file := parse_text(v_code, p.table, p.pref, scope, p.global_scope) if p.pref.is_verbose { println('\n\n') println('>>> vweb template for ${path}:') @@ -115,6 +115,7 @@ fn (mut p Parser) vweb() ast.ComptimeCall { println('>>> end of vweb template END') println('\n\n') } + file = {file| path:html_name} // copy vars from current fn scope into vweb_tmpl scope for stmt in file.stmts { if stmt is ast.FnDecl { @@ -124,12 +125,11 @@ fn (mut p Parser) vweb() ast.ComptimeCall { for _, obj in p.scope.objects { if obj is ast.Var { mut v := obj as ast.Var + v.pos = fn_decl.body_pos tmpl_scope.register(v.name, *v) - // TODO: this is yuck, track idents in parser - // or defer unused var logic to checker - if v_code.contains(v.name) { - v.is_used = true - } + // set the controller action var to used + // if its unused in the template it will warn + v.is_used = true } } break diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index 64837b278e..42308f2724 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -275,29 +275,6 @@ pub fn (mut p Parser) open_scope() { } pub fn (mut p Parser) close_scope() { - // TODO move this to checker since is_changed is set there? - if !p.pref.is_repl && !p.scanner.is_fmt { - for _, obj in p.scope.objects { - match obj { - ast.Var { - if !obj.is_used && obj.name[0] != `_` { - if p.pref.is_prod { - p.error_with_pos('unused variable: `$obj.name`', obj.pos) - } else { - p.warn_with_pos('unused variable: `$obj.name`', obj.pos) - } - } - /* - if it.is_mut && !it.is_changed { - p.warn_with_pos('`$it.name` is declared as mutable, but it was never changed', - it.pos) - } - */ - } - else {} - } - } - } p.scope.end_pos = p.tok.pos p.scope.parent.children << p.scope p.scope = p.scope.parent