brotherton-erpnext/.github/helper/semgrep_rules/frappe_correctness.yml

# This file specifies rules for correctness according to how frappe doctype data model works.

rules:
- id: frappe-modifying-but-not-comitting
  patterns:
    - pattern: |
        def $METHOD(self, ...):
          ...
          self.$ATTR = ...
    - pattern-not: |
        def $METHOD(self, ...):
          ...
          self.$ATTR = ...
          ...
          self.db_set(..., self.$ATTR, ...)
    - pattern-not: |
        def $METHOD(self, ...):
          ...
          self.$ATTR = $SOME_VAR
          ...
          self.db_set(..., $SOME_VAR, ...)
    - pattern-not: |
        def $METHOD(self, ...):
          ...
          self.$ATTR = $SOME_VAR
          ...
          self.save()
    - metavariable-regex:
        metavariable: '$ATTR'
        # this is negative look-ahead, add more attrs to ignore like (ignore|ignore_this_too|ignore_me)
        regex: '^(?!ignore_linked_doctypes|status_updater)(.*)$'
    - metavariable-regex:
        metavariable: "$METHOD"
        regex: "(on_submit|on_cancel)"
  message: |
    DocType modified in self.$METHOD. Please check if modification of self.$ATTR is commited to database.
  languages: [python]
  severity: ERROR

- id: frappe-modifying-but-not-comitting-other-method
  patterns:
  - pattern: |
      class $DOCTYPE(...):
        def $METHOD(self, ...):
          ...
          self.$ANOTHER_METHOD()
          ...

        def $ANOTHER_METHOD(self, ...):
          ...
          self.$ATTR = ...
  - pattern-not: |
      class $DOCTYPE(...):
        def $METHOD(self, ...):
          ...
          self.$ANOTHER_METHOD()
          ...

        def $ANOTHER_METHOD(self, ...):
          ...
          self.$ATTR = ...
          ...
          self.db_set(..., self.$ATTR, ...)
  - pattern-not: |
      class $DOCTYPE(...):
        def $METHOD(self, ...):
          ...
          self.$ANOTHER_METHOD()
          ...

        def $ANOTHER_METHOD(self, ...):
          ...
          self.$ATTR = $SOME_VAR
          ...
          self.db_set(..., $SOME_VAR, ...)
  - pattern-not: |
      class $DOCTYPE(...):
        def $METHOD(self, ...):
          ...
          self.$ANOTHER_METHOD()
          ...
          self.save()
        def $ANOTHER_METHOD(self, ...):
          ...
          self.$ATTR = ...
  - metavariable-regex:
      metavariable: "$METHOD"
      regex: "(on_submit|on_cancel)"
  message: |
    self.$ANOTHER_METHOD is called from self.$METHOD, check if changes to self.$ATTR are commited to database.
  languages: [python]
  severity: ERROR

- id: frappe-print-function-in-doctypes
  pattern: print(...)
  message: |
      Did you mean to leave this print statement in? Consider using msgprint or logger instead of print statement.
  languages: [python]
  severity: WARNING
  paths:
      include:
        - "*/**/doctype/*"

- id: frappe-modifying-child-tables-while-iterating
  pattern-either:
    - pattern: |
        for $ROW in self.$TABLE:
            ...
            self.remove(...)
    - pattern: |
        for $ROW in self.$TABLE:
            ...
            self.append(...)
  message: |
      Child table being modified while iterating on it.
  languages: [python]
  severity: ERROR
  paths:
      include:
        - "*/**/doctype/*"

- id: frappe-same-key-assigned-twice
  pattern-either:
    - pattern: |
        {..., $X: $A, ..., $X: $B, ...}
    - pattern: |
        dict(..., ($X, $A), ..., ($X, $B), ...)
    - pattern: |
        _dict(..., ($X, $A), ..., ($X, $B), ...)
  message: |
      key `$X` is uselessly assigned twice. This could be a potential bug.
  languages: [python]
  severity: ERROR

- id: frappe-manual-commit
  patterns:
    - pattern: frappe.db.commit()
    - pattern-not-inside: |
        try:
          ...
        except ...:
          ...
  message: |
    Manually commiting a transaction is highly discouraged. Read about the transaction model implemented by Frappe Framework before adding manual commits: https://frappeframework.com/docs/user/en/api/database#database-transaction-model If you think manual commit is required then add a comment explaining why and `// nosemgrep` on the same line.
  paths:
      exclude:
        - "**/patches/**"
        - "**/demo/**"
  languages: [python]
  severity: ERROR

- id: frappe-using-db-sql
  pattern-either:
    - pattern: frappe.db.sql(...)
    - pattern: frappe.db.sql_ddl(...)
    - pattern: frappe.db.sql_list(...)
  paths:
    exclude:
      - "test_*.py"
  message: |
    The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database)
  languages: [python]
  severity: ERROR