Skip to content

Commit 5f40d99

Browse files
youknowoneyouknowone
authored
Merge pull request RustPython#1917 from youknowone/test-unpack
Fix unpack error messages and add unittest test_unpack.py
2 parents 596bbfa + be8c71b commit 5f40d99

File tree

4 files changed

+240
-10
lines changed

4 files changed

+240
-10
lines changed

‎Lib/test/test_unpack.py

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
doctests = """
2+
3+
Unpack tuple
4+
5+
>>> t = (1, 2, 3)
6+
>>> a, b, c = t
7+
>>> a == 1 and b == 2 and c == 3
8+
True
9+
10+
Unpack list
11+
12+
>>> l = [4, 5, 6]
13+
>>> a, b, c = l
14+
>>> a == 4 and b == 5 and c == 6
15+
True
16+
17+
Unpack implied tuple
18+
19+
>>> a, b, c = 7, 8, 9
20+
>>> a == 7 and b == 8 and c == 9
21+
True
22+
23+
Unpack string... fun!
24+
25+
>>> a, b, c = 'one'
26+
>>> a == 'o' and b == 'n' and c == 'e'
27+
True
28+
29+
Unpack generic sequence
30+
31+
>>> class Seq:
32+
... def __getitem__(self, i):
33+
... if i >= 0 and i < 3: return i
34+
... raise IndexError
35+
...
36+
>>> a, b, c = Seq()
37+
>>> a == 0 and b == 1 and c == 2
38+
True
39+
40+
Single element unpacking, with extra syntax
41+
42+
>>> st = (99,)
43+
>>> sl = [100]
44+
>>> a, = st
45+
>>> a
46+
99
47+
>>> b, = sl
48+
>>> b
49+
100
50+
51+
Now for some failures
52+
53+
Unpacking non-sequence
54+
55+
>>> a, b, c = 7
56+
Traceback (most recent call last):
57+
...
58+
TypeError: cannot unpack non-iterable int object
59+
60+
Unpacking tuple of wrong size
61+
62+
>>> a, b = t
63+
Traceback (most recent call last):
64+
...
65+
ValueError: too many values to unpack (expected 2)
66+
67+
Unpacking tuple of wrong size
68+
69+
>>> a, b = l
70+
Traceback (most recent call last):
71+
...
72+
ValueError: too many values to unpack (expected 2)
73+
74+
Unpacking sequence too short
75+
76+
>>> a, b, c, d = Seq()
77+
Traceback (most recent call last):
78+
...
79+
ValueError: not enough values to unpack (expected 4, got 3)
80+
81+
Unpacking sequence too long
82+
83+
>>> a, b = Seq()
84+
Traceback (most recent call last):
85+
...
86+
ValueError: too many values to unpack (expected 2)
87+
88+
Unpacking a sequence where the test for too long raises a different kind of
89+
error
90+
91+
>>> class BozoError(Exception):
92+
... pass
93+
...
94+
>>> class BadSeq:
95+
... def __getitem__(self, i):
96+
... if i >= 0 and i < 3:
97+
... return i
98+
... elif i == 3:
99+
... raise BozoError
100+
... else:
101+
... raise IndexError
102+
...
103+
104+
Trigger code while not expecting an IndexError (unpack sequence too long, wrong
105+
error)
106+
107+
>>> a, b, c, d, e = BadSeq()
108+
Traceback (most recent call last):
109+
...
110+
test.test_unpack.BozoError
111+
112+
Trigger code while expecting an IndexError (unpack sequence too short, wrong
113+
error)
114+
115+
>>> a, b, c = BadSeq()
116+
Traceback (most recent call last):
117+
...
118+
test.test_unpack.BozoError
119+
120+
Allow unpacking empty iterables
121+
122+
>>> () = []
123+
>>> [] = ()
124+
>>> [] = []
125+
>>> () = ()
126+
127+
Unpacking non-iterables should raise TypeError
128+
129+
>>> () = 42
130+
Traceback (most recent call last):
131+
...
132+
TypeError: cannot unpack non-iterable int object
133+
134+
Unpacking to an empty iterable should raise ValueError
135+
136+
>>> () = [42]
137+
Traceback (most recent call last):
138+
...
139+
ValueError: too many values to unpack (expected 0)
140+
141+
"""
142+
143+
__test__ = {'doctests' : doctests}
144+
145+
def test_main(verbose=False):
146+
from test import support
147+
from test import test_unpack
148+
support.run_doctest(test_unpack, verbose)
149+
150+
if __name__ == "__main__":
151+
test_main(verbose=True)

