Merge pull request frappe#27288 from ankush/strict_linting

chore: cleanup linting errors and introduce strict linting checks

.git-blame-ignore-revs

@@ -16,3 +16,4 @@
 
 # Whitespace fix throughout codebase
 4551d7d6029b6f587f6c99d4f8df5519241c6a86
+b147b85e6ac19a9220cd1e2958a6ebd99373283a

.github/helper/.flake8_strict

@@ -0,0 +1,72 @@
+[flake8]
+ignore =
+    B007,
+    B950,
+    E101,
+    E111,
+    E114,
+    E116,
+    E117,
+    E121,
+    E122,
+    E123,
+    E124,
+    E125,
+    E126,
+    E127,
+    E128,
+    E131,
+    E201,
+    E202,
+    E203,
+    E211,
+    E221,
+    E222,
+    E223,
+    E224,
+    E225,
+    E226,
+    E228,
+    E231,
+    E241,
+    E242,
+    E251,
+    E261,
+    E262,
+    E265,
+    E266,
+    E271,
+    E272,
+    E273,
+    E274,
+    E301,
+    E302,
+    E303,
+    E305,
+    E306,
+    E401,
+    E402,
+    E501,
+    E502,
+    E701,
+    E702,
+    E703,
+    E741,
+    F401,
+    F403,
+    W191,
+    W291,
+    W292,
+    W293,
+    W391,
+    W503,
+    W504,
+    E711,
+    E129,
+    F841,
+    E713,
+    E712,
+
+
+max-line-length = 200
+exclude=.github/helper/semgrep_rules,test_*.py

.github/workflows/semgrep.yml → .github/workflows/linters.yml

@@ -1,11 +1,12 @@
-name: Semgrep
+name: Linters
 
 on:
   pull_request: { }
 
 jobs:
-  semgrep:
-    name: Frappe Linter
+
+  linters:
+    name: linters
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -16,3 +17,11 @@ jobs:
             config: >-
               r/python.lang.correctness
               .github/helper/semgrep_rules
+
+      - name: Set up Python 3.8
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.8
+
+      - name: Install and Run Pre-commit
+        uses: pre-commit/action@v2.0.0

.pre-commit-config.yaml

@@ -0,0 +1,29 @@
+exclude: 'node_modules|.git'
+default_stages: [commit]
+fail_fast: false
+
+
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.0.1
+    hooks:
+      - id: trailing-whitespace
+        files: "erpnext.*"
+        exclude: ".*json$|.*txt$|.*csv|.*md"
+      - id: check-yaml
+      - id: no-commit-to-branch
+        args: ['--branch', 'develop']
+      - id: check-merge-conflict
+      - id: check-ast
+
+  - repo: https://gitlab.com/pycqa/flake8
+    rev: 3.9.2
+    hooks:
+      - id: flake8
+        args: ['--config', '.github/helper/.flake8_strict']
+        exclude: ".*setup.py$"
+
+ci:
+    autoupdate_schedule: weekly
+    skip: []
+    submodules: false

erpnext/accounts/custom/address.py

@@ -31,7 +31,7 @@ def on_update(self):
 		customers = frappe.db.get_all("Customer", filters=filters, as_list=True)
 		for customer_name in customers:
 			frappe.db.set_value("Customer", customer_name[0], "primary_address", address_display)
-			
+
 @frappe.whitelist()
 def get_shipping_address(company, address = None):
 	filters = [

erpnext/accounts/deferred_revenue.py

@@ -359,7 +359,7 @@ def make_gl_entries(doc, credit_account, debit_account, against,
 		try:
 			make_gl_entries(gl_entries, cancel=(doc.docstatus == 2), merge_entries=True)
 			frappe.db.commit()
-		except:
+		except Exception:
 			frappe.db.rollback()
 			traceback = frappe.get_traceback()
 			frappe.log_error(message=traceback)
@@ -430,7 +430,7 @@ def book_revenue_via_journal_entry(doc, credit_account, debit_account, against,
 
 		if submit:
 			journal_entry.submit()
-	except:
+	except Exception:
 		frappe.db.rollback()
 		traceback = frappe.get_traceback()
 		frappe.log_error(message=traceback)

erpnext/accounts/doctype/bank_transaction/bank_transaction.py

@@ -21,7 +21,7 @@ def on_update_after_submit(self):
 		self.update_allocations()
 		self.clear_linked_payment_entries()
 		self.set_status(update=True)
-	
+
 	def on_cancel(self):
 		self.clear_linked_payment_entries(for_cancel=True)
 		self.set_status(update=True)
@@ -45,7 +45,7 @@ def update_allocations(self):
 			frappe.db.set_value(self.doctype, self.name, "status", "Reconciled")
 
 		self.reload()
-	
+
 	def clear_linked_payment_entries(self, for_cancel=False):
 		for payment_entry in self.payment_entries:
 			if payment_entry.payment_document in ["Payment Entry", "Journal Entry", "Purchase Invoice", "Expense Claim"]:
@@ -77,7 +77,7 @@ def clear_sales_invoice(self, payment_entry, for_cancel=False):
 
 def get_reconciled_bank_transactions(payment_entry):
 	reconciled_bank_transactions = frappe.get_all(
-		'Bank Transaction Payments', 
+		'Bank Transaction Payments',
 		filters = {
 			'payment_entry': payment_entry.payment_entry
 		},

erpnext/accounts/doctype/finance_book/test_finance_book.py

@@ -40,4 +40,4 @@ def create_finance_book():
 	else:
 		finance_book = frappe.get_doc("Finance Book", "_Test Finance Book")
 
-	return finance_book
+	return finance_book

erpnext/accounts/doctype/party_link/party_link.py

@@ -10,14 +10,14 @@ def validate(self):
 		if self.primary_role not in ['Customer', 'Supplier']:
 			frappe.throw(_("Allowed primary roles are 'Customer' and 'Supplier'. Please select one of these roles only."),
 				title=_("Invalid Primary Role"))
-		
+
 		existing_party_link = frappe.get_all('Party Link', {
 			'primary_party': self.secondary_party
 		}, pluck="primary_role")
 		if existing_party_link:
 			frappe.throw(_('{} {} is already linked with another {}')
 				.format(self.secondary_role, self.secondary_party, existing_party_link[0]))
-		
+
 		existing_party_link = frappe.get_all('Party Link', {
 			'secondary_party': self.primary_party
 		}, pluck="primary_role")

erpnext/accounts/doctype/payment_entry/payment_entry.py

@@ -484,7 +484,7 @@ def set_amounts(self):
 
 	def validate_amounts(self):
 		self.validate_received_amount()
-	
+
 	def validate_received_amount(self):
 		if self.paid_from_account_currency == self.paid_to_account_currency:
 			if self.paid_amount != self.received_amount:

erpnext/accounts/doctype/period_closing_voucher/period_closing_voucher.py

@@ -54,7 +54,7 @@ def make_gl_entries(self):
 		if gl_entries:
 			from erpnext.accounts.general_ledger import make_gl_entries
 			make_gl_entries(gl_entries)
-	
+
 	def get_gl_entries(self):
 		gl_entries = []
 		pl_accounts = self.get_pl_balances()
@@ -77,7 +77,7 @@ def get_gl_entries(self):
 			gl_entries += gle_for_net_pl_bal
 
 		return gl_entries
-	
+
 	def get_pnl_gl_entry(self, pl_accounts):
 		company_cost_center = frappe.db.get_value("Company", self.company, "cost_center")
 		gl_entries = []

erpnext/accounts/doctype/pricing_rule/pricing_rule.py

@@ -203,13 +203,13 @@ def apply_pricing_rule(args, doc=None):
 	serialized_items = dict()
 	for item_code, val in query_items:
 		serialized_items.setdefault(item_code, val)
-	
+
 	for item in item_list:
 		args_copy = copy.deepcopy(args)
 		args_copy.update(item)
 		data = get_pricing_rule_for_item(args_copy, item.get('price_list_rate'), doc=doc)
 		out.append(data)
-		
+
 		if serialized_items.get(item.get('item_code')) and not item.get("serial_no") and set_serial_nos_based_on_fifo and not args.get('is_return'):
 			out[0].update(get_serial_no_for_item(args_copy))
 
@@ -315,9 +315,8 @@ def update_args_for_pricing_rule(args):
 	if not (args.item_group and args.brand):
 		try:
 			args.item_group, args.brand = frappe.get_cached_value("Item", args.item_code, ["item_group", "brand"])
-		except TypeError:
-			# invalid item_code
-			return item_details
+		except frappe.DoesNotExistError:
+			return
 		if not args.item_group:
 			frappe.throw(_("Item Group not mentioned in item master for item {0}").format(args.item_code))
 

erpnext/accounts/doctype/pricing_rule/utils.py

@@ -81,7 +81,7 @@ def filter_pricing_rule_based_on_condition(pricing_rules, doc=None):
 				try:
 					if frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()):
 						filtered_pricing_rules.append(pricing_rule)
-				except:
+				except Exception:
 					pass
 			else:
 				filtered_pricing_rules.append(pricing_rule)

erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py

@@ -158,7 +158,7 @@ def get_recipients_and_cc(customer, doc):
 	if doc.cc_to != '':
 		try:
 			cc=[frappe.get_value('User', doc.cc_to, 'email')]
-		except:
+		except Exception:
 			pass
 
 	return recipients, cc

erpnext/accounts/doctype/sales_invoice/sales_invoice.py

@@ -24,7 +24,6 @@
 from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category import get_party_tax_withholding_details
 from frappe.model.utils import get_fetch_values
 from frappe.contacts.doctype.address.address import get_address_display
-from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category import get_party_tax_withholding_details
 
 from erpnext.healthcare.utils import manage_invoice_submit_cancel
 

erpnext/accounts/doctype/sales_taxes_and_charges_template/sales_taxes_and_charges_template.js

@@ -4,4 +4,3 @@
 cur_frm.cscript.tax_table = "Sales Taxes and Charges";
 
 {% include "erpnext/public/js/controllers/accounts.js" %}
-

erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py

@@ -240,7 +240,7 @@ def get_deducted_tax(taxable_vouchers, fiscal_year, tax_details):
 def get_tds_amount(ldc, parties, inv, tax_details, fiscal_year_details, tax_deducted, vouchers):
 	tds_amount = 0
 	invoice_filters = {
-		'name': ('in', vouchers), 
+		'name': ('in', vouchers),
 		'docstatus': 1,
 		'apply_tds': 1
 	}

erpnext/accounts/party.py

@@ -648,7 +648,7 @@ def get_default_contact(doctype, name):
 	if out:
 		try:
 			return out[0][0]
-		except:
+		except Exception:
 			return None
 	else:
 		return None

erpnext/accounts/report/financial_statements.py

@@ -339,7 +339,7 @@ def sort_accounts(accounts, is_root=False, key="name"):
 	"""Sort root types as Asset, Liability, Equity, Income, Expense"""
 
 	def compare_accounts(a, b):
-		if re.split('\W+', a[key])[0].isdigit():
+		if re.split(r'\W+', a[key])[0].isdigit():
 			# if chart of accounts is numbered, then sort by number
 			return cmp(a[key], b[key])
 		elif is_root:

erpnext/agriculture/doctype/disease/disease.py

@@ -6,7 +6,6 @@
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe import _
 
 class Disease(Document):
 	def validate(self):

erpnext/agriculture/doctype/soil_texture/soil_texture.py

@@ -7,7 +7,6 @@
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import flt, cint
-from frappe import _
 
 class SoilTexture(Document):
 	soil_edit_order = [2, 1, 0]

erpnext/agriculture/doctype/water_analysis/water_analysis.py

@@ -6,7 +6,6 @@
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe import _
 
 class WaterAnalysis(Document):
 	@frappe.whitelist()

erpnext/assets/doctype/asset/asset.py

@@ -11,7 +11,7 @@
 from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account
 from erpnext.assets.doctype.asset.depreciation \
 	import get_disposal_account_and_cost_center, get_depreciation_accounts
-from erpnext.accounts.general_ledger import make_gl_entries, make_reverse_gl_entries
+from erpnext.accounts.general_ledger import make_reverse_gl_entries
 from erpnext.accounts.utils import get_account_currency
 from erpnext.controllers.accounts_controller import AccountsController
 
@@ -546,7 +546,7 @@ def get_cwip_account(self, cwip_enabled=False):
 		cwip_account = None
 		try:
 			cwip_account = get_asset_account("capital_work_in_progress_account", self.name, self.asset_category, self.company)
-		except:
+		except Exception:
 			# if no cwip account found in category or company and "cwip is enabled" then raise else silently pass
 			if cwip_enabled:
 				raise

erpnext/controllers/sales_and_purchase_return.py

@@ -63,7 +63,7 @@ def validate_returned_items(doc):
 
 	if doc.doctype in ("Delivery Note", "Sales Invoice"):
 		for d in frappe.db.sql("""select item_code, qty, serial_no, batch_no from `tabPacked Item`
-			where parent = %s""".format(doc.doctype), doc.return_against, as_dict=1):
+			where parent = %s""", doc.return_against, as_dict=1):
 				valid_items = get_ref_item_dict(valid_items, d)
 
 	already_returned_items = get_already_returned_items(doc)

erpnext/controllers/taxes_and_totals.py

@@ -686,7 +686,7 @@ def set_total_amount_to_default_mop(self, total_amount_to_pay):
 				'mode_of_payment': default_mode_of_payment.mode_of_payment,
 				'amount': total_amount_to_pay,
 				'default': 1
-			})	
+			})
 
 def get_itemised_tax_breakup_html(doc):
 	if not doc.taxes:

erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py

@@ -28,10 +28,10 @@ def validate_availability_of_slots(self):
 			to_time = datetime.datetime.strptime(
 				self.min_date+record.to_time, self.format_string)
 			timedelta = to_time-from_time
-			self.validate_from_and_to_time(from_time, to_time)
+			self.validate_from_and_to_time(from_time, to_time, record)
 			self.duration_is_divisible(from_time, to_time)
 
-	def validate_from_and_to_time(self, from_time, to_time):
+	def validate_from_and_to_time(self, from_time, to_time, record):
 		if from_time > to_time:
 			err_msg = _('<b>From Time</b> cannot be later than <b>To Time</b> for {0}').format(record.day_of_week)
 			frappe.throw(_(err_msg))