Fix unpack_ex related messages

youknowone · youknowone · commit 966620891d1c · 2020-05-13T01:48:56.000+09:00
diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs
@@ -1399,7 +1399,14 @@ impl<O: OutputStream> Compiler<O> {
                     }
                 }
             }
-            _ => return Err(self.error(CompileErrorType::Assign(target.name()))),
+            _ => {
+                return Err(self.error(match target.node {
+                    ast::ExpressionType::Starred { .. } => CompileErrorType::SyntaxError(
+                        "starred assignment target must be in a list or tuple".to_owned(),
+                    ),
+                    _ => CompileErrorType::Assign(target.name()),
+                }))
+            }
         }
 
         Ok(())
diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -940,8 +940,13 @@ impl ExecutingFrame<'_> {
         if unpack {
             for obj in self.pop_multiple(size) {
                 // Take all key-value pairs from the dict:
-                let dict: PyDictRef = obj.downcast().expect("Need a dictionary to build a map.");
+                let dict: PyDictRef = obj.downcast().map_err(|obj| {
+                    vm.new_type_error(format!("'{}' object is not a mapping", obj.class().name))
+                })?;
                 for (key, value) in dict {
+                    if !objtype::isinstance(&key, &vm.ctx.str_type()) {
+                        return Err(vm.new_type_error("keywords must be strings".to_owned()));
+                    }
                     if for_call {
                         if map_obj.contains_key(&key, vm) {
                             let key_repr = vm.to_repr(&key)?;
@@ -1014,6 +1019,7 @@ impl ExecutingFrame<'_> {
                 let kwargs = if *has_kwargs {
                     let kw_dict: PyDictRef = match self.pop_value().downcast() {
                         Err(_) => {
+                            // TODO: check collections.abc.Mapping
                             return Err(vm.new_type_error("Kwargs must be a dict.".to_owned()));
                         }
                         Ok(x) => x,
@@ -1137,7 +1143,7 @@ impl ExecutingFrame<'_> {
         let min_expected = before + after;
         if elements.len() < min_expected {
             Err(vm.new_value_error(format!(
-                "Not enough values to unpack (expected at least {}, got {}",
+                "not enough values to unpack (expected at least {}, got {})",
                 min_expected,
                 elements.len()
             )))
diff --git a/vm/src/obj/objiter.rs b/vm/src/obj/objiter.rs
@@ -26,7 +26,7 @@ pub fn get_iter(vm: &VirtualMachine, iter_target: &PyObjectRef) -> PyResult {
         vm.invoke(&method, vec![])
     } else {
         vm.get_method_or_type_error(iter_target.clone(), "__getitem__", || {
-            format!("Cannot iterate over {}", iter_target.class().name)
+            format!("'{}' object is not iterable", iter_target.class().name)
         })?;
         Ok(PySequenceIterator::new_forward(iter_target.clone())
             .into_ref(vm)

Original file line number	Diff line number	Diff line change
`@@ -1399,7 +1399,14 @@ impl<O: OutputStream> Compiler<O> {`
`1399`	`1399`	`}`
`1400`	`1400`	`}`
`1401`	`1401`	`}`
`1402`		`- _ => return Err(self.error(CompileErrorType::Assign(target.name()))),`
	`1402`	`+ _ => {`
	`1403`	`+ return Err(self.error(match target.node {`
	`1404`	`+ ast::ExpressionType::Starred { .. } => CompileErrorType::SyntaxError(`
	`1405`	`+ "starred assignment target must be in a list or tuple".to_owned(),`
	`1406`	`+ ),`
	`1407`	`+ _ => CompileErrorType::Assign(target.name()),`
	`1408`	`+ }))`
	`1409`	`+ }`
`1403`	`1410`	`}`
`1404`	`1411`
`1405`	`1412`	`Ok(())`