Manual commits are frequent source of bugs, confusions or undefined
behaviour.
All new manual commits should be explcitly ignored with explanation on
why it's added. This will only fail for new additions. Existing ones
need to be cleaned up manually.
(cherry picked from commit 06b426e9c3)
152 lines
4.1 KiB
YAML
152 lines
4.1 KiB
YAML
# 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
|