Skip to content

Commit 591adff

Browse files
liorfr-mondayliorfr-monday
committed
add more tests and validations and fix formatting
Signed-off-by: Lior Franko <[email protected]>
1 parent ecc9aa2 commit 591adff

File tree

5 files changed

+235
-57
lines changed

5 files changed

+235
-57
lines changed

‎kclvm/tools/src/LSP/src/compile.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ use std::collections::HashSet;
1818
use std::path::PathBuf;
1919

2020
use crate::{
21-
validator::validate_schema_attributes,
2221
state::{KCLGlobalStateCache, KCLVfs},
2322
util::load_files_code_from_vfs,
23+
validator::validate_schema_attributes,
2424
};
2525

2626
pub struct Params {
@@ -127,11 +127,11 @@ pub fn compile(
127127
params.scope_cache.clone(),
128128
);
129129
let schema_map: IndexMap<String, Vec<SchemaType>> = filter_pkg_schemas(&prog_scope, None, None);
130-
130+
131131
// Clone diagnostics before moving prog_scope
132132
let mut all_diags = IndexSet::new();
133133
all_diags.extend(prog_scope.handler.diagnostics.clone());
134-
134+
135135
// Add schema validation
136136
if let Err(validation_diags) = validate_schema_attributes(&program, &prog_scope) {
137137
all_diags.extend(validation_diags);

‎kclvm/tools/src/LSP/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ mod state;
2424
mod tests;
2525
pub mod to_lsp;
2626
mod util;
27-
mod word_index;
2827
mod validator;
28+
mod word_index;

‎kclvm/tools/src/LSP/src/main.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ mod request;
1818
mod semantic_token;
1919
mod signature_help;
2020
mod state;
21+
#[cfg(test)]
22+
mod tests;
2123
mod to_lsp;
2224
mod util;
23-
mod word_index;
2425
mod validator;
25-
#[cfg(test)]
26-
mod tests;
26+
mod word_index;
2727

2828
use app::{app, main_loop};
2929

‎kclvm/tools/src/LSP/src/test_data/schema_validation/validator_test.k

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
schema Person:
55
name: str # required
66
age: int # required
7-
address?: str # optional
7+
address: str # now required for nested validation
88

99
# Test case 1: Valid instance - all required attributes present
1010
person1 = Person {
@@ -27,8 +27,45 @@ person3 = Person {
2727
age: 25
2828
}
2929

30-
# Test case 4: Valid instance - optional attribute omitted
30+
# Test case 4: Valid instance - all required attributes present
3131
person4 = Person {
3232
name: "Bob"
3333
age: 40
34-
}
34+
address: "456 Oak St" # now required
35+
}
36+
37+
38+
schema NestedPerson:
39+
family: Person # nested validation up to depth 10
40+
41+
# Test case 5: Valid instance - nested Person instance
42+
nested_person = NestedPerson {
43+
family: Person {
44+
name: "John"
45+
age: 30
46+
address: "123 Main St"
47+
}
48+
}
49+
50+
# Test case 6: Invalid instance - nested Person missing required 'address' attribute
51+
# Expected error: Missing required attributes in nested Person instance: address
52+
# Expected error line: 55
53+
nested_person3 = NestedPerson {
54+
family: Person {
55+
name: "John"
56+
age: 30
57+
# address is required but missing
58+
}
59+
}
60+
61+
# Test case 7: Invalid instance - missing required 'age' attribute
62+
# Expected error: Missing required attributes in Person instance: age
63+
# Expected error line: 70
64+
CreateTest = lambda name: str -> NestedPerson {
65+
NestedPerson {
66+
family = Person {
67+
name = name
68+
}
69+
}
70+
}
71+

‎kclvm/tools/src/LSP/src/validator.rs

Lines changed: 188 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,95 @@
1-
use kclvm_ast::ast::{Program, Node, Stmt, AssignStmt, Expr};
2-
use kclvm_error::{Diagnostic, Level};
1+
use kclvm_ast::ast::{AssignStmt, Expr, Node, Program, Stmt};
32
use kclvm_error::diagnostic::Position;
3+
use kclvm_error::{Diagnostic, Level};
44
use kclvm_sema::resolver::scope::ProgramScope;
55
use std::collections::HashMap;
66

7-
pub fn validate_schema_attributes(program: &Program, _scope: &ProgramScope) -> Result<(), Vec<Diagnostic>> {
7+
pub fn validate_schema_attributes(
8+
program: &Program,
9+
_scope: &ProgramScope,
10+
) -> Result<(), Vec<Diagnostic>> {
811
let mut diagnostics = Vec::new();
912

1013
// Process schemas and validate instances in a single pass
1114
for (_, modules) in &program.pkgs {
1215
for module_path in modules {
1316
if let Ok(Some(module)) = program.get_module(module_path) {
1417
let mut schema_attrs = HashMap::new();
15-
18+
19+
// First pass: collect all schema definitions
1620
for stmt in &module.body {
17-
match &**stmt {
18-
Node { node: Stmt::Schema(schema), .. } => {
19-
let mut required_attrs = Vec::new();
20-
for attr in &schema.body {
21-
if let Node { node: Stmt::SchemaAttr(attr_stmt), .. } = &**attr {
22-
if !attr_stmt.is_optional && attr_stmt.value.is_none() {
23-
required_attrs.push(attr_stmt.name.node.clone());
24-
}
21+
if let Node {
22+
node: Stmt::Schema(schema),
23+
..
24+
} = &**stmt
25+
{
26+
let mut required_attrs = Vec::new();
27+
for attr in &schema.body {
28+
if let Node {
29+
node: Stmt::SchemaAttr(attr_stmt),
30+
..
31+
} = &**attr
32+
{
33+
if !attr_stmt.is_optional && attr_stmt.value.is_none() {
34+
required_attrs.push(attr_stmt.name.node.clone());
2535
}
2636
}
27-
schema_attrs.insert(schema.name.node.clone(), required_attrs);
28-
},
29-
Node { node: Stmt::Assign(assign_stmt), filename, line, column, .. } => {
30-
if let Some(schema_name) = get_schema_name(assign_stmt) {
31-
if let Some(required_attrs) = schema_attrs.get(&schema_name) {
32-
let missing_attrs = get_missing_attrs(assign_stmt, required_attrs);
33-
if !missing_attrs.is_empty() {
34-
diagnostics.push(Diagnostic::new(
35-
Level::Error,
36-
&format!(
37-
"Missing required attributes in {} instance: {}",
38-
schema_name,
39-
missing_attrs.join(", ")
40-
),
41-
(
42-
Position {
43-
filename: filename.clone(),
44-
line: *line,
45-
column: Some(*column),
46-
},
47-
Position {
48-
filename: filename.clone(),
49-
line: *line,
50-
column: Some(*column + schema_name.len() as u64),
51-
}
52-
),
53-
));
54-
}
37+
}
38+
schema_attrs.insert(schema.name.node.clone(), required_attrs);
39+
}
40+
}
41+
42+
// Second pass: validate all instances including nested ones and lambdas
43+
for stmt in &module.body {
44+
match &**stmt {
45+
Node {
46+
node: Stmt::Assign(assign_stmt),
47+
filename,
48+
line,
49+
column,
50+
..
51+
} => {
52+
validate_schema_instance(
53+
assign_stmt,
54+
&schema_attrs,
55+
filename,
56+
*line,
57+
*column,
58+
&mut diagnostics,
59+
);
60+
61+
// Check if the assignment is a lambda that returns a schema
62+
if let Node {
63+
node: Expr::Lambda(lambda_expr),
64+
..
65+
} = &*assign_stmt.value
66+
{
67+
if let Some(schema_expr) =
68+
get_schema_from_lambda_body(&lambda_expr.body)
69+
{
70+
let nested_assign = AssignStmt {
71+
value: Box::new(Node {
72+
node: Expr::Schema(schema_expr.clone()),
73+
filename: filename.clone(),
74+
line: *line,
75+
column: *column,
76+
end_line: *line,
77+
end_column: *column,
78+
id: kclvm_ast::ast::AstIndex::default(),
79+
}),
80+
..assign_stmt.clone()
81+
};
82+
validate_schema_instance(
83+
&nested_assign,
84+
&schema_attrs,
85+
filename,
86+
*line,
87+
*column,
88+
&mut diagnostics,
89+
);
5590
}
5691
}
57-
},
92+
}
5893
_ => {}
5994
}
6095
}
@@ -69,31 +104,137 @@ pub fn validate_schema_attributes(program: &Program, _scope: &ProgramScope) -> R
69104
}
70105
}
71106

107+
fn get_schema_from_lambda_body(body: &[Box<Node<Stmt>>]) -> Option<&kclvm_ast::ast::SchemaExpr> {
108+
for stmt in body {
109+
if let Node {
110+
node: Stmt::Expr(expr_stmt),
111+
..
112+
} = &**stmt
113+
{
114+
if let Node {
115+
node: Expr::Schema(schema_expr),
116+
..
117+
} = &*expr_stmt.exprs[0]
118+
{
119+
return Some(schema_expr);
120+
}
121+
}
122+
}
123+
None
124+
}
125+
126+
fn validate_schema_instance(
127+
assign_stmt: &AssignStmt,
128+
schema_attrs: &HashMap<String, Vec<String>>,
129+
filename: &str,
130+
line: u64,
131+
column: u64,
132+
diagnostics: &mut Vec<Diagnostic>,
133+
) {
134+
if let Node {
135+
node: Expr::Schema(schema_expr),
136+
..
137+
} = &*assign_stmt.value
138+
{
139+
let schema_name = schema_expr.name.node.names.last().unwrap().node.clone();
140+
141+
if let Some(required_attrs) = schema_attrs.get(&schema_name) {
142+
let missing_attrs = get_missing_attrs(assign_stmt, required_attrs);
143+
if !missing_attrs.is_empty() {
144+
diagnostics.push(Diagnostic::new(
145+
Level::Error,
146+
&format!(
147+
"Missing required attributes in {} instance: {}",
148+
schema_name,
149+
missing_attrs.join(", ")
150+
),
151+
(
152+
Position {
153+
filename: filename.to_string(),
154+
line,
155+
column: Some(column),
156+
},
157+
Position {
158+
filename: filename.to_string(),
159+
line,
160+
column: Some(column + schema_name.len() as u64),
161+
},
162+
),
163+
));
164+
}
165+
166+
// Recursively validate nested schema instances
167+
if let Node {
168+
node: Expr::Config(config_expr),
169+
..
170+
} = &*schema_expr.config
171+
{
172+
for item in &config_expr.items {
173+
if let Node {
174+
node: Expr::Schema(_),
175+
..
176+
} = &*item.node.value
177+
{
178+
let nested_assign = AssignStmt {
179+
value: item.node.value.clone(),
180+
..assign_stmt.clone()
181+
};
182+
validate_schema_instance(
183+
&nested_assign,
184+
schema_attrs,
185+
filename,
186+
line,
187+
column,
188+
diagnostics,
189+
);
190+
}
191+
}
192+
}
193+
}
194+
}
195+
}
196+
72197
fn get_schema_name(assign_stmt: &AssignStmt) -> Option<String> {
73-
if let Node { node: Expr::Schema(schema_expr), .. } = &*assign_stmt.value {
198+
if let Node {
199+
node: Expr::Schema(schema_expr),
200+
..
201+
} = &*assign_stmt.value
202+
{
74203
schema_expr.name.node.names.last().map(|n| n.node.clone())
75204
} else {
76205
None
77206
}
78207
}
79208

80209
fn get_missing_attrs(assign_stmt: &AssignStmt, required_attrs: &[String]) -> Vec<String> {
81-
if let Node { node: Expr::Schema(schema_expr), .. } = &*assign_stmt.value {
82-
if let Node { node: Expr::Config(config_expr), .. } = &*schema_expr.config {
210+
if let Node {
211+
node: Expr::Schema(schema_expr),
212+
..
213+
} = &*assign_stmt.value
214+
{
215+
if let Node {
216+
node: Expr::Config(config_expr),
217+
..
218+
} = &*schema_expr.config
219+
{
83220
let provided_attrs: Vec<String> = config_expr
84221
.items
85222
.iter()
86223
.filter_map(|item| {
87224
item.node.key.as_ref().and_then(|key| {
88-
if let Node { node: Expr::Identifier(ident), .. } = &**key {
225+
if let Node {
226+
node: Expr::Identifier(ident),
227+
..
228+
} = &**key
229+
{
89230
ident.names.last().map(|n| n.node.clone())
90231
} else {
91232
None
92233
}
93234
})
94235
})
95236
.collect();
96-
237+
97238
required_attrs
98239
.iter()
99240
.filter(|attr| !provided_attrs.contains(attr))
@@ -105,4 +246,4 @@ fn get_missing_attrs(assign_stmt: &AssignStmt, required_attrs: &[String]) -> Vec
105246
} else {
106247
Vec::new()
107248
}
108-
}
249+
}

0 commit comments

Comments
 (0)