GDScript: Implement soft access restriction for members prefixed with _ and __ (as warning by default)

doc/classes/ProjectSettings.xml

@@ -469,12 +469,24 @@
 			Specifies the maximum number of log files allowed (used for rotation). Set to [code]1[/code] to disable log file rotation.
 			If the [code]--log-file <file>[/code] [url=$DOCS_URL/tutorials/editor/command_line_tutorial.html]command line argument[/url] is used, log rotation is always disabled.
 		</member>
+		<member name="debug/gdscript/warnings/accessing_private_member" 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 member beginning with [code]__[/code] is accessed from outside the class.
+		</member>
+		<member name="debug/gdscript/warnings/accessing_protected_member" 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 member beginning with [code]_[/code] is accessed from outside the class or the super class where it is defined.
+		</member>
 		<member name="debug/gdscript/warnings/assert_always_false" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when an [code]assert[/code] call always evaluates to [code]false[/code].
 		</member>
 		<member name="debug/gdscript/warnings/assert_always_true" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when an [code]assert[/code] call always evaluates to [code]true[/code].
 		</member>
+		<member name="debug/gdscript/warnings/calling_private_method" 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 method beginning with [code]__[/code] is called from outside the class.
+		</member>
+		<member name="debug/gdscript/warnings/calling_protected_method" 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 method beginning with [code]_[/code] is called from outside the class or the super class where it is defined.
+		</member>
 		<member name="debug/gdscript/warnings/confusable_capture_reassignment" 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 variable captured by a lambda is reassigned, since this does not modify the outer local variable.
 		</member>
@@ -616,6 +628,9 @@
 		<member name="debug/gdscript/warnings/unused_private_class_variable" 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 private member variable is never used.
 		</member>
+		<member name="debug/gdscript/warnings/unused_protected_class_variable" 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 protected member variable is never used.
+		</member>
 		<member name="debug/gdscript/warnings/unused_signal" 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 signal is declared but never explicitly used in the class.
 		</member>

modules/gdscript/gdscript_analyzer.cpp

@@ -341,6 +341,54 @@ void GDScriptAnalyzer::get_class_node_current_scope_classes(GDScriptParser::Clas
 	}
 }
 
