add more tests and validations and fix formatting

liorfr-monday · liorfr-monday · commit e8a25a6e1a20 · 2025-04-24T15:30:46.000+03:00
diff --git a/kclvm/tools/src/LSP/src/compile.rs b/kclvm/tools/src/LSP/src/compile.rs
@@ -18,9 +18,9 @@ use std::collections::HashSet;
 use std::path::PathBuf;
 
 use crate::{
-    validator::validate_schema_attributes,
     state::{KCLGlobalStateCache, KCLVfs},
     util::load_files_code_from_vfs,
+    validator::validate_schema_attributes,
 };
 
 pub struct Params {
@@ -127,11 +127,11 @@ pub fn compile(
         params.scope_cache.clone(),
     );
     let schema_map: IndexMap<String, Vec<SchemaType>> = filter_pkg_schemas(&prog_scope, None, None);
-    
+
     // Clone diagnostics before moving prog_scope
     let mut all_diags = IndexSet::new();
     all_diags.extend(prog_scope.handler.diagnostics.clone());
-    
+
     // Add schema validation
     if let Err(validation_diags) = validate_schema_attributes(&program, &prog_scope) {
         all_diags.extend(validation_diags);
diff --git a/kclvm/tools/src/LSP/src/lib.rs b/kclvm/tools/src/LSP/src/lib.rs
@@ -24,5 +24,5 @@ mod state;
 mod tests;
 pub mod to_lsp;
 mod util;
-mod word_index;
 mod validator;
+mod word_index;
diff --git a/kclvm/tools/src/LSP/src/main.rs b/kclvm/tools/src/LSP/src/main.rs
@@ -18,12 +18,12 @@ mod request;
 mod semantic_token;
 mod signature_help;
 mod state;
+#[cfg(test)]
+mod tests;
 mod to_lsp;
 mod util;
-mod word_index;
 mod validator;
-#[cfg(test)]
-mod tests;
+mod word_index;
 
 use app::{app, main_loop};
 
diff --git a/kclvm/tools/src/LSP/src/test_data/schema_validation/validator_test.k b/kclvm/tools/src/LSP/src/test_data/schema_validation/validator_test.k
@@ -4,7 +4,7 @@
 schema Person:
     name: str  # required
     age: int   # required
-    address?: str  # optional
+    address: str  # now required for nested validation
 
 # Test case 1: Valid instance - all required attributes present
 person1 = Person {
@@ -27,8 +27,45 @@ person3 = Person {
     age: 25
 }
 
-# Test case 4: Valid instance - optional attribute omitted
+# Test case 4: Valid instance - all required attributes present
 person4 = Person {
     name: "Bob"
     age: 40
-} 
+    address: "456 Oak St"  # now required
+}
+
+
+schema NestedPerson:
+    family: Person  # nested validation up to depth 10
+
+# Test case 5: Valid instance - nested Person instance
+nested_person = NestedPerson {
+    family: Person {
+        name: "John"
+        age: 30
+        address: "123 Main St"
+    }
+}
+
+# Test case 6: Invalid instance - nested Person missing required 'address' attribute
+# Expected error: Missing required attributes in nested Person instance: address
+# Expected error line: 55
+nested_person3 = NestedPerson {
+    family: Person {
+        name: "John"
+        age: 30
+        # address is required but missing
+    }
+}
+
+# Test case 7: Invalid instance - missing required 'age' attribute
+# Expected error: Missing required attributes in Person instance: age
+# Expected error line: 70
+CreateTest = lambda name: str -> NestedPerson {
+    NestedPerson {
+        family = Person {
+            name = name
+        }
+    }
+}
+
diff --git a/kclvm/tools/src/LSP/src/validator.rs b/kclvm/tools/src/LSP/src/validator.rs
@@ -1,60 +1,95 @@
-use kclvm_ast::ast::{Program, Node, Stmt, AssignStmt, Expr};
-use kclvm_error::{Diagnostic, Level};
+use kclvm_ast::ast::{AssignStmt, Expr, Node, Program, Stmt};
 use kclvm_error::diagnostic::Position;
+use kclvm_error::{Diagnostic, Level};
 use kclvm_sema::resolver::scope::ProgramScope;
 use std::collections::HashMap;
 
-pub fn validate_schema_attributes(program: &Program, _scope: &ProgramScope) -> Result<(), Vec<Diagnostic>> {
+pub fn validate_schema_attributes(
+    program: &Program,
+    _scope: &ProgramScope,
+) -> Result<(), Vec<Diagnostic>> {
     let mut diagnostics = Vec::new();
 
     // Process schemas and validate instances in a single pass
     for (_, modules) in &program.pkgs {
         for module_path in modules {
             if let Ok(Some(module)) = program.get_module(module_path) {
                 let mut schema_attrs = HashMap::new();
-                
+
+                // First pass: collect all schema definitions
                 for stmt in &module.body {
-                    match &**stmt {
-                        Node { node: Stmt::Schema(schema), .. } => {
-                            let mut required_attrs = Vec::new();
-                            for attr in &schema.body {
-                                if let Node { node: Stmt::SchemaAttr(attr_stmt), .. } = &**attr {
-                                    if !attr_stmt.is_optional && attr_stmt.value.is_none() {
-                                        required_attrs.push(attr_stmt.name.node.clone());
-                                    }
+                    if let Node {
+                        node: Stmt::Schema(schema),
+                        ..
+                    } = &**stmt
+                    {
+                        let mut required_attrs = Vec::new();
+                        for attr in &schema.body {
+                            if let Node {
+                                node: Stmt::SchemaAttr(attr_stmt),
+                                ..
+                            } = &**attr
+                            {
+                                if !attr_stmt.is_optional && attr_stmt.value.is_none() {
+                                    required_attrs.push(attr_stmt.name.node.clone());
                                 }
                             }
-                            schema_attrs.insert(schema.name.node.clone(), required_attrs);
-                        },
-                        Node { node: Stmt::Assign(assign_stmt), filename, line, column, .. } => {
-                            if let Some(schema_name) = get_schema_name(assign_stmt) {
-                                if let Some(required_attrs) = schema_attrs.get(&schema_name) {
-                                    let missing_attrs = get_missing_attrs(assign_stmt, required_attrs);
-                                    if !missing_attrs.is_empty() {
-                                        diagnostics.push(Diagnostic::new(
-                                            Level::Error,
-                                            &format!(
-                                                "Missing required attributes in {} instance: {}",
-                                                schema_name,
-                                                missing_attrs.join(", ")
-                                            ),
-                                            (
-                                                Position {
-                                                    filename: filename.clone(),
-                                                    line: *line,
-                                                    column: Some(*column),
-                                                },
-                                                Position {
-                                                    filename: filename.clone(),
-                                                    line: *line,
-                                                    column: Some(*column + schema_name.len() as u64),
-                                                }
-                                            ),
-                                        ));
-                                    }
+                        }
+                        schema_attrs.insert(schema.name.node.clone(), required_attrs);
+                    }
+                }
+
+                // Second pass: validate all instances including nested ones and lambdas
+                for stmt in &module.body {
+                    match &**stmt {
+                        Node {
+                            node: Stmt::Assign(assign_stmt),
+                            filename,
+                            line,
+                            column,
+                            ..
+                        } => {
+                            validate_schema_instance(
+                                assign_stmt,
+                                &schema_attrs,
+                                filename,
+                                *line,
+                                *column,
+                                &mut diagnostics,
+                            );
+
+                            // Check if the assignment is a lambda that returns a schema
+                            if let Node {
+                                node: Expr::Lambda(lambda_expr),
+                                ..
+                            } = &*assign_stmt.value
+                            {
+                                if let Some(schema_expr) =
+                                    get_schema_from_lambda_body(&lambda_expr.body)
+                                {
+                                    let nested_assign = AssignStmt {
+                                        value: Box::new(Node {
+                                            node: Expr::Schema(schema_expr.clone()),
+                                            filename: filename.clone(),
+                                            line: *line,
+                                            column: *column,
+                                            end_line: *line,
+                                            end_column: *column,
+                                            id: kclvm_ast::ast::AstIndex::default(),
+                                        }),
+                                        ..assign_stmt.clone()
+                                    };
+                                    validate_schema_instance(
+                                        &nested_assign,
+                                        &schema_attrs,
+                                        filename,
+                                        *line,
+                                        *column,
+                                        &mut diagnostics,
+                                    );
                                 }
                             }
-                        },
+                        }
                         _ => {}
                     }
                 }
@@ -69,31 +104,137 @@ pub fn validate_schema_attributes(program: &Program, _scope: &ProgramScope) -> R
     }
 }
 
+fn get_schema_from_lambda_body(body: &[Box<Node<Stmt>>]) -> Option<&kclvm_ast::ast::SchemaExpr> {
+    for stmt in body {
+        if let Node {
+            node: Stmt::Expr(expr_stmt),
+            ..
+        } = &**stmt
+        {
+            if let Node {
+                node: Expr::Schema(schema_expr),
+                ..
+            } = &*expr_stmt.exprs[0]
+            {
+                return Some(schema_expr);
+            }
+        }
+    }
+    None
+}
+
+fn validate_schema_instance(
+    assign_stmt: &AssignStmt,
+    schema_attrs: &HashMap<String, Vec<String>>,
+    filename: &str,
+    line: u64,
+    column: u64,
+    diagnostics: &mut Vec<Diagnostic>,
+) {
+    if let Node {
+        node: Expr::Schema(schema_expr),
+        ..
+    } = &*assign_stmt.value
+    {
+        let schema_name = schema_expr.name.node.names.last().unwrap().node.clone();
+
+        if let Some(required_attrs) = schema_attrs.get(&schema_name) {
+            let missing_attrs = get_missing_attrs(assign_stmt, required_attrs);
+            if !missing_attrs.is_empty() {
+                diagnostics.push(Diagnostic::new(
+                    Level::Error,
+                    &format!(
+                        "Missing required attributes in {} instance: {}",
+                        schema_name,
+                        missing_attrs.join(", ")
+                    ),
+                    (
+                        Position {
+                            filename: filename.to_string(),
+                            line,
+                            column: Some(column),
+                        },
+                        Position {
+                            filename: filename.to_string(),
+                            line,
+                            column: Some(column + schema_name.len() as u64),
+                        },
+                    ),
+                ));
+            }
+
+            // Recursively validate nested schema instances
+            if let Node {
+                node: Expr::Config(config_expr),
+                ..
+            } = &*schema_expr.config
+            {
+                for item in &config_expr.items {
+                    if let Node {
+                        node: Expr::Schema(_),
+                        ..
+                    } = &*item.node.value
+                    {
+                        let nested_assign = AssignStmt {
+                            value: item.node.value.clone(),
+                            ..assign_stmt.clone()
+                        };
+                        validate_schema_instance(
+                            &nested_assign,
+                            schema_attrs,
+                            filename,
+                            line,
+                            column,
+                            diagnostics,
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
 fn get_schema_name(assign_stmt: &AssignStmt) -> Option<String> {
-    if let Node { node: Expr::Schema(schema_expr), .. } = &*assign_stmt.value {
+    if let Node {
+        node: Expr::Schema(schema_expr),
+        ..
+    } = &*assign_stmt.value
+    {
         schema_expr.name.node.names.last().map(|n| n.node.clone())
     } else {
         None
     }
 }
 
 fn get_missing_attrs(assign_stmt: &AssignStmt, required_attrs: &[String]) -> Vec<String> {
-    if let Node { node: Expr::Schema(schema_expr), .. } = &*assign_stmt.value {
-        if let Node { node: Expr::Config(config_expr), .. } = &*schema_expr.config {
+    if let Node {
+        node: Expr::Schema(schema_expr),
+        ..
+    } = &*assign_stmt.value
+    {
+        if let Node {
+            node: Expr::Config(config_expr),
+            ..
+        } = &*schema_expr.config
+        {
             let provided_attrs: Vec<String> = config_expr
                 .items
                 .iter()
                 .filter_map(|item| {
                     item.node.key.as_ref().and_then(|key| {
-                        if let Node { node: Expr::Identifier(ident), .. } = &**key {
+                        if let Node {
+                            node: Expr::Identifier(ident),
+                            ..
+                        } = &**key
+                        {
                             ident.names.last().map(|n| n.node.clone())
                         } else {
                             None
                         }
                     })
                 })
                 .collect();
-            
+
             required_attrs
                 .iter()
                 .filter(|attr| !provided_attrs.contains(attr))
@@ -105,4 +246,4 @@ fn get_missing_attrs(assign_stmt: &AssignStmt, required_attrs: &[String]) -> Vec
     } else {
         Vec::new()
     }
-}
+}
