fix: fix the error message according to singular or even (#769)

* fix: fix the error message according to singular or even Signed-off-by: zongz <[email protected]> * fix: make fmt Signed-off-by: zongz <[email protected]> * fix: move error message generation to compiler_base_error Signed-off-by: zongz <[email protected]> --------- Signed-off-by: zongz <[email protected]>
kcl-lang · Oct 16, 2023 · 6ac730d · 6ac730d
1 parent cabe3bf
commit 6ac730d
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 14 deletions.
diff --git a/compiler_base/error/src/lib.rs b/compiler_base/error/src/lib.rs
@@ -14,6 +14,7 @@ mod emitter;
 mod tests;
 
 pub mod errors;
+pub mod unit_type;
 
 pub use diagnostic::{
     components, diagnostic_handler, style::DiagnosticStyle, Component, Diagnostic, StyledBuffer,

diff --git a/compiler_base/error/src/unit_type.rs b/compiler_base/error/src/unit_type.rs
@@ -0,0 +1,31 @@
+//! This file provides some of the self-encapsulated types used in handling error messages.
+
+/// [`TyeWithUnit`] is a trait for types that can be converted into a string with a unit.
+pub trait TypeWithUnit {
+    fn into_string_with_unit(self) -> String;
+}
+
+/// [`UnitUsize`] is a [`usize`] type that can be converted into a string with a unit.
+pub struct UnitUsize(pub usize, pub String);
+
+impl TypeWithUnit for UnitUsize {
+    /// [`into_string_with_unit`] converts [`UnitUsize`] into a string with a unit.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use compiler_base_error::unit_type::{TypeWithUnit, UnitUsize};
+    ///
+    /// let unit_usize = UnitUsize(1, "byte".to_string());
+    /// assert_eq!(unit_usize.into_string_with_unit(), "1 byte");
+    /// let unit_usize = UnitUsize(2, "byte".to_string());
+    /// assert_eq!(unit_usize.into_string_with_unit(), "2 bytes");
+    /// ```
+    fn into_string_with_unit(self) -> String {
+        if self.0 > 1 {
+            format!("{} {}s", self.0, self.1)
+        } else {
+            format!("{} {}", self.0, self.1)
+        }
+    }
+}
diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock
diff --git a/kclvm/sema/Cargo.toml b/kclvm/sema/Cargo.toml
@@ -28,7 +28,7 @@ kclvm-span = {path = "../span"}
 compiler_base_span = {path = "../../compiler_base/span", version = "0.0.2"}
 compiler_base_session = {path = "../../compiler_base/session"}
 compiler_base_macros = "0.0.1"
-compiler_base_error = "0.0.8"
+compiler_base_error = {path = "../../compiler_base/error"}
 suggestions = "0.1.1"
 
 [dev-dependencies]

diff --git a/kclvm/sema/src/resolver/arg.rs b/kclvm/sema/src/resolver/arg.rs
@@ -1,5 +1,6 @@
 use crate::resolver::Resolver;
 use crate::ty::{FunctionType, Type};
+use compiler_base_error::unit_type::{TypeWithUnit, UnitUsize};
 use indexmap::IndexSet;
 use kclvm_ast::ast;
 use std::rc::Rc;
@@ -68,8 +69,10 @@ impl<'ctx> Resolver<'ctx> {
             if got_count < expect_count {
                 self.handler.add_compile_error(
                     &format!(
-                        "expected {} positional argument, found {}",
-                        expect_count, got_count
+                        "expected {}, found {}",
+                        UnitUsize(expect_count, "positional argument".to_string())
+                            .into_string_with_unit(),
+                        got_count
                     ),
                     func.get_span_pos(),
                 );
@@ -83,9 +86,10 @@ impl<'ctx> Resolver<'ctx> {
                     if !func_ty.is_variadic {
                         self.handler.add_compile_error(
                             &format!(
-                                "{} takes {} positional argument but {} were given",
+                                "{} takes {} but {} were given",
                                 func_name,
-                                func_ty.params.len(),
+                                UnitUsize(func_ty.params.len(), "positional argument".to_string())
+                                    .into_string_with_unit(),
                                 args.len(),
                             ),
                             args[i].get_span_pos(),

diff --git a/kclvm/sema/src/resolver/test_fail_data/unmatched_args.k b/kclvm/sema/src/resolver/test_fail_data/unmatched_args.k
@@ -4,4 +4,9 @@ schema Foo[a : int]:
 f = lambda x {}
 
 foo = Foo(1,2,3)
-f(1,2)
+f(1,2)
+
+schema Foo2[a : int, b: int]:
+    bar? : int
+
+foo2 = Foo2(1,2,3)
diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs
@@ -228,7 +228,7 @@ fn test_resolve_program_illegal_attr_fail() {
 fn test_resolve_program_unmatched_args_fail() {
     let mut program = parse_program("./src/resolver/test_fail_data/unmatched_args.k").unwrap();
     let scope = resolve_program(&mut program);
-    assert_eq!(scope.handler.diagnostics.len(), 2);
+    assert_eq!(scope.handler.diagnostics.len(), 3);
     let expect_err_msg = "\"Foo\" takes 1 positional argument but 3 were given";
     let diag = &scope.handler.diagnostics[0];
     assert_eq!(
@@ -248,6 +248,16 @@ fn test_resolve_program_unmatched_args_fail() {
     assert_eq!(diag.messages.len(), 1);
     assert_eq!(diag.messages[0].range.0.line, 7);
     assert_eq!(diag.messages[0].message, expect_err_msg);
+
+    let expect_err_msg = "\"Foo2\" takes 2 positional arguments but 3 were given";
+    let diag = &scope.handler.diagnostics[2];
+    assert_eq!(
+        diag.code,
+        Some(DiagnosticId::Error(ErrorKind::CompileError))
+    );
+    assert_eq!(diag.messages.len(), 1);
+    assert_eq!(diag.messages[0].range.0.line, 12);
+    assert_eq!(diag.messages[0].message, expect_err_msg);
 }
 
 #[test]