dissolve_python/

rustpython_visitor.rs

// Copyright (C) 2024 Jelmer Vernooij <jelmer@samba.org>
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//    http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! RustPython-based AST visitor for function call replacement

use anyhow::Result;
use rustpython_ast::{self as ast, Ranged};
use std::cell::RefCell;
use std::collections::{HashMap, HashSet};
use std::rc::Rc;
use thiserror::Error;
use tracing;

#[derive(Debug, Error)]
pub enum TypeIntrospectionError {
    #[error("Pyright query failed: {0}")]
    PyrightError(String),

    #[error("Mypy query failed: {0}")]
    MypyError(String),

    #[error("Failed to determine position in source: {0}")]
    PositionError(String),

    #[error("No type introspection client available")]
    NoClientAvailable,
}

use crate::ast_utils;
use crate::core::{ParameterInfo, ReplaceInfo};
use crate::type_introspection_context::TypeIntrospectionContext;
use crate::types::TypeIntrospectionMethod;

/// Visitor to find and replace function calls using rustpython AST  
pub struct RustPythonFunctionCallReplacer<'a> {
    replacements_info: HashMap<String, ReplaceInfo>,
    replacements: Vec<(std::ops::Range<usize>, String)>, // (source_range, replacement_string)
    type_introspection: TypeIntrospectionMethod,
    file_path: String,
    module_name: String,
    import_map: HashMap<String, String>,
    source_content: String,
    inheritance_map: HashMap<String, Vec<String>>,
    pyright_client: Option<Rc<RefCell<Box<dyn crate::pyright_lsp::PyrightLspClientTrait>>>>,
    mypy_client: Option<Rc<RefCell<crate::mypy_lsp::MypyTypeIntrospector>>>,
    type_cache: RefCell<HashMap<(u32, u32), Option<String>>>,
    _type_context: &'a mut TypeIntrospectionContext,
    builtins: HashSet<String>,
    module_functions: HashSet<String>, // Functions defined in this module
}

#[allow(dead_code)] // Some methods are kept for potential future use
impl<'a> RustPythonFunctionCallReplacer<'a> {
    pub fn new_with_context(
        replacements_info: HashMap<String, ReplaceInfo>,
        type_introspection_context: &'a mut TypeIntrospectionContext,
        file_path: String,
        module_name: String,
        source_content: String,
        inheritance_map: HashMap<String, Vec<String>>,
        builtins: HashSet<String>,
    ) -> Result<Self> {
        let type_introspection = type_introspection_context.method();

        // Get the clients from the context
        let pyright_client = type_introspection_context.pyright_client();
        let mypy_client = type_introspection_context.mypy_client();

        Ok(Self {
            replacements_info,
            replacements: Vec::new(),
            type_introspection,
            file_path,
            module_name,
            import_map: HashMap::new(),
            source_content,
            inheritance_map,
            pyright_client,
            mypy_client,
            type_cache: RefCell::new(HashMap::new()),
            _type_context: type_introspection_context,
            builtins,
            module_functions: HashSet::new(),
        })
    }

    pub fn get_replacements(mut self) -> Vec<(std::ops::Range<usize>, String)> {
        std::mem::take(&mut self.replacements)
    }

    /// Visit a module
    pub fn visit_module(&mut self, module: &[ast::Stmt]) {
        tracing::debug!("Visiting module with {} statements", module.len());

        // First pass: build import map
        for stmt in module {
            if let ast::Stmt::ImportFrom(import) = stmt {
                let level = import.level.as_ref().map_or(0, |i| i.to_usize());
                if let Some(module) = &import.module {
                    // Resolve relative imports - check the level field for relative imports
                    let resolved_module = if level > 0 {
                        // Relative import - resolve to full module path
                        let module_parts: Vec<&str> = self.module_name.split('.').collect();
                        // Go up 'level' directories from current module
                        let parent_parts = if module_parts.len() > level {
                            &module_parts[..module_parts.len() - level]
                        } else {
                            // Can't go up that many levels, use root
                            &module_parts[..1]
                        };

                        let parent = parent_parts.join(".");
                        if module.is_empty() {
                            // from . import X (module is the parent itself)
                            parent
                        } else {
                            // from .submodule import X
                            format!("{}.{}", parent, module)
                        }
                    } else {
                        module.to_string()
                    };

                    for alias in &import.names {
                        let full_name = format!("{}.{}", resolved_module, alias.name);
                        // Only track imports without aliases
                        if alias.asname.is_none() {
                            self.import_map.insert(alias.name.to_string(), full_name);
                        }
                    }
                }
            }
        }

        // Second pass: collect all function definitions in the module
        for stmt in module {
            match stmt {
                ast::Stmt::FunctionDef(func) => {
                    self.module_functions.insert(func.name.to_string());
                }
                ast::Stmt::AsyncFunctionDef(func) => {
                    self.module_functions.insert(func.name.to_string());
                }
                ast::Stmt::ClassDef(class) => {
                    // Also track classes as they might contain methods
                    for body_stmt in &class.body {
                        if let ast::Stmt::FunctionDef(func) = body_stmt {
                            // Track class methods with their qualified name
                            self.module_functions
                                .insert(format!("{}.{}", class.name, func.name));
                        }
                    }
                }
                _ => {}
            }
        }

        tracing::debug!(
            "Found {} module functions: {:?}",
            self.module_functions.len(),
            self.module_functions
        );

        // Third pass: visit statements for replacements
        for (i, stmt) in module.iter().enumerate() {
            tracing::debug!("Visiting statement {}", i);
            self.visit_stmt(stmt);
        }
    }

