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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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/
.
modules/gdscript/gdscript_parser.cpp
Outdated
if (p_allow_property && match(GDScriptTokenizer::Token::IDENTIFIER)) { | ||
StringName identifier_name = previous.get_identifier(); |
There was a problem hiding this comment.
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.
modules/gdscript/gdscript_parser.cpp
Outdated
} else { | ||
push_error(R"(Expected end of statement after variable declaration, found "Identifier" instead.)"); | ||
} |
There was a problem hiding this comment.
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.
modules/gdscript/gdscript_parser.cpp
Outdated
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.)"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
d937f54
to
698a175
Compare
Let me know if this is alright. |
modules/gdscript/tests/scripts/parser/errors/missing_colon_setget_indent.gd
Outdated
Show resolved
Hide resolved
modules/gdscript/tests/scripts/parser/errors/missing_colon_setget_inline.gd
Outdated
Show resolved
Hide resolved
698a175
to
568874c
Compare
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
568874c
to
17de35e
Compare
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