Implement kwargs for builtin open() and _io.FileIO

This makes open() and _io.FileIO() more CPython compliant.
The mode kwarg is fully iplemented.
The encoding kwarg is allowed but not implemented; mainly to allow
the tests to specify encoding for CPython, see #874

8 files changed, 90 insertions, 63 deletions

diff --git a/bare-arm/main.c b/bare-arm/main.c
index 084374d3f1..0724f735ea 100644
--- a/bare-arm/main.c
+++ b/bare-arm/main.c
@@ -70,10 +70,10 @@ mp_import_stat_t mp_import_stat(const char *path) {
     return MP_IMPORT_STAT_NO_EXIST;
 }
 
-mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) {
+mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) {
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open);
+MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open);
 
 void nlr_jump_fail(void *val) {
 }
diff --git a/py/builtin.h b/py/builtin.h
index 01ec22d2b0..a69712bec7 100644
--- a/py/builtin.h
+++ b/py/builtin.h
@@ -25,7 +25,7 @@
  */
 
 mp_obj_t mp_builtin___import__(mp_uint_t n_args, mp_obj_t *args);
-mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args);
+mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs);
 
 MP_DECLARE_CONST_FUN_OBJ(mp_builtin___build_class___obj);
 MP_DECLARE_CONST_FUN_OBJ(mp_builtin___import___obj);
diff --git a/py/qstrdefs.h b/py/qstrdefs.h
index 2c2c45b5be..c10b2fabac 100644
--- a/py/qstrdefs.h
+++ b/py/qstrdefs.h
@@ -446,6 +446,9 @@ Q(StringIO)
 Q(BytesIO)
 Q(getvalue)
 Q(file)
+Q(mode)
+Q(r)
+Q(encoding)
 #endif
 
 #if MICROPY_PY_GC
diff --git a/qemu-arm/main.c b/qemu-arm/main.c
index 7bb8472541..1189ae7b8b 100644
--- a/qemu-arm/main.c
+++ b/qemu-arm/main.c
@@ -70,10 +70,10 @@ mp_import_stat_t mp_import_stat(const char *path) {
     return MP_IMPORT_STAT_NO_EXIST;
 }
 
-mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) {
+mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) {
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open);
+MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open);
 
 void nlr_jump_fail(void *val) {
 }
diff --git a/qemu-arm/test_main.c b/qemu-arm/test_main.c
index 099cd94ba1..fb95130772 100644
--- a/qemu-arm/test_main.c
+++ b/qemu-arm/test_main.c
@@ -78,10 +78,10 @@ mp_import_stat_t mp_import_stat(const char *path) {
     return MP_IMPORT_STAT_NO_EXIST;
 }
 
-mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) {
+mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) {
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open);
+MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open);
 
 void nlr_jump_fail(void *val) {
 }
diff --git a/stmhal/file.c b/stmhal/file.c
index d49a8a259b..16cca79370 100644
--- a/stmhal/file.c
+++ b/stmhal/file.c
@@ -160,49 +160,51 @@ mp_obj_t file_obj_tell(mp_obj_t self_in) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(file_obj_tell_obj, file_obj_tell);
 
-STATIC mp_obj_t file_obj_make_new(mp_obj_t type, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) {
-    mp_arg_check_num(n_args, n_kw, 1, 2, false);
-
-    const char *fname = mp_obj_str_get_str(args[0]);
+// Note: encoding is ignored for now; it's also not a valid kwarg for CPython's FileIO,
+// but by adding it here we can use one single mp_arg_t array for open() and FileIO's constructor
+STATIC const mp_arg_t file_open_args[] = {
+    { MP_QSTR_file, MP_ARG_OBJ | MP_ARG_REQUIRED, {.u_obj = mp_const_none} },
+    { MP_QSTR_mode, MP_ARG_OBJ, {.u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_r)} },
+    { MP_QSTR_encoding, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} },
+};
+#define FILE_OPEN_NUM_ARGS MP_ARRAY_SIZE(file_open_args)
 
