fix: Array.isArray incompatible with mw.loadData

[ElectricalBoy]

Summary

Resolves #7433

https://github.com/wikimedia/mediawiki-extensions-Scribunto/blob/293989c/includes/Engines/LuaCommon/lualib/mw.lua#L844-L845

The table returned by mw.loadData has mw_loadData set to true in its metatable. This PR adds an explicit check for mw_loadData, which is followed by changes proposed in #7435.

How did you test this change?

same as #7435

[hjpalpha]

did some perf tests:
https://liquipedia.net/starcraft2/Module:Hjpalpha/sandbox25

• non loadData array —> run2 (non dev): 433, runDev2 (your dev): 539, runDev3 (my dev): 444
• loadData array —> run (non dev, false result): 1240, runDev (your dev): 287, runDev4 (my dev): 1436
• non array (no loaddata) —> run3 (non dev): 435, runDev5 (your dev): 530, runDev6 (my dev): 554
• non array (loaddata) —> run4 (non dev): 696, runDev7 (your dev): 269, runDev8 (my dev): 817

each test did 1000000 checks
(did only run each test once and one after another due to being on mobile, can likely do each 10 times and calc averages when on pc later today)

my dev: https://liquipedia.net/commons/Module:Array/dev/hjp2

so the question is which perf do we value more

• perf for non loadData array
• perf for loaddata array

[hjpalpha]

did some perf tests: https://liquipedia.net/starcraft2/Module:Hjpalpha/sandbox25

• non loadData array —> run2 (non dev): 433, runDev2 (your dev): 539, runDev3 (my dev): 444
• loadData array —> run (non dev, false result): 1240, runDev (your dev): 287, runDev4 (my dev): 1436
• non array (no loaddata) —> run3 (non dev): 435, runDev5 (your dev): 530, runDev6 (my dev): 554
• non array (loaddata) —> run4 (non dev): 696, runDev7 (your dev): 269, runDev8 (my dev): 817

each test did 1000000 checks (did only run each test once and one after another due to being on mobile, can likely do each 10 times and calc averages when on pc later today)

my dev: https://liquipedia.net/commons/Module:Array/dev/hjp2

so the question is which perf do we value more

• perf for non loadData array
• perf for loaddata array

[Rathoz]

This will loop 3 times (T.size, T.copy, #). You can do just a pair(tbl) and ipair(tbl) and compare the results with just 2 loops.

This can replace T.size and the # in the non-MT case too, since it's the same number of loops.

[ElectricalBoy]

• Lua-Modules/lua/wikis/commons/Table.lua

  Lines 16 to 25 in 139be4d

  	---Get the size of a table
  	---@param tbl table
  	---@return integer
  	function Table.size(tbl)
  	local i = 0
  	for _ in pairs(tbl) do
  	i = i + 1
  	end
  	return i
  	end

  looping with pairs is what Table.size already is doing, so I don't see a need to replace it

• MT case now uses ipairs based counting: 6ba2e4f

• I think using the # operator is still faster for the non-MT case
  C implementation of it uses binary search with linear search fallback

[mbergen]

Do we really want to go this route?
We have many cases where we use the # operator on arrays.
With this, you can then successfully run the isArray check, and end up with something that is not a Lua array still.

(In general, i think we should keep usage of runtime checks like isArray as low as possible, given our constraints)

[ElectricalBoy]

We have many cases where we use the # operator on arrays.
With this, you can then successfully run the isArray check, and end up with something that is not a Lua array still.

The # operator simply does not work for arrays from mw.loadData anyway, and this PR is adding a special case for this only
Operational behavior of other arrays remain the same as live, so Array.isArray won't be returning false positives

[mbergen]

The # operator simply does not work for arrays from mw.loadData anyway, and this PR is adding a special case for this only

Exactly, and that means that Array.isArray should not claim they are arrays, when they aren't valid ones.

When checking Array.isArray, and the result is true, i expect that i can use other functions from Array.
These would still be broken, as lots of them use #.