Skip to content

Commit af3e97f

Browse files
committed
add missing required attribute in schema
Signed-off-by: Lior Franko <[email protected]>
1 parent d9f133a commit af3e97f

File tree

5 files changed

+156
-4
lines changed

5 files changed

+156
-4
lines changed

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

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

2020
use crate::{
21+
validator::validate_schema_attributes,
2122
state::{KCLGlobalStateCache, KCLVfs},
2223
util::load_files_code_from_vfs,
2324
};
@@ -126,7 +127,15 @@ pub fn compile(
126127
params.scope_cache.clone(),
127128
);
128129
let schema_map: IndexMap<String, Vec<SchemaType>> = filter_pkg_schemas(&prog_scope, None, None);
129-
diags.extend(prog_scope.handler.diagnostics);
130+
131+
// Clone diagnostics before moving prog_scope
132+
let mut all_diags = IndexSet::new();
133+
all_diags.extend(prog_scope.handler.diagnostics.clone());
134+
135+
// Add schema validation
136+
if let Err(validation_diags) = validate_schema_attributes(&program, &prog_scope) {
137+
all_diags.extend(validation_diags);
138+
}
130139

131140
let mut default = GlobalState::default();
132141
let mut gs_ref;
@@ -158,8 +167,8 @@ pub fn compile(
158167
Namer::find_symbols(&program, gs);
159168

160169
match AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map) {
161-
Ok(_) => (diags, Ok((program, schema_map, gs.clone()))),
162-
Err(e) => (diags, Err(anyhow::anyhow!("Resolve failed: {:?}", e))),
170+
Ok(_) => (all_diags, Ok((program, schema_map, gs.clone()))),
171+
Err(e) => (all_diags, Err(anyhow::anyhow!("Resolve failed: {:?}", e))),
163172
}
164173
}
165174

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,4 @@ mod tests;
2525
pub mod to_lsp;
2626
mod util;
2727
mod word_index;
28+
mod validator;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ mod state;
2121
mod to_lsp;
2222
mod util;
2323
mod word_index;
24-
24+
mod validator;
2525
#[cfg(test)]
2626
mod tests;
2727

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# This file contains test cases for schema validation
2+
# Each instance below tests different validation scenarios
3+
4+
schema Person:
5+
name: str # required
6+
age: int # required
7+
address?: str # optional
8+
9+
# Test case 1: Valid instance - all required attributes present
10+
person1 = Person {
11+
name: "John"
12+
age: 30
13+
address: "123 Main St"
14+
}
15+
16+
# Test case 2: Invalid instance - missing required 'age' attribute
17+
# Expected error: Missing required attributes in Person instance: age
18+
# Expected error line: 20
19+
person2 = Person {
20+
name: "Jane"
21+
}
22+
23+
# Test case 3: Invalid instance - missing required 'name' attribute
24+
# Expected error: Missing required attributes in Person instance: name
25+
# Expected error line: 26
26+
person3 = Person {
27+
age: 25
28+
}
29+
30+
# Test case 4: Valid instance - optional attribute omitted
31+
person4 = Person {
32+
name: "Bob"
33+
age: 40
34+
}

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
use kclvm_ast::ast::{Program, Node, Stmt, AssignStmt, Expr};
2+
use kclvm_error::{Diagnostic, Level};
3+
use kclvm_error::diagnostic::Position;
4+
use kclvm_sema::resolver::scope::ProgramScope;
5+
use std::collections::HashMap;
6+
7+
pub fn validate_schema_attributes(program: &Program, _scope: &ProgramScope) -> Result<(), Vec<Diagnostic>> {
8+
let mut diagnostics = Vec::new();
9+
10+
// Process schemas and validate instances in a single pass
11+
for (_, modules) in &program.pkgs {
12+
for module_path in modules {
13+
if let Ok(Some(module)) = program.get_module(module_path) {
14+
let mut schema_attrs = HashMap::new();
15+
16+
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+
}
25+
}
26+
}
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+
}
55+
}
56+
}
57+
},
58+
_ => {}
59+
}
60+
}
61+
}
62+
}
63+
}
64+
65+
if diagnostics.is_empty() {
66+
Ok(())
67+
} else {
68+
Err(diagnostics)
69+
}
70+
}
71+
72+
fn get_schema_name(assign_stmt: &AssignStmt) -> Option<String> {
73+
if let Node { node: Expr::Schema(schema_expr), .. } = &*assign_stmt.value {
74+
schema_expr.name.node.names.last().map(|n| n.node.clone())
75+
} else {
76+
None
77+
}
78+
}
79+
80+
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 {
83+
let provided_attrs: Vec<String> = config_expr
84+
.items
85+
.iter()
86+
.filter_map(|item| {
87+
item.node.key.as_ref().and_then(|key| {
88+
if let Node { node: Expr::Identifier(ident), .. } = &**key {
89+
ident.names.last().map(|n| n.node.clone())
90+
} else {
91+
None
92+
}
93+
})
94+
})
95+
.collect();
96+
97+
required_attrs
98+
.iter()
99+
.filter(|attr| !provided_attrs.contains(attr))
100+
.cloned()
101+
.collect()
102+
} else {
103+
required_attrs.to_vec()
104+
}
105+
} else {
106+
Vec::new()
107+
}
108+
}

0 commit comments

Comments
 (0)