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

Add clearer error messages when using set/get incorrectly. #91506

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

Conversation

ArchieVillain
Copy link

It's quite easy for users to forget to use the colon before their setters and getters, but in this situation error messages are unclear about the real nature of the problem.

This commit adds a check for malformed set/get statements and provides error messages containing more direct guidance to users.

Closes #91216

Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test should be added to modules/gdscript/tests/scripts/parser/errors/.

Comment on lines 1101 to 1102
if (p_allow_property && match(GDScriptTokenizer::Token::IDENTIFIER)) {
StringName identifier_name = previous.get_identifier();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think check() and current should be used here instead of match() and previous since match() advances the parser.

} else {
push_error(R"(Expected end of statement after variable declaration, found "Identifier" instead.)");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be removed then.

Comment on lines 1113 to 1124
if (p_allow_property && !malformed_setget_found && match(GDScriptTokenizer::Token::INDENT)) {
if (match(GDScriptTokenizer::Token::IDENTIFIER)) {
StringName identifier_name = previous.get_identifier();
if (identifier_name == SNAME("set") || identifier_name == SNAME("get")) {
malformed_setget_found = true;
}
}

if (malformed_setget_found) {
push_error(vformat(R"(Expected ":" at end of variable declaration when using "set" or "get")"), variable);
} else {
push_error(R"(Unexpected "Indent" after variable declaration.)");
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@ArchieVillain
Copy link
Author

A test should be added to modules/gdscript/tests/scripts/parser/errors/.

Let me know if this is alright.

It's quite easy for users to forget to use the colon before their
setters and getters, but in this situation error messages are unclear about
the real nature of the problem.

This commit adds a check for malformed set/get statements and provides
error messages containing more direct guidance to users.

Closes godotengine#91216
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.

Error creating a property in inner class
3 participants