Skip to content

Commit 02fc58d

Browse files
author
walking devel
authored
orm: add type checker for where (#17179)
1 parent 10261c4 commit 02fc58d

6 files changed

+110
-3
lines changed

vlib/v/ast/types.v

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,6 +1618,12 @@ pub fn (t &TypeSymbol) find_field(name string) ?StructField {
16181618
}
16191619
}
16201620

1621+
pub fn (t &TypeSymbol) has_field(name string) bool {
1622+
t.find_field(name) or { return false }
1623+
1624+
return true
1625+
}
1626+
16211627
fn (a &Aggregate) find_field(name string) ?StructField {
16221628
for mut field in unsafe { a.fields } {
16231629
if field.name == name {

vlib/v/checker/orm.v

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,19 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
4242
} else {
4343
ast.Type(0)
4444
}
45+
4546
mut n := ast.SqlExpr{
4647
pos: node.pos
4748
has_where: true
49+
where_expr: ast.None{}
4850
typ: typ
4951
db_expr: node.db_expr
5052
table_expr: ast.TypeNode{
5153
pos: node.table_expr.pos
5254
typ: typ
5355
}
5456
}
57+
5558
tmp_inside_sql := c.inside_sql
5659
c.sql_expr(mut n)
5760
c.inside_sql = tmp_inside_sql
@@ -100,19 +103,19 @@ fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type {
100103

101104
node.fields = fields
102105
node.sub_structs = sub_structs.move()
106+
field_names := fields.map(it.name)
103107

104108
if node.has_where {
105109
c.expr(node.where_expr)
106110
c.check_expr_has_no_fn_calls_with_non_orm_return_type(&node.where_expr)
111+
c.check_where_expr_has_no_pointless_exprs(sym, field_names, &node.where_expr)
107112
}
108113

109114
if node.has_order {
110115
if mut node.order_expr is ast.Ident {
111116
order_ident_name := node.order_expr.name
112117

113-
sym.find_field(order_ident_name) or {
114-
field_names := fields.map(it.name)
115-
118+
if !sym.has_field(order_ident_name) {
116119
c.orm_error(util.new_suggestion(order_ident_name, field_names).say('`${sym.name}` structure has no field with name `${order_ident_name}`'),
117120
node.order_expr.pos)
118121
return ast.void_type
@@ -402,6 +405,48 @@ fn (mut c Checker) check_expr_has_no_fn_calls_with_non_orm_return_type(expr &ast
402405
}
403406
}
404407

408+
// check_where_expr_has_no_pointless_exprs checks that an expression has no pointless expressions
409+
// which don't affect the result. For example, `where 3` is pointless.
410+
// Also, it checks that the left side of the infix expression is always the structure field.
411+
fn (mut c Checker) check_where_expr_has_no_pointless_exprs(table_type_symbol &ast.TypeSymbol, field_names []string, expr &ast.Expr) {
412+
// Skip type checking for generated subqueries
413+
// that are not linked to scope and vars but only created for cgen.
414+
if expr is ast.None {
415+
return
416+
}
417+
418+
if expr is ast.InfixExpr {
419+
has_no_field_error := "left side of the `${expr.op}` expression must be one of the `${table_type_symbol.name}`'s fields"
420+
421+
if expr.left is ast.Ident {
422+
left_ident_name := expr.left.name
423+
424+
if !table_type_symbol.has_field(left_ident_name) {
425+
c.orm_error(util.new_suggestion(left_ident_name, field_names).say(has_no_field_error),
426+
expr.left.pos)
427+
}
428+
} else if expr.left is ast.InfixExpr || expr.left is ast.ParExpr
429+
|| expr.left is ast.PrefixExpr {
430+
c.check_where_expr_has_no_pointless_exprs(table_type_symbol, field_names,
431+
&expr.left)
432+
} else {
433+
c.orm_error(has_no_field_error, expr.left.pos())
434+
}
435+
436+
if expr.right is ast.InfixExpr || expr.right is ast.ParExpr || expr.right is ast.PrefixExpr {
437+
c.check_where_expr_has_no_pointless_exprs(table_type_symbol, field_names,
438+
&expr.right)
439+
}
440+
} else if expr is ast.ParExpr {
441+
c.check_where_expr_has_no_pointless_exprs(table_type_symbol, field_names, &expr.expr)
442+
} else if expr is ast.PrefixExpr {
443+
c.check_where_expr_has_no_pointless_exprs(table_type_symbol, field_names, &expr.right)
444+
} else {
445+
c.orm_error('`where` expression must have at least one comparison for filtering rows',
446+
expr.pos())
447+
}
448+
}
449+
405450
fn (_ &Checker) fn_return_type_flag_to_string(typ ast.Type) string {
406451
is_result_type := typ.has_flag(.result)
407452
is_option_type := typ.has_flag(.option)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
vlib/v/checker/tests/orm_left_side_expr_in_infix_expr_has_no_struct_field_error.vv:18:26: error: V ORM: left side of the `==` expression must be one of the `User`'s fields.
2+
1 possibility: `id`.
3+
16 |
4+
17 | users := sql db {
5+
18 | select from User where first == second
6+
| ~~~~~
7+
19 | }
8+
20 |
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import db.sqlite
2+
3+
struct User {
4+
id int [primary; sql: serial]
5+
}
6+
7+
fn main() {
8+
mut db := sqlite.connect(':memory:') or { panic(err) }
9+
10+
sql db {
11+
create table User
12+
}
13+
14+
first := 'first'
15+
second := 'second'
16+
17+
users := sql db {
18+
select from User where first == second
19+
}
20+
21+
println(users)
22+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
vlib/v/checker/tests/orm_wrong_where_expr_error.vv:15:26: error: V ORM: `where` expression must have at least one comparison for filtering rows
2+
13 |
3+
14 | users := sql db {
4+
15 | select from User where 3
5+
| ^
6+
16 | }
7+
17 |
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import db.sqlite
2+
3+
struct User {
4+
id int [primary; sql: serial]
5+
}
6+
7+
fn main() {
8+
mut db := sqlite.connect(':memory:') or { panic(err) }
9+
10+
sql db {
11+
create table User
12+
}
13+
14+
users := sql db {
15+
select from User where 3
16+
}
17+
18+
println(users)
19+
}

0 commit comments

Comments
 (0)