Repository: frappe/frappe
Pull request: fix: remove mutable defaults
None of these seem harmful at first but IDK what combination of these will become a bug. 

closes https://github.com/frappe/frappe/issues/14596 

Problem statement:
mutable default arguments in sendmail function
I was reading the source and I see the sendmail function has 3 mutable default arguments:

* recipients=[]
* cc=[]
* bcc=[]

Should be an easy fix for someone, there may be more of these in the codebase.

Hints from issue discussion:
Also more cases of mutable default arguments can be found in:

* accounts/doctype/sales_invoice/pos.py (functions: get_customer_list, make_invoice)
* controller/status_updater.py (functions: get_tolerance_for)
* hub_node/__init__.py (functions: get_list, get_item_favourites)

The email one worries me the most, I have a feeling that if called multiple times it could actually email something to the wrong recipients because recipients is a mutable default argument.

Commit message: fix: mutable defaults

Patch to review:
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 9cc83d63a463..b512b23c7629 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -487,11 +487,11 @@ def get_request_header(key, default=None):
 	:param default: Default value."""
 	return request.headers.get(key, default)
 
-def sendmail(recipients=[], sender="", subject="No Subject", message="No Message",
+def sendmail(recipients=None, sender="", subject="No Subject", message="No Message",
 		as_markdown=False, delayed=True, reference_doctype=None, reference_name=None,
 		unsubscribe_method=None, unsubscribe_params=None, unsubscribe_message=None, add_unsubscribe_link=1,
 		attachments=None, content=None, doctype=None, name=None, reply_to=None, queue_separately=False,
-		cc=[], bcc=[], message_id=None, in_reply_to=None, send_after=None, expose_recipients=None,
+		cc=None, bcc=None, message_id=None, in_reply_to=None, send_after=None, expose_recipients=None,
 		send_priority=1, communication=None, retry=1, now=None, read_receipt=None, is_notification=False,
 		inline_images=None, template=None, args=None, header=None, print_letterhead=False, with_container=False):
 	"""Send email using user's default **Email Account** or global default **Email Account**.
@@ -521,6 +521,14 @@ def sendmail(recipients=[], sender="", subject="No Subject", message="No Message
 	:param header: Append header in email
 	:param with_container: Wraps email inside a styled container
 	"""
+
+	if recipients is None:
+		recipients = []
+	if cc is None:
+		cc = []
+	if bcc is None:
+		bcc = []
+
 	text_content = None
 	if template:
 		message, text_content = get_email_from_template(template, args)
diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py
index ef2bb023982c..cd20a5c0f317 100644
--- a/frappe/core/doctype/data_import/importer.py
+++ b/frappe/core/doctype/data_import/importer.py
@@ -763,7 +763,9 @@ class Column:
 	seen = []
 	fields_column_map = {}
 
-	def __init__(self, index, header, doctype, column_values, map_to_field=None, seen=[]):
+	def __init__(self, index, header, doctype, column_values, map_to_field=None, seen=None):
+		if seen is None:
+			seen = []
 		self.index = index
 		self.column_number = index + 1
 		self.doctype = doctype
diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py
index 3aff3877d64a..4550fdf0e6a2 100644
--- a/frappe/desk/form/linked_with.py
+++ b/frappe/desk/form/linked_with.py
@@ -77,7 +77,7 @@ def get_submitted_linked_docs(doctype, name, docs=None, visited=None):
 
 
 @frappe.whitelist()
-def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=[]):
+def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=None):
 	"""
 	Cancel all linked doctype, optionally ignore doctypes specified in a list.
 
@@ -85,6 +85,8 @@ def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=[]):
 		docs (json str) - It contains list of dictionaries of a linked documents.
 		ignore_doctypes_on_cancel_all (list) - List of doctypes to ignore while cancelling.
 	"""
+	if ignore_doctypes_on_cancel_all is None:
+		ignore_doctypes_on_cancel_all = []
 
 	docs = json.loads(docs)
 	if isinstance(ignore_doctypes_on_cancel_all, str):
@@ -96,7 +98,7 @@ def cancel_all_linked_docs(docs, ignore_doctypes_on_cancel_all=[]):
 		frappe.publish_progress(percent=i/len(docs) * 100, title=_("Cancelling documents"))
 
 
-def validate_linked_doc(docinfo, ignore_doctypes_on_cancel_all=[]):
+def validate_linked_doc(docinfo, ignore_doctypes_on_cancel_all=None):
 	"""
 	Validate a document to be submitted and non-exempted from auto-cancel.
 