+#ifdef DEBUG_ENABLED
+void GDScriptAnalyzer::check_access_private_member(GDScriptParser::IdentifierNode *p_identifier, const bool p_is_call) {
+	if (p_identifier == nullptr) {
+		return;
+	}
+	GDScriptParser::IdentifierNode::IdentifierAccess identifier_access = p_identifier->get_access();
+	if (identifier_access == GDScriptParser::IdentifierNode::IdentifierAccess::ACCESS_PUBLIC) {
+		return;
+	}
+	if (parser->current_class->get_datatype().kind != GDScriptParser::DataType::CLASS && parser->current_class->get_datatype().kind != GDScriptParser::DataType::SCRIPT) {
+		return;
+	}
+	if (parser->current_function && parser->current_function->body && parser->current_function->body->has_local(p_identifier->name)) {
+		return;
+	}
+
+	bool has_member = parser->current_class->has_member(p_identifier->name);
+	if (has_member) {
+		return;
+	}
+	if (identifier_access == GDScriptParser::IdentifierNode::IdentifierAccess::ACCESS_PRIVATE) {
+		parser->push_warning(p_identifier, p_is_call ? GDScriptWarning::CALLING_PRIVATE_METHOD : GDScriptWarning::ACCESSING_PRIVATE_MEMBER, p_identifier->name);
+		return;
+	}
+
+	bool access_valid_protected_member = false;
+	GDScriptParser::ClassNode *base_class = parser->current_class->base_type.class_type;
+	while (base_class != nullptr) {
+		if (base_class->has_member(p_identifier->name)) {
+			access_valid_protected_member = true;
+			break;
+		}
+		base_class = base_class->base_type.class_type;
+	}
+	if (!access_valid_protected_member && identifier_access == GDScriptParser::IdentifierNode::IdentifierAccess::ACCESS_PROTECTED) {
+		parser->push_warning(p_identifier, p_is_call ? GDScriptWarning::CALLING_PROTECTED_METHOD : GDScriptWarning::ACCESSING_PROTECTED_MEMBER, p_identifier->name);
+	}
+}
+
+void GDScriptAnalyzer::warn_unused_private_protected_class_variable(const GDScriptParser::Node *p_node, const GDScriptParser::IdentifierNode *p_identifier) {
+	GDScriptParser::IdentifierNode::IdentifierAccess identifier_access = p_identifier->get_access();
+	if (identifier_access == GDScriptParser::IdentifierNode::IdentifierAccess::ACCESS_PUBLIC) {
+		return;
+	}
+	parser->push_warning(p_node, identifier_access == GDScriptParser::IdentifierNode::IdentifierAccess::ACCESS_PRIVATE ? GDScriptWarning::UNUSED_PRIVATE_CLASS_VARIABLE : GDScriptWarning::UNUSED_PROTECTED_CLASS_VARIABLE, p_identifier->name);
+}
+#endif // DEBUG_ENABLED
+
 Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source) {
 	if (p_source == nullptr && parser->has_class(p_class)) {
 		p_source = p_class;
@@ -1435,8 +1483,8 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
 		GDScriptParser::ClassNode::Member member = p_class->members[i];
 		if (member.type == GDScriptParser::ClassNode::Member::VARIABLE) {
 #ifdef DEBUG_ENABLED
-			if (member.variable->usages == 0 && String(member.variable->identifier->name).begins_with("_")) {
-				parser->push_warning(member.variable->identifier, GDScriptWarning::UNUSED_PRIVATE_CLASS_VARIABLE, member.variable->identifier->name);
+			if (member.variable->usages == 0) {
+				warn_unused_private_protected_class_variable(member.variable->identifier, member.variable->identifier);
 			}
 #endif
 
@@ -3510,10 +3558,19 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 		if (p_call->callee == nullptr && current_lambda != nullptr) {
 			push_error("Cannot use `super()` inside a lambda.", p_call);
 		}
+
+#ifdef DEBUG_ENABLED
+		GDScriptParser::IdentifierNode *identifier = static_cast<GDScriptParser::IdentifierNode *>(p_call->callee);
+		check_access_private_member(identifier, true);
+#endif
 	} else if (callee_type == GDScriptParser::Node::IDENTIFIER) {
 		base_type = parser->current_class->get_datatype();
 		base_type.is_meta_type = false;
 		is_self = true;
+#ifdef DEBUG_ENABLED
+		GDScriptParser::IdentifierNode *identifier = static_cast<GDScriptParser::IdentifierNode *>(p_call->callee);
+		check_access_private_member(identifier, true);
+#endif
 	} else if (callee_type == GDScriptParser::Node::SUBSCRIPT) {
 		GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(p_call->callee);
 		if (subscript->base == nullptr) {
@@ -3547,6 +3604,9 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 			base_type = subscript->base->get_datatype();
 			is_self = subscript->base->type == GDScriptParser::Node::SELF;
 		}
+#ifdef DEBUG_ENABLED
+		check_access_private_member(subscript->attribute, true);
+#endif
 	} else {
 		// Invalid call. Error already sent in parser.
 		// TODO: Could check if Callable here too.
@@ -4468,7 +4528,9 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
 				function_test = function_test->source_lambda->parent_function;
 			}
 		}
-
+#ifdef DEBUG_ENABLED
+		check_access_private_member(p_identifier);
+#endif
 		return;
 	}
 
@@ -4830,6 +4892,9 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri
 			}
 			result_type.kind = GDScriptParser::DataType::VARIANT;
 		}
+#ifdef DEBUG_ENABLED
+		check_access_private_member(p_subscript->attribute);
+#endif
 	} else {
 		if (p_subscript->index == nullptr) {
 			return;

modules/gdscript/gdscript_analyzer.h

@@ -66,6 +66,11 @@ class GDScriptAnalyzer {
 
 	void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source);
 
+#ifdef DEBUG_ENABLED
+	void check_access_private_member(GDScriptParser::IdentifierNode *p_identifier, const bool p_is_call = false);
+	void warn_unused_private_protected_class_variable(const GDScriptParser::Node *p_node, const GDScriptParser::IdentifierNode *p_identifier);
+#endif
+
 	Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr);
 	Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive);
 	GDScriptParser::DataType resolve_datatype(GDScriptParser::TypeNode *p_type);

