Avoid a global state in utils.builtin_lookup and avoid reinstantiating TransformVisitor (#1563)

The global state of `utils.builtin_lookup` could be out-of-date if the builtins module had been rebuilt by AstroidManager.clear_cache(). Also, reinstantiating `TransformVisitor` in `clear_cache()` could lead to to diverging registries of transforms.

Author: jacobtylerwalls

ChangeLog

@@ -17,6 +17,11 @@ Release date: TBA
 
 * Allowed ``AstroidManager.clear_cache`` to reload necessary brain plugins.
 
+* Fixed incorrect inferences after rebuilding the builtins module, e.g. by calling
+  ``AstroidManager.clear_cache``.
+
+  Closes #1559
+
 * Rename ``ModuleSpec`` -> ``module_type`` constructor parameter to match attribute
   name and improve typing. Use ``type`` instead.
 

astroid/manager.py

@@ -9,6 +9,7 @@
 
 from __future__ import annotations
 
+import collections
 import os
 import types
 import zipimport
@@ -377,7 +378,8 @@ def clear_cache(self) -> None:
         clear_inference_tip_cache()
 
         self.astroid_cache.clear()
-        AstroidManager.brain["_transform"] = TransformVisitor()
+        # NB: not a new TransformVisitor()
+        AstroidManager.brain["_transform"].transforms = collections.defaultdict(list)
 
         for lru_cache in (
             LookupMixIn.lookup,

astroid/nodes/scoped_nodes/utils.py

@@ -8,7 +8,6 @@
 
 from __future__ import annotations
 
-import builtins
 from collections.abc import Sequence
 from typing import TYPE_CHECKING
 
@@ -18,18 +17,18 @@
     from astroid import nodes
 
 
-_builtin_astroid: nodes.Module | None = None
-
-
 def builtin_lookup(name: str) -> tuple[nodes.Module, Sequence[nodes.NodeNG]]:
     """Lookup a name in the builtin module.
 
     Return the list of matching statements and the ast for the builtin module
     """
-    # pylint: disable-next=global-statement
-    global _builtin_astroid
-    if _builtin_astroid is None:
-        _builtin_astroid = AstroidManager().ast_from_module(builtins)
+    manager = AstroidManager()
+    try:
+        _builtin_astroid = manager.builtins_module
+    except KeyError:
+        # User manipulated the astroid cache directly! Rebuild everything.
+        manager.clear_cache()
+        _builtin_astroid = manager.builtins_module
     if name == "__dict__":
         return _builtin_astroid, ()
     try:

astroid/transforms.py

@@ -20,6 +20,8 @@ class TransformVisitor:
     :meth:`~visit` with an *astroid* module and the class
     will take care of the rest, walking the tree and running the
     transforms for each encountered node.
+
+    Based on its usage in AstroidManager.brain, it should not be reinstantiated.
     """
 
     def __init__(self):

tests/unittest_manager.py

@@ -318,7 +318,7 @@ def test_borg(self) -> None:
         self.assertIs(built, second_built)
 
 
-class ClearCacheTest(unittest.TestCase, resources.AstroidCacheSetupMixin):
+class ClearCacheTest(unittest.TestCase):
     def test_clear_cache_clears_other_lru_caches(self) -> None:
         lrus = (
             astroid.nodes.node_classes.LookupMixIn.lookup,
@@ -362,6 +362,19 @@ def test_brain_plugins_reloaded_after_clearing_cache(self) -> None:
         inferred = next(format_call.infer())
         self.assertIsInstance(inferred, Const)
 
+    def test_builtins_inference_after_clearing_cache(self) -> None:
+        astroid.MANAGER.clear_cache()
+        isinstance_call = astroid.extract_node("isinstance(1, int)")
+        inferred = next(isinstance_call.infer())
+        self.assertIs(inferred.value, True)
+
+    def test_builtins_inference_after_clearing_cache_manually(self) -> None:
+        # Not recommended to manipulate this, so we detect it and call clear_cache() instead
+        astroid.MANAGER.brain["astroid_cache"].clear()
+        isinstance_call = astroid.extract_node("isinstance(1, int)")
+        inferred = next(isinstance_call.infer())
+        self.assertIs(inferred.value, True)
+
 
 if __name__ == "__main__":
     unittest.main()