From fdf5e909f2e5927100e3c57c2c20920c1e4b6378 Mon Sep 17 00:00:00 2001 From: shayyz-code Date: Wed, 29 Jul 2026 02:13:10 +0630 Subject: [PATCH] refactor(interpreter): resolve strict Clippy findings --- TODO.md | 8 +- src/interpreter.rs | 241 +++++++++++++++++++--------------------- tests/language_specs.rs | 10 ++ 3 files changed, 132 insertions(+), 127 deletions(-) diff --git a/TODO.md b/TODO.md index 7f5864f..7c846b8 100644 --- a/TODO.md +++ b/TODO.md @@ -10,13 +10,13 @@ Snapshot: 2026-07-28. - `cargo fmt --all -- --check` passes and is enforced for pull requests. - `cargo check --all-targets` passes. -- `cargo test` passes and is enforced for pull requests, including all 47 integration specifications and the library doctest. +- `cargo test` passes and is enforced for pull requests, including all 48 integration specifications and the library doctest. - The crate exposes checked file/source execution APIs and typed I/O, parse, and runtime error categories. - Lexer, parser, interpreter, type-inference, examples, mdBook documentation, and cargo-dist release assets exist. ### Quality gaps -- Strict Clippy reports 36 errors across the interpreter, lexer, parser, type inference, and symbol table. +- Strict Clippy reports 16 errors across the lexer, parser, type inference, and symbol table. - Panic recovery wraps rather than removes many panic, `unwrap`, and `expect` paths; the interpreter alone contains roughly 90. - `parser.rs` and `interpreter.rs` are approximately 975 and 1,217 lines and mix several responsibilities. - Checked and unchecked parsing/execution paths duplicate logic. @@ -71,7 +71,9 @@ Snapshot: 2026-07-28. - [x] Open and complete an issue that applies rustfmt, adds `cargo fmt --all -- --check` to CI, and changes no behavior. - [x] Fix the library doctest against the v0.1 API, make `cargo test` green, and require it in CI. -- [ ] Resolve strict Clippy findings without suppressing project-wide lints, then require `-D warnings` in CI. +- [x] Resolve strict Clippy findings in the interpreter without lint suppressions or behavior changes. +- [ ] Resolve the remaining strict Clippy findings in the lexer, parser, type inference, and symbol table. +- [ ] Require `cargo clippy --all-targets --all-features -- -D warnings` in pull-request CI. - [ ] Separate generated mdBook output from sources and define one reproducible documentation build command. - [ ] Reconcile README commands, branch names, CI claims, supported features, and examples with executable behavior. - [ ] Add focused lexer/parser error tests for malformed strings, comments, UTF-8 input, and unexpected EOF. diff --git a/src/interpreter.rs b/src/interpreter.rs index 8b000bb..a477144 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -223,31 +223,29 @@ impl Value { // } } -// Implement `to_string` for Value to handle printing -impl ToString for Value { - fn to_string(&self) -> String { +// Implement `Display` for Value to handle printing and string conversion. +impl fmt::Display for Value { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Value::Null => "null".to_string(), - Value::Int(v) => v.to_string(), - Value::Float(v) => v.to_string(), - Value::Boolean(v) => v.to_string(), - Value::Char(v) => v.to_string(), - Value::String(v) => v.clone(), + Value::Null => f.write_str("null"), + Value::Int(v) => write!(f, "{v}"), + Value::Float(v) => write!(f, "{v}"), + Value::Boolean(v) => write!(f, "{v}"), + Value::Char(v) => write!(f, "{v}"), + Value::String(v) => f.write_str(v), Value::Vector(v) => { let mut vec = Vec::new(); for i in v.iter() { vec.push(i.to_string()) } - format!("{:?}", vec) - } - Value::Map(m) => { - format!("{:?}", m) + write!(f, "{vec:?}") } + Value::Map(m) => write!(f, "{m:?}"), Value::Function(params, body, return_type) => { - format!("{:?}", (params, body, return_type)) + write!(f, "{:?}", (params, body, return_type)) } Value::Struct(_, _, _, _) => panic!("Cannot be displayed"), - Value::BuiltinFunction(_) => "BuiltinFunction".to_string(), + Value::BuiltinFunction(_) => f.write_str("BuiltinFunction"), } } } @@ -264,6 +262,12 @@ pub struct Scope { variables: HashMap, } +impl Default for Scope { + fn default() -> Self { + Self::new() + } +} + impl Scope { pub fn new() -> Self { Scope { @@ -324,6 +328,12 @@ pub struct Interpreter { modules: HashMap>, } +impl Default for Interpreter { + fn default() -> Self { + Self::new() + } +} + impl Interpreter { pub fn new() -> Self { // Start with a global scope @@ -372,7 +382,7 @@ impl Interpreter { "pout".to_string(), Value::BuiltinFunction(|args| { for arg in args { - print!("{}", arg.to_string()); + print!("{arg}"); } Value::Null // Returning a dummy value }), @@ -380,9 +390,9 @@ impl Interpreter { std.insert( "poutln".to_string(), Value::BuiltinFunction(|args| { - if args.len() > 0 { + if !args.is_empty() { for arg in args { - print!("{}", arg.to_string()); + print!("{arg}"); } } println!(); @@ -449,7 +459,7 @@ impl Interpreter { ), }, Value::String(s) => match method_name { - "chars" => Value::Vector(s.chars().map(|c| Value::Char(c)).collect()), + "chars" => Value::Vector(s.chars().map(Value::Char).collect()), "len" => { if !args.is_empty() { panic!("Method 'len' does not take arguments"); @@ -494,7 +504,7 @@ impl Interpreter { panic!("Method 'nth' requires exactly 1 arguments"); } match args[0] { - Value::Int(i) => v.iter().nth(i as usize).unwrap_or(&Value::Null).clone(), + Value::Int(i) => v.get(i as usize).unwrap_or(&Value::Null).clone(), _ => panic!("Method 'nth' needs type Int"), } } @@ -503,84 +513,77 @@ impl Interpreter { }, Value::Map(m) => { let mut function_return_value = Value::Null; // Default dummy value - let get_prototypes = m.get(&"__prototypes__".to_string()); - if get_prototypes.is_some() { - let prototypes = get_prototypes.unwrap(); - if let Value::Map(prototype_vals) = prototypes { - let get_method_from_prototype_vals = - prototype_vals.get(&method_name.to_string()); - if let Some(Value::Function(params, body, return_type)) = - get_method_from_prototype_vals - { - if params.len() != args.len() { - panic!( - "Method {} expects {} arguments, but {} were provided", - method_name, - params.len(), - args.len() - ); - } - // Push a new scope and set the current function context - self.scopes.push(Scope::new()); - - self.current_scope().set_variable( - "__current_function__".to_string(), - Variable { - value: get_method_from_prototype_vals.unwrap().clone(), - is_mutable: false, - var_type: Type::Function( - params.iter().map(|j| j.1.clone()).collect(), - Box::new(return_type.clone()), - ), - }, + if let Some(Value::Map(prototype_vals)) = m.get("__prototypes__") { + let method = prototype_vals.get(method_name); + if let Some(method @ Value::Function(params, body, return_type)) = method { + if params.len() != args.len() { + panic!( + "Method {} expects {} arguments, but {} were provided", + method_name, + params.len(), + args.len() ); + } + // Push a new scope and set the current function context + self.scopes.push(Scope::new()); - let mut m_types = HashMap::new(); - // Set function parameters && m_types for self - for (param, arg_value) in params.iter().zip(args) { - m_types.insert(param.0.clone(), param.1.clone()); - self.current_scope().set_variable( - param.0.clone(), - Variable { - value: arg_value, - is_mutable: true, - var_type: param.1.clone(), - }, - ); - } + self.current_scope().set_variable( + "__current_function__".to_string(), + Variable { + value: method.clone(), + is_mutable: false, + var_type: Type::Function( + params.iter().map(|j| j.1.clone()).collect(), + Box::new(return_type.clone()), + ), + }, + ); - // Set self + let mut m_types = HashMap::new(); + // Set function parameters && m_types for self + for (param, arg_value) in params.iter().zip(args) { + m_types.insert(param.0.clone(), param.1.clone()); self.current_scope().set_variable( - "self".to_string(), + param.0.clone(), Variable { - value: Value::Map(m.clone()), + value: arg_value, is_mutable: true, - var_type: Type::Map(m_types), + var_type: param.1.clone(), }, ); + } - for stmt in body { - self.exec_stmt(&stmt); - if let Some(return_val) = self.return_value.take() { - function_return_value = return_val; - break; - } + // Set self + self.current_scope().set_variable( + "self".to_string(), + Variable { + value: Value::Map(m.clone()), + is_mutable: true, + var_type: Type::Map(m_types), + }, + ); + + for stmt in body { + self.exec_stmt(stmt); + if let Some(return_val) = self.return_value.take() { + function_return_value = return_val; + break; } + } - // Pop the scope after execution - self.scopes.pop(); + // Pop the scope after execution + self.scopes.pop(); - // Validate return type + // Validate return type - if !function_return_value.is_of_type(&return_type) { - panic!( - "Function {} returned a value of mismatched type. Expected {:?}, got {:?}", - method_name, return_type, function_return_value - ); - } - } else { - panic!("Method '{}' not supported for type {:?}", method_name, m); + if !function_return_value.is_of_type(return_type) { + panic!( + "Function {} returned a value of mismatched type. Expected {:?}, got {:?}", + method_name, return_type, function_return_value + ); } + } else { + panic!("Method '{}' not supported for type {:?}", method_name, m); } } function_return_value @@ -604,14 +607,14 @@ impl Interpreter { Expr::Identifier(name) => { let var = self .find_variable(name) - .expect(&format!("Undefined variable: {}", name)); + .unwrap_or_else(|| panic!("Undefined variable: {}", name)); var.value } Expr::Vector(elements, extensor) => { let evaluated_elements: Vec = elements.iter().map(|e| self.eval_expr(e)).collect(); if let Some(extensor_expr) = extensor { - let evaluated_extensor = self.eval_expr(&extensor_expr); + let evaluated_extensor = self.eval_expr(extensor_expr); match evaluated_extensor { Value::Int(i) => { let vector = vec![evaluated_elements[0].clone(); i as usize]; @@ -727,27 +730,23 @@ impl Interpreter { let val = self.eval_expr(prop_expr.1); props.insert(prop_expr.0.clone(), val); } - if struct_var.is_some() { - let struct_val = struct_var.unwrap().value; + if let Some(struct_var) = struct_var { + let struct_val = struct_var.value; if let Value::Struct(prop_types, _, impl_stmts, _) = struct_val { - for prop_type in prop_types.iter() { - match prop_type { - (p_name, p_type) => { - let a = props.get(p_name); - if a.is_none() { - panic!( - "Mismatched key '{}' on struct compound - '{}'", - p_name, struct_name - ) - } - let b = a.unwrap().get_type(); - if b != p_type.clone() { - panic!( - "Mismatched data type, expected '{:?}' but got '{:?}' during initializing '{}'", - p_type, b, struct_name - ) - } - } + for (p_name, p_type) in prop_types.iter() { + let a = props.get(p_name); + if a.is_none() { + panic!( + "Mismatched key '{}' on struct compound - '{}'", + p_name, struct_name + ) + } + let b = a.unwrap().get_type(); + if b != p_type.clone() { + panic!( + "Mismatched data type, expected '{:?}' but got '{:?}' during initializing '{}'", + p_type, b, struct_name + ) } } @@ -825,7 +824,7 @@ impl Interpreter { let mut function_return_value = Value::Null; // Default dummy value for stmt in body { - self.exec_stmt(&stmt); + self.exec_stmt(stmt); if let Some(return_val) = self.return_value.take() { function_return_value = return_val; break; @@ -837,7 +836,7 @@ impl Interpreter { // Validate return type - if !function_return_value.is_of_type(&return_type) { + if !function_return_value.is_of_type(return_type) { panic!( "Function {} returned a value of mismatched type. Expected {:?}, got {:?}", name, return_type, function_return_value @@ -900,19 +899,12 @@ impl Interpreter { let mut props = HashMap::new(); let mut impl_stmts = impl_stmts_map.clone(); for inherit_name in inherit_names.iter() { - let inherit_struct = self.find_variable(&inherit_name); - if inherit_struct.is_none() { - panic!("Undefined Struct for inheritance"); - } else { - let inherit_struct = inherit_struct.unwrap(); + let inherit_struct = self.find_variable(inherit_name); + if let Some(inherit_struct) = inherit_struct { match inherit_struct.value { Value::Struct(inhe_props, _, inhe_impl_stmts_map, _) => { props.extend(inhe_props); - for stmt in inhe_impl_stmts_map - .get(&"Self".to_string()) - .unwrap() - .clone() - { + for stmt in inhe_impl_stmts_map.get("Self").unwrap().clone() { impl_stmts .entry(inherit_name.clone()) .or_insert_with(Vec::new) @@ -921,6 +913,8 @@ impl Interpreter { } _ => panic!("Inheritance from a Struct is only allowed"), } + } else { + panic!("Undefined Struct for inheritance"); } } for property in properties.iter() { @@ -1179,13 +1173,12 @@ impl Interpreter { value: Value::Function(.., return_type), .. }) = current_function + && !return_value.is_of_type(&return_type) { - if !return_value.is_of_type(&return_type) { - panic!( - "Return value type mismatch. Expected {:?}, got {:?}", - return_type, return_value - ); - } + panic!( + "Return value type mismatch. Expected {:?}, got {:?}", + return_type, return_value + ); } self.return_value = Some(return_value); diff --git a/tests/language_specs.rs b/tests/language_specs.rs index 3acc9c8..e07862c 100644 --- a/tests/language_specs.rs +++ b/tests/language_specs.rs @@ -34,6 +34,16 @@ fn run_checked_with_temp_file(label: &str, source: &str) -> Result result } +#[test] +fn spec_value_display_preserves_legacy_output() { + assert_eq!(Value::Int(10).to_string(), "10"); + assert_eq!(format!("{}", Value::String("poo".to_string())), "poo"); + assert_eq!( + Value::Vector(vec![Value::Int(1), Value::Boolean(true)]).to_string(), + r#"["1", "true"]"# + ); +} + fn run_unchecked_with_temp_file_path(label: &str, source: &str) -> (String, Option) { let file_path = unique_temp_file_path(label); fs::write(&file_path, source).expect("failed to write temp source");