Skip to content

Commit

Permalink
bugfix: walk quant expr sub node when got any type of target (#686)
Browse files Browse the repository at this point in the history
  • Loading branch information
He1pa authored Aug 29, 2023
1 parent 34b7fb6 commit abaaefc
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 63 deletions.
2 changes: 1 addition & 1 deletion kclvm/ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ pub struct ParenExpr {
pub expr: NodeRef<Expr>,
}

/// QuantExpr, e.g.
/// QuantExpr, <op> <variables> in <target> {<test> <if_cond>} e.g.
/// ```kcl
/// all x in collection {x > 0}
/// any y in collection {y < 0}
Expand Down
116 changes: 56 additions & 60 deletions kclvm/sema/src/resolver/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,69 +284,65 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {

fn walk_quant_expr(&mut self, quant_expr: &'ctx ast::QuantExpr) -> Self::Result {
let iter_ty = self.expr(&quant_expr.target);
if iter_ty.is_any() {
iter_ty
} else {
let (start, end) = (self.ctx.start_pos.clone(), self.ctx.end_pos.clone());
self.enter_scope(start, end, ScopeKind::Loop);
let (mut key_name, mut val_name) = (None, None);
let mut target_node = None;
for (i, target) in quant_expr.variables.iter().enumerate() {
if target.node.names.is_empty() {
continue;
}
if target.node.names.len() > 1 {
self.handler.add_compile_error(
"loop variables can only be ordinary identifiers",
target.get_span_pos(),
);
}
target_node = Some(target);
let name = &target.node.names[0].node;
if i == 0 {
key_name = Some(name.to_string());
} else if i == 1 {
val_name = Some(name.to_string())
} else {
self.handler.add_compile_error(
&format!(
"the number of loop variables is {}, which can only be 1 or 2",
quant_expr.variables.len()
),
target.get_span_pos(),
);
break;
}
self.ctx.local_vars.push(name.to_string());
let (start, end) = target.get_span_pos();
self.insert_object(
name,
ScopeObject {
name: name.to_string(),
start,
end,
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
let (start, end) = (self.ctx.start_pos.clone(), self.ctx.end_pos.clone());
self.enter_scope(start, end, ScopeKind::Loop);
let (mut key_name, mut val_name) = (None, None);
let mut target_node = None;
for (i, target) in quant_expr.variables.iter().enumerate() {
if target.node.names.is_empty() {
continue;
}
if target.node.names.len() > 1 {
self.handler.add_compile_error(
"loop variables can only be ordinary identifiers",
target.get_span_pos(),
);
}
self.do_loop_type_check(
target_node.unwrap(),
key_name,
val_name,
iter_ty.clone(),
quant_expr.target.get_span_pos(),
);
self.expr_or_any_type(&quant_expr.if_cond);
let item_ty = self.expr(&quant_expr.test);
self.leave_scope();
match &quant_expr.op {
ast::QuantOperation::All | ast::QuantOperation::Any => self.bool_ty(),
ast::QuantOperation::Filter => iter_ty,
ast::QuantOperation::Map => Rc::new(Type::list(item_ty)),
target_node = Some(target);
let name = &target.node.names[0].node;
if i == 0 {
key_name = Some(name.to_string());
} else if i == 1 {
val_name = Some(name.to_string())
} else {
self.handler.add_compile_error(
&format!(
"the number of loop variables is {}, which can only be 1 or 2",
quant_expr.variables.len()
),
target.get_span_pos(),
);
break;
}
self.ctx.local_vars.push(name.to_string());
let (start, end) = target.get_span_pos();
self.insert_object(
name,
ScopeObject {
name: name.to_string(),
start,
end,
ty: self.any_ty(),
kind: ScopeObjectKind::Variable,
used: false,
doc: None,
},
);
}
self.do_loop_type_check(
target_node.unwrap(),
key_name,
val_name,
iter_ty.clone(),
quant_expr.target.get_span_pos(),
);
self.expr_or_any_type(&quant_expr.if_cond);
let item_ty = self.expr(&quant_expr.test);
self.leave_scope();
match &quant_expr.op {
ast::QuantOperation::All | ast::QuantOperation::Any => self.bool_ty(),
ast::QuantOperation::Filter => iter_ty,
ast::QuantOperation::Map => Rc::new(Type::list(item_ty)),
}
}

Expand Down
16 changes: 16 additions & 0 deletions kclvm/sema/src/resolver/test_data/lint.k
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import import_test.a # UnusedImport
import import_test.a # ReImport
import regex

schema Person:
name: str
Expand All @@ -8,3 +9,18 @@ schema Person:
b1 = b._b

import import_test.b # ImportPosition

requires = option("params").requires or []
# Define the validation function
validate_required_labels = lambda item, requires {
if requires:
requires_map = {r.key: r.allowedRegex or "" for r in requires}
labels = item.metadata.labels or {}
if labels:
assert all k, v in labels {
regex.match(v, requires_map[k]) if requires_map[k]
}, "must provide labels with the regex ${requires_map}"
item
}
# Validate All resource
items = [validate_required_labels(i, requires) for i in option("items")]
4 changes: 2 additions & 2 deletions kclvm/sema/src/resolver/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,12 @@ fn test_lint() {
range: (
Position {
filename: filename.clone(),
line: 10,
line: 11,
column: Some(0),
},
Position {
filename: filename.clone(),
line: 10,
line: 11,
column: Some(20),
},
),
Expand Down

0 comments on commit abaaefc

Please sign in to comment.