Arrays in default arguments can be modified

[KoBeWi]

Godot version

4.2 beta4

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 30.0.15.1403) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

If a method has a default argument of type array, you can modify that array and the new content will be displayed in autocompletion tooltip. Easy to test with OS.execute() and an EditorScript.

godot.windows.editor.dev.x86_64_KaygjDglwe.mp4

OS.execute() has an argument used for passing output. It defaults to empty array. If you don't pass the argument, the output will be inserted into the default array, which then shows in autocompletion.

Steps to reproduce

1. Create EditorScript
2. Run OS.execute() with Godot as executable and no other arguments (OS.execute("godot_console", []))
3. Observe the completion popup

Minimal reproduction project

N/A

[chocola-mint]

For those interested in solving this issue:

This seems to be an issue specific to default parameters bound to GDScript from C++ via bind_method. I can't reproduce the same effect with the following tool script:

@tool
extends EditorScript
# run with File > Run
func my_func(arr = []):
	arr.append("hi")
	print(arr)

# Called when the script is executed (using File -> Run in Script Editor).
func _run():
	my_func()
	my_func()
	my_func()
        # Three ["hi"] strings are printed, and code hint isn't changed.

On the other hand, binding the following C++ method is enough to reproduce the issue:

// Using EditorScript as example out of convenience.
void EditorScript::test_array(Array arr) {
	arr.append("hi");
	print_line(arr);
}

// Bind with this:
ClassDB::bind_method(D_METHOD("test_array", "arr"), &EditorScript::test_array, DEFVAL(Array()));

[chocola-mint]

Fixing this doesn't seem trivial at all. I've tried explicitly duplicating default values in all possible call sites, yet to no avail.

There is however a workaround, though. Instead of using Array() as the default value, we could use a read-only array (marked with Array::make_read_only()), and in the method implementation, use Array:is_read_only() to check and avoid modifying the array. A quick search in the codebase suggests that OS::execute is currently the only bound method that breaks this rule, so the amount of modification would be minimal. And besides, modifying a don't-care output array already sounds like a code smell, so this would avoid doing useless work as well.

What do you think?