From cd6f4e9c5868a9ce2729e7017c65fd91c02e34cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matou=C5=A1=20Jan=20Fialka?= Date: Wed, 22 Oct 2025 10:41:26 +0200 Subject: [PATCH] Space Lua: Align `..` (concatenation) operator (#1648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make fallback accept only strings and numbers (as per standard Lua). * Remove `luaToString` from the fallback. It fits well presentation purposes but is too complicated for strict semantic checking in the evaluator and also wrongly coersed non-string and non-number value. It may also return a promise but we require synchronous error path. * Concatenating `nil` now throws "attempt to concatenate nil value" as per standard Lua. * Other types throw "attempt to concatenate a non-string or non-number" which is simplified message diverting from standard Lua which throws typed error but we do not strictly need that for Space Lua (and can be easily added anytime in the future). Signed-off-by: Matouš Jan Fialka --- client/space_lua/eval.ts | 31 +++-- client/space_lua/language_core_test.lua | 146 ++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 10 deletions(-) diff --git a/client/space_lua/eval.ts b/client/space_lua/eval.ts index 5d2358bc..944234d1 100644 --- a/client/space_lua/eval.ts +++ b/client/space_lua/eval.ts @@ -25,7 +25,6 @@ import { luaSet, type LuaStackFrame, LuaTable, - luaToString, luaTruthy, type LuaType, luaTypeOf, @@ -834,15 +833,27 @@ const operatorsMetaMethods: Record { - const aString = luaToString(a); - const bString = luaToString(b); - - if (aString instanceof Promise || bString instanceof Promise) { - return Promise.all([aString, bString]).then(([a, b]) => a + b); - } else { - return aString + bString; - } + nativeImplementation: (a, b, ctx, sf) => { + // Accepts only strings or numbers (coerced to strings) + const coerce = (v: any): string => { + if (v === null || v === undefined) { + throw new LuaRuntimeError( + "attempt to concatenate a nil value", + sf.withCtx(ctx), + ); + } + if (typeof v === "string") { + return v as string; + } + if (typeof v === "number" || v instanceof Number) { + return String(v instanceof Number ? Number(v) : v); + } + throw new LuaRuntimeError( + "attempt to concatenate a non-string or non-number", + sf.withCtx(ctx), + ); + }; + return coerce(a) + coerce(b); }, }, "==": { diff --git a/client/space_lua/language_core_test.lua b/client/space_lua/language_core_test.lua index f81083ae..0c1983f2 100644 --- a/client/space_lua/language_core_test.lua +++ b/client/space_lua/language_core_test.lua @@ -746,3 +746,149 @@ do assertEqual(rawequal(nan, nan), false) end + +-- Some `..` (concatenation) tests + +-- Strings and numbers +assertEqual("a" .. "b", "ab") +assertEqual(1 .. "b", "1b") +assertEqual("a" .. 2, "a2") +assertEqual(4 .. 2, "42") +assertEqual("123" .. 45, "12345") + +-- Multi-return (only first value is used) +do + local function f() + return "X", "Y" + end + + assertEqual("A" .. f(), "AX") + + local function g() + return 1, 2 + end + + assertEqual(g() .. "X", "1X") +end + +local function expect_concat_nil_error(lhs, rhs) + local ok, err = pcall( + function() + return lhs .. rhs + end + ) + + assertEqual(ok, false, "concat must error on nil") +end + +local function expect_concat_type_error(lhs, rhs) + local ok, err = pcall( + function() + return lhs .. rhs + end + ) + + assertEqual(ok, false, "concat must error on non-string/non-number") +end + +-- Nil on either side +expect_concat_nil_error(nil, "x") +expect_concat_nil_error("x", nil) +expect_concat_nil_error(nil, 1) +expect_concat_nil_error(1, nil) +expect_concat_nil_error(nil, nil) + +-- Non-string and non-number must error +expect_concat_type_error(true, "x") +expect_concat_type_error("x", false) +expect_concat_type_error(true, true) + +do + local t = {} + + expect_concat_type_error(t, "x") + expect_concat_type_error("x", t) + expect_concat_type_error(t, t) +end + +-- JS arrays and JS objects behave as tables (no concat) +do + local arr = js.window.JSON.parse("[1, 2]") + + expect_concat_type_error(arr, "x") + expect_concat_type_error("x", arr) + expect_concat_type_error(arr, arr) + + local obj = js.window.JSON.parse('{ "a": 1 }') + + expect_concat_type_error(obj, "x") + expect_concat_type_error("x", obj) + expect_concat_type_error(obj, obj) +end + +-- `__tostring` alone does not relax concat rules +do + local t = {} + + setmetatable( + t, + { + __tostring = function(_) + return "T" + end + } + ) + + expect_concat_type_error(t, "x") + expect_concat_type_error("x", t) + expect_concat_type_error(t, t) +end + +-- `__concat` metamethod cases +do + local L = {} + + setmetatable( + L, + { + __concat = function(a, b) + return "LEFT" + end + } + ) + + assertEqual(L .. "x", "LEFT") + + local R = {} + + setmetatable( + R, + { + __concat = function(a, b) + return "RIGHT" + end + } + ) + + assertEqual("x" .. R, "RIGHT") + + assertEqual(L .. R, "LEFT", + "left-side __concat must be preferred when both defined") +end + +-- `__concat` result handling (only first value used) +do + local M = {} + + setmetatable( + M, + { + __concat = function(a, b) + return "A", "B" + end + } + ) + + assertEqual(M .. "x", "A", + "__concat must use only first return value") +end