resolved conflict

Author: Lazy-Rabbit-2001

doc/classes/ProjectSettings.xml

@@ -509,12 +509,18 @@
 			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/access_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 class member is accessed from external places, such as accessing a private member from other classes that are not derived from the class where the member 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/call_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 class method is called from external places, such as calling a private method from other classes that are not derived from the class where the method 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>

modules/gdscript/gdscript_analyzer.cpp

@@ -2035,6 +2035,33 @@ void GDScriptAnalyzer::decide_suite_type(GDScriptParser::Node *p_suite, GDScript
 	}
 }
 
+#ifdef DEBUG_ENABLED
+void GDScriptAnalyzer::check_access_private_member(GDScriptParser::IdentifierNode *p_identifier, const GDScriptParser::DataType &p_datatype, const bool p_is_call) {
+	if (p_identifier == nullptr) {
+		return;
+	}
+	if (!String(p_identifier->name).begins_with("_")) {
+		return;
+	}
+	if (p_datatype.kind != GDScriptParser::DataType::Kind::CLASS && p_datatype.kind != GDScriptParser::DataType::Kind::SCRIPT) {
+		return; // Accessing from self
+	}
+	if (parser->current_function && parser->current_function->body && parser->current_function->body->has_local(p_identifier->name)) {
+		return;
+	}
+
+	GDScriptParser::DataType target_resolved = type_from_metatype(p_datatype);
+	if (target_resolved.class_type != nullptr && !target_resolved.class_type->has_member(p_identifier->name)) {
+		return; // Accessing an inexisting method
+	}
+	if (is_type_compatible(target_resolved, type_from_metatype(parser->current_class->get_datatype()))) {
+		return;
+	}
+
+	parser->push_warning(p_identifier, p_is_call ? GDScriptWarning::CALL_PRIVATE_METHOD : GDScriptWarning::ACCESS_PRIVATE_MEMBER, p_identifier->name);
+}
+#endif // DEBUG_ENABLED
+
 void GDScriptAnalyzer::resolve_suite(GDScriptParser::SuiteNode *p_suite) {
 	for (int i = 0; i < p_suite->statements.size(); i++) {
 		GDScriptParser::Node *stmt = p_suite->statements[i];
@@ -3538,10 +3565,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, p_call->get_datatype(), 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, p_call->get_datatype(), true);
+#endif
 	} else if (callee_type == GDScriptParser::Node::SUBSCRIPT) {
 		GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(p_call->callee);
 		if (subscript->base == nullptr) {
@@ -3575,6 +3611,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, subscript->base->get_datatype(), true);
+#endif
 	} else {
 		// Invalid call. Error already sent in parser.
 		// TODO: Could check if Callable here too.
@@ -4506,6 +4545,10 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
 			}
 		}
 
+#ifdef DEBUG_ENABLED
+		check_access_private_member(p_identifier, p_identifier->get_datatype());
+#endif
+
 		return;
 	}
 
@@ -4867,6 +4910,10 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri
 			}
 			result_type.kind = GDScriptParser::DataType::VARIANT;
 		}
