test: fix universal type checker based on downstream use and more testing (#28619)

* test: fix universal type checker based on downstream use and more testing

* fix: type validation error reporting

* fix: types; various

* chore: switch off test-time type checking

still too many errors

frappe/__init__.py

@@ -540,6 +540,11 @@ def _strip_html_tags(message):
 	return strip_html_tags(message)
 
 
+ServerAction: TypeAlias = dict
+ClientAction: TypeAlias = dict
+Action: TypeAlias = ServerAction | ClientAction
+
+
 def msgprint(
 	msg: str,
 	title: str | None = None,
@@ -548,7 +553,7 @@ def msgprint(
 	as_list: bool = False,
 	indicator: Literal["blue", "green", "orange", "red", "yellow"] | None = None,
 	alert: bool = False,
-	primary_action: str | None = None,
+	primary_action: Action | None = None,
 	is_minimizable: bool = False,
 	wide: bool = False,
 	*,
@@ -1297,7 +1302,7 @@ def clear_document_cache(doctype: str, name: str | None = None) -> None:
 
 
 def get_cached_value(
-	doctype: str, name: str, fieldname: str | Iterable[str] = "name", as_dict: bool = False
+	doctype: str, name: str | dict, fieldname: str | Iterable[str] = "name", as_dict: bool = False
 ) -> Any:
 	try:
 		doc = get_cached_doc(doctype, name)
@@ -1407,12 +1412,12 @@ def get_meta_module(doctype):
 
 def delete_doc(
 	doctype: str | None = None,
-	name: str | None = None,
+	name: str | dict | None = None,
 	force: bool = False,
 	ignore_doctypes: list[str] | None = None,
 	for_reload: bool = False,
 	ignore_permissions: bool = False,
-	flags: None = None,
+	flags: _dict | None = None,
 	ignore_on_trash: bool = False,
 	ignore_missing: bool = True,
 	delete_permanently: bool = False,
@@ -2146,7 +2151,7 @@ def as_json(obj: dict | list, indent=1, separators=None, ensure_ascii=True) -> s
 
 
 def are_emails_muted():
-	return flags.mute_emails or cint(conf.get("mute_emails"))
+	return flags.mute_emails or cint(conf.get("mute_emails", 0))
 
 
 from frappe.deprecation_dumpster import frappe_get_test_records as get_test_records
@@ -2397,7 +2402,7 @@ def get_desk_link(doctype, name):
 	return html.format(doctype=doctype, name=name, doctype_local=_(doctype), title_local=_(title))
 
 
-def bold(text: str) -> str:
+def bold(text: str | int | float) -> str:
 	"""Return `text` wrapped in `<strong>` tags."""
 	return f"<strong>{text}</strong>"
 

frappe/core/doctype/recorder/recorder.py

@@ -104,8 +104,8 @@ def serialize_request(request):
 		name=request.get("uuid"),
 		number_of_queries=request.get("queries"),
 		time_in_queries=request.get("time_queries"),
-		request_headers=frappe.as_json(request.get("headers"), indent=4),
-		form_dict=frappe.as_json(request.get("form_dict"), indent=4),
+		request_headers=frappe.as_json(request.get("headers", {}), indent=4),
+		form_dict=frappe.as_json(request.get("form_dict", {}), indent=4),
 		sql_queries=request.get("calls"),
 		suggested_indexes=request.get("suggested_indexes"),
 		modified=request.get("time"),

frappe/testing/environment.py

@@ -22,6 +22,7 @@ and tear down the test environment before and after test execution.
 import functools
 import inspect
 import logging
+import pkgutil
 import unittest
 
 import tomllib
@@ -103,7 +104,7 @@ def _decorate_all_methods_and_functions_with_type_checker():
 			logger.warning(f"Failed to parse pyproject.toml for app {app_path}")
 			return {}
 
-	def _decorate_callable(obj, apps, parent_module):
+	def _decorate_callable(obj, parent_module):
 		# whitelisted methods are already checked, see frappe.whitelist
 		if getattr(obj, "__func__", obj) in frappe.whitelisted:
 			return obj
@@ -111,8 +112,9 @@ def _decorate_all_methods_and_functions_with_type_checker():
 		elif hasattr(obj, "_is_decorated_for_validate_argument_types"):
 			return obj
 		elif module := getattr(obj, "__module__", ""):
-			if (app := module.split(".", 1)[0]) and app not in apps:
-				return obj
+			# ensure that the origin skip list is honored on imports; but not the origin
+			# max_depth because they are reimported thus attached to a different namespace
+			app = module.split(".", 1)[0]
 			config = _get_config_from_pyproject(frappe.get_app_source_path(app))
 			skip_namespaces = config.get("skip_namespaces", [])
 			if any(module.startswith(n) for n in skip_namespaces):
@@ -120,47 +122,55 @@ def _decorate_all_methods_and_functions_with_type_checker():
 
 		@functools.wraps(obj)
 		def wrapper(*args, **kwargs):
-			try:
-				return validate_argument_types(obj)(*args, **kwargs)
-			except TypeError as e:
-				# breakpoint()
-				raise e
+			return validate_argument_types(obj)(*args, **kwargs)
 
 		wrapper._is_decorated_for_validate_argument_types = True
 
-		logger.debug(f"... patching {obj.__module__}.{obj.__name__} in {parent_module.__name__}")
+		if obj.__module__ != parent_module.__name__:
+			logger.debug(f"... patching {obj.__module__}.{obj.__name__} (inside {parent_module.__name__})")
+		else:
+			logger.debug(f"... patching {obj.__module__}.{obj.__name__}")
 
 		return wrapper
 
-	def _decorate_module(module, apps, current_depth, max_depth):
-		if current_depth >= max_depth:
-			return
-		if (app := module.__name__.split(".", 1)[0]) and app not in apps:
+	def _decorate_module(app, module, apps, current_depth, max_depth):
+		if current_depth > max_depth:
 			return
 		for name in dir(module):
 			obj = getattr(module, name)
 			if inspect.isfunction(obj):
-				if not hasattr(obj, "__annotations__"):
+				if not getattr(obj, "__annotations__", None):
 					continue
-				setattr(module, name, _decorate_callable(obj, apps, module))
+				# never cross the apps (plural!) boundary for functions
+				if obj.__module__.split(".", 1)[0] not in apps:
+					continue
+				setattr(module, name, _decorate_callable(obj, module))
 			elif inspect.ismodule(obj):
+				# never cross the app (singular!) boundary for modules
+				if obj.__name__.split(".", 1)[0] != app:
+					continue
 				if hasattr(obj, "_is_decorated_for_validate_argument_types"):
 					continue
 				obj._is_decorated_for_validate_argument_types = True
-				_decorate_module(obj, apps, current_depth + 1, max_depth)
+				_decorate_module(app, obj, apps, current_depth + 1, max_depth)
 
 	for app in (apps := frappe.get_installed_apps()):
 		config = _get_config_from_pyproject(frappe.get_app_source_path(app))
 		max_depth = config.get("max_module_depth", float("inf"))
-		logger.info(
-			f"Decorating callables with type validator up to module depth {max_depth+1} in {app!r} ..."
-		)
-		for module_name in frappe.local.app_modules.get(app) or []:
-			try:
-				module = frappe.get_module(f"{app}.{module_name}")
-				_decorate_module(module, apps, 0, max_depth)
-			except ImportError:
-				logger.error(f"Error importing module {app}.{module_name}")
+		skip_namespaces = config.get("skip_namespaces", [])
+		logger.info(f"Adding type validator in {app!r} (up to level {max_depth})...")
+		pkg = frappe.get_module(app)
+		_decorate_module(app, pkg, apps, 1, max_depth)
+
+		for _, submodule_name, _ in pkgutil.walk_packages(path=pkg.__path__, prefix=pkg.__name__ + "."):
+			current_depth = len(submodule_name.split("."))
+			if current_depth > max_depth:
+				continue
+			if any(submodule_name.startswith(n) for n in skip_namespaces):
+				continue
+
+			submodule = frappe.get_module(submodule_name)
+			_decorate_module(app, submodule, apps, current_depth, max_depth)
 
 
 class IntegrationTestPreparation:

frappe/utils/typing_validations.py

@@ -54,15 +54,20 @@ def qualified_name(obj) -> str:
 
 
 def raise_type_error(
-	arg_name: str, arg_type: type, arg_value: object, current_exception: Exception | None = None
+	func: callable,
+	arg_name: str,
+	arg_type: type,
+	arg_value: object,
+	current_exception: Exception | None = None,
 ):
 	"""
 	Raise a TypeError with a message that includes the name of the argument, the expected type
 	and the actual type of the value passed.
 
 	"""
+	module, qualname = func.__module__, func.__qualname__
 	raise FrappeTypeError(
-		f"Argument '{arg_name}' should be of type '{qualified_name(arg_type)}' but got "
+		f"Argument '{arg_name}' in '{module}.{qualname}' should be of type '{qualified_name(arg_type)}' but got "
 		f"'{qualified_name(arg_value)}' instead."
 	) from current_exception
 
@@ -153,10 +158,10 @@ def transform_parameter_types(func: Callable, args: tuple, kwargs: dict):
 		try:
 			current_arg_value_after = TypeAdapter(current_arg_type).validate_python(current_arg_value)
 		except (TypeError, PyValidationError) as e:
-			raise_type_error(current_arg, current_arg_type, current_arg_value, current_exception=e)
+			raise_type_error(func, current_arg, current_arg_type, current_arg_value, current_exception=e)
 
 		if isinstance(current_arg_value_after, EllipsisType):
-			raise_type_error(current_arg, current_arg_type, current_arg_value)
+			raise_type_error(func, current_arg, current_arg_type, current_arg_value)
 
 		# update the args and kwargs with possibly casted value
 		if current_arg in kwargs:

pyproject.toml

@@ -139,7 +139,7 @@ requires = ["flit_core >=3.4,<4"]
 build-backend = "flit_core.buildapi"
 
 [tool.frappe.testing.function_type_validation]
-max_module_depth = 1
+max_module_depth = 0
 skip_namespaces = [
     "frappe.deprecation_dumpster",
     "frappe.utils.typing_validations",