From a59d706b36c3f4f838b258349d5f763c85066e1a Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 21 Sep 2023 11:29:39 +0800 Subject: [PATCH 1/4] fix: raise an error when the asname of import statment is defined multi-times --- kclvm/cmd/src/test_data/multi_import/main.k | 5 +++++ .../cmd/src/test_data/multi_import/sub/main.k | 1 + .../src/test_data/multi_import/sub/sub/main.k | 1 + kclvm/cmd/src/tests.rs | 19 +++++++++++++++++++ kclvm/sema/src/resolver/import.rs | 11 +++++++++++ 5 files changed, 37 insertions(+) create mode 100644 kclvm/cmd/src/test_data/multi_import/main.k create mode 100644 kclvm/cmd/src/test_data/multi_import/sub/main.k create mode 100644 kclvm/cmd/src/test_data/multi_import/sub/sub/main.k diff --git a/kclvm/cmd/src/test_data/multi_import/main.k b/kclvm/cmd/src/test_data/multi_import/main.k new file mode 100644 index 000000000..2534d9819 --- /dev/null +++ b/kclvm/cmd/src/test_data/multi_import/main.k @@ -0,0 +1,5 @@ +import sub as s + +import sub.sub as s + +main = s.sub_sub \ No newline at end of file diff --git a/kclvm/cmd/src/test_data/multi_import/sub/main.k b/kclvm/cmd/src/test_data/multi_import/sub/main.k new file mode 100644 index 000000000..fd778d797 --- /dev/null +++ b/kclvm/cmd/src/test_data/multi_import/sub/main.k @@ -0,0 +1 @@ +sub = "sub" \ No newline at end of file diff --git a/kclvm/cmd/src/test_data/multi_import/sub/sub/main.k b/kclvm/cmd/src/test_data/multi_import/sub/sub/main.k new file mode 100644 index 000000000..7f92e0de0 --- /dev/null +++ b/kclvm/cmd/src/test_data/multi_import/sub/sub/main.k @@ -0,0 +1 @@ +sub_sub = "sub_sub" \ No newline at end of file diff --git a/kclvm/cmd/src/tests.rs b/kclvm/cmd/src/tests.rs index 445889ef9..e1885aa2d 100644 --- a/kclvm/cmd/src/tests.rs +++ b/kclvm/cmd/src/tests.rs @@ -596,3 +596,22 @@ fn test_error_message_fuzz_unmatched() { } } } + +#[test] +fn test_multi_import() { + let test_case_path = PathBuf::from("./src/test_data/multi_import/main.k"); + let matches = app().arg_required_else_help(true).get_matches_from(&[ + ROOT_CMD, + "run", + &test_case_path.canonicalize().unwrap().display().to_string(), + ]); + let settings = must_build_settings(matches.subcommand_matches("run").unwrap()); + let sess = Arc::new(ParseSession::default()); + match exec_program(sess.clone(), &settings.try_into().unwrap()) { + Ok(_) => panic!("unreachable code."), + Err(msg) => { + assert!(msg + .contains("the name 's' is defined multiple times, 's' must be defined only once")) + } + } +} diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs index 1927d64b4..9844f7f34 100644 --- a/kclvm/sema/src/resolver/import.rs +++ b/kclvm/sema/src/resolver/import.rs @@ -91,6 +91,17 @@ impl<'ctx> Resolver<'ctx> { { match self.ctx.import_names.get_mut(&self.ctx.filename) { Some(mapping) => { + // 'import sub as s' and 'import sub.sub as s' will raise this error. + // 'import sub' and 'import sub' will not raise this error. + if mapping.get(&import_stmt.name).is_some() && import_stmt.asname.is_some() { + self.handler.add_compile_error( + &format!( + "the name '{}' is defined multiple times, '{}' must be defined only once", + import_stmt.name, import_stmt.name + ), + stmt.get_span_pos(), + ); + } mapping.insert( import_stmt.name.to_string(), import_stmt.path.to_string(), From e5db9b13b87586963a0c9e0336337ee9677f3feb Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 21 Sep 2023 12:11:53 +0800 Subject: [PATCH 2/4] fix: make fmt --- kclvm/sema/src/resolver/import.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs index 9844f7f34..e65a85ae0 100644 --- a/kclvm/sema/src/resolver/import.rs +++ b/kclvm/sema/src/resolver/import.rs @@ -92,8 +92,10 @@ impl<'ctx> Resolver<'ctx> { match self.ctx.import_names.get_mut(&self.ctx.filename) { Some(mapping) => { // 'import sub as s' and 'import sub.sub as s' will raise this error. - // 'import sub' and 'import sub' will not raise this error. - if mapping.get(&import_stmt.name).is_some() && import_stmt.asname.is_some() { + // 'import sub' and 'import sub' will not raise this error. + if mapping.get(&import_stmt.name).is_some() + && import_stmt.asname.is_some() + { self.handler.add_compile_error( &format!( "the name '{}' is defined multiple times, '{}' must be defined only once", From 1bfc3d33173a69de06ab8b64df0dc5555ccc7b0c Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 21 Sep 2023 14:04:59 +0800 Subject: [PATCH 3/4] fix: move test cases to resolver --- kclvm/cmd/src/tests.rs | 19 --------------- .../test_fail_data/redefine_import}/main.k | 0 .../redefine_import}/sub/main.k | 0 .../redefine_import}/sub/sub/main.k | 0 kclvm/sema/src/resolver/tests.rs | 24 +++++++++++++++++++ 5 files changed, 24 insertions(+), 19 deletions(-) rename kclvm/{cmd/src/test_data/multi_import => sema/src/resolver/test_fail_data/redefine_import}/main.k (100%) rename kclvm/{cmd/src/test_data/multi_import => sema/src/resolver/test_fail_data/redefine_import}/sub/main.k (100%) rename kclvm/{cmd/src/test_data/multi_import => sema/src/resolver/test_fail_data/redefine_import}/sub/sub/main.k (100%) diff --git a/kclvm/cmd/src/tests.rs b/kclvm/cmd/src/tests.rs index e1885aa2d..445889ef9 100644 --- a/kclvm/cmd/src/tests.rs +++ b/kclvm/cmd/src/tests.rs @@ -596,22 +596,3 @@ fn test_error_message_fuzz_unmatched() { } } } - -#[test] -fn test_multi_import() { - let test_case_path = PathBuf::from("./src/test_data/multi_import/main.k"); - let matches = app().arg_required_else_help(true).get_matches_from(&[ - ROOT_CMD, - "run", - &test_case_path.canonicalize().unwrap().display().to_string(), - ]); - let settings = must_build_settings(matches.subcommand_matches("run").unwrap()); - let sess = Arc::new(ParseSession::default()); - match exec_program(sess.clone(), &settings.try_into().unwrap()) { - Ok(_) => panic!("unreachable code."), - Err(msg) => { - assert!(msg - .contains("the name 's' is defined multiple times, 's' must be defined only once")) - } - } -} diff --git a/kclvm/cmd/src/test_data/multi_import/main.k b/kclvm/sema/src/resolver/test_fail_data/redefine_import/main.k similarity index 100% rename from kclvm/cmd/src/test_data/multi_import/main.k rename to kclvm/sema/src/resolver/test_fail_data/redefine_import/main.k diff --git a/kclvm/cmd/src/test_data/multi_import/sub/main.k b/kclvm/sema/src/resolver/test_fail_data/redefine_import/sub/main.k similarity index 100% rename from kclvm/cmd/src/test_data/multi_import/sub/main.k rename to kclvm/sema/src/resolver/test_fail_data/redefine_import/sub/main.k diff --git a/kclvm/cmd/src/test_data/multi_import/sub/sub/main.k b/kclvm/sema/src/resolver/test_fail_data/redefine_import/sub/sub/main.k similarity index 100% rename from kclvm/cmd/src/test_data/multi_import/sub/sub/main.k rename to kclvm/sema/src/resolver/test_fail_data/redefine_import/sub/sub/main.k diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs index 0bf7678ec..a79c4bee2 100644 --- a/kclvm/sema/src/resolver/tests.rs +++ b/kclvm/sema/src/resolver/tests.rs @@ -107,6 +107,30 @@ fn test_resolve_program_fail() { } } +#[test] +fn test_resolve_program_redefine() { + let sess = Arc::new(ParseSession::default()); + let mut program = load_program( + sess.clone(), + &["./src/resolver/test_fail_data/redefine_import/main.k"], + None, + ) + .unwrap(); + + let scope = resolve_program(&mut program); + assert_eq!(scope.handler.diagnostics.len(), 2); + let diag = &scope.handler.diagnostics[0]; + assert_eq!( + diag.code, + Some(DiagnosticId::Error(ErrorKind::CompileError)) + ); + assert_eq!(diag.messages.len(), 1); + assert_eq!( + diag.messages[0].message, + "the name 's' is defined multiple times, 's' must be defined only once" + ); +} + #[test] fn test_resolve_program_mismatch_type_fail() { let mut program = parse_program("./src/resolver/test_fail_data/config_expr.k").unwrap(); From 6a68c3b9db42ad4e3fa317c356fecc8a246f6f64 Mon Sep 17 00:00:00 2001 From: zongz Date: Fri, 22 Sep 2023 14:18:49 +0800 Subject: [PATCH 4/4] fix: fix CR comments --- kclvm/sema/src/resolver/import.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs index e65a85ae0..fe4d59432 100644 --- a/kclvm/sema/src/resolver/import.rs +++ b/kclvm/sema/src/resolver/import.rs @@ -93,16 +93,17 @@ impl<'ctx> Resolver<'ctx> { Some(mapping) => { // 'import sub as s' and 'import sub.sub as s' will raise this error. // 'import sub' and 'import sub' will not raise this error. - if mapping.get(&import_stmt.name).is_some() - && import_stmt.asname.is_some() - { - self.handler.add_compile_error( - &format!( - "the name '{}' is defined multiple times, '{}' must be defined only once", - import_stmt.name, import_stmt.name - ), - stmt.get_span_pos(), - ); + // 'import sub as s' and 'import sub as s' will not raise this error. + if let Some(path) = mapping.get(&import_stmt.name) { + if path != &import_stmt.path { + self.handler.add_compile_error( + &format!( + "the name '{}' is defined multiple times, '{}' must be defined only once", + import_stmt.name, import_stmt.name + ), + stmt.get_span_pos(), + ); + } } mapping.insert( import_stmt.name.to_string(),