From eb3f2a6057262dfa8678663a9560c99f030ae845 Mon Sep 17 00:00:00 2001
From: zaicruvoir1rominet <pouette.vraie@gmail.com>
Date: Mon, 12 Aug 2024 16:36:19 +0200
Subject: [PATCH] Applying PR comments

---
 libcst/codemod/visitors/_add_imports.py    |  6 +++---
 libcst/codemod/visitors/_remove_imports.py |  8 +++-----
 libcst/metadata/type_inference_provider.py | 19 ++++++++++++++-----
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/libcst/codemod/visitors/_add_imports.py b/libcst/codemod/visitors/_add_imports.py
index 6ca8c94a..eeab43ae 100644
--- a/libcst/codemod/visitors/_add_imports.py
+++ b/libcst/codemod/visitors/_add_imports.py
@@ -136,7 +136,7 @@ def add_needed_import(
         """
 
         if module == "__future__" and obj is None:
-            raise ImportError("Cannot import __future__ directly!")
+            raise ValueError("Cannot import __future__ directly!")
         imports = AddImportsVisitor._get_imports_from_context(context)
         imports.append(ImportItem(module, obj, asname, relative))
         context.scratch[AddImportsVisitor.CONTEXT_KEY] = imports
@@ -157,9 +157,9 @@ def __init__(
         # Verify that the imports are valid
         for imp in imps:
             if imp.module == "__future__" and imp.obj_name is None:
-                raise ImportError("Cannot import __future__ directly!")
+                raise ValueError("Cannot import __future__ directly!")
             if imp.module == "__future__" and imp.alias is not None:
-                raise ImportError("Cannot import __future__ objects with aliases!")
+                raise ValueError("Cannot import __future__ objects with aliases!")
 
         # Resolve relative imports if we have a module name
         imps = [imp.resolve_relative(self.context.full_package_name) for imp in imps]
diff --git a/libcst/codemod/visitors/_remove_imports.py b/libcst/codemod/visitors/_remove_imports.py
index 924e7973..56d900df 100644
--- a/libcst/codemod/visitors/_remove_imports.py
+++ b/libcst/codemod/visitors/_remove_imports.py
@@ -46,7 +46,7 @@ def _remove_imports_from_importfrom_stmt(
             self.context.full_package_name, import_node
         )
         if module_name is None:
-            raise ImportError("Cannot look up absolute module from relative import!")
+            raise ValueError("Cannot look up absolute module from relative import!")
 
         # We know any local names will refer to this as an alias if
         # there is one, and as the original name if there is not one
@@ -73,7 +73,7 @@ def _visit_name_attr_alike(self, node: Union[cst.Name, cst.Attribute]) -> None:
         # Look up the scope for this node, remove the import that caused it to exist.
         metadata_wrapper = self.context.wrapper
         if metadata_wrapper is None:
-            raise ImportError(
+            raise ValueError(
                 "Cannot look up import, metadata is not computed for node!"
             )
         scope_provider = metadata_wrapper.resolve(ScopeProvider)
@@ -258,9 +258,7 @@ def remove_unused_import_by_node(
                 context.full_package_name, node
             )
             if module_name is None:
-                raise ImportError(
-                    "Cannot look up absolute module from relative import!"
-                )
+                raise ValueError("Cannot look up absolute module from relative import!")
             for import_alias in names:
                 RemoveImportsVisitor.remove_unused_import(
                     context,
diff --git a/libcst/metadata/type_inference_provider.py b/libcst/metadata/type_inference_provider.py
index 32e74012..8a90c26b 100644
--- a/libcst/metadata/type_inference_provider.py
+++ b/libcst/metadata/type_inference_provider.py
@@ -14,6 +14,11 @@
 from libcst.metadata.position_provider import PositionProvider
 
 
+class TypeInferenceError(Exception):
+    """An attempt to access inferred type annotation
+    (through Pyre Query API) failed."""
+
+
 class Position(TypedDict):
     line: int
     column: int
@@ -61,14 +66,18 @@ def gen_cache(
         params = ",".join(f"path='{root_path / path}'" for path in paths)
         cmd_args = ["pyre", "--noninteractive", "query", f"types({params})"]
 
-        result = subprocess.run(cmd_args, capture_output=True, timeout=timeout)
-        result.check_returncode()
-        stdout, stderr = result.stdout.decode(), result.stderr.decode()
+        result = subprocess.run(
+            cmd_args, capture_output=True, timeout=timeout, text=True
+        )
 
         try:
-            resp = json.loads(stdout)["response"]
+            result.check_returncode()
+            resp = json.loads(result.stdout)["response"]
         except Exception as e:
-            raise Exception(f"{e}\n\nstderr:\n {stderr}\nstdout:\n {stdout}") from e
+            raise TypeInferenceError(
+                f"{e}\n\nstderr:\n {result.stderr}\nstdout:\n {result.stdout}"
+            ) from e
+
         return {path: _process_pyre_data(data) for path, data in zip(paths, resp)}
 
     def __init__(self, cache: PyreData) -> None: