fix: config list subscript type unsoundness (kcl-lang#1591)

Signed-off-by: peefy <[email protected]>
Peefy · Aug 22, 2024 · 16ca833 · 16ca833
1 parent 893aa70
commit 16ca833
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 13 deletions.
diff --git a/kclvm/ast/src/ast.rs b/kclvm/ast/src/ast.rs
@@ -1175,6 +1175,33 @@ pub struct Subscript {
     pub has_question: bool,
 }
 
+impl Subscript {
+    /// Whether the subscript is the a[1] or a[-1] form.
+    pub fn has_name_and_constant_index(&self) -> bool {
+        if let Expr::Identifier(_) = &self.value.node {
+            if let Some(index_node) = &self.index {
+                // Positive index constant
+                if let Expr::NumberLit(number) = &index_node.node {
+                    matches!(number.value, NumberLitValue::Int(_))
+                } else if let Expr::Unary(unary_expr) = &index_node.node {
+                    // Negative index constant
+                    if let Expr::NumberLit(number) = &unary_expr.operand.node {
+                        matches!(number.value, NumberLitValue::Int(_))
+                    } else {
+                        false
+                    }
+                } else {
+                    false
+                }
+            } else {
+                false
+            }
+        } else {
+            false
+        }
+    }
+}
+
 /// Keyword, e.g.
 /// ```kcl
 /// arg=value

diff --git a/kclvm/driver/src/lib.rs b/kclvm/driver/src/lib.rs
@@ -15,7 +15,7 @@ use kclvm_config::{
     },
     path::ModRelativePath,
     settings::{build_settings_pathbuf, DEFAULT_SETTING_FILE},
-    workfile::{load_work_file, WorkFile},
+    workfile::load_work_file,
 };
 use kclvm_parser::LoadProgramOptions;
 use kclvm_utils::path::PathPrefix;

diff --git a/kclvm/sema/src/resolver/config.rs b/kclvm/sema/src/resolver/config.rs
@@ -332,15 +332,20 @@ impl<'ctx> Resolver<'ctx> {
     ) -> Option<TypeRef> {
         if let Some(key) = key {
             if let Some(Some(_)) = self.ctx.config_expr_context.last() {
-                let mut has_index = false;
                 let names: Vec<String> = match &key.node {
                     ast::Expr::Identifier(identifier) => identifier.get_names(),
                     ast::Expr::Subscript(subscript) => {
                         if let ast::Expr::Identifier(identifier) = &subscript.value.node {
                             if let Some(index) = &subscript.index {
                                 if matches!(index.node, ast::Expr::NumberLit(_)) {
-                                    has_index = true;
                                     identifier.get_names()
+                                } else if let ast::Expr::Unary(unary_expr) = &index.node {
+                                    // Negative index constant
+                                    if matches!(unary_expr.operand.node, ast::Expr::NumberLit(_)) {
+                                        identifier.get_names()
+                                    } else {
+                                        return None;
+                                    }
                                 } else {
                                     return None;
                                 }
@@ -366,9 +371,6 @@ impl<'ctx> Resolver<'ctx> {
                 for _ in 0..names.len() - 1 {
                     val_ty = Type::dict_ref(self.str_ty(), val_ty);
                 }
-                if has_index {
-                    val_ty = Type::list_ref(val_ty);
-                }
                 if let Some(Some(obj_last)) = self.ctx.config_expr_context.last() {
                     let ty = obj_last.ty.clone();
                     self.must_assignable_to(
@@ -597,12 +599,14 @@ impl<'ctx> Resolver<'ctx> {
                         val_types.push(val_ty.clone());
                         val_ty
                     }
-                    ast::Expr::Subscript(subscript)
-                        if matches!(subscript.value.node, ast::Expr::Identifier(_)) =>
-                    {
+                    ast::Expr::Subscript(subscript) if subscript.has_name_and_constant_index() => {
                         let val_ty = value_ty.unwrap_or_else(|| self.expr(value));
                         key_types.push(self.str_ty());
-                        val_types.push(Type::list_ref(val_ty.clone()));
+                        if matches!(op, ast::ConfigEntryOperation::Insert) {
+                            val_types.push(val_ty.clone());
+                        } else {
+                            val_types.push(Type::list_ref(val_ty.clone()));
+                        }
                         val_ty
                     }
                     _ => {

diff --git a/kclvm/tools/src/LSP/src/state.rs b/kclvm/tools/src/LSP/src/state.rs
@@ -6,9 +6,7 @@ use crate::word_index::build_word_index;
 use anyhow::Result;
 use crossbeam_channel::{select, unbounded, Receiver, Sender};
 use kclvm_driver::toolchain::{self, Toolchain};
-use kclvm_driver::{
-    lookup_compile_workspaces, lookup_workspace, CompileUnitOptions, WorkSpaceKind,
-};
+use kclvm_driver::{lookup_compile_workspaces, CompileUnitOptions, WorkSpaceKind};
 use kclvm_parser::KCLModuleCache;
 use kclvm_sema::core::global_state::GlobalState;
 use kclvm_sema::resolver::scope::KCLScopeCache;

diff --git a/test/grammar/attr_operator/config_inside/insert/dict_6/main.k b/test/grammar/attr_operator/config_inside/insert/dict_6/main.k
@@ -0,0 +1,5 @@
+config: {str:[int]} = {
+    a += [0, 1, 2]
+    a[0] += [4, 5, 6]
+    a[-1] += [7, 8, 9]
+}
diff --git a/test/grammar/attr_operator/config_inside/insert/dict_6/stdout.golden b/test/grammar/attr_operator/config_inside/insert/dict_6/stdout.golden
@@ -0,0 +1,11 @@
+config:
+  a:
+  - 4
+  - 5
+  - 6
+  - 0
+  - 1
+  - 7
+  - 8
+  - 9
+  - 2
diff --git a/test/grammar/attr_operator/config_inside/insert/dict_fail_0/main.k b/test/grammar/attr_operator/config_inside/insert/dict_fail_0/main.k
@@ -0,0 +1,4 @@
+
+config: {str:[int]} = {
+    a[b] += [1, 2, 3]
+}
diff --git a/test/grammar/attr_operator/config_inside/insert/dict_fail_0/stderr.golden b/test/grammar/attr_operator/config_inside/insert/dict_fail_0/stderr.golden
@@ -0,0 +1,6 @@
+error[E2L23]: CompileError
+ --> ${CWD}/main.k:3:5
+  |
+3 |     a[b] += [1, 2, 3]
+  |     ^ name 'a' is not defined
+  |
-Original file line number
+Diff line change
@@ -0,0 +1,11 @@
+    config:
+      a:
+      - 4
+      - 5
+      - 6
+      - 0
+      - 1
+      - 7
+      - 8
+      - 9
+      - 2