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 an optional untyped_declaration warning #81355

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/classes/ProjectSettings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,9 @@
<member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when returning a call from a [code]void[/code] function when such call cannot be guaranteed to be also [code]void[/code].
</member>
<member name="debug/gdscript/warnings/untyped_declaration" type="int" setter="" getter="" default="0">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a variable or parameter has no static type, or if a function has no static return type.
</member>
<member name="debug/gdscript/warnings/unused_local_constant" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a local constant is never used.
</member>
Expand Down
50 changes: 37 additions & 13 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,11 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
GDScriptParser::ParameterNode *param = member.signal->parameters[j];
GDScriptParser::DataType param_type = type_from_metatype(resolve_datatype(param->datatype_specifier));
param->set_datatype(param_type);
#ifdef DEBUG_ENABLED
if (param->datatype_specifier == nullptr) {
parser->push_warning(param, GDScriptWarning::UNTYPED_DECLARATION, "Parameter", param->identifier->name);
}
#endif
mi.arguments.push_back(param_type.to_property_info(param->identifier->name));
// Signals do not support parameter default values.
}
Expand Down Expand Up @@ -1279,17 +1284,15 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
} else if (member.type == GDScriptParser::ClassNode::Member::VARIABLE && member.variable->property != GDScriptParser::VariableNode::PROP_NONE) {
if (member.variable->property == GDScriptParser::VariableNode::PROP_INLINE) {
if (member.variable->getter != nullptr) {
member.variable->getter->set_datatype(member.variable->datatype);
member.variable->getter->return_type = member.variable->datatype_specifier;
member.variable->getter->set_datatype(member.get_datatype());

resolve_function_body(member.variable->getter);
}
if (member.variable->setter != nullptr) {
resolve_function_signature(member.variable->setter);

if (member.variable->setter->parameters.size() > 0) {
member.variable->setter->parameters[0]->datatype_specifier = member.variable->datatype_specifier;
member.variable->setter->parameters[0]->set_datatype(member.get_datatype());
}
ERR_CONTINUE(member.variable->setter->parameters.is_empty());
member.variable->setter->parameters[0]->datatype_specifier = member.variable->datatype_specifier;
member.variable->setter->parameters[0]->set_datatype(member.get_datatype());

resolve_function_body(member.variable->setter);
}
Expand Down Expand Up @@ -1593,15 +1596,18 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
int default_value_count = 0;
#endif // TOOLS_ENABLED

#ifdef DEBUG_ENABLED
String function_visible_name = function_name;
if (function_name == StringName()) {
function_visible_name = p_is_lambda ? "<anonymous lambda>" : "<unknown function>";
}
#endif

for (int i = 0; i < p_function->parameters.size(); i++) {
resolve_parameter(p_function->parameters[i]);
#ifdef DEBUG_ENABLED
if (p_function->parameters[i]->usages == 0 && !String(p_function->parameters[i]->identifier->name).begins_with("_")) {
String visible_name = function_name;
if (function_name == StringName()) {
visible_name = p_is_lambda ? "<anonymous lambda>" : "<unknown function>";
}
parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, visible_name, p_function->parameters[i]->identifier->name);
parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, function_visible_name, p_function->parameters[i]->identifier->name);
}
is_shadowing(p_function->parameters[i]->identifier, "function parameter", true);
#endif // DEBUG_ENABLED
Expand Down Expand Up @@ -1716,6 +1722,12 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
#endif // TOOLS_ENABLED
}

#ifdef DEBUG_ENABLED
if (p_function->return_type == nullptr) {
parser->push_warning(p_function, GDScriptWarning::UNTYPED_DECLARATION, "Function", function_visible_name);
}
#endif

if (p_function->get_datatype().is_resolving()) {
p_function->set_datatype(prev_datatype);
}
Expand Down Expand Up @@ -1919,6 +1931,13 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
}
}

#ifdef DEBUG_ENABLED
if (!has_specified_type && !p_assignable->infer_datatype && !is_constant) {
const bool is_parameter = p_assignable->type == GDScriptParser::Node::PARAMETER;
parser->push_warning(p_assignable, GDScriptWarning::UNTYPED_DECLARATION, is_parameter ? "Parameter" : "Variable", p_assignable->identifier->name);
}
#endif

