Use a custom error type for symbol-import results (#4688)

crates/ruff/src/importer/mod.rs

@@ -1,6 +1,8 @@
 //! Add and modify import statements to make module members available during fix execution.
 
-use anyhow::{bail, Result};
+use std::error::Error;
+
+use anyhow::Result;
 use libcst_native::{Codegen, CodegenState, ImportAlias, Name, NameOrAttribute};
 use ruff_text_size::TextSize;
 use rustpython_parser::ast::{self, Ranged, Stmt, Suite};
@@ -69,16 +71,10 @@ impl<'a> Importer<'a> {
         member: &str,
         at: TextSize,
         semantic_model: &SemanticModel,
-    ) -> Result<(Edit, String)> {
+    ) -> Result<(Edit, String), ResolutionError> {
         match self.get_symbol(module, member, at, semantic_model) {
+            Some(result) => result,
             None => self.import_symbol(module, member, at, semantic_model),
-            Some(Resolution::Success(edit, binding)) => Ok((edit, binding)),
-            Some(Resolution::LateBinding) => {
-                bail!("Unable to use existing symbol due to late binding")
-            }
-            Some(Resolution::IncompatibleContext) => {
-                bail!("Unable to use existing symbol due to incompatible context")
-            }
         }
     }
 
@@ -89,7 +85,7 @@ impl<'a> Importer<'a> {
         member: &str,
         at: TextSize,
         semantic_model: &SemanticModel,
-    ) -> Option<Resolution> {
+    ) -> Option<Result<(Edit, String), ResolutionError>> {
         // If the symbol is already available in the current scope, use it.
         let imported_name = semantic_model.resolve_qualified_import_name(module, member)?;
 
@@ -101,13 +97,13 @@ impl<'a> Importer<'a> {
         // unclear whether should add an import statement at the top of the file, since it could
         // be shadowed between the import and the current location.
         if imported_name.range().start() > at {
-            return Some(Resolution::LateBinding);
+            return Some(Err(ResolutionError::ImportAfterUsage));
         }
 
         // If the symbol source (i.e., the import statement) is in a typing-only context, but we're
         // in a runtime context, abort.
         if imported_name.context().is_typing() && semantic_model.execution_context().is_runtime() {
-            return Some(Resolution::IncompatibleContext);
+            return Some(Err(ResolutionError::IncompatibleContext));
         }
 
         // We also add a no-op edit to force conflicts with any other fixes that might try to
@@ -130,7 +126,7 @@ impl<'a> Importer<'a> {
             self.locator.slice(imported_name.range()).to_string(),
             imported_name.range(),
         );
-        Some(Resolution::Success(import_edit, imported_name.into_name()))
+        Some(Ok((import_edit, imported_name.into_name())))
     }
 
     /// Generate an [`Edit`] to reference the given symbol. Returns the [`Edit`] necessary to make
@@ -145,16 +141,18 @@ impl<'a> Importer<'a> {
         member: &str,
         at: TextSize,
         semantic_model: &SemanticModel,
-    ) -> Result<(Edit, String)> {
+    ) -> Result<(Edit, String), ResolutionError> {
         if let Some(stmt) = self.find_import_from(module, at) {
             // Case 1: `from functools import lru_cache` is in scope, and we're trying to reference
             // `functools.cache`; thus, we add `cache` to the import, and return `"cache"` as the
             // bound name.
             if semantic_model.is_unbound(member) {
-                let import_edit = self.add_member(stmt, member)?;
+                let Ok(import_edit) = self.add_member(stmt, member) else {
+                    return Err(ResolutionError::InvalidEdit);
+                };
                 Ok((import_edit, member.to_string()))
             } else {
-                bail!("Unable to insert `{member}` into scope due to name conflict")
+                Err(ResolutionError::ConflictingName(member.to_string()))
             }
         } else {
             // Case 2: No `functools` import is in scope; thus, we add `import functools`, and
@@ -163,7 +161,7 @@ impl<'a> Importer<'a> {
                 let import_edit = self.add_import(&AnyImport::Import(Import::module(module)), at);
                 Ok((import_edit, format!("{module}.{member}")))
             } else {
-                bail!("Unable to insert `{module}` into scope due to name conflict")
+                Err(ResolutionError::ConflictingName(module.to_string()))
             }
         }
     }
@@ -224,12 +222,38 @@ impl<'a> Importer<'a> {
     }
 }
 
-enum Resolution {
-    /// The symbol is available for use.
-    Success(Edit, String),
+/// The result of an [`Importer::get_or_import_symbol`] call.
+#[derive(Debug)]
+pub(crate) enum ResolutionError {
     /// The symbol is imported, but the import came after the current location.
-    LateBinding,
+    ImportAfterUsage,
     /// The symbol is imported, but in an incompatible context (e.g., in typing-only context, while
     /// we're in a runtime context).
     IncompatibleContext,
+    /// The symbol can't be imported, because another symbol is bound to the same name.
+    ConflictingName(String),
+    /// The symbol can't be imported due to an error in editing an existing import statement.
+    InvalidEdit,
 }
+
+impl std::fmt::Display for ResolutionError {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ResolutionError::ImportAfterUsage => {
+                fmt.write_str("Unable to use existing symbol due to late binding")
+            }
+            ResolutionError::IncompatibleContext => {
+                fmt.write_str("Unable to use existing symbol due to incompatible context")
+            }
+            ResolutionError::ConflictingName(binding) => std::write!(
+                fmt,
+                "Unable to insert `{binding}` into scope due to name conflict"
+            ),
+            ResolutionError::InvalidEdit => {
+                fmt.write_str("Unable to modify existing import statement")
+            }
+        }
+    }
+}
+
+impl Error for ResolutionError {}