From 43677c6afde25dedf65676a86ef727e8fea89c04 Mon Sep 17 00:00:00 2001
From: matejcik <ja@matejcik.cz>
Date: Tue, 27 Aug 2024 12:00:46 +0200
Subject: [PATCH] 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.
---
 .pylintrc                                     |  1 +
 .../trezor_pylint_plugin.py                   | 48 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

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))