mirror of
https://github.com/trezor/trezor-firmware.git
synced 2024-12-19 12:58:13 +00:00
style(core): add check for utils.INTERNAL_MODEL in (tuple)
The static replacer doesn't understand tuples (it's just a dumb sed, we'd need to teach it the python ast which is a somewhat bigger project that would also make the build slower) so instead we spell out every "utils.INTERNAL_MODEL == xyz" equality check separately. If you don't, you don't get static replacement and you're checking at run-time in every firmware for every device. This pylint will catch the problem.
This commit is contained in:
parent
384615c047
commit
43677c6afd
@ -35,3 +35,4 @@ enable=
|
|||||||
useless-else-on-loop,
|
useless-else-on-loop,
|
||||||
useless-object-inheritance,
|
useless-object-inheritance,
|
||||||
async-awaitable-return,
|
async-awaitable-return,
|
||||||
|
internal-model-tuple-comparison,
|
||||||
|
@ -1,4 +1,4 @@
|
|||||||
from astroid import AsyncFunctionDef
|
from astroid import nodes
|
||||||
from pylint.checkers import BaseChecker
|
from pylint.checkers import BaseChecker
|
||||||
from pylint.checkers.utils import check_messages
|
from pylint.checkers.utils import check_messages
|
||||||
from pylint.interfaces import IAstroidChecker
|
from pylint.interfaces import IAstroidChecker
|
||||||
@ -18,12 +18,56 @@ class AsyncAwaitableChecker(BaseChecker):
|
|||||||
}
|
}
|
||||||
|
|
||||||
@check_messages("async-awaitable-return")
|
@check_messages("async-awaitable-return")
|
||||||
def visit_asyncfunctiondef(self, node: AsyncFunctionDef):
|
def visit_asyncfunctiondef(self, node: nodes.AsyncFunctionDef):
|
||||||
# Check if the return type is explicitly an Awaitable
|
# Check if the return type is explicitly an Awaitable
|
||||||
if node.returns and "Awaitable" in node.returns.as_string():
|
if node.returns and "Awaitable" in node.returns.as_string():
|
||||||
self.add_message("async-awaitable-return", node=node, args=(node.name,))
|
self.add_message("async-awaitable-return", node=node, args=(node.name,))
|
||||||
|
|
||||||
|
|
||||||
|
class InternalModelComparisonChecker(BaseChecker):
|
||||||
|
"""Check that utils.INTERNAL_MODEL is only compared using '==' or '!='.
|
||||||
|
|
||||||
|
This is because the static replacer does not support 'in' or 'not in' comparisons,
|
||||||
|
so the comparison is compiled into all models and performed at runtime. This is
|
||||||
|
typically not what you want.
|
||||||
|
|
||||||
|
When multiple comparisons are necessary, you may need to silence another
|
||||||
|
pylint warning: # pylint: disable=consider-using-in
|
||||||
|
"""
|
||||||
|
|
||||||
|
__implements__ = IAstroidChecker
|
||||||
|
|
||||||
|
name = "internal-model-comparison-checker"
|
||||||
|
priority = -1
|
||||||
|
msgs = {
|
||||||
|
"W9998": (
|
||||||
|
"Only compare INTERNAL_MODEL using '==' or '!='.",
|
||||||
|
"internal-model-tuple-comparison",
|
||||||
|
"Used when utils.INTERNAL_MODEL is compared using 'in' or 'not in' with a tuple.",
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _is_internal_model(node):
|
||||||
|
return (
|
||||||
|
isinstance(node, nodes.Attribute)
|
||||||
|
and node.attrname == "INTERNAL_MODEL"
|
||||||
|
and isinstance(node.expr, nodes.Name)
|
||||||
|
and node.expr.name == "utils"
|
||||||
|
)
|
||||||
|
|
||||||
|
@check_messages("internal-model-tuple-comparison")
|
||||||
|
def visit_compare(self, node: nodes.Compare):
|
||||||
|
if not self._is_internal_model(node.left):
|
||||||
|
return
|
||||||
|
if len(node.ops) != 1:
|
||||||
|
return
|
||||||
|
op, _right = node.ops[0]
|
||||||
|
if op in ("in", "not in"):
|
||||||
|
self.add_message("internal-model-tuple-comparison", node=node)
|
||||||
|
|
||||||
|
|
||||||
def register(linter):
|
def register(linter):
|
||||||
"""Required method to auto register this checker."""
|
"""Required method to auto register this checker."""
|
||||||
linter.register_checker(AsyncAwaitableChecker(linter))
|
linter.register_checker(AsyncAwaitableChecker(linter))
|
||||||
|
linter.register_checker(InternalModelComparisonChecker(linter))
|
||||||
|
Loading…
Reference in New Issue
Block a user