Merge pull request #81355 from ryanabx/features/enforce-static-typing

Add an optional `untyped_declaration` warning
This commit is contained in:
Rémi Verschelde 2023-09-12 08:49:57 +02:00
commit 0545ed5c71
No known key found for this signature in database
GPG Key ID: C3336907360768E1
5 changed files with 53 additions and 13 deletions

View File

@ -549,6 +549,9 @@
<member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="1"> <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]. 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>
<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"> <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. 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> </member>

View File

@ -1000,6 +1000,11 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
GDScriptParser::ParameterNode *param = member.signal->parameters[j]; GDScriptParser::ParameterNode *param = member.signal->parameters[j];
GDScriptParser::DataType param_type = type_from_metatype(resolve_datatype(param->datatype_specifier)); GDScriptParser::DataType param_type = type_from_metatype(resolve_datatype(param->datatype_specifier));
param->set_datatype(param_type); 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)); mi.arguments.push_back(param_type.to_property_info(param->identifier->name));
// Signals do not support parameter default values. // Signals do not support parameter default values.
} }
@ -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) { } 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->property == GDScriptParser::VariableNode::PROP_INLINE) {
if (member.variable->getter != nullptr) { 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); resolve_function_body(member.variable->getter);
} }
if (member.variable->setter != nullptr) { if (member.variable->setter != nullptr) {
resolve_function_signature(member.variable->setter); ERR_CONTINUE(member.variable->setter->parameters.is_empty());
member.variable->setter->parameters[0]->datatype_specifier = member.variable->datatype_specifier;
if (member.variable->setter->parameters.size() > 0) { member.variable->setter->parameters[0]->set_datatype(member.get_datatype());
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); resolve_function_body(member.variable->setter);
} }
@ -1593,15 +1596,18 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
int default_value_count = 0; int default_value_count = 0;
#endif // TOOLS_ENABLED #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++) { for (int i = 0; i < p_function->parameters.size(); i++) {
resolve_parameter(p_function->parameters[i]); resolve_parameter(p_function->parameters[i]);
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (p_function->parameters[i]->usages == 0 && !String(p_function->parameters[i]->identifier->name).begins_with("_")) { if (p_function->parameters[i]->usages == 0 && !String(p_function->parameters[i]->identifier->name).begins_with("_")) {
String visible_name = function_name; parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, function_visible_name, p_function->parameters[i]->identifier->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);
} }
is_shadowing(p_function->parameters[i]->identifier, "function parameter", true); is_shadowing(p_function->parameters[i]->identifier, "function parameter", true);
#endif // DEBUG_ENABLED #endif // DEBUG_ENABLED
@ -1716,6 +1722,12 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
#endif // TOOLS_ENABLED #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()) { if (p_function->get_datatype().is_resolving()) {
p_function->set_datatype(prev_datatype); p_function->set_datatype(prev_datatype);
} }
@ -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_constant = is_constant;
type.is_read_only = false; type.is_read_only = false;
p_assignable->set_datatype(type); p_assignable->set_datatype(type);
@ -2129,13 +2148,18 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
#endif #endif
} }
#ifdef DEBUG_ENABLED #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()); 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 #endif
} }
p_for->variable->set_datatype(specified_type); p_for->variable->set_datatype(specified_type);
} else { } else {
p_for->variable->set_datatype(variable_type); 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
} }
} }

View File

@ -88,6 +88,12 @@ String GDScriptWarning::get_message() const {
case FUNCTION_USED_AS_PROPERTY: case FUNCTION_USED_AS_PROPERTY:
CHECK_SYMBOLS(2); 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]); 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: case UNSAFE_PROPERTY_ACCESS:
CHECK_SYMBOLS(2); 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]); 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]);
@ -208,6 +214,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
"PROPERTY_USED_AS_FUNCTION", "PROPERTY_USED_AS_FUNCTION",
"CONSTANT_USED_AS_FUNCTION", "CONSTANT_USED_AS_FUNCTION",
"FUNCTION_USED_AS_PROPERTY", "FUNCTION_USED_AS_PROPERTY",
"UNTYPED_DECLARATION",
"UNSAFE_PROPERTY_ACCESS", "UNSAFE_PROPERTY_ACCESS",
"UNSAFE_METHOD_ACCESS", "UNSAFE_METHOD_ACCESS",
"UNSAFE_CAST", "UNSAFE_CAST",

View File

@ -64,6 +64,7 @@ public:
PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name. 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. 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. 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_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_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes).
UNSAFE_CAST, // Cast used in an unknown type. UNSAFE_CAST, // Cast used in an unknown type.
@ -112,6 +113,7 @@ public:
WARN, // PROPERTY_USED_AS_FUNCTION WARN, // PROPERTY_USED_AS_FUNCTION
WARN, // CONSTANT_USED_AS_FUNCTION WARN, // CONSTANT_USED_AS_FUNCTION
WARN, // FUNCTION_USED_AS_PROPERTY 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_PROPERTY_ACCESS // Too common in untyped scenarios.
IGNORE, // UNSAFE_METHOD_ACCESS // Too common in untyped scenarios. IGNORE, // UNSAFE_METHOD_ACCESS // Too common in untyped scenarios.
IGNORE, // UNSAFE_CAST // Too common in untyped scenarios. IGNORE, // UNSAFE_CAST // Too common in untyped scenarios.

View File

@ -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. // 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); ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/enable", true);
for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) { 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); String warning_setting = GDScriptWarning::get_settings_path_from_code((GDScriptWarning::Code)i);
ProjectSettings::get_singleton()->set_setting(warning_setting, (int)GDScriptWarning::WARN); ProjectSettings::get_singleton()->set_setting(warning_setting, (int)GDScriptWarning::WARN);
} }