Quick fixes for more compile errors (#1360)

* some more quick fixes Signed-off-by: Shashank Mittal <[email protected]> * fix Signed-off-by: Shashank Mittal <[email protected]> * more fixes Signed-off-by: Shashank Mittal <[email protected]> * fmt Signed-off-by: Shashank Mittal <[email protected]> * grammar test fixed Signed-off-by: Shashank Mittal <[email protected]> * test fix Signed-off-by: Shashank Mittal <[email protected]> * fixes from #1359 Signed-off-by: Shashank Mittal <[email protected]> * fmt Signed-off-by: Shashank Mittal <[email protected]> --------- Signed-off-by: Shashank Mittal <[email protected]>
kcl-lang · May 26, 2024 · 16d0d10 · 16d0d10
1 parent fd4967e
commit 16d0d10
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 52 deletions.
diff --git a/kclvm/sema/src/resolver/arg.rs b/kclvm/sema/src/resolver/arg.rs
@@ -5,6 +5,7 @@ use indexmap::IndexSet;
 use kclvm_ast::ast;
 
 use kclvm_ast::pos::GetPos;
+use kclvm_error::Position;
 
 use crate::ty::TypeRef;
 
@@ -38,17 +39,21 @@ impl<'ctx> Resolver<'ctx> {
         let mut kwarg_types: Vec<(String, TypeRef)> = vec![];
         let mut check_table: IndexSet<String> = IndexSet::default();
         for kw in kwargs {
-            let (suggs, msg) = self.get_arg_kw_err_suggestion(kw, func_ty);
             if !kw.node.arg.node.names.is_empty() {
                 let arg_name = &kw.node.arg.node.names[0].node;
+                let fix_range = kw.get_span_pos();
+                let (start_pos, end_pos) = fix_range;
+                let start_column = start_pos.column.map(|col| col.saturating_sub(2));
+                let modified_start_pos = Position {
+                    column: start_column,
+                    ..start_pos.clone()
+                };
+                let modified_fix_range = (modified_start_pos, end_pos);
                 if check_table.contains(arg_name) {
                     self.handler.add_compile_error_with_suggestions(
-                        &format!(
-                            "{} has duplicated keyword argument {}{}",
-                            func_name, arg_name, msg
-                        ),
-                        kw.get_span_pos(),
-                        Some(suggs),
+                        &format!("{} has duplicated keyword argument {}", func_name, arg_name),
+                        modified_fix_range,
+                        Some(vec![]),
                     );
                 }
                 check_table.insert(arg_name.to_string());
@@ -58,7 +63,7 @@ impl<'ctx> Resolver<'ctx> {
                 kwarg_types.push((arg_name.to_string(), arg_value_type.clone()));
             } else {
                 self.handler
-                    .add_compile_error(&format!("missing argument{}", msg), kw.get_span_pos());
+                    .add_compile_error(&format!("missing argument"), kw.get_span_pos());
             }
         }
         // Do few argument count check
@@ -124,7 +129,7 @@ impl<'ctx> Resolver<'ctx> {
                         "{} got an unexpected keyword argument '{}'{}",
                         func_name, arg_name, msg
                     ),
-                    kwargs[i].get_span_pos(),
+                    kwargs[i].node.arg.get_span_pos(),
                     Some(suggs),
                 );
             }
@@ -147,28 +152,6 @@ impl<'ctx> Resolver<'ctx> {
     }
 
     /// Generate suggestions for keyword argument errors.
-    pub(crate) fn get_arg_kw_err_suggestion(
-        &self,
-        kw: &ast::NodeRef<ast::Keyword>,
-        func_ty: &FunctionType,
-    ) -> (Vec<String>, String) {
-        let attr = &kw.node.arg.node.names[0].node;
-        let valid_params: Vec<&str> = func_ty
-            .params
-            .iter()
-            .map(|param| param.name.as_str())
-            .collect();
-        let suggs = suggestions::provide_suggestions(attr, valid_params.into_iter());
-
-        let suggestion = if !suggs.is_empty() {
-            format!(", did you mean '{}'?", suggs.join(" or "))
-        } else {
-            String::new()
-        };
-
-        (suggs, suggestion)
-    }
-
     pub(crate) fn get_arg_kw_err_suggestion_from_name(
         &self,
         arg_name: &str,

diff --git a/kclvm/sema/src/resolver/global.rs b/kclvm/sema/src/resolver/global.rs
@@ -471,24 +471,30 @@ impl<'ctx> Resolver<'ctx> {
     ) -> SchemaType {
         let name = &schema_stmt.name.node;
         if RESERVED_TYPE_IDENTIFIERS.contains(&name.as_str()) {
-            self.handler.add_compile_error(
+            self.handler.add_compile_error_with_suggestions(
                 &format!(
                     "schema name '{}' cannot be the same as the built-in types ({:?})",
                     name, RESERVED_TYPE_IDENTIFIERS
                 ),
                 schema_stmt.name.get_span_pos(),
+                Some(vec![]),
             );
         }
         if schema_stmt.is_protocol && !name.ends_with(PROTOCOL_SUFFIX) {
-            self.handler.add_error(
-                ErrorKind::CompileError,
-                &[Message {
-                    range: schema_stmt.name.get_span_pos(),
-                    style: Style::LineAndColumn,
-                    message: format!("schema protocol name must end with '{}'", PROTOCOL_SUFFIX),
-                    note: None,
-                    suggested_replacement: None,
-                }],
+            let fix_range = schema_stmt.name.get_span_pos();
+            let (start_pos, end_pos) = fix_range;
+            let start_column = end_pos.column;
+
+            let modified_start_pos = Position {
+                column: start_column,
+                ..start_pos.clone()
+            };
+            let modified_fix_range = (modified_start_pos, end_pos);
+
+            self.handler.add_compile_error_with_suggestions(
+                &format!("schema protocol name must end with '{}'", PROTOCOL_SUFFIX),
+                modified_fix_range,
+                Some(vec![PROTOCOL_SUFFIX.to_string()]),
             );
         }
         if schema_stmt.is_protocol && !schema_stmt.has_only_attribute_definitions() {

diff --git a/kclvm/sema/src/resolver/node.rs b/kclvm/sema/src/resolver/node.rs
@@ -528,13 +528,28 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
         let mut value_ty = self.expr(&selector_expr.value);
         if value_ty.is_module() && selector_expr.has_question {
             let attr = selector_expr.attr.node.get_name();
-            self.handler.add_compile_error(
+            let fix_range = selector_expr.value.get_span_pos();
+            let (start_pos, end_pos) = fix_range;
+            let start_column = end_pos.column;
+            let end_column = end_pos.column.map(|col| col.saturating_add(1));
+            let modified_start_pos = Position {
+                column: start_column,
+                ..start_pos.clone()
+            };
+            let modified_end_pos = Position {
+                column: end_column,
+                ..end_pos.clone()
+            };
+            let modified_fix_range = (modified_start_pos, modified_end_pos);
+
+            self.handler.add_compile_error_with_suggestions(
                 &format!(
                     "For the module type, the use of '?.{}' is unnecessary and it can be modified as '.{}'",
                     attr,
                     attr
                 ),
-                selector_expr.value.get_span_pos(),
+                modified_fix_range,
+                Some(vec![])
             );
         }
         for name in &selector_expr.attr.node.names {
@@ -889,12 +904,22 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
             } else if i == 1 {
                 val_name = Some(name);
             } else {
-                self.handler.add_compile_error(
+                let fix_range = target.get_span_pos();
+                let (start_pos, end_pos) = fix_range;
+                let start_column = start_pos.column.map(|col| col.saturating_sub(2));
+
+                let modified_start_pos = Position {
+                    column: start_column,
+                    ..start_pos.clone()
+                };
+                let modified_fix_range = (modified_start_pos, end_pos);
+                self.handler.add_compile_error_with_suggestions(
                     &format!(
                         "the number of loop variables is {}, which can only be 1 or 2",
                         comp_clause.targets.len()
                     ),
-                    target.get_span_pos(),
+                    modified_fix_range,
+                    Some(vec![]),
                 );
                 break;
             }

diff --git a/kclvm/sema/src/resolver/schema.rs b/kclvm/sema/src/resolver/schema.rs
@@ -225,9 +225,10 @@ impl<'ctx> Resolver<'ctx> {
                         _ => bug!("invalid builtin decorator function type"),
                     },
                     None => {
-                        self.handler.add_compile_error(
+                        self.handler.add_compile_error_with_suggestions(
                             &format!("UnKnown decorator {}", name),
                             decorator.get_span_pos(),
+                            Some(vec![]),
                         );
                     }
                 },

diff --git a/test/grammar/comprehension/dict/invalid_loop_var_fail_0/stderr.golden b/test/grammar/comprehension/dict/invalid_loop_var_fail_0/stderr.golden
@@ -1,6 +1,6 @@
 error[E2L23]: CompileError
- --> ${CWD}/main.k:2:25
+ --> ${CWD}/main.k:2:23
   |
 2 | dataLoop = [i for i, j, k in data]  # error
-  |                         ^ the number of loop variables is 3, which can only be 1 or 2
+  |                       ^ the number of loop variables is 3, which can only be 1 or 2
   |
diff --git a/test/grammar/comprehension/list/invalid_loop_var_fail_0/stderr.golden b/test/grammar/comprehension/list/invalid_loop_var_fail_0/stderr.golden
@@ -1,6 +1,6 @@
 error[E2L23]: CompileError
- --> ${CWD}/main.k:2:25
+ --> ${CWD}/main.k:2:23
   |
 2 | dataLoop = [i for i, j, k in data]  # error
-  |                         ^ the number of loop variables is 3, which can only be 1 or 2
+  |                       ^ the number of loop variables is 3, which can only be 1 or 2
   |
diff --git a/test/grammar/comprehension/str/invalid_loop_var_fail_0/stderr.golden b/test/grammar/comprehension/str/invalid_loop_var_fail_0/stderr.golden
@@ -1,6 +1,6 @@
 error[E2L23]: CompileError
- --> ${CWD}/main.k:2:25
+ --> ${CWD}/main.k:2:23
   |
 2 | dataLoop = [i for i, j, k in dataString]  # error
-  |                         ^ the number of loop variables is 3, which can only be 1 or 2
+  |                       ^ the number of loop variables is 3, which can only be 1 or 2
   |