feat: overridefile supports other types except schema (#1208)

* feat: overridefile supports other types except schema Signed-off-by: zongz <[email protected]> * fix: fix CR comments Signed-off-by: zongz <[email protected]> * fix: fmt json Signed-off-by: zongz <[email protected]> * fix: add some comments and test cases Signed-off-by: zongz <[email protected]> * fix: make fmt Signed-off-by: zongz <[email protected]> * fix: fix failed test case Signed-off-by: zongz <[email protected]> --------- Signed-off-by: zongz <[email protected]>
kcl-lang · Apr 10, 2024 · e3113fc · e3113fc
1 parent acb901d
commit e3113fc
Show file tree

Hide file tree

Showing 20 changed files with 335 additions and 47 deletions.
diff --git a/kclvm/api/src/capi_test.rs b/kclvm/api/src/capi_test.rs
@@ -65,11 +65,33 @@ fn test_c_api_call_exec_program_with_print() {
 
 #[test]
 fn test_c_api_call_override_file() {
-    test_c_api_without_wrapper::<OverrideFileArgs, OverrideFileResult>(
-        "KclvmService.OverrideFile",
-        "override-file.json",
-        "override-file.response.json",
-    );
+    let test_cases = [
+        ("override-file.json", "override-file.response.json"),
+        (
+            "override-file-dict.json",
+            "override-file-dict.response.json",
+        ),
+        (
+            "override-file-dict_0.json",
+            "override-file-dict_0.response.json",
+        ),
+        (
+            "override-file-list.json",
+            "override-file-list.response.json",
+        ),
+        (
+            "override-file-bool.json",
+            "override-file-bool.response.json",
+        ),
+    ];
+
+    for (input, output) in &test_cases {
+        test_c_api_without_wrapper::<OverrideFileArgs, OverrideFileResult>(
+            "KclvmService.OverrideFile",
+            input,
+            output,
+        );
+    }
 }
 
 #[test]

diff --git a/kclvm/api/src/testdata/override-file-bool.json b/kclvm/api/src/testdata/override-file-bool.json
@@ -0,0 +1,7 @@
+{
+	"file": "./src/testdata/override_bool.k",
+	"specs": [
+		"isExist=False"
+	],
+	"import_paths": []
+}
diff --git a/kclvm/api/src/testdata/override-file-bool.response.json b/kclvm/api/src/testdata/override-file-bool.response.json
@@ -0,0 +1,3 @@
+{
+	"result": true
+}
diff --git a/kclvm/api/src/testdata/override-file-dict.json b/kclvm/api/src/testdata/override-file-dict.json
@@ -0,0 +1,7 @@
+{
+	"file": "./src/testdata/override_dict.k",
+	"specs": [
+		"alice1.age=18"
+	],
+	"import_paths": []
+}
diff --git a/kclvm/api/src/testdata/override-file-dict.response.json b/kclvm/api/src/testdata/override-file-dict.response.json
@@ -0,0 +1,3 @@
+{
+	"result": true
+}
diff --git a/kclvm/api/src/testdata/override-file-dict_0.json b/kclvm/api/src/testdata/override-file-dict_0.json
@@ -0,0 +1,7 @@
+{
+	"file": "./src/testdata/override_dict_0.k",
+	"specs": [
+		"alice3={\"age\": 18}"
+	],
+	"import_paths": []
+}
diff --git a/kclvm/api/src/testdata/override-file-dict_0.response.json b/kclvm/api/src/testdata/override-file-dict_0.response.json
@@ -0,0 +1,3 @@
+{
+	"result": true
+}
diff --git a/kclvm/api/src/testdata/override-file-list.json b/kclvm/api/src/testdata/override-file-list.json
@@ -0,0 +1,7 @@
+{
+	"file": "./src/testdata/override_list.k",
+	"specs": [
+		"alice2=[1,2,3]"
+	],
+	"import_paths": []
+}
diff --git a/kclvm/api/src/testdata/override-file-list.response.json b/kclvm/api/src/testdata/override-file-list.response.json
@@ -0,0 +1,3 @@
+{
+	"result": true
+}
diff --git a/kclvm/api/src/testdata/override-file.json b/kclvm/api/src/testdata/override-file.json
@@ -1,8 +1,7 @@
 {
-	"file" : "./src/testdata/test.k",
-	"specs":[
-        "alice.age=18"
+	"file": "./src/testdata/test.k",
+	"specs": [
+		"alice.age=18"
 	],
-	"import_paths":[
-	]
-}
+	"import_paths": []
+}
diff --git a/kclvm/api/src/testdata/override-file.response.json b/kclvm/api/src/testdata/override-file.response.json
@@ -1,3 +1,3 @@
 {
-	"result" : true
-}
+	"result": true
+}
diff --git a/kclvm/api/src/testdata/override_bool.k b/kclvm/api/src/testdata/override_bool.k
@@ -0,0 +1 @@
+isExist=True
diff --git a/kclvm/api/src/testdata/override_dict.k b/kclvm/api/src/testdata/override_dict.k
@@ -0,0 +1 @@
+alice1 = {"age": 1}
diff --git a/kclvm/api/src/testdata/override_dict_0.k b/kclvm/api/src/testdata/override_dict_0.k
@@ -0,0 +1 @@
+alice3 = {"age": 1}
diff --git a/kclvm/api/src/testdata/override_list.k b/kclvm/api/src/testdata/override_list.k
@@ -0,0 +1 @@
+alice2 = [4,5,6]
diff --git a/kclvm/query/src/override.rs b/kclvm/query/src/override.rs
@@ -3,11 +3,12 @@ use std::collections::HashSet;
 use anyhow::{anyhow, Result};
 
 use compiler_base_macros::bug;
+use kclvm_ast::ast;
 use kclvm_ast::config::try_get_config_expr_mut;
 use kclvm_ast::path::get_key_path;
+use kclvm_ast::walk_list_mut;
 use kclvm_ast::walker::MutSelfMutWalker;
 use kclvm_ast::MAIN_PKG;
-use kclvm_ast::{ast, walk_if_mut};
 use kclvm_ast_pretty::print_ast_module;
 use kclvm_parser::parse_expr;
 use kclvm_sema::pre_process::{fix_config_expr_nest_attr, transform_multi_assign};
@@ -108,10 +109,8 @@ pub fn apply_override_on_module(
     // Apply import paths on AST module.
     apply_import_paths_on_module(m, import_paths)?;
     let ss = parse_attribute_path(&o.field_path)?;
-    if ss.len() <= 1 {
-        return Ok(false);
-    }
-    let target_id = &ss[0];
+    let default = String::default();
+    let target_id = ss.get(0).unwrap_or(&default);
     let value = &o.field_value;
     let key = ast::Identifier {
         names: ss[1..]
@@ -240,6 +239,95 @@ struct OverrideTransformer {
 }
 
 impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
+    // When override the global variable, it should be updated in the module level.
+    // Because the delete action may delete the global variable.
+    // TODO: combine the code of walk_module, walk_assign_stmt and walk_unification_stmt
+    fn walk_module(&mut self, module: &'ctx mut ast::Module) {
+        match self.action {
+            // Walk the module body to find the target and override it.
+            ast::OverrideAction::CreateOrUpdate => {
+                module.body.iter_mut().for_each(|stmt| {
+                    if let ast::Stmt::Assign(assign_stmt) = &mut stmt.node {
+                        if assign_stmt.targets.len() == 1 && self.field_paths.len() == 0 {
+                            let target =
+                                &Some(Box::new(ast::Node::dummy_node(ast::Expr::Identifier(
+                                    assign_stmt.targets.get(0).unwrap().node.clone(),
+                                ))));
+                            let target = get_key_path(target);
+                            if target == self.target_id {
+                                let item = assign_stmt.value.clone();
+
+                                let mut value = self.clone_override_value();
+                                // Use position information that needs to override the expression.
+                                value.set_pos(item.pos());
+                                // Override the node value.
+                                assign_stmt.value = value;
+                                self.has_override = true;
+                            }
+                        }
+                    }
+                    if let ast::Stmt::Unification(unification_stmt) = &mut stmt.node {
+                        let target = match unification_stmt.target.node.names.get(0) {
+                            Some(name) => name,
+                            None => bug!(
+                                "Invalid AST unification target names {:?}",
+                                unification_stmt.target.node.names
+                            ),
+                        };
+                        if target.node == self.target_id {
+                            let item = unification_stmt.value.clone();
+
+                            let mut value = self.clone_override_value();
+                            // Use position information that needs to override the expression.
+                            value.set_pos(item.pos());
+
+                            // Unification is only support to override the schema expression.
+                            if let ast::Expr::Schema(schema_expr) = value.node {
+                                self.has_override = true;
+                                unification_stmt.value =
+                                    Box::new(ast::Node::dummy_node(schema_expr));
+                            }
+                        }
+                    }
+                });
+            }
+            ast::OverrideAction::Delete => {
+                // Delete the override target when the action is DELETE.
+                module.body.retain(|stmt| {
+                    if let ast::Stmt::Assign(assign_stmt) = &stmt.node {
+                        if assign_stmt.targets.len() == 1 && self.field_paths.len() == 0 {
+                            let target =
+                                &Some(Box::new(ast::Node::dummy_node(ast::Expr::Identifier(
+                                    assign_stmt.targets.get(0).unwrap().node.clone(),
+                                ))));
+                            let target = get_key_path(target);
+                            if target == self.target_id {
+                                self.has_override = true;
+                                return false;
+                            }
+                        }
+                    }
+                    if let ast::Stmt::Unification(unification_stmt) = &stmt.node {
+                        let target = match unification_stmt.target.node.names.get(0) {
+                            Some(name) => name,
+                            None => bug!(
+                                "Invalid AST unification target names {:?}",
+                                unification_stmt.target.node.names
+                            ),
+                        };
+                        if target.node == self.target_id && self.field_paths.len() == 0 {
+                            self.has_override = true;
+                            return false;
+                        }
+                    }
+                    true
+                });
+            }
+        }
+
+        walk_list_mut!(self, walk_stmt, module.body)
+    }
+
     fn walk_unification_stmt(&mut self, unification_stmt: &'ctx mut ast::UnificationStmt) {
         let name = match unification_stmt.target.node.names.get(0) {
             Some(name) => name,
@@ -248,7 +336,7 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
                 unification_stmt.target.node.names
             ),
         };
-        if name.node != self.target_id {
+        if name.node != self.target_id || self.field_paths.len() == 0 {
             return;
         }
         self.override_target_count = 1;
@@ -257,7 +345,7 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
     }
 
     fn walk_assign_stmt(&mut self, assign_stmt: &'ctx mut ast::AssignStmt) {
-        if let ast::Expr::Schema(_) = &assign_stmt.value.node {
+        if let ast::Expr::Schema(_) | ast::Expr::Config(_) = &assign_stmt.value.node {
             self.override_target_count = 0;
             for target in &assign_stmt.targets {
                 if target.node.names.len() != 1 {
@@ -303,10 +391,11 @@ impl<'ctx> MutSelfMutWalker<'ctx> for OverrideTransformer {
     }
 
     fn walk_config_expr(&mut self, config_expr: &'ctx mut ast::ConfigExpr) {
-        for config_entry in config_expr.items.iter_mut() {
-            walk_if_mut!(self, walk_expr, config_entry.node.key);
-            self.walk_expr(&mut config_entry.node.value.node);
+        // Lookup config all fields and replace if it is matched with the override spec.
+        if !self.lookup_config_and_replace(config_expr) {
+            return;
         }
+        self.override_target_count = 0;
     }
 
     fn walk_if_stmt(&mut self, _: &'ctx mut ast::IfStmt) {

diff --git a/kclvm/query/src/test_data/except.k b/kclvm/query/src/test_data/except.k
@@ -0,0 +1,41 @@
+schema Data:
+    id?: int = 0
+    value?: str = "value"
+
+schema Config:
+    image: str
+    data?: Data
+
+if True:
+    configOther = Config {image = "image/other:v1"}
+
+
+config = Config {
+    image = "image/image:v1"
+    data = {id = 1, value = "override_value"}
+    data = {id = 1, value = "override_value"}
+}
+
+config: Config {
+    image = "image/image:v1"
+    data = {id = 1, value = "override_value"}
+}
+
+dict_config = {"image": "image/image:v2", "data": {"id": 2, "value2": "override_value2"}}
+envs = [{key = "key1", value = "value1"}, {key = "key2", value = "value2"}]
+isfilter = False
+count = 2
+msg = "Hi World"
+dict_delete = {
+    "data": {
+        "id": 1
+        "value": "override_value"
+    }
+}
+insert_config = {key = 1}
+uni_config = {
+    labels: {key1: 1}
+}
+
+config_unification: Config {"image": "image/image:v4"}
+