From 48723f5f8b6ec800ecd75938b20dc2884fa3eef7 Mon Sep 17 00:00:00 2001 From: ychenfo Date: Fri, 26 Nov 2021 04:22:35 +0800 Subject: [PATCH] nac3core: error msg add location in some trivial cases (#112, #110, #108, #87) --- nac3core/src/symbol_resolver.rs | 4 +- nac3core/src/toplevel/composer.rs | 146 ++++++++++++++++------- nac3core/src/toplevel/type_annotation.rs | 80 ++++++++----- 3 files changed, 152 insertions(+), 78 deletions(-) diff --git a/nac3core/src/symbol_resolver.rs b/nac3core/src/symbol_resolver.rs index e8d594d..516260a 100644 --- a/nac3core/src/symbol_resolver.rs +++ b/nac3core/src/symbol_resolver.rs @@ -234,10 +234,10 @@ pub fn parse_type_annotation( } } } else { - Err("unsupported type expression".into()) + Err(format!("unsupported type expression at {}", expr.location)) } } - _ => Err("unsupported type expression".into()), + _ => Err(format!("unsupported type expression at {}", expr.location)), } } diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index b0afaf2..9fcd313 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -551,14 +551,24 @@ impl TopLevelComposer { match &ast.node { ast::StmtKind::ClassDef { name: class_name, body, .. } => { if self.keyword_list.contains(class_name) { - return Err("cannot use keyword as a class name".into()); + return Err(format!( + "cannot use keyword `{}` as a class name ({} at {})", + class_name, + mod_path, + ast.location + )); } if !defined_names.insert({ let mut n = mod_path.clone(); n.push_str(&class_name.to_string()); n }) { - return Err("duplicate definition of class".into()); + return Err(format!( + "duplicate definition of class `{}` ({} at {})", + class_name, + mod_path, + ast.location + )); } let class_name = *class_name; @@ -601,7 +611,12 @@ impl TopLevelComposer { contains_constructor = true; } if self.keyword_list.contains(method_name) { - return Err("cannot use keyword as a method name".into()); + return Err(format!( + "cannot use keyword `{}` as a method name ({} at {})", + method_name, + mod_path, + b.location + )); } let global_class_method_name = { let mut n = mod_path.clone(); @@ -615,7 +630,12 @@ impl TopLevelComposer { n }; if !defined_names.insert(global_class_method_name.clone()) { - return Err("duplicate class method definition".into()); + return Err(format!( + "class method `{}` defined twice ({} at {})", + &global_class_method_name[mod_path.len()..], + mod_path, + b.location + )); } let method_def_id = self.definition_ast_list.len() + { // plus 1 here since we already have the class def @@ -672,12 +692,17 @@ impl TopLevelComposer { // return Err("cannot use keyword as a top level function name".into()); // } let global_fun_name = { - let mut n = mod_path; + let mut n = mod_path.clone(); n.push_str(&name.to_string()); n }; if !defined_names.insert(global_fun_name.clone()) { - return Err("duplicate top level function define".into()); + return Err(format!( + "top level function `{}` defined twice ({} at {})", + &global_fun_name[mod_path.len()..], + mod_path, + ast.location + )); } let fun_name = *name; @@ -703,7 +728,11 @@ impl TopLevelComposer { )) } - _ => Err("only registrations of top level classes/functions are supported".into()), + _ => Err(format!( + "registrations of constructs other than top level classes/functions are not supported ({} at {})", + mod_path, + ast.location + )), } } @@ -765,7 +794,10 @@ impl TopLevelComposer { if !is_generic { is_generic = true; } else { - return Err("Only single Generic[...] can be in bases".into()); + return Err(format!( + "only single Generic[...] is allowed (at {})", + b.location + )); } let type_var_list: Vec<&ast::Expr<()>>; @@ -803,7 +835,10 @@ impl TopLevelComposer { }) }; if !all_unique_type_var { - return Err("expect unique type variables".into()); + return Err(format!( + "duplicate type variable occurs (at {})", + slice.location + )); } // add to TopLevelDef @@ -868,9 +903,11 @@ impl TopLevelComposer { } if has_base { - return Err("a class def can only have at most one base class \ - declaration and one generic declaration" - .into()); + return Err(format!( + "a class definition can only have at most one base class \ + declaration and one generic declaration (at {})", + b.location + )); } has_base = true; @@ -888,7 +925,10 @@ impl TopLevelComposer { if let TypeAnnotation::CustomClass { .. } = &base_ty { class_ancestors.push(base_ty); } else { - return Err("class base declaration can only be custom class".into()); + return Err(format!( + "class base declaration can only be custom class (at {})", + b.location, + )); } } } @@ -1057,14 +1097,14 @@ impl TopLevelComposer { let arg_types = { // make sure no duplicate parameter let mut defined_paramter_name: HashSet<_> = HashSet::new(); - let have_unique_fuction_parameter_name = args.args.iter().all(|x| { - defined_paramter_name.insert(x.node.arg) - && !keyword_list.contains(&x.node.arg) - }); - if !have_unique_fuction_parameter_name { - return Err("top level function must have unique parameter names \ - and names thould not be the same as the keywords" - .into()); + for x in args.args.iter() { + if !defined_paramter_name.insert(x.node.arg) || keyword_list.contains(&x.node.arg) { + return Err(format!( + "top level function must have unique parameter names \ + and names should not be the same as the keywords (at {})", + x.location + )); + } } let arg_with_default: Vec<(&ast::Located>, Option<&ast::Expr>)> = args @@ -1290,20 +1330,22 @@ impl TopLevelComposer { // check method parameters cannot have same name let mut defined_paramter_name: HashSet<_> = HashSet::new(); let zelf: StrRef = "self".into(); - let have_unique_fuction_parameter_name = args.args.iter().all(|x| { - defined_paramter_name.insert(x.node.arg) - && (!keyword_list.contains(&x.node.arg) || x.node.arg == zelf) - }); - if !have_unique_fuction_parameter_name { - return Err("class method must have unique parameter names \ - and names thould not be the same as the keywords" - .into()); + for x in args.args.iter() { + if !defined_paramter_name.insert(x.node.arg) + || (keyword_list.contains(&x.node.arg) && x.node.arg != zelf) { + return Err(format!( + "top level function must have unique parameter names \ + and names should not be the same as the keywords (at {})", + x.location + )) + } } + if name == &"__init__".into() && !defined_paramter_name.contains(&zelf) { - return Err("__init__ function must have a `self` parameter".into()); + return Err(format!("__init__ method must have a `self` parameter (at {})", b.location)); } if !defined_paramter_name.contains(&zelf) { - return Err("currently does not support static method".into()); + return Err(format!("class method must have a `self` parameter (at {})", b.location)); } let mut result = Vec::new(); @@ -1369,7 +1411,7 @@ impl TopLevelComposer { None => None, Some(default) => { if name == "self".into() { - return Err(format!("`self` parameter cannot take default value at {}", x.location)); + return Err(format!("`self` parameter cannot take default value (at {})", x.location)); } Some({ let v = Self::parse_parameter_default_value(default, class_resolver)?; @@ -1481,7 +1523,7 @@ impl TopLevelComposer { }; class_fields_def.push((*attr, dummy_field_type, mutable)); - let annotation = parse_ast_to_type_annotation_kinds( + let parsed_annotation = parse_ast_to_type_annotation_kinds( class_resolver, temp_def_list, unifier, @@ -1491,30 +1533,42 @@ impl TopLevelComposer { )?; // find type vars within this return type annotation let type_vars_within = - get_type_var_contained_in_type_annotation(&annotation); + get_type_var_contained_in_type_annotation(&parsed_annotation); // handle the class type var and the method type var for type_var_within in type_vars_within { if let TypeAnnotation::TypeVar(t) = type_var_within { if !class_type_vars_def.contains(&t) { - return Err("class fields can only use type \ - vars declared as class generic type vars" - .into()); + return Err(format!( + "class fields can only use type \ + vars declared as class generic type vars (at {})", + annotation.location + )); } } else { unreachable!("must be type var annotation"); } } - type_var_to_concrete_def.insert(dummy_field_type, annotation); + type_var_to_concrete_def.insert(dummy_field_type, parsed_annotation); } else { - return Err("same class fields defined twice".into()); + return Err(format!( + "same class fields `{}` defined twice (at {})", + attr, + target.location + )); } } else { - return Err("unsupported statement type in class definition body".into()); + return Err(format!( + "unsupported statement type in class definition body (at {})", + target.location + )); } } ast::StmtKind::Pass { .. } => {} ast::StmtKind::Expr { value: _, .. } => {} // typically a docstring; ignoring all expressions matches CPython behavior - _ => return Err("unsupported statement type in class definition body".into()), + _ => return Err(format!( + "unsupported statement in class definition body (at {})", + b.location + )), } } Ok(()) @@ -1577,7 +1631,10 @@ impl TopLevelComposer { type_var_to_concrete_def, ); if !ok { - return Err("method has same name as ancestors' method, but incompatible type".into()); + return Err(format!( + "method {} has same name as ancestors' method, but incompatible type", + class_method_name + )); } // mark it as added is_override.insert(*class_method_name); @@ -1699,9 +1756,10 @@ impl TopLevelComposer { for (f, _, _) in fields { if !all_inited.contains(f) { return Err(format!( - "fields `{}` of class `{}` not fully initialized", + "fields `{}` of class `{}` not fully initialized in the initializer (at {})", f, - class_name + class_name, + body[0].location, )); } } diff --git a/nac3core/src/toplevel/type_annotation.rs b/nac3core/src/toplevel/type_annotation.rs index 307d879..22ab01e 100644 --- a/nac3core/src/toplevel/type_annotation.rs +++ b/nac3core/src/toplevel/type_annotation.rs @@ -72,7 +72,10 @@ pub fn parse_ast_to_type_annotation_kinds( if let TopLevelDef::Class { type_vars, .. } = &*def_read { type_vars.clone() } else { - return Err("function cannot be used as a type".into()); + return Err(format!( + "function cannot be used as a type (at {})", + expr.location + )); } } else { locked.get(&obj_id).unwrap().clone() @@ -81,8 +84,9 @@ pub fn parse_ast_to_type_annotation_kinds( // check param number here if !type_vars.is_empty() { return Err(format!( - "expect {} type variable parameter but got 0", - type_vars.len() + "expect {} type variable parameter but got 0 (at {})", + type_vars.len(), + expr.location, )); } Ok(TypeAnnotation::CustomClass { id: obj_id, params: vec![] }) @@ -90,10 +94,16 @@ pub fn parse_ast_to_type_annotation_kinds( if let TypeEnum::TVar { .. } = unifier.get_ty(ty).as_ref() { Ok(TypeAnnotation::TypeVar(ty)) } else { - Err("not a type variable identifier".into()) + Err(format!( + "not a type variable identifier at {}", + expr.location + )) } } else { - Err("name cannot be parsed as a type annotation".into()) + Err(format!( + "name cannot be parsed as a type annotation at {}", + expr.location + )) } } @@ -140,24 +150,27 @@ pub fn parse_ast_to_type_annotation_kinds( matches!(&value.node, ast::ExprKind::Name { id, .. } if id == &"tuple".into()) } => { - if let ast::ExprKind::Tuple { elts, .. } = &slice.node { - let type_annotations = elts - .iter() - .map(|e| { - parse_ast_to_type_annotation_kinds( - resolver, - top_level_defs, - unifier, - primitives, - e, - locked.clone(), - ) - }) - .collect::, _>>()?; - Ok(TypeAnnotation::Tuple(type_annotations)) - } else { - Err("Expect multiple elements for tuple".into()) - } + let tup_elts = { + if let ast::ExprKind::Tuple { elts, .. } = &slice.node { + elts.as_slice() + } else { + std::slice::from_ref(slice.as_ref()) + } + }; + let type_annotations = tup_elts + .iter() + .map(|e| { + parse_ast_to_type_annotation_kinds( + resolver, + top_level_defs, + unifier, + primitives, + e, + locked.clone(), + ) + }) + .collect::, _>>()?; + Ok(TypeAnnotation::Tuple(type_annotations)) } // custom class @@ -166,7 +179,7 @@ pub fn parse_ast_to_type_annotation_kinds( if vec!["virtual".into(), "Generic".into(), "list".into(), "tuple".into()] .contains(id) { - return Err("keywords cannot be class name".into()); + return Err(format!("keywords cannot be class name (at {})", value.location)); } let obj_id = resolver .get_identifier_def(*id) @@ -192,13 +205,14 @@ pub fn parse_ast_to_type_annotation_kinds( }; if type_vars.len() != params_ast.len() { return Err(format!( - "expect {} type parameters but got {}", + "expect {} type parameters but got {} (at {})", type_vars.len(), - params_ast.len() + params_ast.len(), + params_ast[0].location )); } let result = params_ast - .into_iter() + .iter() .map(|x| { parse_ast_to_type_annotation_kinds( resolver, @@ -221,19 +235,21 @@ pub fn parse_ast_to_type_annotation_kinds( if no_type_var { result } else { - return Err("application of type vars to generic class \ - is not currently supported" - .into()); + return Err(format!( + "application of type vars to generic class \ + is not currently supported (at {})", + params_ast[0].location + )); } }; Ok(TypeAnnotation::CustomClass { id: obj_id, params: param_type_infos }) } else { - Err("unsupported expression type for class name".into()) + Err(format!("unsupported expression type for class name at {}", value.location)) } } - _ => Err("unsupported expression for type annotation".into()), + _ => Err(format!("unsupported expression for type annotation at {}", expr.location)), } }