diff --git a/.pylintrc b/.pylintrc index e47f2bc6c..cefee09d8 100644 --- a/.pylintrc +++ b/.pylintrc @@ -35,3 +35,4 @@ enable= useless-else-on-loop, useless-object-inheritance, async-awaitable-return, + internal-model-tuple-comparison, diff --git a/tools/trezor-pylint-plugin/trezor_pylint_plugin.py b/tools/trezor-pylint-plugin/trezor_pylint_plugin.py index 4c5879d3d..0cc97e2ab 100644 --- a/tools/trezor-pylint-plugin/trezor_pylint_plugin.py +++ b/tools/trezor-pylint-plugin/trezor_pylint_plugin.py @@ -1,4 +1,4 @@ -from astroid import AsyncFunctionDef +from astroid import nodes from pylint.checkers import BaseChecker from pylint.checkers.utils import check_messages from pylint.interfaces import IAstroidChecker @@ -18,12 +18,56 @@ class AsyncAwaitableChecker(BaseChecker): } @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 if node.returns and "Awaitable" in node.returns.as_string(): 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): """Required method to auto register this checker.""" linter.register_checker(AsyncAwaitableChecker(linter)) + linter.register_checker(InternalModelComparisonChecker(linter))