@@ -109,7 +111,7 @@ def validate_linked_doc(docinfo, ignore_doctypes_on_cancel_all=[]):
 	"""
 
 	#ignore doctype to cancel
-	if docinfo.get("doctype") in ignore_doctypes_on_cancel_all:
+	if docinfo.get("doctype") in (ignore_doctypes_on_cancel_all or []):
 		return False
 
 	# skip non-submittable doctypes since they don't need to be cancelled
diff --git a/frappe/desk/notifications.py b/frappe/desk/notifications.py
index 2a4567ab4fb4..f79c4c6b6b62 100644
--- a/frappe/desk/notifications.py
+++ b/frappe/desk/notifications.py
@@ -216,7 +216,7 @@ def get_filters_for(doctype):
 
 @frappe.whitelist()
 @frappe.read_only()
-def get_open_count(doctype, name, items=[]):
+def get_open_count(doctype, name, items=None):
 	'''Get open count for given transactions and filters
 
 	:param doctype: Reference DocType
diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py
index 6c9fa2e93715..b0d94020dc62 100644
--- a/frappe/desk/reportview.py
+++ b/frappe/desk/reportview.py
@@ -214,11 +214,13 @@ def get_parenttype_and_fieldname(field, data):
 
 	return parenttype, fieldname
 
-def compress(data, args = {}):
+def compress(data, args=None):
 	"""separate keys and values"""
 	from frappe.desk.query_report import add_total_row
 
 	if not data: return data
+	if args is None:
+		args = {}
 	values = []
 	keys = list(data[0])
 	for row in data:
@@ -423,15 +425,20 @@ def delete_bulk(doctype, items):
 
 @frappe.whitelist()
 @frappe.read_only()
-def get_sidebar_stats(stats, doctype, filters=[]):
+def get_sidebar_stats(stats, doctype, filters=None):
+	if filters is None:
+		filters = []
 
 	return {"stats": get_stats(stats, doctype, filters)}
 
 @frappe.whitelist()
 @frappe.read_only()
-def get_stats(stats, doctype, filters=[]):
+def get_stats(stats, doctype, filters=None):
 	"""get tag info"""
 	import json
+
+	if filters is None:
+		filters = []
 	tags = json.loads(stats)
 	if filters:
 		filters = json.loads(filters)
@@ -480,12 +487,11 @@ def get_stats(stats, doctype, filters=[]):
 	return stats
 
 @frappe.whitelist()
-def get_filter_dashboard_data(stats, doctype, filters=[]):
+def get_filter_dashboard_data(stats, doctype, filters=None):
 	"""get tags info"""
 	import json
 	tags = json.loads(stats)
-	if filters:
-		filters = json.loads(filters)
+	filters = json.loads(filters or {})
 	stats = {}
 
 	columns = frappe.db.get_table_columns(doctype)
diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py
index 6a32ae6fd9a0..c25e996bd342 100755
--- a/frappe/email/email_body.py
+++ b/frappe/email/email_body.py
@@ -13,8 +13,8 @@
 
 def get_email(recipients, sender='', msg='', subject='[No Subject]',
 	text_content = None, footer=None, print_html=None, formatted=None, attachments=None,
-	content=None, reply_to=None, cc=[], bcc=[], email_account=None, expose_recipients=None,
-	inline_images=[], header=None):
+	content=None, reply_to=None, cc=None, bcc=None, email_account=None, expose_recipients=None,
+	inline_images=None, header=None):
 	""" Prepare an email with the following format:
 		- multipart/mixed
 			- multipart/alternative