modules/gdscript/gdscript_parser.cpp

@@ -98,6 +98,8 @@ GDScriptParser::GDScriptParser() {
 		register_annotation(MethodInfo("@static_unload"), AnnotationInfo::SCRIPT, &GDScriptParser::static_unload_annotation);
 		// Onready annotation.
 		register_annotation(MethodInfo("@onready"), AnnotationInfo::VARIABLE, &GDScriptParser::onready_annotation);
+
+		// register_annotation(MethodInfo("@virtual"), AnnotationInfo::FUNCTION, &GDScriptParser::virtual_annotation);
 		// Export annotations.
 		register_annotation(MethodInfo("@export"), AnnotationInfo::VARIABLE, &GDScriptParser::export_annotations<PROPERTY_HINT_NONE, Variant::NIL>);
 		register_annotation(MethodInfo("@export_enum", PropertyInfo(Variant::STRING, "names")), AnnotationInfo::VARIABLE, &GDScriptParser::export_annotations<PROPERTY_HINT_ENUM, Variant::NIL>, varray(), true);
@@ -4282,6 +4284,16 @@ bool GDScriptParser::onready_annotation(AnnotationNode *p_annotation, Node *p_ta
 	current_class->onready_used = true;
 	return true;
 }
+// bool GDScriptParser::virtual_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
+//	ERR_FAIL_COND_V_MSG(p_target->type != Node::FUNCTION, false, R"("@virtual" annotation can only be applied to class methods.)");
+//	FunctionNode *method = static_cast<FunctionNode *>(p_target);
+//	if (method->is_virtual) {
+//		push_error(R"("@virtual" annotation can only be applied to a method once.)", p_annotation);
+//		return false;
+//	}
+//	method->is_virtual = true;
+//	return true;
+// }
 
 static String _get_annotation_error_string(const StringName &p_annotation_name, const Vector<Variant::Type> &p_expected_types, const GDScriptParser::DataType &p_provided_type) {
 	Vector<String> types;

modules/gdscript/gdscript_parser.h

@@ -336,8 +336,8 @@ class GDScriptParser {
 			VARIABLE,
 			WHILE,
 		};
-
 		Type type = NONE;
+
 		int start_line = 0, end_line = 0;
 		int start_column = 0, end_column = 0;
 		int leftmost_column = 0, rightmost_column = 0;
@@ -796,6 +796,20 @@ class GDScriptParser {
 			members.push_back(Member(p_annotation_node));
 		}
 
+		Vector<StringName> get_super_class_fqcns() const {
+			Vector<StringName> ret;
+
+			const ClassNode *iterated_class = base_type.class_type;
+			while (iterated_class) {
+				if (iterated_class->datatype.kind == DataType::SCRIPT || iterated_class->datatype.kind == DataType::CLASS) {
+					ret.append(iterated_class->fqcn);
+				}
+				iterated_class = iterated_class->base_type.class_type;
+			}
+
+			return ret;
+		}
+
 		ClassNode() {
 			type = CLASS;
 		}
@@ -855,6 +869,7 @@ class GDScriptParser {
 		SuiteNode *body = nullptr;
 		bool is_static = false; // For lambdas it's determined in the analyzer.
 		bool is_coroutine = false;
+		// bool is_virtual = false;
 		Variant rpc_config;
 		MethodInfo info;
 		LambdaNode *source_lambda = nullptr;
@@ -902,6 +917,12 @@ class GDScriptParser {
 		};
 		Source source = UNDEFINED_SOURCE;
 
+		enum IdentifierAccess {
+			ACCESS_PUBLIC,
+			ACCESS_PRIVATE,
+			ACCESS_PROTECTED,
+		};
+
 		union {
 			ParameterNode *parameter_source = nullptr;
 			IdentifierNode *bind_source;
@@ -916,6 +937,17 @@ class GDScriptParser {
 
 		int usages = 0; // Useful for binds/iterator variable.
 
+		IdentifierAccess get_access() const {
+			String str_name = String(name);
+			if (str_name.begins_with("__")) {
+				return ACCESS_PRIVATE;
+			} else if (str_name.begins_with("_")) {
+				return ACCESS_PROTECTED;
+			} else {
+				return ACCESS_PUBLIC;
+			}
+		}
+
 		IdentifierNode() {
 			type = IDENTIFIER;
 		}
@@ -1509,6 +1541,7 @@ class GDScriptParser {
 	bool icon_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool static_unload_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool onready_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
+	// bool virtual_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	template <PropertyHint t_hint, Variant::Type t_type>
 	bool export_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool export_storage_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);

modules/gdscript/gdscript_warning.cpp

@@ -51,6 +51,7 @@ String GDScriptWarning::get_message() const {
 			CHECK_SYMBOLS(1);
 			return vformat(R"(The local constant "%s" is declared but never used in the block. If this is intended, prefix it with an underscore: "_%s".)", symbols[0], symbols[0]);
 		case UNUSED_PRIVATE_CLASS_VARIABLE:
+		case UNUSED_PROTECTED_CLASS_VARIABLE:
 			CHECK_SYMBOLS(1);
 			return vformat(R"(The class variable "%s" is declared but never used in the class.)", symbols[0]);
 		case UNUSED_PARAMETER:
@@ -162,6 +163,18 @@ String GDScriptWarning::get_message() const {
 			return vformat(R"*(The default value is using "%s" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.)*", symbols[0]);
 		case ONREADY_WITH_EXPORT:
 			return R"("@onready" will set the default value after "@export" takes effect and will override it.)";
+		case ACCESSING_PRIVATE_MEMBER:
+			CHECK_SYMBOLS(1);
+			return vformat(R"(Trying accessing the member "%s" prefixed with "__", which would be a private member.)", symbols[0]);
+		case CALLING_PRIVATE_METHOD:
+			CHECK_SYMBOLS(1);
+			return vformat(R"*(Trying calling the method "%s()" prefixed with "__", which would be a private method.)*", symbols[0]);
+		case ACCESSING_PROTECTED_MEMBER:
+			CHECK_SYMBOLS(1);
+			return vformat(R"(Trying accessing the member "%s" prefixed with "_", which would be a protected member.)", symbols[0]);
+		case CALLING_PROTECTED_METHOD:
+			CHECK_SYMBOLS(1);
+			return vformat(R"*(Trying calling the method "%s()" prefixed with "_", which would be a protected method.)*", symbols[0]);
 #ifndef DISABLE_DEPRECATED
 		// Never produced. These warnings migrated from 3.x by mistake.
 		case PROPERTY_USED_AS_FUNCTION: // There is already an error.
@@ -199,6 +212,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 		"UNUSED_VARIABLE",
 		"UNUSED_LOCAL_CONSTANT",
 		"UNUSED_PRIVATE_CLASS_VARIABLE",
+		"UNUSED_PROTECTED_CLASS_VARIABLE",
 		"UNUSED_PARAMETER",
 		"UNUSED_SIGNAL",
 		"SHADOWED_VARIABLE",
@@ -238,6 +252,10 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 		"NATIVE_METHOD_OVERRIDE",
 		"GET_NODE_DEFAULT_WITHOUT_ONREADY",
 		"ONREADY_WITH_EXPORT",
+		"ACCESSING_PRIVATE_MEMBER",
+		"CALLING_PRIVATE_METHOD",
+		"ACCESSING_PROTECTED_MEMBER",
+		"CALLING_PROTECTED_METHOD",
 #ifndef DISABLE_DEPRECATED
 		"PROPERTY_USED_AS_FUNCTION",
 		"CONSTANT_USED_AS_FUNCTION",

modules/gdscript/gdscript_warning.h

@@ -50,7 +50,8 @@ class GDScriptWarning {
 		UNASSIGNED_VARIABLE_OP_ASSIGN, // Variable never assigned but used in an assignment operation (+=, *=, etc).
 		UNUSED_VARIABLE, // Local variable is declared but never used.
 		UNUSED_LOCAL_CONSTANT, // Local constant is declared but never used.
-		UNUSED_PRIVATE_CLASS_VARIABLE, // Class variable is declared private ("_" prefix) but never used in the class.
+		UNUSED_PRIVATE_CLASS_VARIABLE, // Class variable is declared private ("__" prefix) but never used in the class.
+		UNUSED_PROTECTED_CLASS_VARIABLE, // Class variable is declared private ("__" prefix) but never used in the class.
 		UNUSED_PARAMETER, // Function parameter is never used.
 		UNUSED_SIGNAL, // Signal is defined but never explicitly used in the class.
 		SHADOWED_VARIABLE, // A local variable/constant shadows a current class member.
@@ -90,6 +91,10 @@ class GDScriptWarning {
 		NATIVE_METHOD_OVERRIDE, // The script method overrides a native one, this may not work as intended.
 		GET_NODE_DEFAULT_WITHOUT_ONREADY, // A class variable uses `get_node()` (or the `$` notation) as its default value, but does not use the @onready annotation.
 		ONREADY_WITH_EXPORT, // The `@onready` annotation will set the value after `@export` which is likely not intended.
+		ACCESSING_PRIVATE_MEMBER, // A member prefixed with `__` is being accessed, whether the member exists or not.
+		CALLING_PRIVATE_METHOD, // A method prefixed with `__` is being called, whether the method exists or not.
+		ACCESSING_PROTECTED_MEMBER, // A member prefixed with `_` is being accessed, whether the member exists or not.
+		CALLING_PROTECTED_METHOD, // A method prefixed with `_` is being called, whether the method exists or not.
 #ifndef DISABLE_DEPRECATED
 		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.
@@ -108,6 +113,7 @@ class GDScriptWarning {
 		WARN, // UNUSED_VARIABLE
 		WARN, // UNUSED_LOCAL_CONSTANT
 		WARN, // UNUSED_PRIVATE_CLASS_VARIABLE
+		WARN, // UNUSED_PROTECTED_CLASS_VARIABLE
 		WARN, // UNUSED_PARAMETER
 		WARN, // UNUSED_SIGNAL
 		WARN, // SHADOWED_VARIABLE
@@ -147,6 +153,10 @@ class GDScriptWarning {
 		ERROR, // NATIVE_METHOD_OVERRIDE // May not work as expected.
 		ERROR, // GET_NODE_DEFAULT_WITHOUT_ONREADY // May not work as expected.
 		ERROR, // ONREADY_WITH_EXPORT // May not work as expected.
+		WARN, // ACCESSING_PRIVATE_MEMBER
+		WARN, // CALLING_PRIVATE_METHOD
+		WARN, // ACCESSING_PROTECTED_MEMBER
+		WARN, // CALLING_PROTECTED_METHOD
 #ifndef DISABLE_DEPRECATED
 		WARN, // PROPERTY_USED_AS_FUNCTION
 		WARN, // CONSTANT_USED_AS_FUNCTION

modules/gdscript/tests/scripts/analyzer/features/enum_function_typecheck.gd

@@ -26,7 +26,6 @@ class InnerClass:
 		print("Inner")
 
 		var o := EnumFunctionTypecheckOuterClass.new()
-
 		_d = o.outer_outer_no_class(EnumFunctionTypecheckOuterClass.MyEnum.V1)
 		print()
 		_d = o.outer_outer_class(EnumFunctionTypecheckOuterClass.MyEnum.V1)

modules/gdscript/tests/scripts/analyzer/features/virtual_method_implemented.gd

@@ -5,6 +5,7 @@ class BaseClass:
 class SuperClassMethodsRecognized extends BaseClass:
 	func _init():
 		# Recognizes super class methods.
+		@warning_ignore("calling_protected_method")
 		var _x = _get_property_list()
 
 class SuperMethodsRecognized extends BaseClass:
@@ -16,6 +17,8 @@ class SuperMethodsRecognized extends BaseClass:
 
 func test():
 	var test1 = SuperClassMethodsRecognized.new()
+	@warning_ignore("calling_protected_method")
 	print(test1._get_property_list()) # Calls base class's method.
 	var test2 = SuperMethodsRecognized.new()
+	@warning_ignore("calling_protected_method")
 	print(test2._get_property_list())