Make symlink/copyfile bindings more user-friendly (#532)

* fs_symlink: Allow flags to be a number This is how most flag-accepting function bindings work. * Extract out argument type errors to a helper function (luv_arg_type_error) * fs: Make symlink/copyfile bindings more user-friendly - Callback for the async version can be given as the 3rd parameter if the flags argument is omitted - Flags can be specified as either a table, an integer, or nil, and will throw an error for unexpected types Closes #529
luvit · Feb 22, 2021 · 26d6636 · 26d6636
1 parent 9359dbf
commit 26d6636
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 33 deletions.
diff --git a/docs.md b/docs.md
@@ -2762,19 +2762,19 @@ Equivalent to `link(2)`.
 
 **Returns (async version):** `uv_fs_t userdata`
 
-### `uv.fs_symlink(path, new_path, flags, [callback])`
+### `uv.fs_symlink(path, new_path, [flags], [callback])`
 
 **Parameters:**
 - `path`: `string`
 - `new_path`: `string`
-- `flags`: `table` or `nil`
+- `flags`: `table`, `integer`, or `nil`
   - `dir`: `boolean`
   - `junction`: `boolean`
 - `callback`: `callable` (async version) or `nil` (sync version)
   - `err`: `nil` or `string`
   - `success`: `boolean` or `nil`
 
-Equivalent to `symlink(2)`.
+Equivalent to `symlink(2)`. If the `flags` parameter is omitted, then the 3rd parameter will be treated as the `callback`.
 
 **Returns (sync version):** `boolean` or `fail`
 
@@ -2856,20 +2856,20 @@ Equivalent to `lchown(2)`.
 
 **Returns (async version):** `uv_fs_t userdata`
 
-### `uv.fs_copyfile(path, new_path, flags, [callback])`
+### `uv.fs_copyfile(path, new_path, [flags], [callback])`
 
 **Parameters:**
 - `path`: `string`
 - `new_path`: `string`
-- `flags`: `table` or `nil`
+- `flags`: `table`, `integer`, or `nil`
   - `excl`: `boolean`
   - `ficlone`: `boolean`
   - `ficlone_force`: `boolean`
 - `callback`: `callable` (async version) or `nil` (sync version)
   - `err`: `nil` or `string`
   - `success`: `boolean` or `nil`
 
-Copies a file from path to new_path.
+Copies a file from path to new_path. If the `flags` parameter is omitted, then the 3rd parameter will be treated as the `callback`.
 
 **Returns (sync version):** `boolean` or `fail`
 

diff --git a/src/fs.c b/src/fs.c
@@ -763,15 +763,26 @@ static int luv_fs_symlink(lua_State* L) {
   const char* new_path = luaL_checkstring(L, 2);
   int flags = 0, ref;
   uv_fs_t* req;
-  if (lua_type(L, 3) == LUA_TTABLE) {
-    lua_getfield(L, 3, "dir");
-    if (lua_toboolean(L, -1)) flags |= UV_FS_SYMLINK_DIR;
-    lua_pop(L, 1);
-    lua_getfield(L, 3, "junction");
-    if (lua_toboolean(L, -1)) flags |= UV_FS_SYMLINK_JUNCTION;
-    lua_pop(L, 1);
-  }
-  ref = luv_check_continuation(L, 4);
+  // callback can be the 3rd parameter
+  if (luv_is_callable(L, 3) && lua_isnone(L, 4)) {
+    ref = luv_check_continuation(L, 3);
+  } else {
+    if (lua_type(L, 3) == LUA_TTABLE) {
+      lua_getfield(L, 3, "dir");
+      if (lua_toboolean(L, -1)) flags |= UV_FS_SYMLINK_DIR;
+      lua_pop(L, 1);
+      lua_getfield(L, 3, "junction");
+      if (lua_toboolean(L, -1)) flags |= UV_FS_SYMLINK_JUNCTION;
+      lua_pop(L, 1);
+    }
+    else if (lua_type(L, 3) == LUA_TNUMBER) {
+      flags = lua_tointeger(L, 3);
+    }
+    else if (!lua_isnoneornil(L, 3)) {
+      return luv_arg_type_error(L, 3, "table, integer, or nil expected, got %s");
+    }
+    ref = luv_check_continuation(L, 4);
+  }
   req = (uv_fs_t*)lua_newuserdata(L, sizeof(*req));
   req->data = luv_setup_req(L, ctx, ref);
   // ref the dest path so that we can print it in the error message
@@ -842,23 +853,31 @@ static int luv_fs_copyfile(lua_State*L) {
   const char* new_path = luaL_checkstring(L, 2);
   int flags = 0, ref;
   uv_fs_t* req;
-  if (lua_type(L, 3) == LUA_TTABLE) {
-    lua_getfield(L, 3, "excl");
-    if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_EXCL;
-    lua_pop(L, 1);
+  // callback can be the 3rd parameter
+  if (luv_is_callable(L, 3) && lua_isnone(L, 4)) {
+    ref = luv_check_continuation(L, 3);
+  } else {
+    if (lua_type(L, 3) == LUA_TTABLE) {
+      lua_getfield(L, 3, "excl");
+      if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_EXCL;
+      lua_pop(L, 1);
 #if LUV_UV_VERSION_GEQ(1, 20, 0)
-    lua_getfield(L, 3, "ficlone");
-    if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_FICLONE;
-    lua_pop(L, 1);
-    lua_getfield(L, 3, "ficlone_force");
-    if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_FICLONE_FORCE;
-    lua_pop(L, 1);
+      lua_getfield(L, 3, "ficlone");
+      if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_FICLONE;
+      lua_pop(L, 1);
+      lua_getfield(L, 3, "ficlone_force");
+      if (lua_toboolean(L, -1)) flags |= UV_FS_COPYFILE_FICLONE_FORCE;
+      lua_pop(L, 1);
 #endif
+    }
+    else if (lua_type(L, 3) == LUA_TNUMBER) {
+      flags = lua_tointeger(L, 3);
+    }
+    else if (!lua_isnoneornil(L, 3)) {
+      return luv_arg_type_error(L, 3, "table, integer, or nil expected, got %s");
+    }
+    ref = luv_check_continuation(L, 4);
   }
-  else if (lua_type(L, 3) == LUA_TNUMBER) {
-    flags = lua_tointeger(L, 3);
-  }
-  ref = luv_check_continuation(L, 4);
   req = (uv_fs_t*)lua_newuserdata(L, sizeof(*req));
   req->data = luv_setup_req(L, ctx, ref);
   // ref the dest path so that we can print it in the error message

diff --git a/src/private.h b/src/private.h
@@ -101,6 +101,10 @@ static int luv_is_callable(lua_State* L, int index);
 // Check if the argument is callable and throw an error if it's not
 static void luv_check_callable(lua_State* L, int index);
 
+// Throw an argument error formatted with the type name of the value at the argument's index
+// Example: luv_arg_type_error(L, 1, "expected number or table, got %s");
+static int luv_arg_type_error(lua_State* L, int index, const char* fmt);
+
 static int luv_optboolean(lua_State*L, int idx, int defaultval);
 
 /* From thread.c */

diff --git a/src/util.c b/src/util.c
@@ -73,19 +73,23 @@ static int luv_is_callable(lua_State* L, int index) {
 }
 
 static void luv_check_callable(lua_State* L, int index) {
-  const char *msg;
-  const char *typearg;  /* name for the type of the actual argument */
   if (luv_is_callable(L, index))
     return;
+  else
+    luv_arg_type_error(L, index, "function or callable table expected, got %s");
+}
 
+static int luv_arg_type_error(lua_State* L, int index, const char* fmt) {
+  const char *msg;
+  const char *typearg;  /* name for the type of the actual argument */
   if (luaL_getmetafield(L, index, "__name") == LUA_TSTRING)
     typearg = lua_tostring(L, -1);  /* use the given type name */
   else if (lua_type(L, index) == LUA_TLIGHTUSERDATA)
     typearg = "light userdata";  /* special name for messages */
   else
     typearg = luaL_typename(L, index);  /* standard name */
-  msg = lua_pushfstring(L, "function or callable table expected, got %s", typearg);
-  luaL_argerror(L, index, msg);
+  msg = lua_pushfstring(L, fmt, typearg);
+  return luaL_argerror(L, index, msg);
 }
 
 #if LUV_UV_VERSION_GEQ(1, 10, 0)