Add Error Message Location (#112, #110, #108, #87); Some Minor Error Message Improvements #145

Merged
sb10q merged 5 commits from err_msg_improvement into master 2021-12-22 08:52:19 +08:00
3 changed files with 152 additions and 78 deletions
Showing only changes of commit 48723f5f8b - Show all commits

View File

@ -234,10 +234,10 @@ pub fn parse_type_annotation<T>(
} }
} }
} else { } 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)),
} }
} }

View File

@ -551,14 +551,24 @@ impl TopLevelComposer {
match &ast.node { match &ast.node {
ast::StmtKind::ClassDef { name: class_name, body, .. } => { ast::StmtKind::ClassDef { name: class_name, body, .. } => {
if self.keyword_list.contains(class_name) { 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({ if !defined_names.insert({
let mut n = mod_path.clone(); let mut n = mod_path.clone();
n.push_str(&class_name.to_string()); n.push_str(&class_name.to_string());
n 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; let class_name = *class_name;
@ -601,7 +611,12 @@ impl TopLevelComposer {
contains_constructor = true; contains_constructor = true;
} }
if self.keyword_list.contains(method_name) { 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 global_class_method_name = {
let mut n = mod_path.clone(); let mut n = mod_path.clone();
@ -615,7 +630,12 @@ impl TopLevelComposer {
n n
}; };
if !defined_names.insert(global_class_method_name.clone()) { 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() + { let method_def_id = self.definition_ast_list.len() + {
// plus 1 here since we already have the class def // 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()); // return Err("cannot use keyword as a top level function name".into());
// } // }
let global_fun_name = { let global_fun_name = {
let mut n = mod_path; let mut n = mod_path.clone();
n.push_str(&name.to_string()); n.push_str(&name.to_string());
n n
}; };
if !defined_names.insert(global_fun_name.clone()) { 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; 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 { if !is_generic {
is_generic = true; is_generic = true;
} else { } 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<()>>; let type_var_list: Vec<&ast::Expr<()>>;
@ -803,7 +835,10 @@ impl TopLevelComposer {
}) })
}; };
if !all_unique_type_var { 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 // add to TopLevelDef
@ -868,9 +903,11 @@ impl TopLevelComposer {
} }
if has_base { if has_base {
return Err("a class def can only have at most one base class \ return Err(format!(
declaration and one generic declaration" "a class definition can only have at most one base class \
.into()); declaration and one generic declaration (at {})",
b.location
));
} }
has_base = true; has_base = true;
@ -888,7 +925,10 @@ impl TopLevelComposer {
if let TypeAnnotation::CustomClass { .. } = &base_ty { if let TypeAnnotation::CustomClass { .. } = &base_ty {
class_ancestors.push(base_ty); class_ancestors.push(base_ty);
} else { } 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 = { let arg_types = {
// make sure no duplicate parameter // make sure no duplicate parameter
let mut defined_paramter_name: HashSet<_> = HashSet::new(); let mut defined_paramter_name: HashSet<_> = HashSet::new();
let have_unique_fuction_parameter_name = args.args.iter().all(|x| { for x in args.args.iter() {
defined_paramter_name.insert(x.node.arg) if !defined_paramter_name.insert(x.node.arg) || keyword_list.contains(&x.node.arg) {
&& !keyword_list.contains(&x.node.arg) return Err(format!(
}); "top level function must have unique parameter names \
if !have_unique_fuction_parameter_name { and names should not be the same as the keywords (at {})",
return Err("top level function must have unique parameter names \ x.location
and names thould not be the same as the keywords" ));
.into()); }
} }
let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args
@ -1290,20 +1330,22 @@ impl TopLevelComposer {
// check method parameters cannot have same name // check method parameters cannot have same name
let mut defined_paramter_name: HashSet<_> = HashSet::new(); let mut defined_paramter_name: HashSet<_> = HashSet::new();
let zelf: StrRef = "self".into(); let zelf: StrRef = "self".into();
let have_unique_fuction_parameter_name = args.args.iter().all(|x| { for x in args.args.iter() {
defined_paramter_name.insert(x.node.arg) if !defined_paramter_name.insert(x.node.arg)
&& (!keyword_list.contains(&x.node.arg) || x.node.arg == zelf) || (keyword_list.contains(&x.node.arg) && x.node.arg != zelf) {
}); return Err(format!(
if !have_unique_fuction_parameter_name { "top level function must have unique parameter names \
return Err("class method must have unique parameter names \ and names should not be the same as the keywords (at {})",
and names thould not be the same as the keywords" x.location
.into()); ))
}
} }
if name == &"__init__".into() && !defined_paramter_name.contains(&zelf) { 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) { 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(); let mut result = Vec::new();
@ -1369,7 +1411,7 @@ impl TopLevelComposer {
None => None, None => None,
Some(default) => { Some(default) => {
if name == "self".into() { 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({ Some({
let v = Self::parse_parameter_default_value(default, class_resolver)?; 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)); 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, class_resolver,
temp_def_list, temp_def_list,
unifier, unifier,
@ -1491,30 +1533,42 @@ impl TopLevelComposer {
)?; )?;
// find type vars within this return type annotation // find type vars within this return type annotation
let type_vars_within = 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 // handle the class type var and the method type var
for type_var_within in type_vars_within { for type_var_within in type_vars_within {
if let TypeAnnotation::TypeVar(t) = type_var_within { if let TypeAnnotation::TypeVar(t) = type_var_within {
if !class_type_vars_def.contains(&t) { if !class_type_vars_def.contains(&t) {
return Err("class fields can only use type \ return Err(format!(
vars declared as class generic type vars" "class fields can only use type \
.into()); vars declared as class generic type vars (at {})",
annotation.location
));
} }
} else { } else {
unreachable!("must be type var annotation"); 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 { } else {
return Err("same class fields defined twice".into()); return Err(format!(
"same class fields `{}` defined twice (at {})",
attr,
target.location
));
} }
} else { } 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::Pass { .. } => {}
ast::StmtKind::Expr { value: _, .. } => {} // typically a docstring; ignoring all expressions matches CPython behavior 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(()) Ok(())
@ -1577,7 +1631,10 @@ impl TopLevelComposer {
type_var_to_concrete_def, type_var_to_concrete_def,
); );
if !ok { 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 // mark it as added
is_override.insert(*class_method_name); is_override.insert(*class_method_name);
@ -1699,9 +1756,10 @@ impl TopLevelComposer {
for (f, _, _) in fields { for (f, _, _) in fields {
if !all_inited.contains(f) { if !all_inited.contains(f) {
return Err(format!( return Err(format!(
"fields `{}` of class `{}` not fully initialized", "fields `{}` of class `{}` not fully initialized in the initializer (at {})",
f, f,
class_name class_name,
body[0].location,
)); ));
} }
} }

View File

@ -72,7 +72,10 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if let TopLevelDef::Class { type_vars, .. } = &*def_read { if let TopLevelDef::Class { type_vars, .. } = &*def_read {
type_vars.clone() type_vars.clone()
} else { } 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 { } else {
locked.get(&obj_id).unwrap().clone() locked.get(&obj_id).unwrap().clone()
@ -81,8 +84,9 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
// check param number here // check param number here
if !type_vars.is_empty() { if !type_vars.is_empty() {
return Err(format!( return Err(format!(
"expect {} type variable parameter but got 0", "expect {} type variable parameter but got 0 (at {})",
type_vars.len() type_vars.len(),
expr.location,
)); ));
} }
Ok(TypeAnnotation::CustomClass { id: obj_id, params: vec![] }) Ok(TypeAnnotation::CustomClass { id: obj_id, params: vec![] })
@ -90,10 +94,16 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if let TypeEnum::TVar { .. } = unifier.get_ty(ty).as_ref() { if let TypeEnum::TVar { .. } = unifier.get_ty(ty).as_ref() {
Ok(TypeAnnotation::TypeVar(ty)) Ok(TypeAnnotation::TypeVar(ty))
} else { } else {
Err("not a type variable identifier".into()) Err(format!(
"not a type variable identifier at {}",
expr.location
))
} }
} else { } 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<T>(
matches!(&value.node, ast::ExprKind::Name { id, .. } if id == &"tuple".into()) matches!(&value.node, ast::ExprKind::Name { id, .. } if id == &"tuple".into())
} => } =>
{ {
if let ast::ExprKind::Tuple { elts, .. } = &slice.node { let tup_elts = {
let type_annotations = elts if let ast::ExprKind::Tuple { elts, .. } = &slice.node {
.iter() elts.as_slice()
.map(|e| { } else {
parse_ast_to_type_annotation_kinds( std::slice::from_ref(slice.as_ref())
resolver, }
top_level_defs, };
unifier, let type_annotations = tup_elts
primitives, .iter()
e, .map(|e| {
locked.clone(), parse_ast_to_type_annotation_kinds(
) resolver,
}) top_level_defs,
.collect::<Result<Vec<_>, _>>()?; unifier,
Ok(TypeAnnotation::Tuple(type_annotations)) primitives,
} else { e,
Err("Expect multiple elements for tuple".into()) locked.clone(),
} )
})
.collect::<Result<Vec<_>, _>>()?;
Ok(TypeAnnotation::Tuple(type_annotations))
} }
// custom class // custom class
@ -166,7 +179,7 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if vec!["virtual".into(), "Generic".into(), "list".into(), "tuple".into()] if vec!["virtual".into(), "Generic".into(), "list".into(), "tuple".into()]
.contains(id) .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 let obj_id = resolver
.get_identifier_def(*id) .get_identifier_def(*id)
@ -192,13 +205,14 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
}; };
if type_vars.len() != params_ast.len() { if type_vars.len() != params_ast.len() {
return Err(format!( return Err(format!(
"expect {} type parameters but got {}", "expect {} type parameters but got {} (at {})",
type_vars.len(), type_vars.len(),
params_ast.len() params_ast.len(),
params_ast[0].location
)); ));
} }
let result = params_ast let result = params_ast
.into_iter() .iter()
.map(|x| { .map(|x| {
parse_ast_to_type_annotation_kinds( parse_ast_to_type_annotation_kinds(
resolver, resolver,
@ -221,19 +235,21 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if no_type_var { if no_type_var {
result result
} else { } else {
return Err("application of type vars to generic class \ return Err(format!(
is not currently supported" "application of type vars to generic class \
.into()); is not currently supported (at {})",
params_ast[0].location
));
} }
}; };
Ok(TypeAnnotation::CustomClass { id: obj_id, params: param_type_infos }) Ok(TypeAnnotation::CustomClass { id: obj_id, params: param_type_infos })
} else { } 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)),
} }
} }