‎vm/src/frame.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -667,13 +667,35 @@ impl ExecutingFrame<'_> {
667667
}
668668
bytecode::Instruction::UnpackSequence { size } => {
669669
let value = self.pop_value();
670-
let elements = vm.extract_elements(&value)?;
671-
if elements.len() != *size {
672-
Err(vm.new_value_error("Wrong number of values to unpack".to_owned()))
673-
} else {
674-
for element in elements.into_iter().rev() {
675-
self.push_value(element);
670+
let elements = vm.extract_elements(&value).map_err(|e| {
671+
if e.class().is(&vm.ctx.exceptions.type_error) {
672+
vm.new_type_error(format!(
673+
"cannot unpack non-iterable {} object",
674+
value.class().name
675+
))
676+
} else {
677+
e
678+
}
679+
})?;
680+
let msg = match elements.len().cmp(size) {
681+
std::cmp::Ordering::Equal => {
682+
for element in elements.into_iter().rev() {
683+
self.push_value(element);
684+
}
685+
None
686+
}
687+
std::cmp::Ordering::Greater => {
688+
Some(format!("too many values to unpack (expected {})", size))
676689
}
690+
std::cmp::Ordering::Less => Some(format!(
691+
"not enough values to unpack (expected {}, got {})",
692+
size,
693+
elements.len()
694+
)),
695+
};
696+
if let Some(msg) = msg {
697+
Err(vm.new_value_error(msg))
698+
} else {
677699
Ok(None)
678700
}
679701
}

‎vm/src/obj/objstr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ impl PyString {
327327
}
328328

329329
#[pymethod(name = "__repr__")]
330-
fn repr(&self, vm: &VirtualMachine) -> PyResult<String> {
330+
pub(crate) fn repr(&self, vm: &VirtualMachine) -> PyResult<String> {
331331
let in_len = self.value.len();
332332
let mut out_len = 0usize;
333333
// let mut max = 127;

‎vm/src/stdlib/io.rs

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::obj::objbyteinner::PyBytesLike;
1616
use crate::obj::objbytes::PyBytesRef;
1717
use crate::obj::objint;
1818
use crate::obj::objiter;
19-
use crate::obj::objstr::{self, PyStringRef};
19+
use crate::obj::objstr::{self, PyString, PyStringRef};
2020
use crate::obj::objtype::{self, PyClassRef};
2121
use crate::pyobject::{
2222
BufferProtocol, Either, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
@@ -812,12 +812,69 @@ fn buffered_writer_seekable(_self: PyObjectRef) -> bool {
812812
true
813813
}
814814

815+
#[derive(FromArgs)]
816+
struct TextIOWrapperArgs {
817+
#[pyarg(positional_or_keyword, optional = false)]
818+
buffer: PyObjectRef,
819+
#[pyarg(positional_or_keyword, default = "None")]
820+
encoding: Option<PyStringRef>,
821+
#[pyarg(positional_or_keyword, default = "None")]
822+
errors: Option<PyStringRef>,
823+
#[pyarg(positional_or_keyword, default = "None")]
824+
newline: Option<PyStringRef>,
825+
}
826+
827+
impl TextIOWrapperArgs {
828+
fn validate_newline(&self, vm: &VirtualMachine) -> PyResult<()> {
829+
if let Some(pystr) = &self.newline {
830+
match pystr.as_str() {
831+
"" | "\n" | "\r" | "\r\n" => Ok(()),
832+
_ => {
833+
Err(vm.new_value_error(format!("illegal newline value: '{}'", pystr.repr(vm)?)))
834+
}
835+
}
836+
} else {
837+
Ok(())
838+
}
839+
}
840+
}
841+
815842
fn text_io_wrapper_init(
816843
instance: PyObjectRef,
817-
buffer: PyObjectRef,
844+
args: TextIOWrapperArgs,
818845
vm: &VirtualMachine,
819846
) -> PyResult<()> {
820-
vm.set_attr(&instance, "buffer", buffer.clone())?;
847+
args.validate_newline(vm)?;
848+
849+
let mut encoding: Option<PyStringRef> = args.encoding.clone();
850+
let mut self_encoding = None; // TODO: Try os.device_encoding(fileno)
851+
if encoding.is_none() && self_encoding.is_none() {
852+
// TODO: locale module
853+
self_encoding = Some("utf-8");
854+
}
855+
if let Some(self_encoding) = self_encoding {
856+
encoding = Some(PyString::from(self_encoding).into_ref(vm));
857+
} else if let Some(ref encoding) = encoding {
858+
self_encoding = Some(encoding.as_str())
859+
} else {
860+
return Err(vm.new_os_error("could not determine default encoding".to_owned()));
861+
}
862+
let _ = encoding; // TODO: check codec
863+
864+
let errors = args
865+
.errors
866+
.map_or_else(|| vm.ctx.new_str("strict"), |o| o.into_object());
867+
868+
// let readuniversal = args.newline.map_or_else(true, |s| s.as_str().is_empty());
869+
870+
vm.set_attr(
871+
&instance,
872+
"encoding",
873+
self_encoding.map_or_else(|| vm.get_none(), |s| vm.ctx.new_str(s)),
874+
)?;
875+
vm.set_attr(&instance, "errors", errors)?;
876+
vm.set_attr(&instance, "buffer", args.buffer.clone())?;
877+
821878
Ok(())
822879
}
823880

0 commit comments

Comments
 (0)