+
+#ifdef DEBUG_ENABLED
+		check_access_private_member(p_subscript->attribute, p_subscript->base->get_datatype());
+#endif
 	} else {
 		if (p_subscript->index == nullptr) {
 			return;

modules/gdscript/gdscript_analyzer.h

@@ -71,6 +71,10 @@ class GDScriptAnalyzer {
 
 	void decide_suite_type(GDScriptParser::Node *p_suite, GDScriptParser::Node *p_statement);
 
+#ifdef DEBUG_ENABLED
+	void check_access_private_member(GDScriptParser::IdentifierNode *p_identifier, const GDScriptParser::DataType &p_datatype, const bool p_is_call = false);
+#endif
+
 	void resolve_annotation(GDScriptParser::AnnotationNode *p_annotation);
 	void resolve_class_member(GDScriptParser::ClassNode *p_class, const StringName &p_name, const GDScriptParser::Node *p_source = nullptr);
 	void resolve_class_member(GDScriptParser::ClassNode *p_class, int p_index, const GDScriptParser::Node *p_source = nullptr);

modules/gdscript/gdscript_warning.cpp

@@ -162,6 +162,12 @@ String GDScriptWarning::get_message() const {
 			return vformat(R"*(The default value uses "%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 ACCESS_PRIVATE_MEMBER:
+			CHECK_SYMBOLS(1);
+			return vformat(R"(Trying to access a private member "%s" from an external place, which would cause problems during runtime.)", symbols[0]);
+		case CALL_PRIVATE_METHOD:
+			CHECK_SYMBOLS(1);
+			return vformat(R"*(Trying to call a private method "%s()" from an external place, which would cause problems during runtime.)*", 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.
@@ -194,50 +200,52 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 	ERR_FAIL_COND_V(p_code < 0 || p_code >= WARNING_MAX, String());
 
 	static const char *names[] = {
-		PNAME("UNASSIGNED_VARIABLE"),
-		PNAME("UNASSIGNED_VARIABLE_OP_ASSIGN"),
-		PNAME("UNUSED_VARIABLE"),
-		PNAME("UNUSED_LOCAL_CONSTANT"),
-		PNAME("UNUSED_PRIVATE_CLASS_VARIABLE"),
-		PNAME("UNUSED_PARAMETER"),
-		PNAME("UNUSED_SIGNAL"),
-		PNAME("SHADOWED_VARIABLE"),
-		PNAME("SHADOWED_VARIABLE_BASE_CLASS"),
-		PNAME("SHADOWED_GLOBAL_IDENTIFIER"),
-		PNAME("UNREACHABLE_CODE"),
-		PNAME("UNREACHABLE_PATTERN"),
-		PNAME("STANDALONE_EXPRESSION"),
-		PNAME("STANDALONE_TERNARY"),
-		PNAME("INCOMPATIBLE_TERNARY"),
-		PNAME("UNTYPED_DECLARATION"),
-		PNAME("INFERRED_DECLARATION"),
-		PNAME("UNSAFE_PROPERTY_ACCESS"),
-		PNAME("UNSAFE_METHOD_ACCESS"),
-		PNAME("UNSAFE_CAST"),
-		PNAME("UNSAFE_CALL_ARGUMENT"),
-		PNAME("UNSAFE_VOID_RETURN"),
-		PNAME("RETURN_VALUE_DISCARDED"),
-		PNAME("STATIC_CALLED_ON_INSTANCE"),
-		PNAME("MISSING_TOOL"),
-		PNAME("REDUNDANT_STATIC_UNLOAD"),
-		PNAME("REDUNDANT_AWAIT"),
-		PNAME("ASSERT_ALWAYS_TRUE"),
-		PNAME("ASSERT_ALWAYS_FALSE"),
-		PNAME("INTEGER_DIVISION"),
-		PNAME("NARROWING_CONVERSION"),
-		PNAME("INT_AS_ENUM_WITHOUT_CAST"),
-		PNAME("INT_AS_ENUM_WITHOUT_MATCH"),
-		PNAME("ENUM_VARIABLE_WITHOUT_DEFAULT"),
-		PNAME("EMPTY_FILE"),
-		PNAME("DEPRECATED_KEYWORD"),
-		PNAME("CONFUSABLE_IDENTIFIER"),
-		PNAME("CONFUSABLE_LOCAL_DECLARATION"),
-		PNAME("CONFUSABLE_LOCAL_USAGE"),
-		PNAME("CONFUSABLE_CAPTURE_REASSIGNMENT"),
-		PNAME("INFERENCE_ON_VARIANT"),
-		PNAME("NATIVE_METHOD_OVERRIDE"),
-		PNAME("GET_NODE_DEFAULT_WITHOUT_ONREADY"),
-		PNAME("ONREADY_WITH_EXPORT"),
+		"UNASSIGNED_VARIABLE",
+		"UNASSIGNED_VARIABLE_OP_ASSIGN",
+		"UNUSED_VARIABLE",
+		"UNUSED_LOCAL_CONSTANT",
+		"UNUSED_PRIVATE_CLASS_VARIABLE",
+		"UNUSED_PARAMETER",
+		"UNUSED_SIGNAL",
+		"SHADOWED_VARIABLE",
+		"SHADOWED_VARIABLE_BASE_CLASS",
+		"SHADOWED_GLOBAL_IDENTIFIER",
+		"UNREACHABLE_CODE",
+		"UNREACHABLE_PATTERN",
+		"STANDALONE_EXPRESSION",
+		"STANDALONE_TERNARY",
+		"INCOMPATIBLE_TERNARY",
+		"UNTYPED_DECLARATION",
+		"INFERRED_DECLARATION",
+		"UNSAFE_PROPERTY_ACCESS",
+		"UNSAFE_METHOD_ACCESS",
+		"UNSAFE_CAST",
+		"UNSAFE_CALL_ARGUMENT",
+		"UNSAFE_VOID_RETURN",
+		"RETURN_VALUE_DISCARDED",
+		"STATIC_CALLED_ON_INSTANCE",
+		"MISSING_TOOL",
+		"REDUNDANT_STATIC_UNLOAD",
+		"REDUNDANT_AWAIT",
+		"ASSERT_ALWAYS_TRUE",
+		"ASSERT_ALWAYS_FALSE",
+		"INTEGER_DIVISION",
+		"NARROWING_CONVERSION",
+		"INT_AS_ENUM_WITHOUT_CAST",
+		"INT_AS_ENUM_WITHOUT_MATCH",
+		"ENUM_VARIABLE_WITHOUT_DEFAULT",
+		"EMPTY_FILE",
+		"DEPRECATED_KEYWORD",
+		"CONFUSABLE_IDENTIFIER",
+		"CONFUSABLE_LOCAL_DECLARATION",
+		"CONFUSABLE_LOCAL_USAGE",
+		"CONFUSABLE_CAPTURE_REASSIGNMENT",
+		"INFERENCE_ON_VARIANT",
+		"NATIVE_METHOD_OVERRIDE",
+		"GET_NODE_DEFAULT_WITHOUT_ONREADY",
+		"ONREADY_WITH_EXPORT",
+		"ACCESS_PRIVATE_MEMBER",
+		"CALL_PRIVATE_METHOD",
 #ifndef DISABLE_DEPRECATED
 		"PROPERTY_USED_AS_FUNCTION",
 		"CONSTANT_USED_AS_FUNCTION",

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

@@ -15,6 +15,7 @@ class SuperMethodsRecognized extends BaseClass:
 		return result
 
 func test():
+	@warning_ignore_start("call_private_method")
 	var test1 = SuperClassMethodsRecognized.new()
 	print(test1._get_property_list()) # Calls base class's method.
 	var test2 = SuperMethodsRecognized.new()

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

@@ -151,6 +151,30 @@ func test_unsafe_void_return() -> void:
 func get_class():
 	pass
 
+class PrivA:
+    var _t = 1
+
+    func _priv_method():
+        _t = 5
+
+class PrivB:
+    var a = PrivA.new()
+
+    func _foo():
+		@warning_ignore("access_private_member")
+        a._t = 2
+		@warning_ignore("call_private_method")
+        a._priv_method()
+
+class PrivC extends PrivA:
+    var a = PrivA.new()
+
+    func _foo():
+		@warning_ignore("access_private_member")
+        a._t = 2
+		@warning_ignore("call_private_method")
+        a._priv_method()
+
 # We don't want to execute it because of errors, just analyze.
 func test():
 	pass

modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.out

@@ -1 +1,2 @@
-GDTEST_OK
+GDTEST_PARSER_ERROR
+Used space character for indentation instead of tab as used before in the file.

modules/gdscript/tests/scripts/analyzer/warnings/access_modifier_soft.gd

@@ -0,0 +1,22 @@
+class A:
+    var _t = 1
+
+    func _priv_method():
+        _t = 5
+
+class B:
+    var a = A.new()
+
+    func _foo():
+        a._t = 2
+        a._priv_method()
+
+class C extends A:
+    var a = A.new()
+
+    func _foo():
+        a._t = 2
+        a._priv_method()
+
+func test():
+    pass

modules/gdscript/tests/scripts/analyzer/warnings/access_modifier_soft.out

@@ -0,0 +1,3 @@
+GDTEST_OK
+~~ WARNING at line 11: (ACCESS_PRIVATE_MEMBER) Trying to access a private member "_t" from an external place, which would cause problems during runtime.
+~~ WARNING at line 12: (CALL_PRIVATE_METHOD) Trying to call a private method "_priv_method()" from an external place, which would cause problems during runtime.

modules/gdscript/tests/scripts/runtime/features/onready_base_before_subclass.gd

@@ -14,5 +14,6 @@ class B extends A:
 
 func test():
 	var node := B.new()
+	@warning_ignore("call_private_method")
 	node._ready()
 	node.free()