type.is_constant = is_constant;
type.is_read_only = false;
p_assignable->set_datatype(type);
Expand Down Expand Up @@ -2129,13 +2148,18 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
#endif
}
#ifdef DEBUG_ENABLED
} else {
} else if (variable_type.is_hard_type()) {
parser->push_warning(p_for->datatype_specifier, GDScriptWarning::REDUNDANT_FOR_VARIABLE_TYPE, p_for->variable->name, variable_type.to_string(), specified_type.to_string());
#endif
}
p_for->variable->set_datatype(specified_type);
} else {
p_for->variable->set_datatype(variable_type);
#ifdef DEBUG_ENABLED
if (!variable_type.is_hard_type()) {
parser->push_warning(p_for->variable, GDScriptWarning::UNTYPED_DECLARATION, R"("for" iterator variable)", p_for->variable->name);
}
#endif
}
}

Expand Down
7 changes: 7 additions & 0 deletions modules/gdscript/gdscript_warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ String GDScriptWarning::get_message() const {
case FUNCTION_USED_AS_PROPERTY:
CHECK_SYMBOLS(2);
return vformat(R"(The property "%s" was not found in base "%s" but there's a method with the same name. Did you mean to call it?)", symbols[0], symbols[1]);
case UNTYPED_DECLARATION:
CHECK_SYMBOLS(2);
if (symbols[0] == "Function") {
return vformat(R"*(%s "%s()" has no static return type.)*", symbols[0], symbols[1]);
}
return vformat(R"(%s "%s" has no static type.)", symbols[0], symbols[1]);
case UNSAFE_PROPERTY_ACCESS:
CHECK_SYMBOLS(2);
return vformat(R"(The property "%s" is not present on the inferred type "%s" (but may be present on a subtype).)", symbols[0], symbols[1]);
Expand Down Expand Up @@ -208,6 +214,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
"PROPERTY_USED_AS_FUNCTION",
"CONSTANT_USED_AS_FUNCTION",
"FUNCTION_USED_AS_PROPERTY",
"UNTYPED_DECLARATION",
"UNSAFE_PROPERTY_ACCESS",
"UNSAFE_METHOD_ACCESS",
"UNSAFE_CAST",
Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/gdscript_warning.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class GDScriptWarning {
PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name.
CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name.
FUNCTION_USED_AS_PROPERTY, // Property not found, but there's a function with the same name.
UNTYPED_DECLARATION, // Variable/parameter/function has no static type, explicitly specified or inferred (`:=`).
UNSAFE_PROPERTY_ACCESS, // Property not found in the detected type (but can be in subtypes).
UNSAFE_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes).
UNSAFE_CAST, // Cast used in an unknown type.
Expand Down Expand Up @@ -112,6 +113,7 @@ class GDScriptWarning {
WARN, // PROPERTY_USED_AS_FUNCTION
WARN, // CONSTANT_USED_AS_FUNCTION
WARN, // FUNCTION_USED_AS_PROPERTY
IGNORE, // UNTYPED_DECLARATION // Static typing is optional, we don't want to spam warnings.
IGNORE, // UNSAFE_PROPERTY_ACCESS // Too common in untyped scenarios.
IGNORE, // UNSAFE_METHOD_ACCESS // Too common in untyped scenarios.
IGNORE, // UNSAFE_CAST // Too common in untyped scenarios.
Expand Down
4 changes: 4 additions & 0 deletions modules/gdscript/tests/gdscript_test_runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ GDScriptTestRunner::GDScriptTestRunner(const String &p_source_dir, bool p_init_l
// Set all warning levels to "Warn" in order to test them properly, even the ones that default to error.
ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/enable", true);
for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) {
if (i == GDScriptWarning::UNTYPED_DECLARATION) {
// TODO: Add ability for test scripts to specify which warnings to enable/disable for testing.
continue;
}
String warning_setting = GDScriptWarning::get_settings_path_from_code((GDScriptWarning::Code)i);
ProjectSettings::get_singleton()->set_setting(warning_setting, (int)GDScriptWarning::WARN);
}
Expand Down