From d9572f99603562561b6ff4f86c11c4a464479ccf Mon Sep 17 00:00:00 2001 From: dak2 Date: Sun, 8 Mar 2026 14:48:15 +0900 Subject: [PATCH] Add keyword argument support to type inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby's keyword arguments (def foo(name:, age: 0)) are widely used but were previously ignored by the type checker, causing incomplete type propagation when methods were called with keyword arguments. This change implements full keyword argument support across the type inference pipeline: - Parse keyword parameters (required and optional) in method definitions and register them as named vertices in the method registry - Separate positional and keyword arguments at call sites by detecting KeywordHashNode in the argument list - Propagate keyword argument types to parameter vertices by name matching in MethodCallBox, including through .new → initialize forwarding Design based on v0.1.8-D-keyword-args.md specification. Co-Authored-By: Claude Opus 4.6 --- rust/src/analyzer/attributes.rs | 3 +- rust/src/analyzer/calls.rs | 10 ++- rust/src/analyzer/definitions.rs | 8 +- rust/src/analyzer/dispatch.rs | 148 +++++++++++++++++++++++++++++-- rust/src/analyzer/parameters.rs | 59 ++++++++++-- rust/src/env/global_env.rs | 12 ++- rust/src/env/method_registry.rs | 10 ++- rust/src/graph/box.rs | 139 +++++++++++++++++++++++++++-- 8 files changed, 363 insertions(+), 26 deletions(-) diff --git a/rust/src/analyzer/attributes.rs b/rust/src/analyzer/attributes.rs index 1086af9..2971bc7 100644 --- a/rust/src/analyzer/attributes.rs +++ b/rust/src/analyzer/attributes.rs @@ -39,7 +39,7 @@ pub(crate) fn process_attr_declaration( // Register getter (attr_reader / attr_accessor) if matches!(kind, AttrKind::Reader | AttrKind::Accessor) { - genv.register_user_method(recv_ty.clone(), &attr_name, ivar_vtx, vec![]); + genv.register_user_method(recv_ty.clone(), &attr_name, ivar_vtx, vec![], None); } // Register setter (attr_writer / attr_accessor) @@ -51,6 +51,7 @@ pub(crate) fn process_attr_declaration( &format!("{}=", attr_name), ivar_vtx, vec![param_vtx], + None, ); } } diff --git a/rust/src/analyzer/calls.rs b/rust/src/analyzer/calls.rs index 19e5499..242a04f 100644 --- a/rust/src/analyzer/calls.rs +++ b/rust/src/analyzer/calls.rs @@ -5,6 +5,8 @@ //! - Managing return value vertices //! - Attaching source location for error reporting +use std::collections::HashMap; + use crate::env::GlobalEnv; use crate::graph::{MethodCallBox, VertexId}; use crate::source_map::SourceLocation; @@ -15,6 +17,7 @@ pub fn install_method_call( recv_vtx: VertexId, method_name: String, arg_vtxs: Vec, + kwarg_vtxs: Option>, location: Option, ) -> VertexId { // Create Vertex for return value @@ -22,7 +25,8 @@ pub fn install_method_call( // Create MethodCallBox with location and argument vertices let box_id = genv.alloc_box_id(); - let call_box = MethodCallBox::new(box_id, recv_vtx, method_name, ret_vtx, arg_vtxs, location); + let call_box = + MethodCallBox::new(box_id, recv_vtx, method_name, ret_vtx, arg_vtxs, kwarg_vtxs, location); genv.register_box(box_id, Box::new(call_box)); ret_vtx @@ -39,7 +43,7 @@ mod tests { let recv_vtx = genv.new_source(Type::string()); let ret_vtx = - install_method_call(&mut genv, recv_vtx, "upcase".to_string(), vec![], None); + install_method_call(&mut genv, recv_vtx, "upcase".to_string(), vec![], None, None); // Return vertex should exist assert!(genv.get_vertex(ret_vtx).is_some()); @@ -51,7 +55,7 @@ mod tests { let recv_vtx = genv.new_source(Type::string()); let _ret_vtx = - install_method_call(&mut genv, recv_vtx, "upcase".to_string(), vec![], None); + install_method_call(&mut genv, recv_vtx, "upcase".to_string(), vec![], None, None); // Box should be added assert_eq!(genv.box_count(), 1); diff --git a/rust/src/analyzer/definitions.rs b/rust/src/analyzer/definitions.rs index 22f9da9..4a464b7 100644 --- a/rust/src/analyzer/definitions.rs +++ b/rust/src/analyzer/definitions.rs @@ -6,6 +6,8 @@ //! - Method definition scope management (def baz ... end) //! - Extracting class/module names from AST nodes (including qualified names like Api::User) +use std::collections::HashMap; + use crate::env::{GlobalEnv, LocalEnv}; use crate::graph::{ChangeSet, VertexId}; use crate::types::Type; @@ -78,10 +80,10 @@ pub(crate) fn process_def_node( let merge_vtx = genv.scope_manager.current_method_return_vertex(); // Process parameters BEFORE processing body - let param_vtxs = if let Some(params_node) = def_node.parameters() { + let (param_vtxs, keyword_param_vtxs) = if let Some(params_node) = def_node.parameters() { install_parameters(genv, lenv, changes, source, ¶ms_node) } else { - vec![] + (vec![], HashMap::new()) }; let mut last_vtx = None; @@ -108,11 +110,13 @@ pub(crate) fn process_def_node( } else { Type::instance(&name) }; + let kw_params = (!keyword_param_vtxs.is_empty()).then_some(keyword_param_vtxs); genv.register_user_method( recv_type, &method_name, ret_vtx, param_vtxs, + kw_params, ); } } diff --git a/rust/src/analyzer/dispatch.rs b/rust/src/analyzer/dispatch.rs index 27838c9..e83dbc3 100644 --- a/rust/src/analyzer/dispatch.rs +++ b/rust/src/analyzer/dispatch.rs @@ -3,6 +3,8 @@ //! This module handles the pattern matching of Ruby AST nodes //! and dispatches them to specialized handlers. +use std::collections::HashMap; + use crate::env::{GlobalEnv, LocalEnv}; use crate::graph::{BlockParameterTypeBox, ChangeSet, VertexId}; use crate::source_map::SourceLocation; @@ -294,10 +296,31 @@ fn process_method_call_common<'a>( return Some(super::operators::process_not_operator(genv)); } - let arg_vtxs: Vec = arguments - .iter() - .filter_map(|arg| super::install::install_node(genv, lenv, changes, source, arg)) - .collect(); + // Separate positional arguments and keyword arguments + let mut positional_arg_vtxs: Vec = Vec::new(); + let mut keyword_arg_vtxs: HashMap = HashMap::new(); + + for arg in &arguments { + if let Some(kw_hash) = arg.as_keyword_hash_node() { + for element in kw_hash.elements().iter() { + let assoc = match element.as_assoc_node() { + Some(a) => a, + None => continue, + }; + let name = match assoc.key().as_symbol_node() { + Some(sym) => bytes_to_name(sym.unescaped()), + None => continue, + }; + if let Some(vtx) = + super::install::install_node(genv, lenv, changes, source, &assoc.value()) + { + keyword_arg_vtxs.insert(name, vtx); + } + } + } else if let Some(vtx) = super::install::install_node(genv, lenv, changes, source, arg) { + positional_arg_vtxs.push(vtx); + } + } if let Some(block_node) = block { if let Some(block) = block_node.as_block_node() { @@ -318,8 +341,19 @@ fn process_method_call_common<'a>( } } + let kwarg_vtxs = if keyword_arg_vtxs.is_empty() { + None + } else { + Some(keyword_arg_vtxs) + }; + Some(finish_method_call( - genv, recv_vtx, method_name, arg_vtxs, location, + genv, + recv_vtx, + method_name, + positional_arg_vtxs, + kwarg_vtxs, + location, )) } @@ -329,9 +363,10 @@ fn finish_method_call( recv_vtx: VertexId, method_name: String, arg_vtxs: Vec, + kwarg_vtxs: Option>, location: SourceLocation, ) -> VertexId { - install_method_call(genv, recv_vtx, method_name, arg_vtxs, Some(location)) + install_method_call(genv, recv_vtx, method_name, arg_vtxs, kwarg_vtxs, Some(location)) } #[cfg(test)] @@ -984,4 +1019,105 @@ end genv.type_errors ); } + + // === Keyword argument tests === + + // Test 26: Required keyword argument type propagation + #[test] + fn test_keyword_arg_required_propagation() { + let source = r#" +class Greeter + def greet(name:) + name + end +end + +Greeter.new.greet(name: "Alice") +"#; + let genv = analyze(source); + + let info = genv + .resolve_method(&Type::instance("Greeter"), "greet") + .expect("Greeter#greet should be registered"); + let ret_vtx = info.return_vertex.unwrap(); + assert_eq!(get_type_show(&genv, ret_vtx), "String"); + } + + // Test 27: Optional keyword argument with default type + #[test] + fn test_keyword_arg_optional_default_type() { + let source = r#" +class Counter + def count(step: 1) + step + end +end +"#; + let genv = analyze(source); + + let info = genv + .resolve_method(&Type::instance("Counter"), "count") + .expect("Counter#count should be registered"); + let ret_vtx = info.return_vertex.unwrap(); + // step has Integer type from default value + assert_eq!(get_type_show(&genv, ret_vtx), "Integer"); + } + + // Test 28: Positional and keyword arguments mixed + #[test] + fn test_positional_and_keyword_mixed() { + let source = r#" +class User + def initialize(id, name:) + @id = id + @name = name + end +end + +User.new(1, name: "Alice") +"#; + let genv = analyze(source); + assert!(genv.type_errors.is_empty()); + } + + // Test 29: Keyword argument via .new propagation to initialize + #[test] + fn test_keyword_arg_via_new_to_initialize() { + let source = r#" +class Config + def initialize(debug:) + @debug = debug + end + + def debug? + @debug + end +end + +Config.new(debug: true) +"#; + let genv = analyze(source); + assert!(genv.type_errors.is_empty()); + } + + // Test 30: Multiple keyword arguments + #[test] + fn test_multiple_keyword_args() { + let source = r#" +class User + def profile(name:, age:) + name + end +end + +User.new.profile(name: "Alice", age: 30) +"#; + let genv = analyze(source); + + let info = genv + .resolve_method(&Type::instance("User"), "profile") + .expect("User#profile should be registered"); + let ret_vtx = info.return_vertex.unwrap(); + assert_eq!(get_type_show(&genv, ret_vtx), "String"); + } } diff --git a/rust/src/analyzer/parameters.rs b/rust/src/analyzer/parameters.rs index 5c76ed9..30dd729 100644 --- a/rust/src/analyzer/parameters.rs +++ b/rust/src/analyzer/parameters.rs @@ -5,6 +5,8 @@ //! - Creating vertices for parameters //! - Registering parameters as local variables in method scope +use std::collections::HashMap; + use crate::env::{GlobalEnv, LocalEnv}; use crate::graph::{ChangeSet, VertexId}; use crate::types::Type; @@ -117,16 +119,18 @@ pub fn install_keyword_rest_parameter( /// Install method parameters as local variables /// -/// Returns a Vec of VertexId for required and optional parameters (positional), -/// which can be used for argument-to-parameter type propagation. +/// Returns a tuple of: +/// - Vec: positional parameter vertices (required and optional) +/// - HashMap: keyword parameter vertices (name → vertex) pub(crate) fn install_parameters( genv: &mut GlobalEnv, lenv: &mut LocalEnv, changes: &mut ChangeSet, source: &str, params_node: &ruby_prism::ParametersNode, -) -> Vec { +) -> (Vec, HashMap) { let mut param_vtxs = Vec::new(); + let mut keyword_param_vtxs: HashMap = HashMap::new(); // Required parameters: def foo(a, b) for node in params_node.requireds().iter() { @@ -165,8 +169,30 @@ pub(crate) fn install_parameters( } } + // Keyword parameters: def foo(name:, age: 0) + // Reuses install_required_parameter / install_optional_parameter + // since the logic is identical for positional and keyword parameters. + for node in params_node.keywords().iter() { + if let Some(req_kw) = node.as_required_keyword_parameter_node() { + let name = bytes_to_name(req_kw.name().as_slice()); + let vtx = install_required_parameter(genv, lenv, name.clone()); + keyword_param_vtxs.insert(name, vtx); + } else if let Some(opt_kw) = node.as_optional_keyword_parameter_node() { + let name = bytes_to_name(opt_kw.name().as_slice()); + let default_value = opt_kw.value(); + let vtx = if let Some(default_vtx) = + super::install::install_node(genv, lenv, changes, source, &default_value) + { + install_optional_parameter(genv, lenv, changes, name.clone(), default_vtx) + } else { + install_required_parameter(genv, lenv, name.clone()) + }; + keyword_param_vtxs.insert(name, vtx); + } + } + // Keyword rest parameter: def foo(**kwargs) - // Not included in param_vtxs (keyword args need special handling) + // Not included in keyword_param_vtxs (collects all remaining keywords) if let Some(kwrest_node) = params_node.keyword_rest() { if let Some(kwrest_param) = kwrest_node.as_keyword_rest_parameter_node() { if let Some(name_id) = kwrest_param.name() { @@ -176,7 +202,7 @@ pub(crate) fn install_parameters( } } - param_vtxs + (param_vtxs, keyword_param_vtxs) } #[cfg(test)] @@ -217,4 +243,27 @@ mod tests { assert_ne!(vtx_b, vtx_c); assert_ne!(vtx_a, vtx_c); } + + #[test] + fn test_install_optional_parameter_inherits_default_type() { + let mut genv = GlobalEnv::new(); + let mut lenv = LocalEnv::new(); + let mut changes = ChangeSet::new(); + + // Default value: 0 (Integer) + let default_vtx = genv.new_source(Type::integer()); + let vtx = install_optional_parameter( + &mut genv, + &mut lenv, + &mut changes, + "age".to_string(), + default_vtx, + ); + + assert_eq!(lenv.get_var("age"), Some(vtx)); + + // Type should propagate from default value + let vertex = genv.get_vertex(vtx).unwrap(); + assert_eq!(vertex.show(), "Integer"); + } } diff --git a/rust/src/env/global_env.rs b/rust/src/env/global_env.rs index de4a1f5..d6c82e8 100644 --- a/rust/src/env/global_env.rs +++ b/rust/src/env/global_env.rs @@ -3,6 +3,8 @@ //! This module provides a unified interface for managing vertices, boxes, //! methods, type errors, and scopes during type inference. +use std::collections::HashMap; + use crate::env::box_manager::BoxManager; use crate::env::method_registry::{MethodInfo, MethodRegistry}; use crate::env::scope::{Scope, ScopeId, ScopeKind, ScopeManager}; @@ -184,9 +186,15 @@ impl GlobalEnv { method_name: &str, return_vertex: VertexId, param_vertices: Vec, + keyword_param_vertices: Option>, ) { - self.method_registry - .register_user_method(recv_ty, method_name, return_vertex, param_vertices); + self.method_registry.register_user_method( + recv_ty, + method_name, + return_vertex, + param_vertices, + keyword_param_vertices, + ); } // ===== Type Errors ===== diff --git a/rust/src/env/method_registry.rs b/rust/src/env/method_registry.rs index 5b1cad6..7859dda 100644 --- a/rust/src/env/method_registry.rs +++ b/rust/src/env/method_registry.rs @@ -1,9 +1,10 @@ //! Method registration and resolution +use std::collections::HashMap; + use crate::graph::VertexId; use crate::types::Type; use smallvec::SmallVec; -use std::collections::HashMap; const OBJECT_CLASS: &str = "Object"; const KERNEL_MODULE: &str = "Kernel"; @@ -15,6 +16,7 @@ pub struct MethodInfo { pub block_param_types: Option>, pub return_vertex: Option, pub param_vertices: Option>, + pub keyword_param_vertices: Option>, } /// Registry for method definitions @@ -51,6 +53,7 @@ impl MethodRegistry { block_param_types, return_vertex: None, param_vertices: None, + keyword_param_vertices: None, }, ); } @@ -62,6 +65,7 @@ impl MethodRegistry { method_name: &str, return_vertex: VertexId, param_vertices: Vec, + keyword_param_vertices: Option>, ) { self.methods.insert( (recv_ty, method_name.to_string()), @@ -70,6 +74,7 @@ impl MethodRegistry { block_param_types: None, return_vertex: Some(return_vertex), param_vertices: Some(param_vertices), + keyword_param_vertices, }, ); } @@ -134,7 +139,7 @@ mod tests { fn test_register_user_method_and_resolve() { let mut registry = MethodRegistry::new(); let return_vtx = VertexId(42); - registry.register_user_method(Type::instance("User"), "name", return_vtx, vec![]); + registry.register_user_method(Type::instance("User"), "name", return_vtx, vec![], None); let info = registry.resolve(&Type::instance("User"), "name").unwrap(); assert_eq!(info.return_vertex, Some(VertexId(42))); @@ -151,6 +156,7 @@ mod tests { "add", return_vtx, param_vtxs, + None, ); let info = registry.resolve(&Type::instance("Calc"), "add").unwrap(); diff --git a/rust/src/graph/box.rs b/rust/src/graph/box.rs index f165eff..bba893b 100644 --- a/rust/src/graph/box.rs +++ b/rust/src/graph/box.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use crate::env::GlobalEnv; use crate::graph::change_set::ChangeSet; use crate::graph::vertex::VertexId; @@ -28,6 +30,22 @@ fn propagate_arguments( } } +/// Propagate keyword argument types to keyword parameter vertices by name +fn propagate_keyword_arguments( + kwarg_vtxs: Option<&HashMap>, + kw_param_vtxs: Option<&HashMap>, + changes: &mut ChangeSet, +) { + let (Some(args), Some(params)) = (kwarg_vtxs, kw_param_vtxs) else { + return; + }; + for (name, arg_vtx) in args { + if let Some(¶m_vtx) = params.get(name) { + changes.add_edge(*arg_vtx, param_vtx); + } + } +} + /// Box representing a method call #[allow(dead_code)] pub struct MethodCallBox { @@ -36,6 +54,7 @@ pub struct MethodCallBox { method_name: String, ret: VertexId, arg_vtxs: Vec, + kwarg_vtxs: Option>, location: Option, // Source code location /// Number of times this box has been rescheduled reschedule_count: u8, @@ -51,6 +70,7 @@ impl MethodCallBox { method_name: String, ret: VertexId, arg_vtxs: Vec, + kwarg_vtxs: Option>, location: Option, ) -> Self { Self { @@ -59,6 +79,7 @@ impl MethodCallBox { method_name, ret, arg_vtxs, + kwarg_vtxs, location, reschedule_count: 0, } @@ -90,6 +111,11 @@ impl MethodCallBox { method_info.param_vertices.as_deref(), changes, ); + propagate_keyword_arguments( + self.kwarg_vtxs.as_ref(), + method_info.keyword_param_vertices.as_ref(), + changes, + ); } else { // Builtin/RBS method: create source with fixed return type let ret_src_id = genv.new_source(method_info.return_type.clone()); @@ -118,10 +144,17 @@ impl MethodCallBox { let ret_src = genv.new_source(instance_type.clone()); changes.add_edge(ret_src, self.ret); - let init_params = genv - .resolve_method(&instance_type, "initialize") - .and_then(|info| info.param_vertices.as_deref()); - propagate_arguments(&self.arg_vtxs, init_params, changes); + let init_info = genv.resolve_method(&instance_type, "initialize"); + propagate_arguments( + &self.arg_vtxs, + init_info.and_then(|info| info.param_vertices.as_deref()), + changes, + ); + propagate_keyword_arguments( + self.kwarg_vtxs.as_ref(), + init_info.and_then(|info| info.keyword_param_vertices.as_ref()), + changes, + ); } else { self.report_type_error(recv_ty, genv); } @@ -325,6 +358,7 @@ mod tests { "upcase".to_string(), ret_vtx, vec![], + None, None, // No location in test ); @@ -357,6 +391,7 @@ mod tests { "unknown_method".to_string(), ret_vtx, vec![], + None, None, // No location in test ); @@ -552,7 +587,7 @@ mod tests { let body_src = genv.new_source(Type::string()); // Register user-defined method User#name with return_vertex - genv.register_user_method(Type::instance("User"), "name", body_src, vec![]); + genv.register_user_method(Type::instance("User"), "name", body_src, vec![], None); // Simulate: user.name (receiver has type User) let recv_vtx = genv.new_vertex(); @@ -568,6 +603,7 @@ mod tests { ret_vtx, vec![], None, + None, ); genv.register_box(box_id, Box::new(call_box)); @@ -598,6 +634,7 @@ mod tests { inner_ret_vtx, vec![], None, + None, ); genv.register_box(inner_box_id, Box::new(inner_call)); @@ -607,6 +644,7 @@ mod tests { "format", inner_ret_vtx, vec![param_vtx], + None, ); // 5. Simulate call: Formatter.new.format(42) @@ -625,6 +663,7 @@ mod tests { call_ret_vtx, vec![arg_vtx], None, + None, ); genv.register_box(call_box_id, Box::new(call_box)); @@ -637,4 +676,94 @@ mod tests { // Return type should be String (Integer#to_s -> String) assert_eq!(genv.get_vertex(call_ret_vtx).unwrap().show(), "String"); } + + #[test] + fn test_keyword_arg_propagation() { + let mut genv = GlobalEnv::new(); + + // Simulate: def greet(name:); name; end + let param_vtx = genv.new_vertex(); + let mut kw_params = HashMap::new(); + kw_params.insert("name".to_string(), param_vtx); + + genv.register_user_method( + Type::instance("Greeter"), + "greet", + param_vtx, // return vertex = param (returns name) + vec![], + Some(kw_params), + ); + + // Simulate call: Greeter.new.greet(name: "Alice") + let recv_vtx = genv.new_vertex(); + let recv_src = genv.new_source(Type::instance("Greeter")); + genv.add_edge(recv_src, recv_vtx); + + let arg_vtx = genv.new_source(Type::string()); + let mut kwarg_vtxs = HashMap::new(); + kwarg_vtxs.insert("name".to_string(), arg_vtx); + + let ret_vtx = genv.new_vertex(); + let box_id = genv.alloc_box_id(); + let call_box = MethodCallBox::new( + box_id, + recv_vtx, + "greet".to_string(), + ret_vtx, + vec![], + Some(kwarg_vtxs), + None, + ); + genv.register_box(box_id, Box::new(call_box)); + + genv.run_all(); + + // param_vtx should have String type (propagated from keyword argument) + assert_eq!(genv.get_vertex(param_vtx).unwrap().show(), "String"); + assert_eq!(genv.get_vertex(ret_vtx).unwrap().show(), "String"); + } + + #[test] + fn test_keyword_arg_name_mismatch_skipped() { + let mut genv = GlobalEnv::new(); + + let param_vtx = genv.new_vertex(); + let mut kw_params = HashMap::new(); + kw_params.insert("name".to_string(), param_vtx); + + genv.register_user_method( + Type::instance("Greeter"), + "greet", + param_vtx, + vec![], + Some(kw_params), + ); + + let recv_vtx = genv.new_vertex(); + let recv_src = genv.new_source(Type::instance("Greeter")); + genv.add_edge(recv_src, recv_vtx); + + // Wrong keyword name: "title" instead of "name" + let arg_vtx = genv.new_source(Type::string()); + let mut kwarg_vtxs = HashMap::new(); + kwarg_vtxs.insert("title".to_string(), arg_vtx); + + let ret_vtx = genv.new_vertex(); + let box_id = genv.alloc_box_id(); + let call_box = MethodCallBox::new( + box_id, + recv_vtx, + "greet".to_string(), + ret_vtx, + vec![], + Some(kwarg_vtxs), + None, + ); + genv.register_box(box_id, Box::new(call_box)); + + genv.run_all(); + + // param_vtx should remain untyped (name mismatch → no propagation) + assert_eq!(genv.get_vertex(param_vtx).unwrap().show(), "untyped"); + } }