Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tests] Add check for argument name validity #94798

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Jul 26, 2024

Checks that all method and signal arguments have valid names

See:

I'd say a check in unit tests should suffice, but we could add checks in the API dump as well but we'd need to add some more for class, method, and signal names etc., which would double with the unit tests so I'd say the unit tests should cover the API

@AThousandShips
Copy link
Member Author

Will move to the arg check and add the vararg check to prevent issues with empty cases, will push once CI is done

@AThousandShips AThousandShips force-pushed the api_check_args branch 2 times, most recently from d8e12b8 to 81d043b Compare July 26, 2024 14:03
@OffsetMOSFET
Copy link

This is a good fix.

I think the only thing that could be added is checking against reserved C++ keywords, but at least those should compile error for godot-cpp.

@AThousandShips
Copy link
Member Author

C++ keywords are already sanitized in godot-cpp:
https://github.com/godotengine/godot-cpp/blob/f2b521f55a0a2ba7080418ba898b97028da6134a/binding_generator.py#L2775-L2795

Checks that all method and signal arguments have valid names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants