Preventing typical mistakes when developing Frappe Framework apps

Over last year we have developed 10s of rules for preventing typical mistakes that developers make while creating Frappe Framework apps: GitHub - frappe/semgrep-rules: Semgrep rules specific to Frappe Framework

Some are silly, some are very specific to implementation details that not many are aware of.
Examples:

frappe_correctness.py
   frappe-breaks-multitenancy
      variable global variable does not respect database multitenancy, consider wrapping it in
      function or method call.

        9| variable = frappe.db.get_value("ABC", "x", "y")
         |----------------------------------------
   frappe-modifying-but-not-comitting-other-method
      self.tainted_method is called from self.on_submit, check if changes to self.status are
      commited to database.

       86| class DoctypeClass(Document):
       87|    def on_submit(self):
       88|            self.good_method()
       89|            self.tainted_method()
       90|
       91|    def tainted_method(self):
       92|            self.status = "uptate"
         |----------------------------------------
   frappe-overriding-local-proxies
      frappe.db is a local proxy, assigning it to another object will remove the proxying and
      replace it with another object. Use frappe.local.db instead.

      123| frappe.db = Database()
         |----------------------------------------
   frappe-query-debug-statement
      Did you mean to leave this debug statement in?

      117| frappe.db.get_value("DocType", "name", debug=True)
         |----------------------------------------
      120| frappe.db.get_value("DocType", "name", debug=1)
         |----------------------------------------
   frappe-single-value-type-safety
      If "System Settings" is a single doctype then using `frappe.db.get_value` is discouraged for
      fetching value from single doctypes. frappe.db.get_value for single doctype is not type
      safe, use `frappe.db.get_single_value` instead.

      135| duration = frappe.db.get_value("System Settings", None, "duration") or 24


You can also use this in your custom apps:


If you know any more such rules/heuristics that can be added feel free to send a PR. If you’re unfamiliar with semgrep you can still create an issue with the description of the rule and if it’s valid we will convert it to semgrep rule.

5 Likes