diff --git a/kclvm/tools/src/LSP/src/compile.rs b/kclvm/tools/src/LSP/src/compile.rs index 492c0d5dd..4c095e758 100644 --- a/kclvm/tools/src/LSP/src/compile.rs +++ b/kclvm/tools/src/LSP/src/compile.rs @@ -20,6 +20,7 @@ use std::path::PathBuf; use crate::{ state::{KCLGlobalStateCache, KCLVfs}, util::load_files_code_from_vfs, + validator::validate_schema_attributes, }; pub struct Params { @@ -126,7 +127,15 @@ pub fn compile( params.scope_cache.clone(), ); let schema_map: IndexMap> = filter_pkg_schemas(&prog_scope, None, None); - diags.extend(prog_scope.handler.diagnostics); + + // 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); + } let mut default = GlobalState::default(); let mut gs_ref; @@ -158,8 +167,8 @@ pub fn compile( Namer::find_symbols(&program, gs); match AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map) { - Ok(_) => (diags, Ok((program, schema_map, gs.clone()))), - Err(e) => (diags, Err(anyhow::anyhow!("Resolve failed: {:?}", e))), + Ok(_) => (all_diags, Ok((program, schema_map, gs.clone()))), + Err(e) => (all_diags, Err(anyhow::anyhow!("Resolve failed: {:?}", e))), } } diff --git a/kclvm/tools/src/LSP/src/lib.rs b/kclvm/tools/src/LSP/src/lib.rs index 0a306581b..943c4e5bb 100644 --- a/kclvm/tools/src/LSP/src/lib.rs +++ b/kclvm/tools/src/LSP/src/lib.rs @@ -24,4 +24,5 @@ mod state; mod tests; pub mod to_lsp; mod util; +mod validator; mod word_index; diff --git a/kclvm/tools/src/LSP/src/main.rs b/kclvm/tools/src/LSP/src/main.rs index 7f26bff06..d550609e4 100644 --- a/kclvm/tools/src/LSP/src/main.rs +++ b/kclvm/tools/src/LSP/src/main.rs @@ -18,13 +18,13 @@ mod request; mod semantic_token; mod signature_help; mod state; +#[cfg(test)] +mod tests; mod to_lsp; mod util; +mod validator; mod word_index; -#[cfg(test)] -mod tests; - use app::{app, main_loop}; /// Main entry point for the `kcl-language-server` executable. diff --git a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__completion__tests__import_external_pkg_test.snap.new b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__completion__tests__import_external_pkg_test.snap.new new file mode 100644 index 000000000..3f380af42 --- /dev/null +++ b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__completion__tests__import_external_pkg_test.snap.new @@ -0,0 +1,6 @@ +--- +source: tools/src/LSP/src/completion.rs +assertion_line: 2189 +expression: "format! (\"{:?}\", got_labels)" +--- +[] 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 new file mode 100644 index 000000000..455414f6e --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/schema_validation/validator_test.k @@ -0,0 +1,71 @@ +# This file contains test cases for schema validation +# Each instance below tests different validation scenarios + +schema Person: + name: str # required + age: int # required + address: str # now required for nested validation + +# Test case 1: Valid instance - all required attributes present +person1 = Person { + name: "John" + age: 30 + address: "123 Main St" +} + +# Test case 2: Invalid instance - missing required 'age' attribute +# Expected error: Missing required attributes in Person instance: age +# Expected error line: 20 +person2 = Person { + name: "Jane" +} + +# Test case 3: Invalid instance - missing required 'name' attribute +# Expected error: Missing required attributes in Person instance: name +# Expected error line: 26 +person3 = Person { + age: 25 +} + +# 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 in lambda return +# 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 new file mode 100644 index 000000000..e112e3348 --- /dev/null +++ b/kclvm/tools/src/LSP/src/validator.rs @@ -0,0 +1,252 @@ +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> { + 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 { + 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); + } + } + + // 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, + ); + } + } + } + _ => {} + } + } + } + } + } + + if diagnostics.is_empty() { + Ok(()) + } else { + Err(diagnostics) + } +} + +fn get_schema_from_lambda_body(body: &[Box>]) -> 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>, + filename: &str, + line: u64, + column: u64, + diagnostics: &mut Vec, +) { + 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(_), + filename, + line, + column, + .. + } = &*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 { + 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 { + 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 = config_expr + .items + .iter() + .filter_map(|item| { + item.node.key.as_ref().and_then(|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)) + .cloned() + .collect() + } else { + required_attrs.to_vec() + } + } else { + Vec::new() + } +}