From 2994e7150f0631e403097e512f65082d0d37e50b Mon Sep 17 00:00:00 2001 From: pancake Date: Sun, 8 Nov 2020 09:14:24 +0100 Subject: [PATCH] vweb: check function and route parameter count (#6761) --- vlib/net/http/backend_windows.c.v | 2 +- vlib/net/http/download_windows.c.v | 2 +- vlib/v/ast/ast.v | 2 + vlib/v/checker/checker.v | 61 +++++++++++++++++--- vlib/v/checker/tests/vweb_routing_checks.out | 14 +++++ vlib/v/checker/tests/vweb_routing_checks.vv | 50 ++++++++++++++++ vlib/v/parser/fn.v | 6 +- vlib/v/table/table.v | 6 +- vlib/vweb/vweb.v | 10 +++- 9 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 vlib/v/checker/tests/vweb_routing_checks.out create mode 100644 vlib/v/checker/tests/vweb_routing_checks.vv diff --git a/vlib/net/http/backend_windows.c.v b/vlib/net/http/backend_windows.c.v index 62e5aadd8c..c099b437b4 100644 --- a/vlib/net/http/backend_windows.c.v +++ b/vlib/net/http/backend_windows.c.v @@ -8,7 +8,7 @@ module http #include "vschannel.c" fn C.new_tls_context() C.TlsContext -fn (req &Request) ssl_do(port int, method Method, host_name, path string) ?Response { +fn (req &Request) ssl_do(port int, method Method, host_name string, path string) ?Response { mut ctx := C.new_tls_context() C.vschannel_init(&ctx) mut buff := malloc(C.vsc_init_resp_buff_size) diff --git a/vlib/net/http/download_windows.c.v b/vlib/net/http/download_windows.c.v index ebe24ad53e..61f1f74ded 100644 --- a/vlib/net/http/download_windows.c.v +++ b/vlib/net/http/download_windows.c.v @@ -8,7 +8,7 @@ module http #include -fn download_file_with_progress(url, out string, cb, cb_finished voidptr) { +fn download_file_with_progress(url string, out string, cb voidptr, cb_finished voidptr) { } /* diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index baa9c245c7..14833d3b79 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -260,6 +260,7 @@ pub: receiver Field receiver_pos token.Position is_method bool + method_idx int rec_mut bool // is receiver mutable rec_share table.ShareType language table.Language @@ -275,6 +276,7 @@ pub mut: stmts []Stmt return_type table.Type comments []Comment // comments *after* the header, but *before* `{`; used for InterfaceDecl + source_file &File = 0 } // break, continue diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index e4acc862d1..f0a935da0e 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -31,7 +31,7 @@ pub struct Checker { pref &pref.Preferences // Preferences shared from V struct pub mut: table &table.Table - file ast.File + file &ast.File = 0 nr_errors int nr_warnings int errors []errors.Error @@ -61,6 +61,7 @@ mut: error_details []string generic_funcs []&ast.FnDecl vmod_file_content string // needed for @VMOD_FILE, contents of the file, *NOT its path* + vweb_gen_types []table.Type // vweb route checks } pub fn new_checker(table &table.Table, pref &pref.Preferences) Checker { @@ -71,7 +72,7 @@ pub fn new_checker(table &table.Table, pref &pref.Preferences) Checker { } } -pub fn (mut c Checker) check(ast_file ast.File) { +pub fn (mut c Checker) check(ast_file &ast.File) { c.file = ast_file for i, ast_import in ast_file.imports { for j in 0 .. i { @@ -112,7 +113,7 @@ pub fn (mut c Checker) check_scope_vars(sc &ast.Scope) { } // not used right now -pub fn (mut c Checker) check2(ast_file ast.File) []errors.Error { +pub fn (mut c Checker) check2(ast_file &ast.File) []errors.Error { c.file = ast_file for stmt in ast_file.stmts { c.stmt(stmt) @@ -147,6 +148,7 @@ pub fn (mut c Checker) check_files(ast_files []ast.File) { has_main_fn = true } } + c.verify_all_vweb_routes() // Make sure fn main is defined in non lib builds if c.pref.build_mode == .build_module || c.pref.is_test { return @@ -4207,17 +4209,17 @@ fn (mut c Checker) post_process_generic_fns() { // Loop thru each generic function concrete type. // Check each specific fn instantiation. for i in 0 .. c.generic_funcs.len { - mut node := c.generic_funcs[i] if c.table.fn_gen_types.len == 0 { // no concrete types, so just skip: continue } - // eprintln('>> post_process_generic_fns $c.file.path | $node.name , c.table.fn_gen_types.len: $c.table.fn_gen_types.len') + mut node := c.generic_funcs[i] for gen_type in c.table.fn_gen_types[node.name] { c.cur_generic_type = gen_type - // sym:=c.table.get_type_symbol(gen_type) - // println('\ncalling check for $node.name for type $sym.source_name') c.fn_decl(mut node) + if node.name in ['vweb.run_app', 'vweb.run'] { + c.vweb_gen_types << gen_type + } } c.cur_generic_type = 0 c.generic_funcs[i] = 0 @@ -4266,6 +4268,8 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) { c.error('cannot define new methods on non-local `$sym.source_name` (' + 'current module is `$c.mod`, `$sym.source_name` is from `$sym.mod`)', node.pos) } + // needed for proper error reporting during vweb route checking + sym.methods[node.method_idx].source_fn = voidptr(node) } if node.language == .v { // Make sure all types are valid @@ -4322,6 +4326,7 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) { c.error('missing return at end of function `$node.name`', node.pos) } c.returns = false + node.source_file = c.file } fn has_top_return(stmts []ast.Stmt) bool { @@ -4342,3 +4347,45 @@ fn has_top_return(stmts []ast.Stmt) bool { } return false } + +fn (mut c Checker) verify_vweb_params_for_method(m table.Fn) (bool, int, int) { + margs := m.params.len - 1 // first arg is the receiver/this + if m.attrs.len == 0 { + // allow non custom routed methods, with 1:1 mapping + return true, -1, margs + } + mut route_attributes := 0 + for a in m.attrs { + if a.name.starts_with('/') { + route_attributes += a.name.count(':') + } + } + return route_attributes == margs, route_attributes, margs +} + +fn (mut c Checker) verify_all_vweb_routes() { + if c.vweb_gen_types.len == 0 { + return + } + typ_vweb_result := c.table.find_type_idx('vweb.Result') + for vgt in c.vweb_gen_types { + sym_app := c.table.get_type_symbol(vgt) + for m in sym_app.methods { + if m.return_type_source_name == 'vweb.Result' { + is_ok, nroute_attributes, nargs := c.verify_vweb_params_for_method(m) + if !is_ok { + f := &ast.FnDecl(m.source_fn) + if isnil(f) { + continue + } + if f.return_type == typ_vweb_result && + f.receiver.typ == m.params[0].typ && f.name == m.name { + c.file = f.source_file // setup of file path for the warning + c.warn('mismatched parameters count between vweb method `${sym_app.name}.$m.name` ($nargs) and route attribute $m.attrs ($nroute_attributes)', + f.pos) + } + } + } + } + } +} diff --git a/vlib/v/checker/tests/vweb_routing_checks.out b/vlib/v/checker/tests/vweb_routing_checks.out new file mode 100644 index 0000000000..8ba67b5b59 --- /dev/null +++ b/vlib/v/checker/tests/vweb_routing_checks.out @@ -0,0 +1,14 @@ +vlib/v/checker/tests/vweb_routing_checks.vv:22:1: error: mismatched parameters count between vweb method `App.bar` (1) and route attribute ['/bar'] (0) + 20 | // segfault because path taks 0 vars and fcn takes 1 arg + 21 | ['/bar'] + 22 | pub fn (mut app App) bar(a string) vweb.Result { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 23 | app.vweb.html('works') + 24 | return vweb.Result{} +vlib/v/checker/tests/vweb_routing_checks.vv:29:1: error: mismatched parameters count between vweb method `App.cow` (0) and route attribute ['/cow/:low'] (1) + 27 | // no segfault, but it shouldnt compile + 28 | ['/cow/:low'] + 29 | pub fn (mut app App) cow() vweb.Result { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 30 | app.vweb.html('works') + 31 | return vweb.Result{} diff --git a/vlib/v/checker/tests/vweb_routing_checks.vv b/vlib/v/checker/tests/vweb_routing_checks.vv new file mode 100644 index 0000000000..a86af53e3f --- /dev/null +++ b/vlib/v/checker/tests/vweb_routing_checks.vv @@ -0,0 +1,50 @@ +import vweb + +struct App { + pub mut: + vweb vweb.Context +} + +pub fn (mut app App) no_attributes(a string) vweb.Result { + return vweb.Result{} +} + +// works fine, as long as fcn gets 1 arg and route takes 1 var +['/foo/:bar'] +pub fn (mut app App) foo(a string) vweb.Result { + eprintln('foo') + app.vweb.html('works') + return vweb.Result{} +} + +// segfault because path taks 0 vars and fcn takes 1 arg +['/bar'] +pub fn (mut app App) bar(a string) vweb.Result { + app.vweb.html('works') + return vweb.Result{} +} + +// no segfault, but it shouldnt compile +['/cow/:low'] +pub fn (mut app App) cow() vweb.Result { + app.vweb.html('works') + return vweb.Result{} +} + +pub fn (app App) init_once() { + // +} + +pub fn (app App) init() { + // +} + +pub fn (mut app App) index() { + app.vweb.html('hello') +} + +fn main() { + port := 8181 + mut app := App{} + vweb.run_app(mut app, port) +} diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 13cafe18fc..b66729be87 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -258,15 +258,15 @@ fn (mut p Parser) fn_decl() ast.FnDecl { mut return_type := table.void_type if p.tok.kind.is_start_of_type() || (p.tok.kind == .key_fn && p.tok.line_nr == p.prev_tok.line_nr) { - end_pos = p.tok.position() return_type = p.parse_type() } + mut type_sym_method_idx := 0 // Register if is_method { mut type_sym := p.table.get_type_symbol(rec_type) ret_type_sym := p.table.get_type_symbol(return_type) // p.warn('reg method $type_sym.name . $name ()') - type_sym.register_method(table.Fn{ + type_sym_method_idx = type_sym.register_method(table.Fn{ name: name params: params return_type: return_type @@ -307,6 +307,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl { language: language }) } + end_pos = p.prev_tok.position() // Body p.cur_fn_name = name mut stmts := []ast.Stmt{} @@ -336,6 +337,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl { } receiver_pos: receiver_pos is_method: is_method + method_idx: type_sym_method_idx rec_mut: rec_mut language: language no_body: no_body diff --git a/vlib/v/table/table.v b/vlib/v/table/table.v index 9af62367f4..19e0f24d7e 100644 --- a/vlib/v/table/table.v +++ b/vlib/v/table/table.v @@ -38,6 +38,7 @@ pub: attrs []Attr pub mut: name string + source_fn voidptr // set in the checker, while processing fn declarations } fn (f &Fn) method_equals(o &Fn) bool { @@ -162,9 +163,12 @@ pub fn (mut t Table) register_fn(new_fn Fn) { t.fns[new_fn.name] = new_fn } -pub fn (mut t TypeSymbol) register_method(new_fn Fn) { +pub fn (mut t TypeSymbol) register_method(new_fn Fn) int { + // returns a method index, stored in the ast.FnDecl + // for faster lookup in the checker's fn_decl method // println('reg me $new_fn.name nr_args=$new_fn.args.len') t.methods << new_fn + return t.methods.len - 1 } pub fn (t &Table) register_aggregate_method(mut sym TypeSymbol, name string) ?Fn { diff --git a/vlib/vweb/vweb.v b/vlib/vweb/vweb.v index 1e106213db..fe5e4efd70 100644 --- a/vlib/vweb/vweb.v +++ b/vlib/vweb/vweb.v @@ -203,7 +203,7 @@ pub fn run_app(mut app T, port int) { $if method.return_type is Result { // check routes for validity } - } + } //app.reset() for { conn := l.accept() or { panic('accept() failed') } @@ -380,7 +380,7 @@ fn handle_conn(conn net.Socket, mut app T) { mut vars := []string{cap: route_words_a.len} mut action := '' $for method in T.methods { - $if method.return_type is Result { + $if method.return_type is Result { attrs := method.attrs route_words_a = [][]string{} if attrs.len == 0 { @@ -485,7 +485,11 @@ fn handle_conn(conn net.Socket, mut app T) { // search again for method if action == method.name && method.attrs.len > 0 { // call action method - app.$method(vars) + if method.args.len == vars.len { + app.$method(vars) + } else { + eprintln('warning: uneven parameters count (${method.args.len}) in `$method.name`, compared to the vweb route `$method.attrs` (${vars.len})') + } } } }