mirror of
https://github.com/vlang/v.git
synced 2023-08-10 21:13:21 +03:00
checker: add type checking for ORM limit
, offset
, and order by
. (#17095)
This commit is contained in:
parent
b487c9d38e
commit
d563739264
@ -3,6 +3,10 @@
|
|||||||
import time
|
import time
|
||||||
import db.sqlite
|
import db.sqlite
|
||||||
|
|
||||||
|
const (
|
||||||
|
offset_const = 2
|
||||||
|
)
|
||||||
|
|
||||||
struct Module {
|
struct Module {
|
||||||
id int [primary; sql: serial]
|
id int [primary; sql: serial]
|
||||||
name string
|
name string
|
||||||
@ -233,16 +237,17 @@ fn test_orm() {
|
|||||||
assert y.len == 2
|
assert y.len == 2
|
||||||
assert y[0].id == 2
|
assert y[0].id == 2
|
||||||
|
|
||||||
offset_const := 2
|
|
||||||
z := sql db {
|
z := sql db {
|
||||||
select from User order by id limit 2 offset offset_const
|
select from User order by id limit 2 offset offset_const
|
||||||
}
|
}
|
||||||
assert z.len == 2
|
assert z.len == 2
|
||||||
assert z[0].id == 3
|
assert z[0].id == 3
|
||||||
|
|
||||||
oldest := sql db {
|
oldest := sql db {
|
||||||
select from User order by age desc limit 1
|
select from User order by age desc limit 1
|
||||||
}
|
}
|
||||||
assert oldest.age == 34
|
assert oldest.age == 34
|
||||||
|
|
||||||
offs := 1
|
offs := 1
|
||||||
second_oldest := sql db {
|
second_oldest := sql db {
|
||||||
select from User order by age desc limit 1 offset offs
|
select from User order by age desc limit 1 offset offs
|
||||||
|
@ -1,7 +1,6 @@
|
|||||||
module checker
|
module checker
|
||||||
|
|
||||||
import v.ast
|
import v.ast
|
||||||
import v.pref
|
|
||||||
import v.util
|
import v.util
|
||||||
import v.token
|
import v.token
|
||||||
import strings
|
import strings
|
||||||
@ -147,7 +146,10 @@ fn (mut c Checker) get_comptime_number_value(mut expr ast.Expr) ?i64 {
|
|||||||
return expr.val.i64()
|
return expr.val.i64()
|
||||||
}
|
}
|
||||||
if mut expr is ast.Ident {
|
if mut expr is ast.Ident {
|
||||||
if mut obj := c.table.global_scope.find_const(expr.name) {
|
has_expr_mod_in_name := expr.name.contains('.')
|
||||||
|
expr_name := if has_expr_mod_in_name { expr.name } else { '${expr.mod}.${expr.name}' }
|
||||||
|
|
||||||
|
if mut obj := c.table.global_scope.find_const(expr_name) {
|
||||||
if obj.typ == 0 {
|
if obj.typ == 0 {
|
||||||
obj.typ = c.expr(obj.expr)
|
obj.typ = c.expr(obj.expr)
|
||||||
}
|
}
|
||||||
|
@ -4,9 +4,11 @@ module checker
|
|||||||
|
|
||||||
import v.ast
|
import v.ast
|
||||||
import v.token
|
import v.token
|
||||||
|
import v.util
|
||||||
|
|
||||||
const (
|
const (
|
||||||
fkey_attr_name = 'fkey'
|
fkey_attr_name = 'fkey'
|
||||||
|
v_orm_prefix = 'V ORM'
|
||||||
)
|
)
|
||||||
|
|
||||||
fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
|
fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
|
||||||
@ -22,7 +24,7 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
|
|||||||
c.cur_orm_ts = old_ts
|
c.cur_orm_ts = old_ts
|
||||||
}
|
}
|
||||||
if sym.info !is ast.Struct {
|
if sym.info !is ast.Struct {
|
||||||
c.error('the table symbol `${sym.name}` has to be a struct', node.table_expr.pos)
|
c.orm_error('the table symbol `${sym.name}` has to be a struct', node.table_expr.pos)
|
||||||
return ast.void_type
|
return ast.void_type
|
||||||
}
|
}
|
||||||
info := sym.info as ast.Struct
|
info := sym.info as ast.Struct
|
||||||
@ -103,20 +105,42 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
|
|||||||
c.expr(node.where_expr)
|
c.expr(node.where_expr)
|
||||||
c.check_expr_has_no_fn_calls_with_non_orm_return_type(&node.where_expr)
|
c.check_expr_has_no_fn_calls_with_non_orm_return_type(&node.where_expr)
|
||||||
}
|
}
|
||||||
if node.has_offset {
|
|
||||||
c.expr(node.offset_expr)
|
if node.has_order {
|
||||||
|
if mut node.order_expr is ast.Ident {
|
||||||
|
order_ident_name := node.order_expr.name
|
||||||
|
|
||||||
|
sym.find_field(order_ident_name) or {
|
||||||
|
field_names := fields.map(it.name)
|
||||||
|
|
||||||
|
c.orm_error(util.new_suggestion(order_ident_name, field_names).say('`${sym.name}` structure has no field with name `${order_ident_name}`'),
|
||||||
|
node.order_expr.pos)
|
||||||
|
return ast.void_type
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
c.orm_error("expected `${sym.name}` structure's field", node.order_expr.pos())
|
||||||
|
return ast.void_type
|
||||||
|
}
|
||||||
|
|
||||||
|
c.expr(node.order_expr)
|
||||||
}
|
}
|
||||||
|
|
||||||
if node.has_limit {
|
if node.has_limit {
|
||||||
c.expr(node.limit_expr)
|
c.expr(node.limit_expr)
|
||||||
|
c.check_sql_value_expr_is_comptime_with_natural_number_or_expr_with_int_type(mut node.limit_expr,
|
||||||
|
'limit')
|
||||||
}
|
}
|
||||||
if node.has_order {
|
|
||||||
c.expr(node.order_expr)
|
if node.has_offset {
|
||||||
|
c.expr(node.offset_expr)
|
||||||
|
c.check_sql_value_expr_is_comptime_with_natural_number_or_expr_with_int_type(mut node.offset_expr,
|
||||||
|
'offset')
|
||||||
}
|
}
|
||||||
c.expr(node.db_expr)
|
c.expr(node.db_expr)
|
||||||
|
|
||||||
if node.or_expr.kind == .block {
|
if node.or_expr.kind == .block {
|
||||||
if node.or_expr.stmts.len == 0 {
|
if node.or_expr.stmts.len == 0 {
|
||||||
c.error('or block needs to return a default value', node.or_expr.pos)
|
c.orm_error('or block needs to return a default value', node.or_expr.pos)
|
||||||
}
|
}
|
||||||
if node.or_expr.stmts.len > 0 && node.or_expr.stmts.last() is ast.ExprStmt {
|
if node.or_expr.stmts.len > 0 && node.or_expr.stmts.last() is ast.ExprStmt {
|
||||||
c.expected_or_type = node.typ
|
c.expected_or_type = node.typ
|
||||||
@ -176,6 +200,7 @@ fn (mut c Checker) sql_stmt_line(mut node ast.SqlStmtLine) ast.Type {
|
|||||||
c.error('unknown type `${table_sym.name}`', node.pos)
|
c.error('unknown type `${table_sym.name}`', node.pos)
|
||||||
return ast.void_type
|
return ast.void_type
|
||||||
}
|
}
|
||||||
|
|
||||||
info := table_sym.info as ast.Struct
|
info := table_sym.info as ast.Struct
|
||||||
fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, table_sym.name)
|
fields := c.fetch_and_verify_orm_fields(info, node.table_expr.pos, table_sym.name)
|
||||||
mut sub_structs := map[int]ast.SqlStmtLine{}
|
mut sub_structs := map[int]ast.SqlStmtLine{}
|
||||||
@ -216,7 +241,7 @@ fn (mut c Checker) sql_stmt_line(mut node ast.SqlStmtLine) ast.Type {
|
|||||||
for i, column in node.updated_columns {
|
for i, column in node.updated_columns {
|
||||||
x := node.fields.filter(it.name == column)
|
x := node.fields.filter(it.name == column)
|
||||||
if x.len == 0 {
|
if x.len == 0 {
|
||||||
c.error('type `${table_sym.name}` has no field named `${column}`', node.pos)
|
c.orm_error('type `${table_sym.name}` has no field named `${column}`', node.pos)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
field := x[0]
|
field := x[0]
|
||||||
@ -241,19 +266,19 @@ fn (mut c Checker) check_orm_struct_field_attributes(field ast.StructField) {
|
|||||||
for attr in field.attrs {
|
for attr in field.attrs {
|
||||||
if attr.name == checker.fkey_attr_name {
|
if attr.name == checker.fkey_attr_name {
|
||||||
if field_type.kind != .array && field_type.kind != .struct_ {
|
if field_type.kind != .array && field_type.kind != .struct_ {
|
||||||
c.error('The `${checker.fkey_attr_name}` attribute must be used only with arrays and structures',
|
c.orm_error('the `${checker.fkey_attr_name}` attribute must be used only with arrays and structures',
|
||||||
attr.pos)
|
attr.pos)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if !attr.has_arg {
|
if !attr.has_arg {
|
||||||
c.error('The `${checker.fkey_attr_name}` attribute must have an argument',
|
c.orm_error('the `${checker.fkey_attr_name}` attribute must have an argument',
|
||||||
attr.pos)
|
attr.pos)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if attr.kind != .string {
|
if attr.kind != .string {
|
||||||
c.error('`${checker.fkey_attr_name}` attribute must be string. Try [${checker.fkey_attr_name}: \'${attr.arg}\'] instead of [${checker.fkey_attr_name}: ${attr.arg}]',
|
c.orm_error("`${checker.fkey_attr_name}` attribute must be string. Try [${checker.fkey_attr_name}: '${attr.arg}'] instead of [${checker.fkey_attr_name}: ${attr.arg}]",
|
||||||
attr.pos)
|
attr.pos)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -265,7 +290,7 @@ fn (mut c Checker) check_orm_struct_field_attributes(field ast.StructField) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
field_struct_type.find_field(attr.arg) or {
|
field_struct_type.find_field(attr.arg) or {
|
||||||
c.error('`${field_struct_type.name}` struct has no field with name `${attr.arg}`',
|
c.orm_error('`${field_struct_type.name}` struct has no field with name `${attr.arg}`',
|
||||||
attr.pos)
|
attr.pos)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -275,7 +300,7 @@ fn (mut c Checker) check_orm_struct_field_attributes(field ast.StructField) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if field_type.kind == .array && !has_fkey_attr {
|
if field_type.kind == .array && !has_fkey_attr {
|
||||||
c.error('A field that holds an array must be defined with the `${checker.fkey_attr_name}` attribute',
|
c.orm_error('a field that holds an array must be defined with the `${checker.fkey_attr_name}` attribute',
|
||||||
field.pos)
|
field.pos)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -288,15 +313,61 @@ fn (mut c Checker) fetch_and_verify_orm_fields(info ast.Struct, pos token.Pos, t
|
|||||||
&& c.table.sym(c.table.sym(it.typ).array_info().elem_type).kind == .struct_))
|
&& c.table.sym(c.table.sym(it.typ).array_info().elem_type).kind == .struct_))
|
||||||
&& !it.attrs.contains('skip'))
|
&& !it.attrs.contains('skip'))
|
||||||
if fields.len == 0 {
|
if fields.len == 0 {
|
||||||
c.error('V orm: select: empty fields in `${table_name}`', pos)
|
c.orm_error('select: empty fields in `${table_name}`', pos)
|
||||||
return []ast.StructField{}
|
return []ast.StructField{}
|
||||||
}
|
}
|
||||||
if fields[0].name != 'id' {
|
if fields[0].name != 'id' {
|
||||||
c.error('V orm: `id int` must be the first field in `${table_name}`', pos)
|
c.orm_error('`id int` must be the first field in `${table_name}`', pos)
|
||||||
}
|
}
|
||||||
return fields
|
return fields
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// check_sql_value_expr_is_comptime_with_natural_number_or_expr_with_int_type checks that an expression is compile-time
|
||||||
|
// and contains an integer greater than or equal to zero or it is a runtime expression with an integer type.
|
||||||
|
fn (mut c Checker) check_sql_value_expr_is_comptime_with_natural_number_or_expr_with_int_type(mut expr ast.Expr, sql_keyword string) {
|
||||||
|
comptime_number := c.get_comptime_number_value(mut expr) or {
|
||||||
|
c.check_sql_expr_type_is_int(expr, sql_keyword)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if comptime_number < 0 {
|
||||||
|
c.orm_error('`${sql_keyword}` must be greater than or equal to zero', expr.pos())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn (mut c Checker) check_sql_expr_type_is_int(expr &ast.Expr, sql_keyword string) {
|
||||||
|
if expr is ast.Ident {
|
||||||
|
if expr.obj.typ.is_int() {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
} else if expr is ast.CallExpr {
|
||||||
|
if expr.return_type == 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
type_symbol := c.table.sym(expr.return_type)
|
||||||
|
is_error_type := expr.return_type.has_flag(.result) || expr.return_type.has_flag(.option)
|
||||||
|
is_acceptable_type := type_symbol.is_int() && !is_error_type
|
||||||
|
|
||||||
|
if !is_acceptable_type {
|
||||||
|
error_type_symbol := c.fn_return_type_flag_to_string(expr.return_type)
|
||||||
|
c.orm_error('function calls in `${sql_keyword}` must return only an integer type, but `${expr.name}` returns `${error_type_symbol}${type_symbol.name}`',
|
||||||
|
expr.pos)
|
||||||
|
}
|
||||||
|
|
||||||
|
return
|
||||||
|
} else if expr is ast.ParExpr {
|
||||||
|
c.check_sql_expr_type_is_int(&expr.expr, sql_keyword)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
c.orm_error('the type of `${sql_keyword}` must be an integer type', expr.pos())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn (mut c Checker) orm_error(message string, pos token.Pos) {
|
||||||
|
c.error('${checker.v_orm_prefix}: ${message}', pos)
|
||||||
|
}
|
||||||
|
|
||||||
// check_expr_has_no_fn_calls_with_non_orm_return_type checks that an expression has no function calls
|
// check_expr_has_no_fn_calls_with_non_orm_return_type checks that an expression has no function calls
|
||||||
// that return complex types which can't be transformed into SQL.
|
// that return complex types which can't be transformed into SQL.
|
||||||
fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast.Expr) {
|
fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast.Expr) {
|
||||||
@ -312,23 +383,15 @@ fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast
|
|||||||
}
|
}
|
||||||
|
|
||||||
type_symbol := c.table.sym(expr.return_type)
|
type_symbol := c.table.sym(expr.return_type)
|
||||||
is_result_type := expr.return_type.has_flag(.result)
|
|
||||||
is_option_type := expr.return_type.has_flag(.option)
|
|
||||||
is_time := type_symbol.cname == 'time__Time'
|
is_time := type_symbol.cname == 'time__Time'
|
||||||
is_not_pointer := !type_symbol.is_pointer()
|
is_not_pointer := !type_symbol.is_pointer()
|
||||||
is_error_type := is_result_type || is_option_type
|
is_error_type := expr.return_type.has_flag(.result) || expr.return_type.has_flag(.option)
|
||||||
is_acceptable_type := (type_symbol.is_primitive() || is_time) && is_not_pointer
|
is_acceptable_type := (type_symbol.is_primitive() || is_time) && is_not_pointer
|
||||||
&& !is_error_type
|
&& !is_error_type
|
||||||
|
|
||||||
if !is_acceptable_type {
|
if !is_acceptable_type {
|
||||||
error_type_symbol := if is_result_type {
|
error_type_symbol := c.fn_return_type_flag_to_string(expr.return_type)
|
||||||
'!'
|
c.orm_error('function calls must return only primitive types and time.Time, but `${expr.name}` returns `${error_type_symbol}${type_symbol.name}`',
|
||||||
} else if is_option_type {
|
|
||||||
'?'
|
|
||||||
} else {
|
|
||||||
''
|
|
||||||
}
|
|
||||||
c.error('V ORM: function calls must return only primitive types and time.Time, but `${expr.name}` returns `${error_type_symbol}${type_symbol.name}`',
|
|
||||||
expr.pos)
|
expr.pos)
|
||||||
}
|
}
|
||||||
} else if expr is ast.ParExpr {
|
} else if expr is ast.ParExpr {
|
||||||
@ -338,3 +401,16 @@ fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast
|
|||||||
c.check_expr_has_no_fn_calls_with_non_orm_return_type(&expr.right)
|
c.check_expr_has_no_fn_calls_with_non_orm_return_type(&expr.right)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn (_ &Checker) fn_return_type_flag_to_string(typ ast.Type) string {
|
||||||
|
is_result_type := typ.has_flag(.result)
|
||||||
|
is_option_type := typ.has_flag(.option)
|
||||||
|
|
||||||
|
return if is_result_type {
|
||||||
|
'!'
|
||||||
|
} else if is_option_type {
|
||||||
|
'?'
|
||||||
|
} else {
|
||||||
|
''
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -1,4 +1,4 @@
|
|||||||
vlib/v/checker/tests/orm_empty_struct.vv:9:15: error: V orm: select: empty fields in `Person`
|
vlib/v/checker/tests/orm_empty_struct.vv:9:15: error: V ORM: select: empty fields in `Person`
|
||||||
7 | db := sqlite.connect(':memory:')!
|
7 | db := sqlite.connect(':memory:')!
|
||||||
8 | _ := sql db {
|
8 | _ := sql db {
|
||||||
9 | select from Person
|
9 | select from Person
|
||||||
|
7
vlib/v/checker/tests/orm_limit_less_than_zero_error.out
Normal file
7
vlib/v/checker/tests/orm_limit_less_than_zero_error.out
Normal file
@ -0,0 +1,7 @@
|
|||||||
|
vlib/v/checker/tests/orm_limit_less_than_zero_error.vv:18:26: error: V ORM: `limit` must be greater than or equal to zero
|
||||||
|
16 |
|
||||||
|
17 | users := sql db {
|
||||||
|
18 | select from User limit user_limit
|
||||||
|
| ~~~~~~~~~~
|
||||||
|
19 | }
|
||||||
|
20 |
|
22
vlib/v/checker/tests/orm_limit_less_than_zero_error.vv
Normal file
22
vlib/v/checker/tests/orm_limit_less_than_zero_error.vv
Normal file
@ -0,0 +1,22 @@
|
|||||||
|
module main
|
||||||
|
|
||||||
|
import db.sqlite
|
||||||
|
|
||||||
|
const (
|
||||||
|
user_limit = -10
|
||||||
|
)
|
||||||
|
|
||||||
|
struct User {
|
||||||
|
id int [primary; sql: serial]
|
||||||
|
username string
|
||||||
|
}
|
||||||
|
|
||||||
|
fn main() {
|
||||||
|
db := sqlite.connect(':memory:') or { panic(err) }
|
||||||
|
|
||||||
|
users := sql db {
|
||||||
|
select from User limit user_limit
|
||||||
|
}
|
||||||
|
|
||||||
|
println(users)
|
||||||
|
}
|
@ -1,4 +1,4 @@
|
|||||||
vlib/v/checker/tests/orm_no_default_value.vv:11:4: error: or block needs to return a default value
|
vlib/v/checker/tests/orm_no_default_value.vv:11:4: error: V ORM: or block needs to return a default value
|
||||||
9 | _ := sql db {
|
9 | _ := sql db {
|
||||||
10 | select from Person
|
10 | select from Person
|
||||||
11 | } or {
|
11 | } or {
|
||||||
|
@ -1,4 +1,4 @@
|
|||||||
vlib/v/checker/tests/orm_not_a_struct.vv:10:15: error: the table symbol `Person` has to be a struct
|
vlib/v/checker/tests/orm_not_a_struct.vv:10:15: error: V ORM: the table symbol `Person` has to be a struct
|
||||||
8 | db := sqlite.connect(':memory:')!
|
8 | db := sqlite.connect(':memory:')!
|
||||||
9 | _ := sql db {
|
9 | _ := sql db {
|
||||||
10 | select from Person
|
10 | select from Person
|
||||||
|
@ -0,0 +1,14 @@
|
|||||||
|
vlib/v/checker/tests/orm_using_non_struct_field_in_order_by_error.vv:14:29: error: V ORM: `User` structure has no field with name `database`.
|
||||||
|
2 possibilities: `id`, `username`.
|
||||||
|
12 |
|
||||||
|
13 | users := sql db {
|
||||||
|
14 | select from User order by database
|
||||||
|
| ~~~~~~~~
|
||||||
|
15 | }
|
||||||
|
16 |
|
||||||
|
vlib/v/checker/tests/orm_using_non_struct_field_in_order_by_error.vv:17:2: error: `println` can not print void expressions
|
||||||
|
15 | }
|
||||||
|
16 |
|
||||||
|
17 | println(users)
|
||||||
|
| ~~~~~~~~~~~~~~
|
||||||
|
18 | }
|
@ -0,0 +1,18 @@
|
|||||||
|
module main
|
||||||
|
|
||||||
|
import db.sqlite
|
||||||
|
|
||||||
|
struct User {
|
||||||
|
id int [primary; sql: serial]
|
||||||
|
username string
|
||||||
|
}
|
||||||
|
|
||||||
|
fn main() {
|
||||||
|
db := sqlite.connect(':memory:') or { panic(err) }
|
||||||
|
|
||||||
|
users := sql db {
|
||||||
|
select from User order by database
|
||||||
|
}
|
||||||
|
|
||||||
|
println(users)
|
||||||
|
}
|
@ -307,7 +307,7 @@ fn (mut g Gen) sql_expr_to_orm_primitive(expr ast.Expr) {
|
|||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
eprintln(expr)
|
eprintln(expr)
|
||||||
verror('Unknown expr')
|
verror('V ORM: ${expr.type_name()} is not supported')
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user