+STATIC mp_obj_t file_open(mp_obj_t type, mp_arg_val_t *args) {
     int mode = 0;
-    if (n_args == 1) {
-        mode = FA_READ;
-    } else {
-        const char *mode_s = mp_obj_str_get_str(args[1]);
-        // TODO make sure only one of r, w, x, a, and b, t are specified
-        while (*mode_s) {
-            switch (*mode_s++) {
-                case 'r':
-                    mode |= FA_READ;
-                    break;
-                case 'w':
-                    mode |= FA_WRITE | FA_CREATE_ALWAYS;
-                    break;
-                case 'x':
-                    mode |= FA_WRITE | FA_CREATE_NEW;
-                    break;
-                case 'a':
-                    mode |= FA_WRITE | FA_OPEN_ALWAYS;
-                    break;
-                case '+':
-                    mode |= FA_READ | FA_WRITE;
-                    break;
-                #if MICROPY_PY_IO_FILEIO
-                case 'b':
-                    type = (mp_obj_t)&mp_type_fileio;
-                    break;
-                #endif
-                case 't':
-                    type = (mp_obj_t)&mp_type_textio;
-                    break;
-            }
+    const char *mode_s = mp_obj_str_get_str(args[1].u_obj);
+    // TODO make sure only one of r, w, x, a, and b, t are specified
+    while (*mode_s) {
+        switch (*mode_s++) {
+            case 'r':
+                mode |= FA_READ;
+                break;
+            case 'w':
+                mode |= FA_WRITE | FA_CREATE_ALWAYS;
+                break;
+            case 'x':
+                mode |= FA_WRITE | FA_CREATE_NEW;
+                break;
+            case 'a':
+                mode |= FA_WRITE | FA_OPEN_ALWAYS;
+                break;
+            case '+':
+                mode |= FA_READ | FA_WRITE;
+                break;
+            #if MICROPY_PY_IO_FILEIO
+            case 'b':
+                type = (mp_obj_t)&mp_type_fileio;
+                break;
+            #endif
+            case 't':
+                type = (mp_obj_t)&mp_type_textio;
+                break;
         }
     }
 
     pyb_file_obj_t *o = m_new_obj_with_finaliser(pyb_file_obj_t);
     o->base.type = type;
 
+    const char *fname = mp_obj_str_get_str(args[0].u_obj);
     FRESULT res = f_open(&o->fp, fname, mode);
     if (res != FR_OK) {
         m_del_obj(pyb_file_obj_t, o);
@@ -217,6 +219,12 @@ STATIC mp_obj_t file_obj_make_new(mp_obj_t type, mp_uint_t n_args, mp_uint_t n_k
     return o;
 }
 
+STATIC mp_obj_t file_obj_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) {
+    mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS];
+    mp_arg_parse_all_kw_array(n_args, n_kw, args, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals);
+    return file_open(type_in, arg_vals);
+}
+
 // TODO gc hook to close the file if not already closed
 
 STATIC const mp_map_elem_t rawfile_locals_dict_table[] = {
@@ -273,9 +281,10 @@ const mp_obj_type_t mp_type_textio = {
 };
 
 // Factory function for I/O stream classes
-STATIC mp_obj_t pyb_io_open(mp_uint_t n_args, const mp_obj_t *args) {
-    // TODO: analyze mode and buffering args and instantiate appropriate type
-    return file_obj_make_new((mp_obj_t)&mp_type_textio, n_args, 0, args);
+mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs) {
+    // TODO: analyze buffering args and instantiate appropriate type
+    mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS];
+    mp_arg_parse_all(n_args, args, kwargs, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals);
+    return file_open((mp_obj_t)&mp_type_textio, arg_vals);
 }
-
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, pyb_io_open);
+MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open);
diff --git a/unix/file.c b/unix/file.c
index bb7ed247f0..21b2726848 100644
--- a/unix/file.c
+++ b/unix/file.c
@@ -118,16 +118,19 @@ STATIC mp_obj_t fdfile_fileno(mp_obj_t self_in) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(fdfile_fileno_obj, fdfile_fileno);
 
-STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) {
+// Note: encoding is ignored for now; it's also not a valid kwarg for CPython's FileIO,
+// but by adding it here we can use one single mp_arg_t array for open() and FileIO's constructor
+STATIC const mp_arg_t file_open_args[] = {
+    { MP_QSTR_file, MP_ARG_OBJ | MP_ARG_REQUIRED, {.u_obj = mp_const_none} },
+    { MP_QSTR_mode, MP_ARG_OBJ, {.u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_r)} },
+    { MP_QSTR_encoding, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} },
+};
+#define FILE_OPEN_NUM_ARGS MP_ARRAY_SIZE(file_open_args)
+
+STATIC mp_obj_t fdfile_open(mp_obj_t type_in, mp_arg_val_t *args) {
     mp_obj_fdfile_t *o = m_new_obj(mp_obj_fdfile_t);
     mp_const_obj_t type = type_in;
-
-    const char *mode_s;
-    if (n_args > 1) {
-        mode_s = mp_obj_str_get_str(args[1]);
-    } else {
-        mode_s = "r";
-    }
+    const char *mode_s = mp_obj_str_get_str(args[1].u_obj);
 
     int mode = 0;
     while (*mode_s) {
@@ -159,12 +162,14 @@ STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_
 
     o->base.type = type;
 
-    if (MP_OBJ_IS_SMALL_INT(args[0])) {
-        o->fd = MP_OBJ_SMALL_INT_VALUE(args[0]);
+    mp_obj_t fid = args[0].u_obj;
+
+    if (MP_OBJ_IS_SMALL_INT(fid)) {
+        o->fd = MP_OBJ_SMALL_INT_VALUE(fid);
         return o;
     }
 
-    const char *fname = mp_obj_str_get_str(args[0]);
+    const char *fname = mp_obj_str_get_str(fid);
     int fd = open(fname, mode, 0644);
     if (fd == -1) {
         nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(errno)));
@@ -173,6 +178,12 @@ STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_
     return o;
 }
 
+STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) {
+    mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS];
+    mp_arg_parse_all_kw_array(n_args, n_kw, args, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals);
+    return fdfile_open(type_in, arg_vals);
+}
+
 STATIC const mp_map_elem_t rawfile_locals_dict_table[] = {
     { MP_OBJ_NEW_QSTR(MP_QSTR_fileno), (mp_obj_t)&fdfile_fileno_obj },
     { MP_OBJ_NEW_QSTR(MP_QSTR_read), (mp_obj_t)&mp_stream_read_obj },
@@ -225,11 +236,13 @@ const mp_obj_type_t mp_type_textio = {
 };
 
 // Factory function for I/O stream classes
-mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args) {
-    // TODO: analyze mode and buffering args and instantiate appropriate type
-    return fdfile_make_new((mp_obj_t)&mp_type_textio, n_args, 0, args);
+mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs) {
+    // TODO: analyze buffering args and instantiate appropriate type
+    mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS];
+    mp_arg_parse_all(n_args, args, kwargs, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals);
+    return fdfile_open((mp_obj_t)&mp_type_textio, arg_vals);
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open);
+MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open);
 
 const mp_obj_fdfile_t mp_sys_stdin_obj  = { .base = {&mp_type_textio}, .fd = STDIN_FILENO };
 const mp_obj_fdfile_t mp_sys_stdout_obj = { .base = {&mp_type_textio}, .fd = STDOUT_FILENO };
diff --git a/unix/modsocket.c b/unix/modsocket.c
index 7f9184db64..82d833653f 100644
--- a/unix/modsocket.c
+++ b/unix/modsocket.c
@@ -257,7 +257,9 @@ STATIC mp_obj_t socket_makefile(mp_uint_t n_args, const mp_obj_t *args) {
     mp_obj_t *new_args = alloca(n_args * sizeof(mp_obj_t));
     memcpy(new_args + 1, args + 1, (n_args - 1) * sizeof(mp_obj_t));
     new_args[0] = MP_OBJ_NEW_SMALL_INT(self->fd);
-    return mp_builtin_open(n_args, new_args);
+    mp_map_t kwargs;
+    mp_map_init(&kwargs, 0);
+    return mp_builtin_open(n_args, new_args, &kwargs);
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(socket_makefile_obj, 1, 3, socket_makefile);