Lua: Clean up pass

Fixing constness, removing unused function, adding LuaEngine::variadicToStringList

Change-Id: If567ac83c04e5ce6f973c819f303c9cb790b3948
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: hjk <hjk@qt.io>

11 files changed, 70 insertions, 108 deletions

diff --git a/src/plugins/lua/bindings/action.cpp b/src/plugins/lua/bindings/action.cpp
index 93ae6f5188..015dd9b068 100644
--- a/src/plugins/lua/bindings/action.cpp
+++ b/src/plugins/lua/bindings/action.cpp
@@ -24,7 +24,7 @@ void addActionModule()
                         "CA_NonConfigurable",
                         Core::Command::CA_NonConfigurable);
 
-        result["create"] = [](const std::string &actionId, sol::table options) {
+        result["create"] = [](const std::string &actionId, const sol::table &options) {
             Core::ActionBuilder b(nullptr, Id::fromString(QString::fromStdString(actionId)));
 
             for (const auto &[k, v] : options) {
diff --git a/src/plugins/lua/bindings/fetch.cpp b/src/plugins/lua/bindings/fetch.cpp
index 8580957350..f8ea05d2d4 100644
--- a/src/plugins/lua/bindings/fetch.cpp
+++ b/src/plugins/lua/bindings/fetch.cpp
@@ -53,7 +53,7 @@ void addFetchModule()
 
         static QNetworkAccessManager networkAccessManager;
 
-        fetch["fetch_cb"] = [](sol::table options, sol::function callback, sol::this_state s) {
+        fetch["fetch_cb"] = [](const sol::table &options, const sol::function &callback, const sol::this_state &thisState) {
             auto url = options.get<QString>("url");
 
             auto method = (options.get_or<QString>("method", "GET")).toLower();
@@ -77,7 +77,7 @@ void addFetchModule()
                 throw std::runtime_error("Unknown method: " + method.toStdString());
 
             if (convertToTable) {
-                QObject::connect(reply, &QNetworkReply::finished, reply, [reply, s, callback]() {
+                QObject::connect(reply, &QNetworkReply::finished, reply, [reply, thisState, callback]() {
                     reply->deleteLater();
 
                     if (reply->error() != QNetworkReply::NoError) {
@@ -93,11 +93,11 @@ void addFetchModule()
                         return;
                     }
                     if (doc.isObject()) {
-                        callback(LuaEngine::toTable(s, doc.object()));
+                        callback(LuaEngine::toTable(thisState, doc.object()));
                     } else if (doc.isArray()) {
-                        callback(LuaEngine::toTable(s, doc.array()));
+                        callback(LuaEngine::toTable(thisState, doc.array()));
                     } else {
-                        sol::state_view lua(s);
+                        sol::state_view lua(thisState);
                         callback(lua.create_table());
                     }
                 });
diff --git a/src/plugins/lua/bindings/hook.cpp b/src/plugins/lua/bindings/hook.cpp
index cb94e54582..261e6fe115 100644
--- a/src/plugins/lua/bindings/hook.cpp
+++ b/src/plugins/lua/bindings/hook.cpp
@@ -34,7 +34,7 @@ void addHookModule()
             "Hook",
             sol::no_constructor,
             "connect",
-            [](Hook *hook, sol::function func) -> QMetaObject::Connection {
+            [](Hook *hook, const sol::function &func) -> QMetaObject::Connection {
                 QMetaObject::Connection con
                     = QObject::connect(hook, &Hook::trigger, [func](sol::table args) {
                           auto res = LuaEngine::void_safe_call(func, args);
@@ -46,12 +46,12 @@ void addHookModule()
             [](Hook *, QMetaObject::Connection con) { QObject::disconnect(con); });
     });
 
-    LuaEngine::registerHook("editors.documentOpened", [](sol::function func) {
+    LuaEngine::registerHook("editors.documentOpened", [](const sol::function &func) {
         QObject::connect(Core::EditorManager::instance(),
                          &Core::EditorManager::documentOpened,
                          [func](Core::IDocument *document) { func(document); });
     });
-    LuaEngine::registerHook("editors.documentClosed", [](sol::function func) {
+    LuaEngine::registerHook("editors.documentClosed", [](const sol::function &func) {
         QObject::connect(Core::EditorManager::instance(),
                          &Core::EditorManager::documentClosed,
                          [func](Core::IDocument *document) { func(document); });
diff --git a/src/plugins/lua/bindings/layout.cpp b/src/plugins/lua/bindings/layout.cpp
index 3d396161c7..6225bce9f3 100644
--- a/src/plugins/lua/bindings/layout.cpp
+++ b/src/plugins/lua/bindings/layout.cpp
@@ -13,10 +13,10 @@ using namespace Utils;
 
 namespace Lua::Internal {
 
-static void processChildren(LayoutItem *item, sol::table children)
+static void processChildren(LayoutItem *item, const sol::table &children)
 {
     for (size_t i = 1; i <= children.size(); ++i) {
-        sol::object v = children[i];
+        const sol::object v = children[i];
 
         if (v.is<LayoutItem *>()) {
             item->addItem(*v.as<LayoutItem *>());
@@ -25,7 +25,7 @@ static void processChildren(LayoutItem *item, sol::table children)
         } else if (v.is<QString>()) {
             item->addItem(v.as<QString>());
         } else if (v.is<sol::function>()) {
-            sol::function f = v.as<sol::function>();
+            const sol::function f = v.as<sol::function>();
             auto res = LuaEngine::safe_call<LayoutItem *>(f);
             QTC_ASSERT_EXPECTED(res, continue);
             item->addItem(**res);
@@ -37,7 +37,7 @@ static void processChildren(LayoutItem *item, sol::table children)
 }
 
 template<class T, typename... Args>
-static std::unique_ptr<T> construct(Args &&...args, sol::table children)
+static std::unique_ptr<T> construct(Args &&...args, const sol::table &children)
 {
     std::unique_ptr<T> item(new T(std::forward<Args>(args)..., {}));
 
@@ -157,13 +157,13 @@ void addLayoutModule()
         layout["fieldGrowthPolicy"] = &fieldGrowthPolicy;
         layout["id"] = &id;
         layout["setText"] = &setText;
-        layout["onClicked"] = [](sol::function f) {
+        layout["onClicked"] = [](const sol::function &f) {
             return onClicked([f]() {
                 auto res = LuaEngine::void_safe_call(f);
                 QTC_CHECK_EXPECTED(res);
             });
         };
-        layout["onTextChanged"] = [](sol::function f) {
+        layout["onTextChanged"] = [](const sol::function &f) {
             return onTextChanged([f](const QString &text) {
                 auto res = LuaEngine::void_safe_call(f, text);
                 QTC_CHECK_EXPECTED(res);
diff --git a/src/plugins/lua/bindings/messagemanager.cpp b/src/plugins/lua/bindings/messagemanager.cpp
index 4e284c0b50..f6b8e874ad 100644
--- a/src/plugins/lua/bindings/messagemanager.cpp
+++ b/src/plugins/lua/bindings/messagemanager.cpp
@@ -7,47 +7,19 @@
 
 namespace Lua::Internal {
 
-static QString variadicToString(sol::state_view lua, sol::variadic_args vargs)
-{
-    sol::function tostring = lua["tostring"];
-    QStringList msg;
-    for (auto v : vargs) {
-        if (v.get_type() != sol::type::string) {
-            lua_getglobal(lua.lua_state(), "tostring");
-            v.push();
-            if (lua_pcall(lua.lua_state(), 1, 1, 0) != LUA_OK) {
-                msg.append("<invalid>");
-                continue;
-            }
-            if (lua_isstring(lua.lua_state(), -1) != 1) {
-                msg.append("<invalid>");
-                continue;
-            }
-            auto str = sol::stack::pop<QString>(lua.lua_state());
-            msg.append(str);
-        } else {
-            msg.append(v.get<QString>());
-        }
-    }
-    return msg.join("");
-}
-
 void addMessageManagerModule()
 {
     LuaEngine::registerProvider("MessageManager", [](sol::state_view lua) -> sol::object {
         sol::table mm = lua.create_table();
 
-        mm.set_function("writeFlashing", [](sol::variadic_args vargs, sol::this_state s) {
-            sol::state_view lua(s);
-            Core::MessageManager::writeFlashing(variadicToString(lua, vargs));
+        mm.set_function("writeFlashing", [](const sol::variadic_args &vargs) {
+            Core::MessageManager::writeFlashing(LuaEngine::variadicToStringList(vargs).join(""));
         });
-        mm.set_function("writeDisrupting", [](sol::variadic_args vargs, sol::this_state s) {
-            sol::state_view lua(s);
-            Core::MessageManager::writeDisrupting(variadicToString(lua, vargs));
+        mm.set_function("writeDisrupting", [](const sol::variadic_args &vargs) {
+            Core::MessageManager::writeDisrupting(LuaEngine::variadicToStringList(vargs).join(""));
         });
-        mm.set_function("writeSilently", [](sol::variadic_args vargs, sol::this_state s) {
-            sol::state_view lua(s);
-            Core::MessageManager::writeSilently(variadicToString(lua, vargs));
+        mm.set_function("writeSilently", [](const sol::variadic_args &vargs) {
+            Core::MessageManager::writeSilently(LuaEngine::variadicToStringList(vargs).join(""));
         });
 
         return mm;
diff --git a/src/plugins/lua/bindings/qtcprocess.cpp b/src/plugins/lua/bindings/qtcprocess.cpp
index f40f22f415..3c6c7eef7e 100644
--- a/src/plugins/lua/bindings/qtcprocess.cpp
+++ b/src/plugins/lua/bindings/qtcprocess.cpp
@@ -15,7 +15,7 @@ void addProcessModule()
     LuaEngine::registerProvider("__process", [](sol::state_view lua) -> sol::object {
         sol::table process = lua.create_table();
 
-        process["runInTerminal_cb"] = [](const QString &cmdline, sol::function cb) {
+        process["runInTerminal_cb"] = [](const QString &cmdline, const sol::function &cb) {
             Process *p = new Process;
             p->setTerminalMode(TerminalMode::Run);
             p->setCommand(CommandLine::fromUserInput((cmdline)));
diff --git a/src/plugins/lua/bindings/settings.cpp b/src/plugins/lua/bindings/settings.cpp
index 9097987840..b1c388605f 100644
--- a/src/plugins/lua/bindings/settings.cpp
+++ b/src/plugins/lua/bindings/settings.cpp
@@ -27,7 +27,7 @@ public:
         return it->second;
     }
 
-    void dynamic_set(const std::string &key, sol::stack_object value)
+    void dynamic_set(const std::string &key, const sol::stack_object &value)
     {
         if (!value.is<BaseAspect>())
             throw std::runtime_error("AspectContainer can only contain BaseAspect instances");
@@ -50,7 +50,7 @@ public:
     std::unordered_map<std::string, sol::object> m_entries;
 };
 
-std::unique_ptr<LuaAspectContainer> aspectContainerCreate(sol::table options)
+std::unique_ptr<LuaAspectContainer> aspectContainerCreate(const sol::table &options)
 {
     auto container = std::make_unique<LuaAspectContainer>();
 
@@ -224,7 +224,7 @@ void typedAspectCreate(BoolAspect *aspect, const std::string &key, const sol::ob
 
 template<class T>
 std::unique_ptr<T> createAspectFromTable(
-    sol::table options, const std::function<void(T *, const std::string &, sol::object)> &f)
+    const sol::table &options, const std::function<void(T *, const std::string &, sol::object)> &f)
 {
     auto aspect = std::make_unique<T>();
 
@@ -265,7 +265,9 @@ sol::usertype<T> addTypedAspect(sol::table &lua, const QString &name)
     return lua.new_usertype<T>(
         name,
         "create",
-        [](sol::table options) { return createAspectFromTable<T>(options, &typedAspectCreate<T>); },
+        [](const sol::table &options) {
+            return createAspectFromTable<T>(options, &typedAspectCreate<T>);
+        },
         sol::base_classes,
         sol::bases<TypedAspect<typename T::valueType>, BaseAspect>());
 }
@@ -315,7 +317,7 @@ void addSettingsModule()
         settings.new_usertype<ToggleAspect>(
             "ToggleAspect",
             "create",
-            [](sol::table options) {
+            [](const sol::table &options) {
                 return createAspectFromTable<ToggleAspect>(
                     options,
                     [](ToggleAspect *aspect, const std::string &key, const sol::object &value) {
@@ -363,7 +365,7 @@ void addSettingsModule()
         settings.new_usertype<TriStateAspect>(
             "TriStateAspect",
             "create",
-            [](sol::table options) {
+            [](const sol::table &options) {
                 return createAspectFromTable<TriStateAspect>(
                     options,
                     [](TriStateAspect *aspect, const std::string &key, const sol::object &value) {
@@ -376,10 +378,9 @@ void addSettingsModule()
                     });
             },
             "value",
-            sol::property([](TriStateAspect *a) { return triStateToString(a->value()); },
-                          [](TriStateAspect *a, const QString &v) {
-                              a->setValue(triStateFromString(v));
-                          }),
+            sol::property(
+                [](TriStateAspect *a) { return triStateToString(a->value()); },
+                [](TriStateAspect *a, const QString &v) { a->setValue(triStateFromString(v)); }),
             "volatileValue",
             sol::property(
                 [](TriStateAspect *a) {
@@ -396,7 +397,7 @@ void addSettingsModule()
         settings.new_usertype<TextDisplay>(
             "TextDisplay",
             "create",
-            [](sol::table options) {
+            [](const sol::table &options) {
                 return createAspectFromTable<TextDisplay>(
                     options,
                     [](TextDisplay *aspect, const std::string &key, const sol::object &value) {
@@ -430,7 +431,7 @@ void addSettingsModule()
         settings.new_usertype<AspectList>(
             "AspectList",
             "create",
-            [](sol::table options) {
+            [](const sol::table &options) {
                 return createAspectFromTable<AspectList>(
                     options,
                     [](AspectList *aspect, const std::string &key, const sol::object &value) {
@@ -462,14 +463,14 @@ void addSettingsModule()
             "createAndAddItem",
             &AspectList::createAndAddItem,
             "foreach",
-            [](AspectList *a, sol::function clbk) {
+            [](AspectList *a, const sol::function &clbk) {
                 a->forEachItem<BaseAspect>([clbk](std::shared_ptr<BaseAspect> item) {
                     auto res = Lua::LuaEngine::void_safe_call(clbk, item);
                     QTC_CHECK_EXPECTED(res);
                 });
             },
             "enumerate",
-            [](AspectList *a, sol::function clbk) {
+            [](AspectList *a, const sol::function &clbk) {
                 a->forEachItem<BaseAspect>([clbk](std::shared_ptr<BaseAspect> item, int idx) {
                     auto res = Lua::LuaEngine::void_safe_call(clbk, item, idx);
                     QTC_CHECK_EXPECTED(res);
@@ -494,7 +495,7 @@ void addSettingsModule()
             }
         };
 
-        settings.new_usertype<OptionsPage>("OptionsPage", "create", [](sol::table options) {
+        settings.new_usertype<OptionsPage>("OptionsPage", "create", [](const sol::table &options) {
             return std::make_unique<OptionsPage>(options);
         });
 
diff --git a/src/plugins/lua/bindings/utils.cpp b/src/plugins/lua/bindings/utils.cpp
index 8609beb4f2..370fb53fed 100644
--- a/src/plugins/lua/bindings/utils.cpp
+++ b/src/plugins/lua/bindings/utils.cpp
@@ -17,7 +17,7 @@ void addUtilsModule()
     LuaEngine::registerProvider("__utils", [](sol::state_view lua) -> sol::object {
         sol::table utils = lua.create_table();
 
-        utils.set_function("waitms_cb", [](int ms, sol::function cb) {
+        utils.set_function("waitms_cb", [](int ms, const sol::function &cb) {
             QTimer *timer = new QTimer();
             timer->setSingleShot(true);
             timer->setInterval(ms);
@@ -73,7 +73,7 @@ return {
             "isExecutableFile",
             &FilePath::isExecutableFile,
             "dirEntries",
-            [](sol::this_state s, const FilePath &p, sol::table options) -> sol::table {
+            [](sol::this_state s, const FilePath &p, const sol::table &options) -> sol::table {
                 sol::state_view lua(s);
                 sol::table result = lua.create_table();
                 const QStringList nameFilters = options.get_or<QStringList>("nameFilters", {});
diff --git a/src/plugins/lua/luaengine.cpp b/src/plugins/lua/luaengine.cpp
index b9c67c9956..38aac217da 100644
--- a/src/plugins/lua/luaengine.cpp
+++ b/src/plugins/lua/luaengine.cpp
@@ -43,17 +43,18 @@ void LuaEngine::registerProvider(const QString &packageName, const PackageProvid
     instance().d->m_providers[packageName] = provider;
 }
 
-void LuaEngine::autoRegister(std::function<void(sol::state_view)> registerFunction)
+void LuaEngine::autoRegister(const std::function<void(sol::state_view)> &registerFunction)
 {
     instance().d->m_autoProviders.append(registerFunction);
 }
 
-void LuaEngine::registerHook(QString name, std::function<void(sol::function)> hook)
+void LuaEngine::registerHook(QString name, const std::function<void(sol::function)> &hook)
 {
     instance().d->m_hooks.insert("." + name, hook);
 }
 
-expected_str<void> LuaEngine::connectHooks(sol::state_view lua, const sol::table &table, QString path)
+expected_str<void> LuaEngine::connectHooks(
+    sol::state_view lua, const sol::table &table, const QString &path)
 {
     for (const auto &[k, v] : table) {
         if (v.get_type() == sol::type::table) {
@@ -100,21 +101,11 @@ expected_str<LuaPluginSpec *> LuaEngine::loadPlugin(const Utils::FilePath &path)
                        sol::lib::io);
 
     lua["print"] = [prefix = path.fileName()](sol::variadic_args va) {
-        QStringList strings;
-        int n = va.size();
-        int i;
-        for (i = 1; i <= n; i++) {
-            size_t l;
-            const char *s = luaL_tolstring(va.lua_state(), i, &l);
-            if (s != nullptr)
-                strings.append(QString::fromUtf8(s, l));
-        }
-
-        qDebug().noquote() << "[" << prefix << "]" << strings.join("\t");
+        qDebug().noquote() << "[" << prefix << "]" << variadicToStringList(va).join("\t");
     };
 
     for (const auto &[name, func] : d->m_providers.asKeyValueRange()) {
-        lua["package"]["preload"][name.toStdString()] = [func = func](sol::this_state s) {
+        lua["package"]["preload"][name.toStdString()] = [func = func](const sol::this_state &s) {
             return func(s);
         };
     }
@@ -165,7 +156,7 @@ static void setFromJson(sol::table &t, KeyType k, const QJsonValue &v)
         t[k] = LuaEngine::toTable(t.lua_state(), v);
 }
 
-sol::table LuaEngine::toTable(sol::state_view lua, const QJsonValue &v)
+sol::table LuaEngine::toTable(const sol::state_view &lua, const QJsonValue &v)
 {
     sol::table table(lua, sol::create);
 
@@ -186,9 +177,9 @@ sol::table LuaEngine::toTable(sol::state_view lua, const QJsonValue &v)
     return table;
 }
 
-QJsonValue toJsonValue(sol::object object);
+QJsonValue toJsonValue(const sol::object &object);
 
-QJsonValue toJsonValue(sol::table table)
+QJsonValue toJsonValue(const sol::table &table)
 {
     if (table.get<std::optional<sol::object>>(1)) {
         // Is Array
@@ -212,7 +203,7 @@ QJsonValue toJsonValue(sol::table table)
     return obj;
 }
 
-QJsonValue toJsonValue(sol::object object)
+QJsonValue toJsonValue(const sol::object &object)
 {
     switch (object.get_type()) {
     case sol::type::lua_nil:
@@ -235,15 +226,19 @@ QJsonValue LuaEngine::toJson(const sol::table &table)
     return toJsonValue(table);
 }
 
-expected_str<int> LuaEngine::resumeImpl(sol::this_state s, int nArgs)
+QStringList LuaEngine::variadicToStringList(const sol::variadic_args &vargs)
 {
-    int res;
-    auto success = lua_resume(s.lua_state(), nullptr, nArgs, &res);
-
-    if (success == LUA_OK || success == LUA_YIELD)
-        return res;
+    QStringList strings;
+    int n = vargs.size();
+    int i;
+    for (i = 1; i <= n; i++) {
+        size_t l;
+        const char *s = luaL_tolstring(vargs.lua_state(), i, &l);
+        if (s != nullptr)
+            strings.append(QString::fromUtf8(s, l));
+    }
 
-    return make_unexpected((sol::stack::pop<QString>(s.lua_state())));
+    return strings;
 }
 
 } // namespace Lua
diff --git a/src/plugins/lua/luaengine.h b/src/plugins/lua/luaengine.h
index 3246c7de19..0ea78e0211 100644
--- a/src/plugins/lua/luaengine.h
+++ b/src/plugins/lua/luaengine.h
@@ -43,27 +43,20 @@ public:
     Utils::expected_str<LuaPluginSpec *> loadPlugin(const Utils::FilePath &path);
 
     static void registerProvider(const QString &packageName, const PackageProvider &provider);
-    static void autoRegister(std::function<void(sol::state_view)> registerFunction);
-    static void registerHook(QString name, std::function<void(sol::function)> hookProvider);
+    static void autoRegister(const std::function<void(sol::state_view)> &registerFunction);
+    static void registerHook(QString name, const std::function<void(sol::function)> &hookProvider);
 
     static Utils::expected_str<void> connectHooks(sol::state_view lua, const sol::table &hookTable);
 
     static bool isCoroutine(lua_State *state);
 
-    static sol::table toTable(sol::state_view lua, const QJsonValue &v);
+    static sol::table toTable(const sol::state_view &lua, const QJsonValue &v);
     static QJsonValue toJson(const sol::table &t);
 
-    static Utils::expected_str<int> resumeImpl(sol::this_state s, int nargs);
-
-    template<typename... Args>
-    static Utils::expected_str<int> resume(sol::this_state s, Args &&...args)
-    {
-        sol::stack::push(s, std::forward<Args>(args)...);
-        return resumeImpl(s, sizeof...(Args));
-    }
+    static QStringList variadicToStringList(const sol::variadic_args &vargs);
 
     template<typename R, typename... Args>
-    static Utils::expected_str<R> safe_call(sol::protected_function function, Args &&...args)
+    static Utils::expected_str<R> safe_call(const sol::protected_function &function, Args &&...args)
     {
         sol::protected_function_result result = function(std::forward<Args>(args)...);
         if (!result.valid()) {
@@ -78,7 +71,8 @@ public:
     }
 
     template<typename... Args>
-    static Utils::expected_str<void> void_safe_call(sol::protected_function function, Args &&...args)
+    static Utils::expected_str<void> void_safe_call(
+        const sol::protected_function &function, Args &&...args)
     {
         sol::protected_function_result result = function(std::forward<Args>(args)...);
         if (!result.valid()) {
@@ -90,7 +84,7 @@ public:
 
 protected:
     Utils::expected_str<void> connectHooks(
-        sol::state_view lua, const sol::table &table, QString path);
+        sol::state_view lua, const sol::table &table, const QString &path);
 
 private:
     std::unique_ptr<LuaEnginePrivate> d;
diff --git a/src/plugins/lua/luauibindings.cpp b/src/plugins/lua/luauibindings.cpp
index 5649a40dfc..3c99085bda 100644
--- a/src/plugins/lua/luauibindings.cpp
+++ b/src/plugins/lua/luauibindings.cpp
@@ -12,7 +12,7 @@
 namespace Lua::Internal {
 
 template<class T = QObject>
-sol::object qobject_index_get(sol::this_state s, QObject *obj, const char *key)
+sol::object qobject_index_get(const sol::this_state &s, QObject *obj, const char *key)
 {
     auto &metaObject = T::staticMetaObject;
     int iProp = metaObject.indexOfProperty(key);