From 16d0d100cb633da751338c18e611c2c92932ed93 Mon Sep 17 00:00:00 2001 From: Shashank Mittal Date: Sun, 26 May 2024 23:36:58 +0530 Subject: [PATCH] Quick fixes for more compile errors (#1360) * some more quick fixes Signed-off-by: Shashank Mittal * fix Signed-off-by: Shashank Mittal * more fixes Signed-off-by: Shashank Mittal * fmt Signed-off-by: Shashank Mittal * grammar test fixed Signed-off-by: Shashank Mittal * test fix Signed-off-by: Shashank Mittal * fixes from #1359 Signed-off-by: Shashank Mittal * fmt Signed-off-by: Shashank Mittal --------- Signed-off-by: Shashank Mittal --- kclvm/sema/src/resolver/arg.rs | 45 ++++++------------- kclvm/sema/src/resolver/global.rs | 26 ++++++----- kclvm/sema/src/resolver/node.rs | 33 ++++++++++++-- kclvm/sema/src/resolver/schema.rs | 3 +- .../invalid_loop_var_fail_0/stderr.golden | 4 +- .../invalid_loop_var_fail_0/stderr.golden | 4 +- .../str/invalid_loop_var_fail_0/stderr.golden | 4 +- 7 files changed, 67 insertions(+), 52 deletions(-) diff --git a/kclvm/sema/src/resolver/arg.rs b/kclvm/sema/src/resolver/arg.rs index 7c8ccecbf..2e4e6c9e2 100644 --- 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 = 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, - func_ty: &FunctionType, - ) -> (Vec, 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 index 9fb1780a9..866f73443 100644 --- 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 index 3296f9115..6edc0c3e6 100644 --- 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 index ab4c31736..b09d042f1 100644 --- 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 index d1103b7ec..4fa9db858 100644 --- 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 | \ No newline at end of file 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 index d1103b7ec..4fa9db858 100644 --- 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 | \ No newline at end of file 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 index d5c33854e..72dadd0a2 100644 --- 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 | \ No newline at end of file