Add a check for unnamed arguments to `make_rst.py`

[YuriSizov]

Sometimes developers forget to change the argument information in method bindings when changing the method itself. This results in unnamed arguments in the documentation, which doesn't make good docs. We sometimes fix it by hand, but with this change we should start catching the issue on CI for every PR.

It would look something like this and the Static Checks task would fail.: https://github.com/YuriSizov/godot/runs/7706112708?check_suite_focus=true

I've decided to change the default name of the unnamed argument/parameter for better grepping. I don't think it would break anything, and if it does then name your parameters 😈

---

I briefly played with an option to fail in the doctool itself, so developers can test it locally while running it to sync up the docs, but we don't have a way to fail CI from the doctool at the moment (even if you return a non-zero exit code). It is still possible to add checks to the doctool without making it fail, something like this (if we still want to report an exit code; otherwise swap for the ERR_CONTINUE_MSG macro):

// Log as error, but don't fail.
if (arginfo.name.is_empty() || arginfo.name.begins_with("_unnamed_arg")) {
	exit_code = EXIT_FAILURE;
	ERR_PRINT(vformat("In '%s' the '%s' method has unnamed argument in position %d; check '%s::_bind_methods'.", lang->get_name(), md.name, j, lang->get_name()));
}

We also don't typically check anything about classes in doctools, it just creates XML and make_rst.py does the checking on CI. So this would be an extra option, on top of this PR, if we want that.

For the reference, make_rst.py can be run locally with --dry-run to not produce any files and only check for errors. Can probably include in pre-commit hooks as a simpler solution?

[akien-mga]

While having a check in make_rst.py is nice to have, I think this should be checked by test_class_db.h directly.

As a matter of fact, this PR doesn't catch some methods which do lack argument names, because they're setters and their argument name isn't listed in the XML. But you can see them in the GDExtension API:

$ rg arg0
godot-headers/extension_api.json
34240:                                                  "name": "arg0",
67272:                                                  "name": "arg0",
212688:                                                 "name": "arg0",

[YuriSizov]

Ah, I knew about setters, but didn't think about them in the context of the API generation. Makes sense. But it doesn't look like test_class_db.h would be enough. We also need to test the global scope (servers/singletons, variant utilities), and each script language built-ins.

[akien-mga]

Also for the record, in GDNative in 3.x the problem is much worse as all the "private" underscore methods bound for signals/UndoRedo/call also often don't have argument names, since we don't really care and they don't show in the docs. So there's plenty of arg0 and arg1 in the 3.x GDNative JSON: https://raw.githubusercontent.com/godotengine/godot-headers/3.x/api.json

[YuriSizov]

Well, thank god we don't have tests in 3.x then 😛

[YuriSizov]

From test_class_db.h testing alone, there are 17 cases of unnamed parameters, mostly setters:

===============================================================================
[doctest] test cases:     596 |     595 passed |  1 failed | 1 skipped
[doctest] assertions: 2298748 | 2298731 passed | 17 failed |
[doctest] Status: FAILURE!

===============================================================================
C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(833):
TEST SUITE: [ClassDB]
TEST CASE:  [ClassDB] Add exposed classes, builtin types, and global enums
  [ClassDB] Find exposed class

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'AnimationPlayer.set_movie_quit_on_finish_enabled'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'BitMap._set_data'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'ConfirmationDialog.set_cancel_button_text'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'Curve2D._set_data'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'Curve3D._set_data'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'MultiMesh._set_transform_array'.       

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'MultiMesh._set_transform_2d_array'.    

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'MultiMesh._set_color_array'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'MultiMesh._set_custom_data_array'.     

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'OptionButton._select_int'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'PackedDataContainer._set_data'.        

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'PackedScene._set_bundled_scene'.       

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'PolygonPathFinder._set_data'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'ResourcePreloader._set_resources'.     

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'SpriteFrames._set_animations'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'TextEdit.set_fit_content_height_enabled'.

C:\Projects\godot-engine\master\tests/core/object/test_class_db.h(377): ERROR: CHECK_FALSE( (p_arg.name.is_empty() || p_arg.name.begins_with("_unnamed_arg")) ) is NOT correct!       
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of method 'Translation._set_messages'.

---

Nothing from test_variant.h, but if an error is introduced, it is correctly detected like so:

===============================================================================
C:\Projects\godot-engine\master\tests/core/variant/test_variant.h(931):
TEST CASE:  [Variant] Utility functions
  [Variant] Validate utility functions

C:\Projects\godot-engine\master\tests/core/variant/test_variant.h(970): ERROR: CHECK_FALSE( (arg.name.is_empty() || arg.name.begins_with("_unnamed_arg")) ) is NOT correct!
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 2 of function 'lerp'.

===============================================================================
[doctest] test cases:     597 |     595 passed |  2 failed | 1 skipped
[doctest] assertions: 2298883 | 2298865 passed | 18 failed |
[doctest] Status: FAILURE!

---

Nothing from modules/gdscript/tests/gdscript_test_runner_suite.h, but if an error is introduced, it is correctly detected:

===============================================================================
C:\Projects\godot-engine\master\modules\gdscript\tests\gdscript_test_runner_suite.h(72):
TEST CASE:  [Modules][GDScript] Validate built-in API
  [Modules][GDScript] Validate built-in methods

C:\Projects\godot-engine\master\modules\gdscript\tests\gdscript_test_runner_suite.h(85): ERROR: CHECK_FALSE( (arg.name.is_empty() || arg.name.begins_with("_unnamed_arg")) ) is NOT correct!
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of built-in method 'dict2inst'.

===============================================================================
C:\Projects\godot-engine\master\modules\gdscript\tests\gdscript_test_runner_suite.h(72):   
TEST CASE:  [Modules][GDScript] Validate built-in API
  [Modules][GDScript] Validate built-in annotations

C:\Projects\godot-engine\master\modules\gdscript\tests\gdscript_test_runner_suite.h(101): ERROR: CHECK_FALSE( (arg.name.is_empty() || arg.name.begins_with("_unnamed_arg")) ) is NOT correct!
  values: CHECK_FALSE( true )
  logged: Unnamed argument in position 0 of built-in annotation '@export_group'.

---

Finally, servers and other singletons should already be registered with ClassDB, so they are covered by previous tests.

[YuriSizov]

@akien-mga Done! I hope this is to your satisfaction. I've added a second commit that adds tests to ClassDB, Variant, and GDScript, and fixes what these tests have unearthed.

[akien-mga]

Thanks!