fix(server): Add deprecated/removed lua table functions (#901)

* fix(server): Add deprecated/removed lua table functions

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>

* fix(server): Add table.foreachi type check

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>

---------

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>

src/core/interpreter.cc

@@ -11,6 +11,8 @@
 #include <cstring>
 #include <optional>
 
+#include "core/interpreter_polyfill.h"
+
 extern "C" {
 #include <lauxlib.h>
 #include <lua.h>
@@ -272,12 +274,8 @@ debug = nil
   lua_pushnil(lua);
   lua_setglobal(lua, "dofile");
 
-  // unpack was a global function until Lua 5.2, but was moved into the table module.
-  // Register it globally to maintain compatibility.
-  lua_getglobal(lua, "table");
-  lua_getfield(lua, -1, "unpack");
-  lua_remove(lua, -2);
-  lua_setglobal(lua, "unpack");
+  // Register deprecated or removed functions to maintain compatibility with 5.1
+  register_polyfills(lua);
 }
 
 // dest must have at least 41 chars.

src/core/interpreter_polyfill.h

@@ -0,0 +1,91 @@
+// Copyright 2022, DragonflyDB authors.  All rights reserved.
+// See LICENSE for licensing terms.
+//
+// This header contains implementations of deprecated, removed or renamed lua functions.
+
+#pragma once
+
+extern "C" {
+#include <lauxlib.h>
+#include <lua.h>
+#include <lualib.h>
+
+// TODO: Fix checktab
+#define aux_getn(L, n, w) (luaL_len(L, n))
+
+LUA_API void lua_len(lua_State* L, int idx);
+
+static int polyfill_table_getn(lua_State* L) {
+  lua_len(L, 1);
+  return 1;
+}
+
+static int polyfill_table_setn(lua_State* L) {
+  // From Lua 5.1, ltablib.c
+  luaL_checktype(L, 1, LUA_TTABLE);
+  luaL_error(L, "setn is obsolete");
+  lua_pushvalue(L, 1);
+  return 1;
+}
+
+static int polyfill_table_foreach(lua_State* L) {
+  // From Lua 5.1, ltablib.c
+  luaL_checktype(L, 1, LUA_TTABLE);
+  luaL_checktype(L, 2, LUA_TFUNCTION);
+  lua_pushnil(L); /* first key */
+  while (lua_next(L, 1)) {
+    lua_pushvalue(L, 2);  /* function */
+    lua_pushvalue(L, -3); /* key */
+    lua_pushvalue(L, -3); /* value */
+    lua_call(L, 2, 1);
+    if (!lua_isnil(L, -1))
+      return 1;
+    lua_pop(L, 2); /* remove value and result */
+  }
+  return 0;
+}
+
+static int polyfill_table_foreachi(lua_State* L) {
+  luaL_checktype(L, 1, LUA_TTABLE);  // Check type here because aux_getn is stripped
+  // From Lua 5.1, ltablib.c
+  int i;
+  int n = aux_getn(L, 1, 0b11);
+  luaL_checktype(L, 2, LUA_TFUNCTION);
+  for (i = 1; i <= n; i++) {
+    lua_pushvalue(L, 2);   /* function */
+    lua_pushinteger(L, i); /* 1st argument */
+    lua_rawgeti(L, 1, i);  /* 2nd argument */
+    lua_call(L, 2, 1);
+    if (!lua_isnil(L, -1))
+      return 1;
+    lua_pop(L, 1); /* remove nil result */
+  }
+  return 0;
+}
+
+static void register_polyfills(lua_State* lua) {
+  lua_getglobal(lua, "table");
+
+  // unpack was a global function until Lua 5.2
+  lua_getfield(lua, -1, "unpack");
+  lua_setglobal(lua, "unpack");
+
+  // table.getn - removed, length operator # should be used instead
+  lua_pushcfunction(lua, polyfill_table_getn);
+  lua_setfield(lua, -2, "getn");
+
+  // table.setn - removed, freely resizing a table is no longer possible
+  lua_pushcfunction(lua, polyfill_table_setn);
+  lua_setfield(lua, -2, "setn");
+
+  // table.getn - removed, instead the length operator # should be used
+  lua_pushcfunction(lua, polyfill_table_foreach);
+  lua_setfield(lua, -2, "foreach");
+
+  // table.forachi - removed, use for loops should be used instead
+  lua_pushcfunction(lua, polyfill_table_foreachi);
+  lua_setfield(lua, -2, "foreachi");
+
+  lua_remove(lua, -1);
+}
+}

src/core/interpreter_test.cc

@@ -10,6 +10,7 @@ extern "C" {
 }
 
 #include <absl/strings/str_cat.h>
+#include <absl/strings/str_replace.h>
 #include <gmock/gmock.h>
 
 #include "base/gtest.h"
@@ -321,13 +322,35 @@ TEST_F(InterpreterTest, Modules) {
   EXPECT_EQ("str(\x1\x2test)", ser_.res);
 }
 
-// Since Lua 5.2 global functions were moved to separate namespaces.
-// We need to register them globally to maintain 5.1 compatibility.
-TEST_F(InterpreterTest, OutdatedGlobals) {
-  // table.unpack is used in Laravel:
-  // https://github.com/laravel/framework/blob/6a5c2ec92200cc485983f26b284f7e78470b885f/src/Illuminate/Queue/LuaScripts.php#L118
+// Check compatibility with Lua 5.1
+TEST_F(InterpreterTest, Compatibility) {
+  // unpack is no longer global
   EXPECT_TRUE(Execute("return unpack{1,2,3}"));
   EXPECT_EQ("i(1)", ser_.res);
+
+  string_view test_foreach_template =
+      "local t = {1,'two',3;four='yes'}; local out = {};"
+      "table.{TESTF} (t, function(k, v) table.insert(out, {k, v}) end); "
+      "return out; ";
+
+  // table.foreach was removed
+  string test_foreach = absl::StrReplaceAll(test_foreach_template, {{"{TESTF}", "foreach"}});
+  EXPECT_TRUE(Execute(test_foreach));
+  EXPECT_EQ("[[i(1) i(1)] [i(2) str(two)] [i(3) i(3)] [str(four) str(yes)]]", ser_.res);
+
+  // table.foreachi was removed
+  string test_foreachi = absl::StrReplaceAll(test_foreach_template, {{"{TESTF}", "foreachi"}});
+  EXPECT_TRUE(Execute(test_foreachi));
+  EXPECT_EQ("[[i(1) i(1)] [i(2) str(two)] [i(3) i(3)]]", ser_.res);
+
+  EXPECT_FALSE(Execute("table.foreachi('not-a-table', print);"));  // check invalid args
+
+  // table.getn was replaced with length operator
+  EXPECT_TRUE(Execute("return table.getn{1, 2, 3};"));
+  EXPECT_EQ("i(3)", ser_.res);
+
+  // table.setn was removed, resizing is no longer needed, it thows an error
+  EXPECT_FALSE(Execute("local t = {}; local a = 1; table.setn(t, 100); return a+123;"));
 }
 
 }  // namespace dfly