@@ -25,6 +25,14 @@ def get_email(recipients, sender='', msg='', subject='[No Subject]',
 				- attachment
 	"""
 	content = content or msg
+
+	if cc is None:
+		cc = []
+	if bcc is None:
+		bcc = []
+	if inline_images is None:
+		inline_images = []
+
 	emailobj = EMail(sender, recipients, subject, reply_to=reply_to, cc=cc, bcc=bcc, email_account=email_account, expose_recipients=expose_recipients)
 
 	if not content.strip().startswith("<"):
diff --git a/frappe/email/receive.py b/frappe/email/receive.py
index a755ec5e74a0..7fab90bee3cf 100644
--- a/frappe/email/receive.py
+++ b/frappe/email/receive.py
@@ -340,7 +340,7 @@ def make_error_msg(self, msg_num, incoming_mail):
 
 		return error_msg
 
-	def update_flag(self, uid_list={}):
+	def update_flag(self, uid_list=None):
 		""" set all uids mails the flag as seen  """
 
 		if not uid_list:
diff --git a/frappe/frappeclient.py b/frappe/frappeclient.py
index e57f82b60a14..ab5897920327 100644
--- a/frappe/frappeclient.py
+++ b/frappe/frappeclient.py
@@ -286,12 +286,16 @@ def migrate_single(self, doctype):
 		doc.modified = frappe.db.get_single_value(doctype, "modified")
 		frappe.get_doc(doc).insert()
 
-	def get_api(self, method, params={}):
+	def get_api(self, method, params=None):
+		if params is None:
+			params = {}
 		res = self.session.get(self.url + "/api/method/" + method + "/",
 			params=params, verify=self.verify, headers=self.headers)
 		return self.post_process(res)
 
-	def post_api(self, method, params={}):
+	def post_api(self, method, params=None):
+		if params is None:
+			params = {}
 		res = self.session.post(self.url + "/api/method/" + method + "/",
 			params=params, verify=self.verify, headers=self.headers)
 		return self.post_process(res)
diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py
index c7f723bbdcd0..1a6892d30d8f 100644
--- a/frappe/parallel_test_runner.py
+++ b/frappe/parallel_test_runner.py
@@ -238,7 +238,9 @@ def print_result(self):
 		self.call_orchestrator('test-completed')
 		return super().print_result()
 
-	def call_orchestrator(self, endpoint, data={}):
+	def call_orchestrator(self, endpoint, data=None):
+		if data is None:
+			data = {}
 		# add repo token header
 		# build id in header
 		headers = {
diff --git a/frappe/twofactor.py b/frappe/twofactor.py
index 0df4baec500d..6ae53ed71737 100644
--- a/frappe/twofactor.py
+++ b/frappe/twofactor.py
@@ -11,9 +11,9 @@
 
 class ExpiredLoginException(Exception): pass
 
-def toggle_two_factor_auth(state, roles=[]):
+def toggle_two_factor_auth(state, roles=None):
 	'''Enable or disable 2FA in site_config and roles'''
-	for role in roles:
+	for role in roles or []:
 		role = frappe.get_doc('Role', {'role_name': role})
 		role.two_factor_auth = cint(state)
 		role.save(ignore_permissions=True)
@@ -417,4 +417,4 @@ def reset_otp_secret(user):
 		enqueue(method=frappe.sendmail, queue='short', timeout=300, event=None, is_async=True, job_name=None, now=False, **email_args)
 		return frappe.msgprint(_("OTP Secret has been reset. Re-registration will be required on next login."))
 	else:
-		return frappe.throw(_("OTP secret can only be reset by the Administrator."))
\ No newline at end of file
+		return frappe.throw(_("OTP secret can only be reset by the Administrator."))


Code review comments:
diff_hunk: @@ -480,12 +487,11 @@ def get_stats(stats, doctype, filters=[]):
 	return stats
 
 @frappe.whitelist()
-def get_filter_dashboard_data(stats, doctype, filters=[]):
+def get_filter_dashboard_data(stats, doctype, filters=None):
 	"""get tags info"""
 	import json
 	tags = json.loads(stats)
-	if filters:
-		filters = json.loads(filters)
+	filters = json.loads(filters or {})
path: frappe/desk/reportview.py
text: @user1:
```suggestion
	filters = json.loads(filters or [])
```

Type consistency?