From 16ca833814b04d020c3d8cffb237424eb4929eb2 Mon Sep 17 00:00:00 2001 From: Peefy Date: Thu, 22 Aug 2024 11:45:01 +0800 Subject: [PATCH] fix: config list subscript type unsoundness (#1591) Signed-off-by: peefy --- kclvm/ast/src/ast.rs | 27 +++++++++++++++++++ kclvm/driver/src/lib.rs | 2 +- kclvm/sema/src/resolver/config.rs | 22 ++++++++------- kclvm/tools/src/LSP/src/state.rs | 4 +-- .../config_inside/insert/dict_6/main.k | 5 ++++ .../config_inside/insert/dict_6/stdout.golden | 11 ++++++++ .../config_inside/insert/dict_fail_0/main.k | 4 +++ .../insert/dict_fail_0/stderr.golden | 6 +++++ 8 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 test/grammar/attr_operator/config_inside/insert/dict_6/main.k create mode 100644 test/grammar/attr_operator/config_inside/insert/dict_6/stdout.golden create mode 100644 test/grammar/attr_operator/config_inside/insert/dict_fail_0/main.k create mode 100644 test/grammar/attr_operator/config_inside/insert/dict_fail_0/stderr.golden diff --git a/kclvm/ast/src/ast.rs b/kclvm/ast/src/ast.rs index c8f0ead3d..900026100 100644 --- 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 index c80022bf6..7de00fe33 100644 --- 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 index a670d7bcc..212c078ce 100644 --- a/kclvm/sema/src/resolver/config.rs +++ b/kclvm/sema/src/resolver/config.rs @@ -332,15 +332,20 @@ impl<'ctx> Resolver<'ctx> { ) -> Option { if let Some(key) = key { if let Some(Some(_)) = self.ctx.config_expr_context.last() { - let mut has_index = false; let names: Vec = 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 index 0bf4fa1e3..d8bb2e334 100644 --- 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 new file mode 100644 index 000000000..9befa820a --- /dev/null +++ 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 new file mode 100644 index 000000000..2e0992740 --- /dev/null +++ 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 new file mode 100644 index 000000000..a2c7912f5 --- /dev/null +++ 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 new file mode 100644 index 000000000..cf420e1ea --- /dev/null +++ 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 + | \ No newline at end of file