    /// Visit a statement
    fn visit_stmt(&mut self, stmt: &ast::Stmt) {
        match stmt {
            ast::Stmt::FunctionDef(func) => {
                // Skip functions decorated with @replace_me
                let has_replace_me = func.decorator_list.iter().any(|dec| match dec {
                    ast::Expr::Name(name) => name.id.as_str() == "replace_me",
                    ast::Expr::Call(call) => {
                        if let ast::Expr::Name(name) = &*call.func {
                            name.id.as_str() == "replace_me"
                        } else {
                            false
                        }
                    }
                    _ => false,
                });

                if !has_replace_me {
                    for s in &func.body {
                        self.visit_stmt(s);
                    }
                }
            }
            ast::Stmt::AsyncFunctionDef(func) => {
                tracing::debug!("Visiting async function: {}", func.name);
                for s in &func.body {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::ClassDef(class) => {
                for s in &class.body {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::For(for_stmt) => {
                // Visit the target and iter expressions
                self.visit_expr(&for_stmt.target);
                self.visit_expr(&for_stmt.iter);
                for s in &for_stmt.body {
                    self.visit_stmt(s);
                }
                for s in &for_stmt.orelse {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::While(while_stmt) => {
                // Visit the test expression
                self.visit_expr(&while_stmt.test);
                for s in &while_stmt.body {
                    self.visit_stmt(s);
                }
                for s in &while_stmt.orelse {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::If(if_stmt) => {
                // Visit the test expression (this is where walrus operators can be)
                self.visit_expr(&if_stmt.test);
                for s in &if_stmt.body {
                    self.visit_stmt(s);
                }
                for s in &if_stmt.orelse {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::With(with) => {
                // Visit the context expressions
                for item in &with.items {
                    self.visit_expr(&item.context_expr);
                    if let Some(optional_vars) = &item.optional_vars {
                        self.visit_expr(optional_vars);
                    }
                }
                for s in &with.body {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::AsyncWith(with) => {
                // Visit the context expressions
                for item in &with.items {
                    self.visit_expr(&item.context_expr);
                    if let Some(optional_vars) = &item.optional_vars {
                        self.visit_expr(optional_vars);
                    }
                }
                for s in &with.body {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::Try(try_stmt) => {
                for s in &try_stmt.body {
                    self.visit_stmt(s);
                }
                for handler in &try_stmt.handlers {
                    let ast::ExceptHandler::ExceptHandler(h) = handler;
                    for s in &h.body {
                        self.visit_stmt(s);
                    }
                }
                for s in &try_stmt.orelse {
                    self.visit_stmt(s);
                }
                for s in &try_stmt.finalbody {
                    self.visit_stmt(s);
                }
            }
            ast::Stmt::Expr(expr_stmt) => {
                self.visit_expr(&expr_stmt.value);
            }
            ast::Stmt::Return(ret) => {
                if let Some(value) = &ret.value {
                    self.visit_expr(value);
                }
            }
            ast::Stmt::Delete(del) => {
                for target in &del.targets {
                    self.visit_expr(target);
                }
            }
            ast::Stmt::Assign(assign) => {
                self.visit_expr(&assign.value);
            }
            ast::Stmt::AugAssign(aug) => {
                self.visit_expr(&aug.value);
            }
            ast::Stmt::AnnAssign(ann) => {
                if let Some(value) = &ann.value {
                    self.visit_expr(value);
                }
            }
            ast::Stmt::Import(import) => {
                for alias in &import.names {
                    if let Some(asname) = &alias.asname {
                        self.import_map
                            .insert(asname.to_string(), alias.name.to_string());
                    } else {
                        let parts: Vec<&str> = alias.name.split('.').collect();
                        if let Some(first) = parts.first() {
                            self.import_map
                                .insert(first.to_string(), alias.name.to_string());
                        }
                    }
                }
            }
            ast::Stmt::ImportFrom(_) => {
                // Import mapping is handled in the first pass of visit_module
            }
            _ => {}
        }
    }

    /// Visit an expression
    fn visit_expr(&mut self, expr: &ast::Expr) {
        match expr {
            ast::Expr::Call(call) => {
                let func_name = self.expr_to_string(&call.func);
                tracing::info!("Visiting call expression: {}", func_name);

                // Check if this is a magic method call
                if let Some(replacement_str) = self.check_magic_method_call_direct(expr, call) {
                    tracing::debug!("Found magic method replacement");
                    let range = expr.range();
                    let source_range = range.start().to_usize()..range.end().to_usize();
                    self.replacements.push((source_range, replacement_str));
                    return;
                }

                // Check if this is a regular function call that needs replacement
                if let Some(replacement_str) =
                    self.check_function_call_direct(&call.func, &call.args, &call.keywords)
                {
                    tracing::debug!(
                        "Found function replacement for {}: {}",
                        func_name,
                        replacement_str
                    );
                    let range = expr.range();
                    let source_range = range.start().to_usize()..range.end().to_usize();
                    tracing::debug!("Adding replacement at range {:?}", source_range);
                    self.replacements.push((source_range, replacement_str));
                    return;
                }

                // Visit nested expressions
                self.visit_expr(&call.func);
                for arg in &call.args {
                    self.visit_expr(arg);
                }
                for keyword in &call.keywords {
                    self.visit_expr(&keyword.value);
                }
            }
            ast::Expr::BinOp(binop) => {
                tracing::debug!(
                    "Visiting binary operation: {} op {}",
                    self.expr_to_string(&binop.left),
                    self.expr_to_string(&binop.right)
                );
                self.visit_expr(&binop.left);
                self.visit_expr(&binop.right);
            }
            ast::Expr::UnaryOp(unary) => {
                self.visit_expr(&unary.operand);
            }
            ast::Expr::IfExp(ifexp) => {
                self.visit_expr(&ifexp.test);
                self.visit_expr(&ifexp.body);
                self.visit_expr(&ifexp.orelse);
            }
            ast::Expr::Dict(dict) => {
                for key in dict.keys.iter().flatten() {
                    self.visit_expr(key);
                }
                for value in &dict.values {
                    self.visit_expr(value);
                }
            }
            ast::Expr::Set(set) => {
                for elt in &set.elts {
                    self.visit_expr(elt);
                }
            }
            ast::Expr::List(list) => {
                for elt in &list.elts {
                    self.visit_expr(elt);
                }
            }
            ast::Expr::Tuple(tuple) => {
                for elt in &tuple.elts {
                    self.visit_expr(elt);
                }
            }
            ast::Expr::ListComp(comp) => {
                self.visit_expr(&comp.elt);
                for gen in &comp.generators {
                    self.visit_expr(&gen.iter);
                    for if_expr in &gen.ifs {
                        self.visit_expr(if_expr);
                    }
                }
            }
            ast::Expr::SetComp(comp) => {
                self.visit_expr(&comp.elt);
                for gen in &comp.generators {
                    self.visit_expr(&gen.iter);
                    for if_expr in &gen.ifs {
                        self.visit_expr(if_expr);
                    }
                }
            }
            ast::Expr::DictComp(comp) => {
                self.visit_expr(&comp.key);
                self.visit_expr(&comp.value);
                for gen in &comp.generators {
                    self.visit_expr(&gen.iter);
                    for if_expr in &gen.ifs {
                        self.visit_expr(if_expr);
                    }
                }
            }
            ast::Expr::GeneratorExp(comp) => {
                self.visit_expr(&comp.elt);
                for gen in &comp.generators {
                    self.visit_expr(&gen.iter);
                    for if_expr in &gen.ifs {
                        self.visit_expr(if_expr);
                    }
                }
            }
            ast::Expr::Lambda(lambda) => {
                self.visit_expr(&lambda.body);
            }
            ast::Expr::Subscript(sub) => {
                self.visit_expr(&sub.value);
                self.visit_expr(&sub.slice);
            }
            ast::Expr::Compare(comp) => {
                self.visit_expr(&comp.left);
                for comparator in &comp.comparators {
                    self.visit_expr(comparator);
                }
            }
            ast::Expr::NamedExpr(named) => {
                tracing::debug!(
                    "Visiting NamedExpr, value: {:?}",
                    self.expr_to_string(&named.value)
                );
                self.visit_expr(&named.target);
                self.visit_expr(&named.value);
            }
            ast::Expr::Await(await_expr) => {
                tracing::debug!("Visiting await expression");

                // Check if the inner expression is a call that needs replacement
                if let ast::Expr::Call(call) = &*await_expr.value {
                    // Check if this call needs replacement
                    if let Some(replacement_str) =
                        self.check_function_call_direct(&call.func, &call.args, &call.keywords)
                    {
                        // The replacement already includes await if needed
                        // Replace the entire await expression with the replacement
                        let range = expr.range();
                        let source_range = range.start().to_usize()..range.end().to_usize();
                        self.replacements.push((source_range, replacement_str));
                        return;
                    }
                }

                // Otherwise, visit the inner expression normally
                self.visit_expr(&await_expr.value);
            }
            _ => {}
        }
    }

    /// Check if this is a magic method call and return replacement string directly
    fn check_magic_method_call_direct(
        &mut self,
        _call_expr: &ast::Expr,
        call: &ast::ExprCall,
    ) -> Option<String> {
        // Check if it's a builtin function call with exactly one argument
        if call.args.len() == 1 {
            if let ast::Expr::Name(name) = &*call.func {
                let builtin_name = name.id.as_str();
                tracing::debug!("Checking builtin function: {}", builtin_name);

                let magic_method = match builtin_name {
                    "int" => "__int__",
                    "str" => "__str__",
                    "float" => "__float__",
                    "bool" => "__bool__",
                    "len" => "__len__",
                    "abs" => "__abs__",
                    "hash" => "__hash__",
                    "bytes" => "__bytes__",
                    "format" => "__format__",
                    "repr" => "__repr__",
                    _ => {
                        tracing::debug!("Unknown builtin: {}", builtin_name);
                        return None;
                    }
                };
                tracing::debug!("Matched magic method: {}", magic_method);

                // Get the type of the argument using type introspection
                let type_name = match self.get_expression_type(&call.args[0]) {
                    Ok(Some(t)) => t,
                    Ok(None) => {
                        tracing::debug!(
                            "No type information available for {} expression",
                            builtin_name
                        );
                        return None;
                    }
                    Err(e) => {
                        tracing::debug!(
                            "Failed to get type for {} expression: {}",
                            builtin_name,
                            e
                        );
                        return None;
                    }
                };

                tracing::debug!("Type of expression for {}: {}", builtin_name, type_name);
                // println!("DEBUG: Type of '{}' is '{}'", self.expr_to_string(&call.args[0]), type_name);

                // Look up the replacement for this type's magic method
                let method_key = format!("{}.{}", type_name, magic_method);
                let method_key_with_module = if !type_name.contains('.') {
                    Some(format!(
                        "{}.{}.{}",
                        self.module_name, type_name, magic_method
                    ))
                } else {
                    None
                };

                let replace_info = self.replacements_info.get(&method_key).or_else(|| {
                    method_key_with_module
                        .as_ref()
                        .and_then(|k| self.replacements_info.get(k))
                });

                if let Some(info) = replace_info {
                    tracing::debug!(
                        "Found replacement info for {}: {}",
                        method_key,
                        info.replacement_expr
                    );

                    // Use AST-based replacement by walking the AST directly
                    let replacement_ast = info
                        .replacement_ast
                        .as_ref()
                        .expect("replacement_ast must be present for proper transformation");

                    // Walk the replacement AST directly with parameter substitution
                    // Extract base module name from the replacement info
                    // For "module.Class.method", we want just "module"
                    let source_module = info
                        .old_name
                        .find('.')
                        .map(|first_dot_pos| &info.old_name[..first_dot_pos]);

                    let replacement = self.walk_replacement_ast(
                        replacement_ast,
                        &call.args,
                        &call.keywords,
                        &info.parameters,
                        source_module,
                    );

                    // Strip module prefix if we're replacing within the same module
                    let replacement = if let Some(src_mod) = source_module {
                        if src_mod == self.module_name {
                            // Strip "module." prefix from the replacement
                            replacement.replace(&format!("{}.", src_mod), "")
                        } else {
                            replacement
                        }
                    } else {
                        replacement
                    };

                    // For magic methods, check if we should unwrap the builtin call
                    // Walk the replacement AST to decide based on structure, not string patterns
                    if self.should_unwrap_magic_method_call(replacement_ast, builtin_name) {
                        // Extract the inner content by walking the AST
                        if let rustpython_ast::Expr::Call(builtin_call) = replacement_ast.as_ref() {
                            if builtin_call.args.len() == 1 {
                                // Walk the inner expression with the ORIGINAL call's args, not the builtin's
                                let inner_replacement = self.walk_replacement_ast_impl(
                                    &builtin_call.args[0],
                                    &call.args,
                                    &call.keywords,
                                    &info.parameters,
                                    source_module,
                                );
                                return Some(inner_replacement);
                            }
                        }
                    }

                    return Some(replacement);
                } else {
                    tracing::debug!("No replacement found for {}", method_key);
                    return None;
                }
            }
        }
        None
    }

    /// Check if this is a regular function call that needs replacement (returns string directly)
    fn check_function_call_direct(
        &mut self,
        func: &ast::Expr,
        args: &[ast::Expr],
        keywords: &[ast::Keyword],
    ) -> Option<String> {
        // Get the function name
        let func_name = self.get_func_name(func)?;
        println!(
            "DEBUG: check_function_call_direct - func_name={}",
            func_name
        );

        // Check if this function needs replacement
        let replace_info = self.replacements_info.get(&func_name);
        if replace_info.is_none() {
            println!("DEBUG: No replacement found for {}", func_name);
            println!(
                "DEBUG: Available replacements: {:?}",
                self.replacements_info.keys().collect::<Vec<_>>()
            );
            return None;
        }
        let replace_info = replace_info.unwrap();

        // If we have an AST for the replacement, use it for proper handling
        if let Some(ref replacement_ast) = replace_info.replacement_ast {
            // Build a map of parameters to actual arguments
            let mut param_map = HashMap::new();

            // Check if this is a method call based on both syntax and construct type
            // Static methods are NOT method calls - they don't have implicit self/cls
            let is_method_call = matches!(func, ast::Expr::Attribute(_)) &&
                matches!(replace_info.construct_type,
                    crate::core::ConstructType::ClassMethod |
                    crate::core::ConstructType::Function |
                    crate::core::ConstructType::AsyncFunction
                ) &&
                // Additional check: if it's a regular Function in an attribute context,
                // check if it's actually a method by looking at the parameters
                (replace_info.construct_type != crate::core::ConstructType::Function ||
                 replace_info.parameters.first().is_some_and(|p| p.name == "self"));

            if is_method_call {
                // For method calls, we need to handle different types
                if let ast::Expr::Attribute(attr_expr) = func {
                    match replace_info.construct_type {
                        crate::core::ConstructType::ClassMethod => {
                            // For classmethods, first parameter receives the class itself
                            if let Some(first_param) = replace_info.parameters.first() {
                                if !first_param.is_vararg && !first_param.is_kwarg {
                                    param_map.insert(first_param.name.clone(), &*attr_expr.value);
                                }
                            }

                            // Map other arguments (skipping first parameter)
                            for (i, arg) in args.iter().enumerate() {
                                if let Some(param) = replace_info.parameters.get(i + 1) {
                                    if !param.is_vararg && !param.is_kwarg {
                                        param_map.insert(param.name.clone(), arg);
                                    }
                                }
                            }
                        }
                        crate::core::ConstructType::StaticMethod => {
                            // Static methods don't have implicit self/cls
                            for (i, arg) in args.iter().enumerate() {
                                if let Some(param) = replace_info.parameters.get(i) {
                                    if !param.is_vararg && !param.is_kwarg {
                                        param_map.insert(param.name.clone(), arg);
                                    }
                                }
                            }
                        }
                        _ => {
                            // Regular instance methods - first parameter receives the instance
                            if let Some(first_param) = replace_info.parameters.first() {
                                if !first_param.is_vararg && !first_param.is_kwarg {
                                    param_map.insert(first_param.name.clone(), &*attr_expr.value);
                                }
                            }

                            // Map other arguments (skipping first parameter)
                            for (i, arg) in args.iter().enumerate() {
                                if let Some(param) = replace_info.parameters.get(i + 1) {
                                    if !param.is_vararg && !param.is_kwarg {
                                        param_map.insert(param.name.clone(), arg);
                                    }
                                }
                            }
                        }
                    }
                }
            } else {
                // For regular functions, handle *args and **kwargs specially
                let mut regular_params = Vec::new();
                let mut vararg_param = None;
                let mut kwarg_param = None;

                for param in &replace_info.parameters {
                    if param.is_vararg {
                        vararg_param = Some(param.name.clone());
                    } else if param.is_kwarg {
                        kwarg_param = Some(param.name.clone());
                    } else {
                        regular_params.push(param);
                    }
                }

                // Check if we have dict unpacking but no **kwargs in replacement
                let has_dict_unpack = keywords.iter().any(|kw| kw.arg.is_none());
                let has_kwarg_param = kwarg_param.is_some();

                if has_dict_unpack && !has_kwarg_param {
                    // Special case: preserve dict unpacking when replacement doesn't have **kwargs
                    // Just map positional args
                    for (i, arg) in args.iter().enumerate() {
                        if i < regular_params.len() {
                            param_map.insert(regular_params[i].name.clone(), arg);
                        }
                    }
                    // Don't map the dict unpacking - we'll preserve it as-is
                } else {
                    // Map positional arguments
                    for (i, arg) in args.iter().enumerate() {
                        if i < regular_params.len() {
                            param_map.insert(regular_params[i].name.clone(), arg);
                        } else if vararg_param.is_some() {
                            // Extra positional args go to *args - we'll handle this below
                            break;
                        }
                    }

                    // Map keyword arguments
                    for keyword in keywords {
                        if let Some(arg_name) = &keyword.arg {
                            param_map.insert(arg_name.to_string(), &keyword.value);
                        }
                    }
                }
            }

            // Transform the replacement AST by walking it directly
            // For method calls, we need to build the proper argument list that includes self
            let mut augmented_args = Vec::new();
            let mut augmented_keywords = Vec::new();

            if is_method_call {
                // For method calls, add the instance as the first argument
                if let ast::Expr::Attribute(attr_expr) = func {
                    augmented_args.push((*attr_expr.value).clone());
                }
                // Add the regular arguments
                augmented_args.extend_from_slice(args);
            } else {
                // For regular function calls, use arguments as-is
                augmented_args.extend_from_slice(args);
            }

            // Keywords remain the same for both cases
            augmented_keywords.extend_from_slice(keywords);

            // Extract base module name from the replacement info
            // For "module.Class.method", we want just "module"
            let source_module = replace_info
                .old_name
                .find('.')
                .map(|first_dot_pos| &replace_info.old_name[..first_dot_pos]);

            let replacement_str = self.walk_replacement_ast(
                replacement_ast,
                &augmented_args,
                &augmented_keywords,
                &replace_info.parameters,
                source_module,
            );

            // Strip module prefix if we're replacing within the same module
            let replacement_str = if let Some(src_mod) = source_module {
                if src_mod == self.module_name {
                    // Strip "module." prefix from the replacement
                    replacement_str.replace(&format!("{}.", src_mod), "")
                } else {
                    replacement_str
                }
            } else {
                replacement_str
            };

            return Some(replacement_str);
        }

        // Fall back to string-based replacement if no AST
        // Build argument strings for parameter substitution
        let mut arg_map = HashMap::new();

        // Check if this is a method call (func is an Attribute expression)
        let is_method_call = matches!(func, ast::Expr::Attribute(_));

        if is_method_call {
            // For method calls, extract the base object for 'self'
            if let ast::Expr::Attribute(attr_expr) = func {
                let self_str = self.expr_to_string(&attr_expr.value);
                arg_map.insert("self".to_string(), self_str);

                // Map the actual arguments to parameters (skipping 'self')
                let method_params: Vec<_> = replace_info
                    .parameters
                    .iter()
                    .filter(|p| p.name != "self")
                    .collect();

                for (i, arg) in args.iter().enumerate() {
                    if i < method_params.len() {
                        let param = method_params[i];
                        let arg_str = self.expr_to_string(arg);
                        arg_map.insert(param.name.clone(), arg_str);
                    }
                }
            }
        } else {
            // For regular function calls, map arguments normally
            for (i, arg) in args.iter().enumerate() {
                if i < replace_info.parameters.len() {
                    let param = &replace_info.parameters[i];
                    let arg_str = self.expr_to_string(arg);
                    arg_map.insert(param.name.clone(), arg_str);
                }
            }
        }

        // Map keyword arguments
        for keyword in keywords {
            if let Some(arg_name) = &keyword.arg {
                let arg_str = self.expr_to_string(&keyword.value);
                arg_map.insert(arg_name.to_string(), arg_str);
            }
        }

        // Perform parameter substitution in the replacement expression
        let mut replacement = replace_info.replacement_expr.clone();
        for (param_name, arg_value) in arg_map {
            let placeholder = format!("{{{}}}", param_name);
            replacement = replacement.replace(&placeholder, &arg_value);
        }

        Some(replacement)
    }

    /// Get the fully qualified name of a function
    fn get_func_name(&mut self, func: &ast::Expr) -> Option<String> {
        match func {
            ast::Expr::Name(name) => {
                // Check if it's imported
                if let Some(full_name) = self.import_map.get(name.id.as_str()) {
                    Some(full_name.clone())
                } else {
                    // For local functions, try with module prefix first
                    let full_name = format!("{}.{}", self.module_name, name.id);
                    #[allow(clippy::map_entry)]
                    // This is conditional lookup, not contains_key + insert
                    if self.replacements_info.contains_key(&full_name) {
                        Some(full_name)
                    } else {
                        Some(name.id.to_string())
                    }
                }
            }
            ast::Expr::Attribute(attr_expr) => {
                // First check if this is a module.function call
                let base_str = self.expr_to_string(&attr_expr.value);

                // Resolve base through import map if it's an imported module
                let resolved_base = if let Some(imported_path) = self.import_map.get(&base_str) {
                    imported_path.clone()
                } else {
                    base_str.clone()
                };

                let full_path = format!("{}.{}", resolved_base, attr_expr.attr);

                // Check if this full path needs replacement (e.g., library.utils.old_function)
                if self.replacements_info.contains_key(&full_path) {
                    tracing::debug!("Found replacement for module function: {}", full_path);
                    return Some(full_path);
                }

                // For method calls, we need to resolve the type of the base object
                // Use type introspection to get the actual type
                tracing::debug!(
                    "Attempting to get type for attribute base: {:?}",
                    self.expr_to_string(&attr_expr.value)
                );
                let type_result = self.get_expression_type(&attr_expr.value);
                println!("DEBUG: get_expression_type result: {:?}", type_result);
                if let Ok(Some(base_type)) = type_result {
                    tracing::debug!("Got type for attribute base: {}", base_type);

                    // Check if this type was imported - if so, use the full import path
                    let full_type = if let Some(imported_path) = self.import_map.get(&base_type) {
                        tracing::debug!("Type {} was imported as {}", base_type, imported_path);
                        imported_path.clone()
                    } else if base_type.contains('.') {
                        // Already fully qualified
                        base_type.clone()
                    } else {
                        // Local class, use current module
                        let result = format!("{}.{}", self.module_name, base_type);
                        result
                    };

                    let full_method_name = format!("{}.{}", full_type, attr_expr.attr);
                    tracing::debug!("Checking for method replacement: {}", full_method_name);

                    if self.replacements_info.contains_key(&full_method_name) {
                        tracing::debug!("Found replacement for method: {}", full_method_name);
                        return Some(full_method_name);
                    }

                    // Check parent classes if not found directly
                    tracing::debug!(
                        "Checking inheritance map for {}: {:?}",
                        full_type,
                        self.inheritance_map.get(&full_type)
                    );

                    // Try to find inheritance entry - try both the resolved import path and the full module path
                    let mut parent_classes = self.inheritance_map.get(&full_type);

                    // If not found and this looks like a relative import, try to find the full module path
                    if parent_classes.is_none() && !full_type.starts_with("dulwich.") {
                        // Check if there's a corresponding entry with the full module path
                        for (full_class_name, parents) in &self.inheritance_map {
                            // Check if this full class name ends with our relative type
                            if full_class_name
                                .ends_with(&format!(".{}", full_type.replace("repo.", "")))
                            {
                                parent_classes = Some(parents);
                                break;
                            }
                        }
                    }

                    if let Some(parent_classes) = parent_classes {
                        for parent in parent_classes {
                            // parent already includes module prefix (e.g., "test_module.BaseRepo")
                            let parent_method_name = format!("{}.{}", parent, attr_expr.attr);
                            tracing::debug!("Checking parent class method: {}", parent_method_name);
                            if self.replacements_info.contains_key(&parent_method_name) {
                                tracing::debug!(
                                    "Found replacement in parent class: {}",
                                    parent_method_name
                                );
                                return Some(parent_method_name);
                            }
                        }
                    }

                    // Also try without module prefix in case the type already includes it
                    let method_name = format!("{}.{}", base_type, attr_expr.attr);
                    if self.replacements_info.contains_key(&method_name) {
                        return Some(method_name);
                    }
                }

                // Return the literal path
                Some(full_path)
            }
            _ => None,
        }
    }

    /// Get the type of an expression
    fn get_expression_type(
        &mut self,
        expr: &ast::Expr,
    ) -> Result<Option<String>, TypeIntrospectionError> {
        // Get the range of the expression from the AST
        let range = expr.range();
        let start_byte = range.start().to_usize();
        let end_byte = range.end().to_usize();

        // For attribute expressions, query at the exact position of the attribute name
        let query_byte = match expr {
            ast::Expr::Attribute(_attr_expr) => {
                // For attribute access like `obj.attr`, we want to query at the position of `attr`
                // Find the position of the dot and query just after it
                let expr_text =
                    &self.source_content[start_byte..end_byte.min(self.source_content.len())];
                if let Some(dot_pos) = expr_text.rfind('.') {
                    // Position right after the dot, at the start of the attribute name
                    start_byte + dot_pos + 1
                } else {
                    // Fallback to end of expression if no dot found
                    end_byte.saturating_sub(1)
                }
            }
            _ => start_byte,
        };

        // Validate byte position
        if query_byte > self.source_content.len() {
            return Err(TypeIntrospectionError::PositionError(format!(
                "Byte position {} exceeds source length {}",
                query_byte,
                self.source_content.len()
            )));
        }

        // Calculate line and column from byte position
        let line = self.source_content[..query_byte].matches('\n').count() + 1;
        let line_start = self.source_content[..query_byte]
            .rfind('\n')
            .map(|p| p + 1)
            .unwrap_or(0);
        let column = query_byte - line_start;

        let position = (line as u32, column as u32);

        // Debug: show position and text at that position (comment out for production)
        // let expr_end = end_byte.min(self.source_content.len());
        // let expr_text = &self.source_content[start_byte..expr_end];
        // eprintln!("DEBUG: Querying type at position {}:{} (byte {}) for expr: '{}'", line, column, query_byte, expr_text);

        // Check cache first
        if let Some(cached) = self.type_cache.borrow().get(&position) {
            return Ok(cached.clone());
        }

        // Use type introspection based on the configured method
        let result = match self.type_introspection {
            TypeIntrospectionMethod::PyrightLsp => {
                if let Some(pyright) = &self.pyright_client {
                    match pyright.borrow_mut().query_type(
                        &self.file_path,
                        &self.source_content,
                        line as u32,
                        column as u32,
                    ) {
                        Ok(type_opt) => type_opt,
                        Err(e) => {
                            tracing::warn!(
                                "Pyright type query failed at {}:{}: {}",
                                line,
                                column,
                                e
                            );
                            return Err(TypeIntrospectionError::PyrightError(e.to_string()));
                        }
                    }
                } else {
                    return Err(TypeIntrospectionError::NoClientAvailable);
                }
            }
            TypeIntrospectionMethod::MypyDaemon => {
                if let Some(mypy) = &self.mypy_client {
                    match mypy
                        .borrow_mut()
                        .get_type_at_position(&self.file_path, line, column + 1) // dmypy expects 1-based columns
                    {
                        Ok(type_opt) => type_opt,
                        Err(e) => {
                            tracing::warn!("Mypy type query failed at {}:{}: {}", line, column, e);
                            return Err(TypeIntrospectionError::MypyError(e));
                        }
                    }
                } else {
                    return Err(TypeIntrospectionError::NoClientAvailable);
                }
            }
            TypeIntrospectionMethod::PyrightWithMypyFallback => {
                // Try Pyright first
                let pyright_result = if let Some(pyright) = &self.pyright_client {
                    match pyright.borrow_mut().query_type(
                        &self.file_path,
                        &self.source_content,
                        line as u32,
                        column as u32,
                    ) {
                        Ok(type_opt) => type_opt,
                        Err(e) => {
                            tracing::debug!("Pyright failed, trying Mypy: {}", e);
                            None
                        }
                    }
                } else {
                    None
                };

                if pyright_result.is_some() {
                    pyright_result
                } else if let Some(mypy) = &self.mypy_client {
                    match mypy
                        .borrow_mut()
                        .get_type_at_position(&self.file_path, line, column + 1) // dmypy expects 1-based columns
                    {
                        Ok(type_opt) => type_opt,
                        Err(e) => {
                            tracing::warn!(
                                "Both Pyright and Mypy failed at {}:{}: {}",
                                line,
                                column,
                                e
                            );
                            return Err(TypeIntrospectionError::MypyError(e));
                        }
                    }
                } else {
                    return Err(TypeIntrospectionError::NoClientAvailable);
                }
            }
        };

        // Clean up the type string if needed
        let cleaned_result = result.map(|t| {
            // Remove module prefixes if present for simpler matching
            if let Some(last_dot) = t.rfind('.') {
                t[last_dot + 1..].to_string()
            } else {
                t
            }
        });

        // Cache the result
        self.type_cache
            .borrow_mut()
            .insert(position, cleaned_result.clone());

        Ok(cleaned_result)
    }

    /// Convert an expression to a string
    fn expr_to_string(&self, expr: &ast::Expr) -> String {
        // Reconstruct the string from the AST
        self.expr_to_string_with_placeholders(expr, &HashSet::new())
    }

    /// Convert an expression to a string with placeholders
    #[allow(clippy::only_used_in_recursion)] // self is needed for recursive method calls
    fn expr_to_string_with_placeholders<R>(
        &self,
        expr: &rustpython_ast::Expr<R>,
        param_names: &HashSet<String>,
    ) -> String {
        match expr {
            rustpython_ast::Expr::Name(name) => {
                ast_utils::format_name_with_placeholders(&name.id, param_names)
            }
            rustpython_ast::Expr::Constant(c) => ast_utils::format_constant(&c.value),
            rustpython_ast::Expr::Call(call) => {
                let func_str = self.expr_to_string_with_placeholders(&*call.func, param_names);
                let mut arg_strs = Vec::new();
                for arg in &call.args {
                    arg_strs.push(self.expr_to_string_with_placeholders(arg, param_names));
                }
                for keyword in &call.keywords {
                    if let Some(arg) = &keyword.arg {
                        arg_strs.push(format!(
                            "{}={}",
                            arg,
                            self.expr_to_string_with_placeholders(&keyword.value, param_names)
                        ));
                    }
                }
                format!("{}({})", func_str, arg_strs.join(", "))
            }
            rustpython_ast::Expr::Attribute(attr) => {
                let value_str = self.expr_to_string_with_placeholders(&*attr.value, param_names);
                format!("{}.{}", value_str, attr.attr)
            }
            rustpython_ast::Expr::BinOp(binop) => {
                let left = self.expr_to_string_with_placeholders(&*binop.left, param_names);
                let right = self.expr_to_string_with_placeholders(&*binop.right, param_names);
                let op = match &binop.op {
                    rustpython_ast::Operator::Add => "+",
                    rustpython_ast::Operator::Sub => "-",
                    rustpython_ast::Operator::Mult => "*",
                    rustpython_ast::Operator::Div => "/",
                    rustpython_ast::Operator::Mod => "%",
                    rustpython_ast::Operator::Pow => "**",
                    rustpython_ast::Operator::FloorDiv => "//",
                    rustpython_ast::Operator::LShift => "<<",
                    rustpython_ast::Operator::RShift => ">>",
                    rustpython_ast::Operator::BitOr => "|",
                    rustpython_ast::Operator::BitXor => "^",
                    rustpython_ast::Operator::BitAnd => "&",
                    rustpython_ast::Operator::MatMult => "@",
                };
                format!("{} {} {}", left, op, right)
            }
            rustpython_ast::Expr::BoolOp(boolop) => {
                let values: Vec<String> = boolop
                    .values
                    .iter()
                    .map(|v| self.expr_to_string_with_placeholders(v, param_names))
                    .collect();
                let op = match &boolop.op {
                    rustpython_ast::BoolOp::And => " and ",
                    rustpython_ast::BoolOp::Or => " or ",
                };
                values.join(op)
            }
            rustpython_ast::Expr::UnaryOp(unaryop) => {
                let operand = self.expr_to_string_with_placeholders(&*unaryop.operand, param_names);
                let op = match &unaryop.op {
                    rustpython_ast::UnaryOp::Not => "not ",
                    rustpython_ast::UnaryOp::UAdd => "+",
                    rustpython_ast::UnaryOp::USub => "-",
                    rustpython_ast::UnaryOp::Invert => "~",
                };
                format!("{}{}", op, operand)
            }
            rustpython_ast::Expr::Compare(compare) => {
                let mut result = self.expr_to_string_with_placeholders(&*compare.left, param_names);
                for (op, comparator) in compare.ops.iter().zip(&compare.comparators) {
                    let op_str = match op {
                        rustpython_ast::CmpOp::Eq => " == ",
                        rustpython_ast::CmpOp::NotEq => " != ",
                        rustpython_ast::CmpOp::Lt => " < ",
                        rustpython_ast::CmpOp::LtE => " <= ",
                        rustpython_ast::CmpOp::Gt => " > ",
                        rustpython_ast::CmpOp::GtE => " >= ",
                        rustpython_ast::CmpOp::Is => " is ",
                        rustpython_ast::CmpOp::IsNot => " is not ",
                        rustpython_ast::CmpOp::In => " in ",
                        rustpython_ast::CmpOp::NotIn => " not in ",
                    };
                    result.push_str(op_str);
                    result
                        .push_str(&self.expr_to_string_with_placeholders(comparator, param_names));
                }
                result
            }
            rustpython_ast::Expr::JoinedStr(joined) => {
                // F-string
                let mut result = String::from("f\"");
                for value in &joined.values {
                    match value {
                        rustpython_ast::Expr::Constant(c) => {
                            if let ast::Constant::Str(s) = &c.value {
                                result.push_str(s);
                            }
                        }
                        rustpython_ast::Expr::FormattedValue(fval) => {
                            result.push('{');
                            result.push_str(
                                &self.expr_to_string_with_placeholders(&*fval.value, param_names),
                            );
                            if let Some(spec) = &fval.format_spec {
                                result.push(':');
                                // Format spec is itself a JoinedStr expression
                                if let rustpython_ast::Expr::JoinedStr(spec_joined) = &**spec {
                                    for spec_part in &spec_joined.values {
                                        if let rustpython_ast::Expr::Constant(c) = spec_part {
                                            if let ast::Constant::Str(s) = &c.value {
                                                result.push_str(s);
                                            }
                                        }
                                    }
                                }
                            }
                            result.push('}');
                        }
                        _ => {}
                    }
                }
                result.push('"');
                result
            }
            rustpython_ast::Expr::List(list) => {
                let items: Vec<String> = list
                    .elts
                    .iter()
                    .map(|e| self.expr_to_string_with_placeholders(e, param_names))
                    .collect();
                format!("[{}]", items.join(", "))
            }
            rustpython_ast::Expr::Tuple(tuple) => {
                let items: Vec<String> = tuple
                    .elts
                    .iter()
                    .map(|e| self.expr_to_string_with_placeholders(e, param_names))
                    .collect();
                if items.len() == 1 {
                    format!("({},)", items[0])
                } else {
                    format!("({})", items.join(", "))
                }
            }
            rustpython_ast::Expr::Dict(dict) => {
                let mut items = Vec::new();
                for (key_opt, value) in dict.keys.iter().zip(&dict.values) {
                    if let Some(key) = key_opt {
                        let key_str = self.expr_to_string_with_placeholders(key, param_names);
                        let value_str = self.expr_to_string_with_placeholders(value, param_names);
                        items.push(format!("{}: {}", key_str, value_str));
                    } else {
                        // None key means dictionary unpacking
                        let value_str = self.expr_to_string_with_placeholders(value, param_names);
                        items.push(format!("**{}", value_str));
                    }
                }
                format!("{{{}}}", items.join(", "))
            }
            rustpython_ast::Expr::Set(set) => {
                let items: Vec<String> = set
                    .elts
                    .iter()
                    .map(|e| self.expr_to_string_with_placeholders(e, param_names))
                    .collect();
                format!("{{{}}}", items.join(", "))
            }
            rustpython_ast::Expr::Subscript(sub) => {
                let value = self.expr_to_string_with_placeholders(&*sub.value, param_names);
                let slice = self.expr_to_string_with_placeholders(&*sub.slice, param_names);
                format!("{}[{}]", value, slice)
            }
            rustpython_ast::Expr::Slice(slice) => {
                let lower = slice
                    .lower
                    .as_ref()
                    .map(|e| self.expr_to_string_with_placeholders(&**e, param_names))
                    .unwrap_or_default();
                let upper = slice
                    .upper
                    .as_ref()
                    .map(|e| self.expr_to_string_with_placeholders(&**e, param_names))
                    .unwrap_or_default();
                let step = slice
                    .step
                    .as_ref()
                    .map(|e| {
                        format!(
                            ":{}",
                            self.expr_to_string_with_placeholders(&**e, param_names)
                        )
                    })
                    .unwrap_or_default();
                format!("{}:{}{}", lower, upper, step)
            }
            rustpython_ast::Expr::Lambda(lambda) => {
                let args: Vec<String> = lambda
                    .args
                    .args
                    .iter()
                    .map(|arg| arg.def.arg.to_string())
                    .collect();
                let body = self.expr_to_string_with_placeholders(&*lambda.body, param_names);
                if args.is_empty() {
                    format!("lambda: {}", body)
                } else {
                    format!("lambda {}: {}", args.join(", "), body)
                }
            }
            rustpython_ast::Expr::ListComp(comp) => {
                let elt = self.expr_to_string_with_placeholders(&*comp.elt, param_names);
                let mut comp_str = String::new();
                for gen in &comp.generators {
                    let target = self.expr_to_string_with_placeholders(&gen.target, param_names);
                    let iter = self.expr_to_string_with_placeholders(&gen.iter, param_names);
                    let async_keyword = if gen.is_async { "async " } else { "" };
                    comp_str.push_str(&format!(" {}for {} in {}", async_keyword, target, iter));
                    for if_clause in &gen.ifs {
                        let if_expr = self.expr_to_string_with_placeholders(if_clause, param_names);
                        comp_str.push_str(&format!(" if {}", if_expr));
                    }
                }
                format!("[{}{}]", elt, comp_str)
            }
            rustpython_ast::Expr::SetComp(comp) => {
                let elt = self.expr_to_string_with_placeholders(&*comp.elt, param_names);
                let mut comp_str = String::new();
                for gen in &comp.generators {
                    let target = self.expr_to_string_with_placeholders(&gen.target, param_names);
                    let iter = self.expr_to_string_with_placeholders(&gen.iter, param_names);
                    let async_keyword = if gen.is_async { "async " } else { "" };
                    comp_str.push_str(&format!(" {}for {} in {}", async_keyword, target, iter));
                    for if_clause in &gen.ifs {
                        let if_expr = self.expr_to_string_with_placeholders(if_clause, param_names);
                        comp_str.push_str(&format!(" if {}", if_expr));
                    }
                }
                format!("{{{}{}}}", elt, comp_str)
            }
            rustpython_ast::Expr::DictComp(comp) => {
                let key = self.expr_to_string_with_placeholders(&*comp.key, param_names);
                let value = self.expr_to_string_with_placeholders(&*comp.value, param_names);
                let mut comp_str = String::new();
                for gen in &comp.generators {
                    let target = self.expr_to_string_with_placeholders(&gen.target, param_names);
                    let iter = self.expr_to_string_with_placeholders(&gen.iter, param_names);
                    let async_keyword = if gen.is_async { "async " } else { "" };
                    comp_str.push_str(&format!(" {}for {} in {}", async_keyword, target, iter));
                    for if_clause in &gen.ifs {
                        let if_expr = self.expr_to_string_with_placeholders(if_clause, param_names);
                        comp_str.push_str(&format!(" if {}", if_expr));
                    }
                }
                format!("{{{}: {}{}}}", key, value, comp_str)
            }
            rustpython_ast::Expr::GeneratorExp(comp) => {
                let elt = self.expr_to_string_with_placeholders(&*comp.elt, param_names);
                let mut comp_str = String::new();
                for gen in &comp.generators {
                    let target = self.expr_to_string_with_placeholders(&gen.target, param_names);
                    let iter = self.expr_to_string_with_placeholders(&gen.iter, param_names);
                    let async_keyword = if gen.is_async { "async " } else { "" };
                    comp_str.push_str(&format!(" {}for {} in {}", async_keyword, target, iter));
                    for if_clause in &gen.ifs {
                        let if_expr = self.expr_to_string_with_placeholders(if_clause, param_names);
                        comp_str.push_str(&format!(" if {}", if_expr));
                    }
                }
                format!("({}{})", elt, comp_str)
            }
            rustpython_ast::Expr::IfExp(if_exp) => {
                let body = self.expr_to_string_with_placeholders(&*if_exp.body, param_names);
                let test = self.expr_to_string_with_placeholders(&*if_exp.test, param_names);
                let orelse = self.expr_to_string_with_placeholders(&*if_exp.orelse, param_names);
                format!("{} if {} else {}", body, test, orelse)
            }
            rustpython_ast::Expr::NamedExpr(named) => {
                let target = self.expr_to_string_with_placeholders(&*named.target, param_names);
                let value = self.expr_to_string_with_placeholders(&*named.value, param_names);
                format!("({} := {})", target, value)
            }
            rustpython_ast::Expr::Starred(starred) => {
                let value = self.expr_to_string_with_placeholders(&*starred.value, param_names);
                format!("*{}", value)
            }
            rustpython_ast::Expr::Await(await_expr) => {
                let value = self.expr_to_string_with_placeholders(&*await_expr.value, param_names);
                format!("await {}", value)
            }
            _ => "".to_string(),
        }
    }

    /// Transform a replacement AST with parameter substitution, returning a new AST
    fn transform_replacement_ast_to_expr(
        &self,
        replacement_ast: &rustpython_ast::Expr,
        obj_expr: &ast::Expr,
    ) -> ast::Expr {
        // Transform the replacement AST by replacing 'self' with obj_expr
        self.transform_expr_replacing_self(replacement_ast, obj_expr)
    }

    /// Transform a replacement AST with general parameter substitution
    fn transform_replacement_ast_with_params(
        &self,
        replacement_ast: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
    ) -> ast::Expr {
        self.transform_expr_with_params(replacement_ast, param_map)
    }

    /// Recursively transform an expression, replacing parameters with their values
    fn transform_expr_with_params(
        &self,
        expr: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
    ) -> ast::Expr {
        match expr {
            ast::Expr::Name(name) => {
                // Check if this name should be replaced with a parameter value
                if let Some(replacement_expr) = param_map.get(name.id.as_str()) {
                    (*replacement_expr).clone()
                } else {
                    ast::Expr::Name(ast::ExprName {
                        id: name.id.clone(),
                        ctx: ast::ExprContext::Load,
                        range: Default::default(),
                    })
                }
            }
            ast::Expr::Attribute(attr) => {
                // Transform the value part
                let value = self.transform_expr_with_params(&attr.value, param_map);
                ast::Expr::Attribute(ast::ExprAttribute {
                    value: Box::new(value),
                    attr: attr.attr.clone(),
                    ctx: ast::ExprContext::Load,
                    range: Default::default(),
                })
            }
            ast::Expr::Call(call) => {
                // Transform function and arguments
                let func = self.transform_expr_with_params(&call.func, param_map);
                let args: Vec<ast::Expr> = call
                    .args
                    .iter()
                    .map(|arg| self.transform_expr_with_params(arg, param_map))
                    .collect();
                let keywords: Vec<ast::Keyword> = call
                    .keywords
                    .iter()
                    .map(|kw| ast::Keyword {
                        arg: kw.arg.clone(),
                        value: self.transform_expr_with_params(&kw.value, param_map),
                        range: Default::default(),
                    })
                    .collect();

                ast::Expr::Call(ast::ExprCall {
                    func: Box::new(func),
                    args,
                    keywords,
                    range: Default::default(),
                })
            }
            ast::Expr::BinOp(binop) => {
                let left = self.transform_expr_with_params(&binop.left, param_map);
                let right = self.transform_expr_with_params(&binop.right, param_map);
                ast::Expr::BinOp(ast::ExprBinOp {
                    left: Box::new(left),
                    op: binop.op,
                    right: Box::new(right),
                    range: Default::default(),
                })
            }
            ast::Expr::Await(await_expr) => {
                let value = self.transform_expr_with_params(&await_expr.value, param_map);
                ast::Expr::Await(ast::ExprAwait {
                    value: Box::new(value),
                    range: Default::default(),
                })
            }
            // Handle other expressions by converting them directly
            _ => self.convert_expr(expr),
        }
    }

    /// Recursively transform an expression, replacing 'self' with the given expression
    fn transform_expr_replacing_self(
        &self,
        expr: &rustpython_ast::Expr,
        obj_expr: &ast::Expr,
    ) -> ast::Expr {
        match expr {
            ast::Expr::Name(name) if name.id.as_str() == "self" => {
                // Replace 'self' with the object expression
                obj_expr.clone()
            }
            ast::Expr::Attribute(attr) => {
                // Transform the value part
                let value = self.transform_expr_replacing_self(&attr.value, obj_expr);
                ast::Expr::Attribute(ast::ExprAttribute {
                    value: Box::new(value),
                    attr: attr.attr.clone(),
                    ctx: ast::ExprContext::Load,
                    range: Default::default(),
                })
            }
            ast::Expr::Call(call) => {
                // Transform function and arguments
                let func = self.transform_expr_replacing_self(&call.func, obj_expr);
                let args: Vec<ast::Expr> = call
                    .args
                    .iter()
                    .map(|arg| self.transform_expr_replacing_self(arg, obj_expr))
                    .collect();
                let keywords: Vec<ast::Keyword> = call
                    .keywords
                    .iter()
                    .map(|kw| ast::Keyword {
                        arg: kw.arg.clone(),
                        value: self.transform_expr_replacing_self(&kw.value, obj_expr),
                        range: Default::default(),
                    })
                    .collect();

                ast::Expr::Call(ast::ExprCall {
                    func: Box::new(func),
                    args,
                    keywords,
                    range: Default::default(),
                })
            }
            // For other expressions, convert them directly
            _ => {
                // Convert the expression - now mostly an identity function
                // but ensures proper range fields are set
                self.convert_expr(expr)
            }
        }
    }

    /// Convert an expression to a regular expression (identity function now)
    fn convert_expr(&self, expr: &rustpython_ast::Expr) -> ast::Expr {
        match expr {
            ast::Expr::Name(name) => ast::Expr::Name(ast::ExprName {
                id: name.id.clone(),
                ctx: ast::ExprContext::Load,
                range: Default::default(),
            }),
            ast::Expr::Constant(c) => ast::Expr::Constant(ast::ExprConstant {
                value: c.value.clone(),
                kind: c.kind.clone(),
                range: Default::default(),
            }),
            _ => {
                // Fallback: parse the string representation
                let expr_str = self.expr_to_string_with_placeholders(expr, &HashSet::new());
                match rustpython_parser::parse(
                    &expr_str,
                    rustpython_parser::Mode::Expression,
                    "<conversion>",
                ) {
                    Ok(ast::Mod::Expression(expr_mod)) => *expr_mod.body,
                    _ => ast::Expr::Name(ast::ExprName {
                        id: "ERROR".to_string().into(),
                        ctx: ast::ExprContext::Load,
                        range: Default::default(),
                    }),
                }
            }
        }
    }

    /// Transform a replacement AST with actual arguments
    fn transform_replacement_ast_with_args(
        &self,
        ast: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
        all_args: &[ast::Expr],
        all_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
    ) -> String {
        // Convert the rustpython AST to string, substituting parameters with actual arguments
        self.rustpython_expr_to_string_with_args(ast, param_map, all_args, all_keywords, parameters)
    }

    /// Convert rustpython AST expression to string with argument substitution
    fn rustpython_expr_to_string_with_args(
        &self,
        expr: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
        all_args: &[ast::Expr],
        all_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
    ) -> String {
        self.rustpython_expr_to_string_with_args_impl(
            expr,
            param_map,
            all_args,
            all_keywords,
            parameters,
            true,
        )
    }

    /// Walk replacement AST directly without template parsing - this is the clean approach
    fn walk_replacement_ast(
        &self,
        expr: &rustpython_ast::Expr,
        call_args: &[ast::Expr],
        call_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
        source_module: Option<&str>,
    ) -> String {
        self.walk_replacement_ast_impl(expr, call_args, call_keywords, parameters, source_module)
    }

    fn walk_replacement_ast_impl(
        &self,
        expr: &rustpython_ast::Expr,
        call_args: &[ast::Expr],
        call_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
        source_module: Option<&str>,
    ) -> String {
        use rustpython_ast as rpy_ast;

        match expr {
            rpy_ast::Expr::Name(name) => {
                let name_str = name.id.as_str();
                // Check if this is a vararg parameter - these should NOT be mapped by index
                // Note: kwarg parameters are handled separately below
                if let Some(_param) = parameters
                    .iter()
                    .find(|p| p.name == name_str && p.is_vararg)
                {
                    // For *args, we don't substitute the name itself
                    // It will be expanded when we encounter it in a Starred context
                    return name_str.to_string();
                }

                // Map regular parameter names to actual call arguments
                if let Some(param_index) = parameters
                    .iter()
                    .position(|p| p.name == name_str && !p.is_vararg && !p.is_kwarg)
                {
                    if param_index < call_args.len() {
                        return self.expr_to_string(&call_args[param_index]);
                    }

                    // If we don't have a positional argument at this index,
                    // check if there's a starred keyword argument that could fill this parameter.
                    // This handles the case where **kwargs should fill a missing positional parameter.
                    if param_index == call_args.len() && !call_keywords.is_empty() {
                        // Look for the first starred keyword argument (**kwargs style)
                        for keyword in call_keywords.iter() {
                            if keyword.arg.is_none() {
                                // This is a **kwargs expansion - use it for this parameter
                                return format!("**{}", self.expr_to_string(&keyword.value));
                            }
                        }
                    }
                }

                // Check if this is a **kwargs parameter that should collect individual keyword arguments
                if let Some(_param) = parameters.iter().find(|p| p.is_kwarg && p.name == name_str) {
                    // This is a **kwargs parameter - collect all keyword arguments
                    let individual_kwargs: Vec<String> = call_keywords
                        .iter()
                        .map(|kw| {
                            if let Some(arg) = &kw.arg {
                                format!("{}={}", arg, self.expr_to_string(&kw.value))
                            } else {
                                // This is a starred expansion like **dict, preserve it with **
                                format!("**{}", self.expr_to_string(&kw.value))
                            }
                        })
                        .collect();

                    if !individual_kwargs.is_empty() {
                        return individual_kwargs.join(", ");
                    } else {
                        // No kwargs to expand - return empty string
                        return String::new();
                    }
                }

                // Check for keyword arguments with names
                if let Some(keyword) = call_keywords
                    .iter()
                    .find(|kw| kw.arg.as_ref().map(|arg| arg.as_str()) == Some(name_str))
                {
                    return self.expr_to_string(&keyword.value);
                }

                // Check if this is a parameter that wasn't provided but has a default value
                if let Some(param) = parameters.iter().find(|p| p.name == name_str) {
                    if let Some(param_index) = parameters.iter().position(|p| p.name == name_str) {
                        let is_provided = param_index < call_args.len()
                            || call_keywords.iter().any(|kw| {
                                kw.arg.as_ref().map(|arg| arg.as_str()) == Some(name_str)
                            });

                        if !is_provided && param.has_default {
                            // Parameter not provided and has default - use a placeholder that will be filtered out
                            return "__OMIT_PARAMETER__".to_string();
                        }
                    }
                }

                // If it's not a parameter, check if we need to qualify the name with the source module
                if let Some(source_mod) = source_module {
                    // Function is not available in current scope
                    // Check if it's imported from the source module
                    let full_name = format!("{}.{}", source_mod, name_str);
                    if self.import_map.values().any(|v| v == &full_name) {
                        // It's imported with its full name, use the short name
                        name_str.to_string()
                    } else {
                        // Not imported - use the fully qualified name from source module
                        full_name
                    }
                } else {
                    // No source module information, use the name as-is
                    name_str.to_string()
                }
            }

            rpy_ast::Expr::Attribute(attr) => {
                let value = self.walk_replacement_ast_impl(
                    &attr.value,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("{}.{}", value, attr.attr)
            }

            rpy_ast::Expr::Call(call) => {
                // Handle the function name with potential name resolution
                let func = match call.func.as_ref() {
                    rpy_ast::Expr::Name(name) => {
                        let func_name = name.id.as_str();
                        // Check if this is a parameter that should be substituted
                        if let Some(param_index) =
                            parameters.iter().position(|p| p.name == func_name)
                        {
                            if param_index < call_args.len() {
                                self.expr_to_string(&call_args[param_index])
                            } else {
                                // Apply name resolution for functions that aren't parameters
                                if !self.import_map.contains_key(func_name)
                                    && !self.builtins.contains(func_name)
                                {
                                    format!("{}.{}", self.module_name, func_name)
                                } else {
                                    func_name.to_string()
                                }
                            }
                        } else {
                            // Not a parameter - check if this function exists in scope
                            // Only apply name resolution if the function is not already available

                            // Check if this is imported or a builtin
                            if self.import_map.contains_key(func_name)
                                || self.builtins.contains(func_name)
                            {
                                // Function is imported or is a builtin - use it directly
                                func_name.to_string()
                            } else if self.module_functions.contains(func_name) {
                                // Function is defined in this module - use it directly
                                func_name.to_string()
                            } else if let Some(source_mod) = source_module {
                                // Function is not available in current scope
                                // Check if it's imported from the source module
                                let full_name = format!("{}.{}", source_mod, func_name);
                                if self.import_map.values().any(|v| v == &full_name) {
                                    // It's imported with its full name, use the short name
                                    func_name.to_string()
                                } else {
                                    // Not imported - use the fully qualified name from source module
                                    full_name
                                }
                            } else {
                                // No source module info - use as-is
                                func_name.to_string()
                            }
                        }
                    }
                    _ => self.walk_replacement_ast_impl(
                        &call.func,
                        call_args,
                        call_keywords,
                        parameters,
                        source_module,
                    ),
                };
                let args: Vec<String> = call
                    .args
                    .iter()
                    .filter_map(|arg| {
                        let value = self.walk_replacement_ast_impl(
                            arg,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        );

                        // Skip empty strings (e.g., from empty *args expansion)
                        if value.is_empty() {
                            return None;
                        }

                        // Check if this is an unsubstituted parameter name
                        // If so, skip it (don't include unprovided parameters in the call)
                        if let rpy_ast::Expr::Name(name) = arg {
                            let name_str = name.id.as_str();
                            // Only filter out if:
                            // 1. The value equals the parameter name (unsubstituted)
                            // 2. The parameter exists in the replacement signature
                            // 3. AND there's no actual argument provided for this parameter
                            if value == name_str && parameters.iter().any(|p| p.name == name_str) {
                                // Check if this parameter has a corresponding argument
                                if let Some(param_index) =
                                    parameters.iter().position(|p| p.name == name_str)
                                {
                                    let param = &parameters[param_index];
                                    let is_provided = param_index < call_args.len()
                                        || call_keywords.iter().any(|kw| {
                                            kw.arg
                                                .as_ref()
                                                .is_some_and(|arg| arg.as_str() == name_str)
                                        });

                                    if !is_provided {
                                        // Parameter not provided - only skip if it has defaults
                                        if param.has_default {
                                            // This is an unsubstituted parameter with default, skip it
                                            return None;
                                        } else {
                                            // This is an unsubstituted parameter without default, keep it
                                            // (This handles required parameters like 'self' in methods)
                                        }
                                    }
                                }
                            }
                        }

                        // Skip parameters that were marked for omission
                        if value == "__OMIT_PARAMETER__" {
                            return None;
                        }

                        Some(value)
                    })
                    .collect();
                let keywords: Vec<String> = call
                    .keywords
                    .iter()
                    .filter_map(|kw| {
                        let value = self.walk_replacement_ast_impl(
                            &kw.value,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        );
                        if let Some(arg) = &kw.arg {
                            // Skip parameters that were marked for omission
                            if value == "__OMIT_PARAMETER__" {
                                return None;
                            }

                            // Check if this keyword argument's value is an unsubstituted parameter
                            let is_unsubstituted = value == arg.as_str()
                                || (source_module.is_some()
                                    && value
                                        == format!("{}.{}", source_module.unwrap(), arg.as_str()));

                            if is_unsubstituted {
                                // Check if this parameter was actually provided in the original call
                                if let Some(param_idx) =
                                    parameters.iter().position(|p| p.name == arg.as_str())
                                {
                                    let param = &parameters[param_idx];
                                    let is_provided = param_idx < call_args.len()
                                        || call_keywords.iter().any(|kw| {
                                            kw.arg
                                                .as_ref()
                                                .is_some_and(|a| a.as_str() == arg.as_str())
                                        });

                                    if is_provided {
                                        // Parameter was provided, keep it as-is
                                        return Some(format!("{}={}", arg, value));
                                    } else if param.has_default {
                                        // Parameter has default and wasn't provided, omit it
                                        return None;
                                    } else {
                                        // Parameter doesn't have default and wasn't provided - keep it as-is
                                        // (This handles required parameters like 'self' in methods)
                                        return Some(format!("{}={}", arg, value));
                                    }
                                }
                                // Unknown parameter, keep original behavior (skip it)
                                return None;
                            }
                            Some(format!("{}={}", arg, value))
                        } else {
                            // This is a starred expression like **kwargs

                            // If the value is empty (no kwargs to expand), skip it
                            if value.is_empty() {
                                return None;
                            }

                            // Check if the value is already expanded individual arguments (contains =)
                            if value.contains("=") {
                                // Value is already expanded (like "a=1, b=2" or just "a=1"), don't add **
                                Some(value)
                            } else if value.starts_with("**") {
                                // Value already has ** prefix, don't add another one
                                Some(value)
                            } else {
                                // Value is a single expression like "dict_var" or "kwargs", add **
                                Some(format!("**{}", value))
                            }
                        }
                    })
                    .collect();

                let mut all_args = args;
                all_args.extend(keywords);
                format!("{}({})", func, all_args.join(", "))
            }

            rpy_ast::Expr::Constant(constant) => {
                // Handle constants like strings, numbers, etc.
                self.format_constant(&constant.value)
            }

            // For all other expression types, recursively handle them
            rpy_ast::Expr::IfExp(if_exp) => {
                let body = self.walk_replacement_ast_impl(
                    &if_exp.body,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let test = self.walk_replacement_ast_impl(
                    &if_exp.test,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let orelse = self.walk_replacement_ast_impl(
                    &if_exp.orelse,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("{} if {} else {}", body, test, orelse)
            }

            rpy_ast::Expr::BoolOp(boolop) => {
                let values: Vec<String> = boolop
                    .values
                    .iter()
                    .map(|v| {
                        self.walk_replacement_ast_impl(
                            v,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .collect();
                let op = match &boolop.op {
                    rustpython_ast::BoolOp::And => " and ",
                    rustpython_ast::BoolOp::Or => " or ",
                };
                values.join(op)
            }

            rpy_ast::Expr::BinOp(binop) => {
                let left = self.walk_replacement_ast_impl(
                    &binop.left,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let right = self.walk_replacement_ast_impl(
                    &binop.right,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let op = match &binop.op {
                    rustpython_ast::Operator::Add => "+",
                    rustpython_ast::Operator::Sub => "-",
                    rustpython_ast::Operator::Mult => "*",
                    rustpython_ast::Operator::Div => "/",
                    rustpython_ast::Operator::Mod => "%",
                    rustpython_ast::Operator::Pow => "**",
                    rustpython_ast::Operator::FloorDiv => "//",
                    rustpython_ast::Operator::LShift => "<<",
                    rustpython_ast::Operator::RShift => ">>",
                    rustpython_ast::Operator::BitOr => "|",
                    rustpython_ast::Operator::BitXor => "^",
                    rustpython_ast::Operator::BitAnd => "&",
                    rustpython_ast::Operator::MatMult => "@",
                };
                format!("{} {} {}", left, op, right)
            }

            rpy_ast::Expr::Subscript(sub) => {
                let value = self.walk_replacement_ast_impl(
                    &sub.value,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let slice = self.walk_replacement_ast_impl(
                    &sub.slice,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("{}[{}]", value, slice)
            }

            rpy_ast::Expr::Compare(compare) => {
                let mut result = self.walk_replacement_ast_impl(
                    &compare.left,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                for (op, comparator) in compare.ops.iter().zip(&compare.comparators) {
                    let op_str = match op {
                        rustpython_ast::CmpOp::Eq => " == ",
                        rustpython_ast::CmpOp::NotEq => " != ",
                        rustpython_ast::CmpOp::Lt => " < ",
                        rustpython_ast::CmpOp::LtE => " <= ",
                        rustpython_ast::CmpOp::Gt => " > ",
                        rustpython_ast::CmpOp::GtE => " >= ",
                        rustpython_ast::CmpOp::Is => " is ",
                        rustpython_ast::CmpOp::IsNot => " is not ",
                        rustpython_ast::CmpOp::In => " in ",
                        rustpython_ast::CmpOp::NotIn => " not in ",
                    };
                    result.push_str(op_str);
                    result.push_str(&self.walk_replacement_ast_impl(
                        comparator,
                        call_args,
                        call_keywords,
                        parameters,
                        source_module,
                    ));
                }
                result
            }

            rpy_ast::Expr::UnaryOp(unaryop) => {
                let operand = self.walk_replacement_ast_impl(
                    &unaryop.operand,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let op = match &unaryop.op {
                    rustpython_ast::UnaryOp::Not => "not ",
                    rustpython_ast::UnaryOp::UAdd => "+",
                    rustpython_ast::UnaryOp::USub => "-",
                    rustpython_ast::UnaryOp::Invert => "~",
                };
                format!("{}{}", op, operand)
            }

            rpy_ast::Expr::List(list) => {
                let items: Vec<String> = list
                    .elts
                    .iter()
                    .map(|e| {
                        self.walk_replacement_ast_impl(
                            e,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .collect();
                format!("[{}]", items.join(", "))
            }

            rpy_ast::Expr::Tuple(tuple) => {
                let items: Vec<String> = tuple
                    .elts
                    .iter()
                    .map(|e| {
                        self.walk_replacement_ast_impl(
                            e,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .collect();
                if items.len() == 1 {
                    format!("({},)", items[0])
                } else {
                    format!("({})", items.join(", "))
                }
            }

            rpy_ast::Expr::Dict(dict) => {
                let mut items = Vec::new();
                for (key_opt, value) in dict.keys.iter().zip(&dict.values) {
                    if let Some(key) = key_opt {
                        let key_str = self.walk_replacement_ast_impl(
                            key,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        );
                        let value_str = self.walk_replacement_ast_impl(
                            value,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        );
                        items.push(format!("{}: {}", key_str, value_str));
                    } else {
                        // None key means dictionary unpacking
                        let value_str = self.walk_replacement_ast_impl(
                            value,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        );
                        items.push(format!("**{}", value_str));
                    }
                }
                format!("{{{}}}", items.join(", "))
            }

            rpy_ast::Expr::Set(set) => {
                let items: Vec<String> = set
                    .elts
                    .iter()
                    .map(|e| {
                        self.walk_replacement_ast_impl(
                            e,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .collect();
                format!("{{{}}}", items.join(", "))
            }

            rpy_ast::Expr::NamedExpr(named) => {
                let target = self.walk_replacement_ast_impl(
                    &named.target,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                let value = self.walk_replacement_ast_impl(
                    &named.value,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("({} := {})", target, value)
            }

            rpy_ast::Expr::Starred(starred) => {
                // Check if this is *args that needs expansion
                if let rpy_ast::Expr::Name(name) = &*starred.value {
                    if let Some(_param) = parameters
                        .iter()
                        .find(|p| p.is_vararg && p.name == name.id.as_str())
                    {
                        // This is *args - expand to actual arguments
                        let regular_param_count = parameters
                            .iter()
                            .filter(|p| !p.is_vararg && !p.is_kwarg)
                            .count();

                        // For functions with only *args and **kwargs, all positional args go to *args
                        if regular_param_count == 0 {
                            let args_strs: Vec<String> = call_args
                                .iter()
                                .map(|arg| self.expr_to_string(arg))
                                .collect();
                            return args_strs.join(", ");
                        }

                        // For functions with some regular params, extra args go to *args
                        if call_args.len() > regular_param_count {
                            let extra_args: Vec<String> = call_args[regular_param_count..]
                                .iter()
                                .map(|arg| self.expr_to_string(arg))
                                .collect();
                            return extra_args.join(", ");
                        }
                        return String::new(); // No extra args
                    }
                }

                // Not *args - just a regular starred expression
                let value = self.walk_replacement_ast_impl(
                    &starred.value,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("*{}", value)
            }

            rpy_ast::Expr::Await(await_expr) => {
                let value = self.walk_replacement_ast_impl(
                    &await_expr.value,
                    call_args,
                    call_keywords,
                    parameters,
                    source_module,
                );
                format!("await {}", value)
            }

            rpy_ast::Expr::Slice(slice) => {
                let lower = slice
                    .lower
                    .as_ref()
                    .map(|e| {
                        self.walk_replacement_ast_impl(
                            e,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .unwrap_or_default();
                let upper = slice
                    .upper
                    .as_ref()
                    .map(|e| {
                        self.walk_replacement_ast_impl(
                            e,
                            call_args,
                            call_keywords,
                            parameters,
                            source_module,
                        )
                    })
                    .unwrap_or_default();
                if let Some(step) = &slice.step {
                    let step_str = self.walk_replacement_ast_impl(
                        step,
                        call_args,
                        call_keywords,
                        parameters,
                        source_module,
                    );
                    format!("{}:{}:{}", lower, upper, step_str)
                } else {
                    format!("{}:{}", lower, upper)
                }
            }

            // For other complex expression types that don't need parameter substitution,
            // use the regular expr_to_string_with_placeholders
            _ => {
                // For now, use the comprehensive handler for other types
                self.expr_to_string_with_placeholders(expr, &HashSet::new())
            }
        }
    }

    #[allow(clippy::only_used_in_recursion)] // self is needed for recursive method calls
    fn format_constant(&self, constant: &rustpython_ast::Constant) -> String {
        use rustpython_ast::Constant;
        match constant {
            Constant::Str(s) => format!("\"{}\"", s.replace("\\", "\\\\").replace("\"", "\\\"")),
            Constant::Bytes(b) => format!("b\"{}\"", String::from_utf8_lossy(b)),
            Constant::Int(i) => i.to_string(),
            Constant::Float(f) => f.to_string(),
            Constant::Complex { real, imag } => {
                if *real == 0.0 {
                    format!("{}j", imag)
                } else if *imag == 0.0 {
                    format!("{}", real)
                } else if *imag < 0.0 {
                    format!("{}{}j", real, imag)
                } else {
                    format!("{}+{}j", real, imag)
                }
            }
            Constant::Bool(b) => {
                if *b {
                    "True".to_string()
                } else {
                    "False".to_string()
                }
            }
            Constant::None => "None".to_string(),
            Constant::Ellipsis => "...".to_string(),
            Constant::Tuple(items) => {
                let elements: Vec<String> = items
                    .iter()
                    .map(|item| self.format_constant(item))
                    .collect();
                format!("({})", elements.join(", "))
            }
        }
    }

    fn rustpython_expr_to_string_for_template(
        &self,
        expr: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
        all_args: &[ast::Expr],
        all_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
    ) -> String {
        self.rustpython_expr_to_string_with_args_impl(
            expr,
            param_map,
            all_args,
            all_keywords,
            parameters,
            false,
        )
    }

    fn rustpython_expr_to_string_with_args_impl(
        &self,
        expr: &rustpython_ast::Expr,
        param_map: &HashMap<String, &ast::Expr>,
        all_args: &[ast::Expr],
        all_keywords: &[ast::Keyword],
        parameters: &[ParameterInfo],
        apply_name_resolution: bool,
    ) -> String {
        use rustpython_ast as rpy_ast;

        match expr {
            rpy_ast::Expr::Name(name) => {
                let name_str = name.id.as_str();

                // Check if this is a parameter that should be substituted
                if let Some(arg_expr) = param_map.get(name_str) {
                    return self.expr_to_string(arg_expr);
                }

                // Check if this is *args - don't substitute, let Starred handle it
                if parameters.iter().any(|p| p.is_vararg && p.name == name_str) {
                    return name_str.to_string();
                }

                // Check if this is **kwargs - don't substitute, let the dictionary unpacking handle it
                if parameters.iter().any(|p| p.is_kwarg && p.name == name_str) {
                    return name_str.to_string();
                }

                // This name doesn't have a substitution value
                // Check if it's supposed to be a parameter that should have been provided
                // Only return empty if we're actively doing substitution (param_map has entries)
                // and this is a regular parameter that should have been mapped
                if !param_map.is_empty()
                    && parameters
                        .iter()
                        .any(|p| p.name == name_str && !p.is_vararg && !p.is_kwarg)
                {
                    // We're doing parameter substitution and this parameter wasn't provided
                    return String::new();
                }

                // Otherwise, return the name as-is
                name_str.to_string()
            }
            rpy_ast::Expr::Call(call) => {
                // Handle function calls in the replacement - need to check if function needs module prefix
                let func_str = match &*call.func {
                    rpy_ast::Expr::Name(name) => {
                        let func_name = name.id.as_str();
                        // First check if this is a parameter that should be substituted
                        if let Some(param_expr) = param_map.get(func_name) {
                            self.expr_to_string(param_expr)
                        } else if let Some(full_name) = self.import_map.get(func_name) {
                            // Check if this function name is imported
                            full_name.clone()
                        } else {
                            // Check if it's a builtin function
                            if apply_name_resolution && !self.builtins.contains(func_name) {
                                // Not imported and not a builtin, add module prefix
                                format!("{}.{}", self.module_name, func_name)
                            } else {
                                func_name.to_string()
                            }
                        }
                    }
                    _ => {
                        // For attribute calls or other complex expressions, use normal processing
                        self.rustpython_expr_to_string_with_args_impl(
                            &call.func,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                            apply_name_resolution,
                        )
                    }
                };

                let mut args = Vec::new();
                for arg in &call.args {
                    let arg_str = self.rustpython_expr_to_string_with_args(
                        arg,
                        param_map,
                        all_args,
                        all_keywords,
                        parameters,
                    );
                    if !arg_str.is_empty() {
                        args.push(arg_str);
                    }
                }

                // Handle keywords in the replacement (including **kwargs expansion)
                for keyword in &call.keywords {
                    if let Some(arg_name) = &keyword.arg {
                        let value = self.rustpython_expr_to_string_with_args(
                            &keyword.value,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        );
                        // Skip if the value is just the parameter name (meaning it wasn't provided)
                        if !value.is_empty() && value != arg_name.as_str() {
                            args.push(format!("{}={}", arg_name, value));
                        }
                    } else {
                        // This is **something - check if it's **kwargs placeholder
                        if let rpy_ast::Expr::Name(name) = &keyword.value {
                            if parameters
                                .iter()
                                .any(|p| p.is_kwarg && p.name == name.id.as_str())
                            {
                                // This is **kwargs - check if we have actual dict unpacking
                                let has_dict_unpack =
                                    all_keywords.iter().any(|kw| kw.arg.is_none());

                                if has_dict_unpack {
                                    // We have **dict unpacking in the actual call - preserve it
                                    for kw in all_keywords {
                                        if kw.arg.is_none() {
                                            // This is dict unpacking
                                            args.push(format!(
                                                "**{}",
                                                self.expr_to_string(&kw.value)
                                            ));
                                        } else {
                                            // Regular keyword argument
                                            args.push(format!(
                                                "{}={}",
                                                kw.arg.as_ref().unwrap(),
                                                self.expr_to_string(&kw.value)
                                            ));
                                        }
                                    }
                                } else {
                                    // No dict unpacking, just regular keyword args
                                    for kw in all_keywords {
                                        if let Some(arg_name) = &kw.arg {
                                            args.push(format!(
                                                "{}={}",
                                                arg_name,
                                                self.expr_to_string(&kw.value)
                                            ));
                                        }
                                    }
                                }
                                continue;
                            }
                        }
                        // Regular ** unpacking
                        let value = self.rustpython_expr_to_string_with_args(
                            &keyword.value,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        );
                        if !value.is_empty() {
                            args.push(format!("**{}", value));
                        }
                    }
                }

                // Check if we need to add preserved dict unpacking
                let has_dict_unpack = all_keywords.iter().any(|kw| kw.arg.is_none());
                let has_kwarg_param = parameters.iter().any(|p| p.is_kwarg);

                if has_dict_unpack && !has_kwarg_param {
                    // Add the preserved dict unpacking
                    for kw in all_keywords {
                        if kw.arg.is_none() {
                            args.push(format!("**{}", self.expr_to_string(&kw.value)));
                        }
                    }
                }

                format!("{}({})", func_str, args.join(", "))
            }
            rpy_ast::Expr::Attribute(attr) => {
                let value = self.rustpython_expr_to_string_with_args(
                    &attr.value,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("{}.{}", value, attr.attr)
            }
            rpy_ast::Expr::BinOp(binop) => {
                let left = self.rustpython_expr_to_string_with_args(
                    &binop.left,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let right = self.rustpython_expr_to_string_with_args(
                    &binop.right,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let op = match &binop.op {
                    rpy_ast::Operator::Add => "+",
                    rpy_ast::Operator::Sub => "-",
                    rpy_ast::Operator::Mult => "*",
                    rpy_ast::Operator::Div => "/",
                    rpy_ast::Operator::FloorDiv => "//",
                    rpy_ast::Operator::Mod => "%",
                    rpy_ast::Operator::Pow => "**",
                    rpy_ast::Operator::LShift => "<<",
                    rpy_ast::Operator::RShift => ">>",
                    rpy_ast::Operator::BitOr => "|",
                    rpy_ast::Operator::BitXor => "^",
                    rpy_ast::Operator::BitAnd => "&",
                    rpy_ast::Operator::MatMult => "@",
                };
                format!("{} {} {}", left, op, right)
            }
            rpy_ast::Expr::Starred(starred) => {
                // Check if this is *args
                if let rpy_ast::Expr::Name(name) = &*starred.value {
                    if let Some(_param) = parameters
                        .iter()
                        .find(|p| p.is_vararg && p.name == name.id.as_str())
                    {
                        // This is *args - expand to actual arguments
                        let regular_param_count = parameters
                            .iter()
                            .filter(|p| !p.is_vararg && !p.is_kwarg)
                            .count();

                        // For functions with only *args and **kwargs, all positional args go to *args
                        if regular_param_count == 0 {
                            let args_strs: Vec<String> = all_args
                                .iter()
                                .map(|arg| self.expr_to_string(arg))
                                .collect();
                            return args_strs.join(", ");
                        }

                        // For functions with some regular params, extra args go to *args
                        if all_args.len() > regular_param_count {
                            let extra_args: Vec<String> = all_args[regular_param_count..]
                                .iter()
                                .map(|arg| self.expr_to_string(arg))
                                .collect();
                            return extra_args.join(", ");
                        }
                        return String::new(); // No extra args
                    }
                }

                // Regular starred expression
                let value = self.rustpython_expr_to_string_with_args(
                    &starred.value,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("*{}", value)
            }
            // Handle dictionary unpacking for **kwargs
            rpy_ast::Expr::Dict(dict) if dict.keys.is_empty() && dict.values.len() == 1 => {
                // This might be **kwargs
                if let rpy_ast::Expr::Name(name) = &dict.values[0] {
                    if let Some(_param) = parameters
                        .iter()
                        .find(|p| p.is_kwarg && p.name == name.id.as_str())
                    {
                        // This is **kwargs - expand to actual keyword arguments
                        let kwargs: Vec<String> = all_keywords
                            .iter()
                            .map(|kw| {
                                if let Some(arg_name) = &kw.arg {
                                    format!("{}={}", arg_name, self.expr_to_string(&kw.value))
                                } else {
                                    format!("**{}", self.expr_to_string(&kw.value))
                                }
                            })
                            .collect();
                        return kwargs.join(", ");
                    }
                }
                // Fall through to default handling
                let param_names: HashSet<String> = param_map.keys().cloned().collect();
                self.expr_to_string_with_placeholders(expr, &param_names)
            }
            rpy_ast::Expr::Await(await_expr) => {
                // Handle await expressions by recursively processing the value
                let value = self.rustpython_expr_to_string_with_args(
                    &await_expr.value,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("await {}", value)
            }
            rpy_ast::Expr::Subscript(subscript) => {
                // Handle subscript expressions like dict[key]
                let value = self.rustpython_expr_to_string_with_args(
                    &subscript.value,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let slice = self.rustpython_expr_to_string_with_args(
                    &subscript.slice,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("{}[{}]", value, slice)
            }
            rpy_ast::Expr::IfExp(if_exp) => {
                // Handle conditional expressions like (x if condition else y)
                let body = self.rustpython_expr_to_string_with_args(
                    &if_exp.body,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let test = self.rustpython_expr_to_string_with_args(
                    &if_exp.test,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let orelse = self.rustpython_expr_to_string_with_args(
                    &if_exp.orelse,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("{} if {} else {}", body, test, orelse)
            }
            rpy_ast::Expr::UnaryOp(unary) => {
                // Handle unary operations like -x, not x
                let operand = self.rustpython_expr_to_string_with_args(
                    &unary.operand,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let op = match &unary.op {
                    rustpython_ast::UnaryOp::Not => "not ",
                    rustpython_ast::UnaryOp::UAdd => "+",
                    rustpython_ast::UnaryOp::USub => "-",
                    rustpython_ast::UnaryOp::Invert => "~",
                };
                format!("{}{}", op, operand)
            }
            rpy_ast::Expr::Compare(compare) => {
                // Handle comparison operations like x > 0
                let mut result = self.rustpython_expr_to_string_with_args(
                    &compare.left,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                for (op, comparator) in compare.ops.iter().zip(&compare.comparators) {
                    let op_str = match op {
                        rustpython_ast::CmpOp::Eq => " == ",
                        rustpython_ast::CmpOp::NotEq => " != ",
                        rustpython_ast::CmpOp::Lt => " < ",
                        rustpython_ast::CmpOp::LtE => " <= ",
                        rustpython_ast::CmpOp::Gt => " > ",
                        rustpython_ast::CmpOp::GtE => " >= ",
                        rustpython_ast::CmpOp::Is => " is ",
                        rustpython_ast::CmpOp::IsNot => " is not ",
                        rustpython_ast::CmpOp::In => " in ",
                        rustpython_ast::CmpOp::NotIn => " not in ",
                    };
                    result.push_str(op_str);
                    result.push_str(&self.rustpython_expr_to_string_with_args(
                        comparator,
                        param_map,
                        all_args,
                        all_keywords,
                        parameters,
                    ));
                }
                result
            }
            rpy_ast::Expr::NamedExpr(named) => {
                // Handle walrus operator (x := value)
                let target = self.rustpython_expr_to_string_with_args(
                    &named.target,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                let value = self.rustpython_expr_to_string_with_args(
                    &named.value,
                    param_map,
                    all_args,
                    all_keywords,
                    parameters,
                );
                format!("({} := {})", target, value)
            }
            rpy_ast::Expr::BoolOp(boolop) => {
                // Handle boolean operations like (a or b)
                let values: Vec<String> = boolop
                    .values
                    .iter()
                    .map(|v| {
                        self.rustpython_expr_to_string_with_args(
                            v,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        )
                    })
                    .collect();
                let op = match &boolop.op {
                    rustpython_ast::BoolOp::And => " and ",
                    rustpython_ast::BoolOp::Or => " or ",
                };
                values.join(op)
            }
            rpy_ast::Expr::Dict(dict) => {
                // Handle dictionary literals like {}
                let mut items = Vec::new();
                for (key_opt, value) in dict.keys.iter().zip(&dict.values) {
                    if let Some(key) = key_opt {
                        let key_str = self.rustpython_expr_to_string_with_args(
                            key,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        );
                        let value_str = self.rustpython_expr_to_string_with_args(
                            value,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        );
                        items.push(format!("{}: {}", key_str, value_str));
                    } else {
                        // None key means dictionary unpacking
                        let value_str = self.rustpython_expr_to_string_with_args(
                            value,
                            param_map,
                            all_args,
                            all_keywords,
                            parameters,
                        );
                        items.push(format!("**{}", value_str));
                    }
                }
                format!("{{{}}}", items.join(", "))
            }
            // For other expression types, convert using the existing method
            _ => {
                // Check if this is a **kwargs reference in a Call's keywords
                if let rpy_ast::Expr::Name(name) = expr {
                    if let Some(_param) = parameters
                        .iter()
                        .find(|p| p.is_kwarg && p.name == name.id.as_str())
                    {
                        // This is **kwargs in a keyword context - expand it
                        let kwargs: Vec<String> = all_keywords
                            .iter()
                            .map(|kw| {
                                if let Some(arg_name) = &kw.arg {
                                    format!("{}={}", arg_name, self.expr_to_string(&kw.value))
                                } else {
                                    format!("**{}", self.expr_to_string(&kw.value))
                                }
                            })
                            .collect();
                        if !kwargs.is_empty() {
                            return kwargs.join(", ");
                        }
                    }
                }

                // Use the existing expr_to_string_with_placeholders but with a custom param_names set
                let param_names: HashSet<String> = param_map.keys().cloned().collect();
                self.expr_to_string_with_placeholders(expr, &param_names)
            }
        }
    }

    /// Transform a replacement AST with parameter substitution
    /// Determine if a magic method call should be unwrapped based on AST structure
    fn should_unwrap_magic_method_call(
        &self,
        replacement_ast: &rustpython_ast::Expr,
        builtin_name: &str,
    ) -> bool {
        use rustpython_ast::Expr;

        tracing::debug!(
            "should_unwrap_magic_method_call: builtin_name={}",
            builtin_name
        );

        // Check if this is a call to the same builtin function
        if let Expr::Call(call) = replacement_ast {
            if let Expr::Name(func_name) = &*call.func {
                if func_name.id.as_str() == builtin_name && call.args.len() == 1 {
                    tracing::debug!("Found matching builtin call, checking argument type");
                    // This is builtin(something) - check what the something is
                    match &call.args[0] {
                        // Simple parameter reference like {self} -> unwrap only if it's a direct parameter
                        Expr::Name(name) => {
                            let should_unwrap = name.id.as_str() == "self"; // Only unwrap if it's literally 'self'
                            tracing::debug!(
                                "Name argument '{}', should_unwrap: {}",
                                name.id,
                                should_unwrap
                            );
                            should_unwrap
                        }

                        // Attribute access like {self}.value -> DON'T unwrap, keep the builtin call
                        Expr::Attribute(_) => {
                            tracing::debug!(
                                "Attribute argument - NOT unwrapping to preserve builtin call"
                            );
                            false
                        }

                        // Call expression like {self}.method() -> unwrap if it's a method call on self
                        Expr::Call(call_expr) => {
                            // Check if this is a method call on self (self.method())
                            if let Expr::Attribute(attr) = &*call_expr.func {
                                if let Expr::Name(base_name) = &*attr.value {
                                    if base_name.id.as_str() == "self" {
                                        // This is self.method() - unwrap since the method should return the right type
                                        tracing::debug!("Method call on self - unwrapping");
                                        return true;
                                    }
                                }
                            }
                            tracing::debug!("Call argument - NOT unwrapping");
                            false
                        }

                        // Other complex expressions -> don't unwrap
                        _ => {
                            tracing::debug!("Other expression type - NOT unwrapping");
                            false
                        }
                    }
                } else {
                    tracing::debug!("Not matching builtin call or wrong arg count");
                    false
                }
            } else {
                tracing::debug!("Call func is not a simple name");
                false
            }
        } else {
            tracing::debug!("Replacement is not a call expression");
            false
        }
    }
}