diff --git a/docs/STRESS_TEST_FINDINGS.md b/docs/STRESS_TEST_FINDINGS.md index 7117bdc..7394586 100644 --- a/docs/STRESS_TEST_FINDINGS.md +++ b/docs/STRESS_TEST_FINDINGS.md @@ -2,48 +2,43 @@ This document summarizes issues found during systematic stress testing of the Lux parser, typechecker, and REPL. -## Critical Bugs +## Critical Bugs (All Fixed) -### 1. Record Equality Returns False for Equal Records +### 1. ~~Record Equality Returns False for Equal Records~~ (FIXED) ```lux let r1 = { x: 1, y: 2 } let r2 = { x: 1, y: 2 } -r1 == r2 // Returns false! Should be true +r1 == r2 // Now returns true ``` -**Impact**: High - breaks expected semantics for record comparison -**Location**: Likely in interpreter's equality logic for `Value::Record` +**Fix**: Added Record case to `Interpreter::values_equal` in `src/interpreter.rs` -### 2. Invalid Escape Sequences Silently Accepted +### 2. ~~Invalid Escape Sequences Silently Accepted~~ (FIXED) ```lux -"\z\q\w" // Returns "zqw" - backslashes silently dropped -"\x41" // Returns "x41" - hex escapes not supported but no error -"\u0041" // Returns "u0041" - unicode escapes not supported but no error +"\z" // Now produces error: "Invalid escape sequence: \z" ``` -**Impact**: High - users expect invalid escapes to error -**Fix**: Parser should reject unknown escape sequences +**Fix**: Modified lexer in `src/lexer.rs` to error on unknown escape sequences -### 3. Unknown Effects Silently Accepted in Declarations +### 3. ~~Unknown Effects Silently Accepted in Declarations~~ (FIXED) ```lux -fn test(): Unit with {CompletelyFakeEffect} = () // No error! +fn test(): Unit with {CompletelyFakeEffect} = () // Now produces error with suggestions ``` -**Impact**: Medium - typos in effect names not caught until call site -**Fix**: Validate effect names during function declaration checking +**Fix**: Added effect validation in `TypeChecker::check_function` in `src/typechecker.rs` -### 4. No Forward References for Mutual Recursion +### 4. ~~No Forward References for Mutual Recursion~~ (FIXED/DOCUMENTED) ```lux -fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1) // Error: isOdd undefined +fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1) fn isOdd(n: Int): Bool = if n == 0 then false else isEven(n - 1) +// Works in files (two-pass type checking), REPL limitation documented ``` -**Impact**: Medium - common pattern not supported -**Fix**: Two-pass type checking or explicit forward declarations +**Note**: Works when loaded from files due to two-pass type checking. REPL processes line-by-line, so forward references don't work there. -### 5. Circular Type Definitions Silently Fail +### 5. ~~Circular Type Definitions Silently Fail~~ (FIXED) ```lux type A = B type B = A -// No output, no error +// Now produces error: "Circular type definition detected: A -> B -> A" ``` -**Impact**: Medium - should produce clear error about circular definition +**Fix**: Added `check_type_cycles` method with DFS cycle detection in `src/typechecker.rs`. Also fixed parser to require `|` for enum variants, allowing `type A = B` to be parsed as a type alias. ## Parser Issues diff --git a/src/interpreter.rs b/src/interpreter.rs index 1ad194f..87af7ab 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -2810,6 +2810,11 @@ impl Interpreter { (Value::Tuple(a), Value::Tuple(b)) => { a.len() == b.len() && a.iter().zip(b).all(|(x, y)| self.values_equal(x, y)) } + (Value::Record(a), Value::Record(b)) => { + a.len() == b.len() && a.iter().all(|(k, v)| { + b.get(k).map(|bv| self.values_equal(v, bv)).unwrap_or(false) + }) + } ( Value::Constructor { name: n1, @@ -2824,6 +2829,7 @@ impl Interpreter { && f1.len() == f2.len() && f1.iter().zip(f2).all(|(x, y)| self.values_equal(x, y)) } + (Value::Json(a), Value::Json(b)) => a == b, _ => false, } } diff --git a/src/lexer.rs b/src/lexer.rs index 0a7b8e4..1e370b8 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -484,13 +484,21 @@ impl<'a> Lexer<'a> { current_literal.push('}'); } _ => { + let escape_start = self.pos; let escaped = match self.advance() { Some('n') => '\n', Some('r') => '\r', Some('t') => '\t', Some('\\') => '\\', Some('"') => '"', - Some(c) => c, + Some('0') => '\0', + Some('\'') => '\'', + Some(c) => { + return Err(LexError { + message: format!("Invalid escape sequence: \\{}", c), + span: Span::new(escape_start - 1, self.pos), + }); + } None => { return Err(LexError { message: "Unterminated string".into(), diff --git a/src/main.rs b/src/main.rs index 5af2750..ec99373 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1813,6 +1813,46 @@ c")"#; assert_eq!(eval(r#"let x = typeOf("hello")"#).unwrap(), r#""String""#); } + // Bug fix tests + #[test] + fn test_record_equality() { + assert_eq!(eval("let x = { a: 1, b: 2 } == { a: 1, b: 2 }").unwrap(), "true"); + assert_eq!(eval("let x = { a: 1 } == { a: 2 }").unwrap(), "false"); + assert_eq!(eval("let x = { a: 1, b: 2 } == { a: 1, b: 3 }").unwrap(), "false"); + } + + #[test] + fn test_invalid_escape_sequence() { + let result = eval(r#"let x = "\z""#); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Invalid escape sequence")); + } + + #[test] + fn test_unknown_effect_error() { + let result = eval("fn test(): Unit with {FakeEffect} = ()"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Unknown effect")); + } + + #[test] + fn test_circular_type_definitions() { + let result = eval("type A = B\ntype B = A"); + assert!(result.is_err(), "Should detect circular type definitions"); + let err = result.unwrap_err(); + assert!(err.contains("Circular"), "Error should mention 'Circular': {}", err); + } + + #[test] + fn test_mutual_recursion() { + let source = r#" + fn isEven(n: Int): Bool = if n == 0 then true else isOdd(n - 1) + fn isOdd(n: Int): Bool = if n == 0 then false else isEven(n - 1) + let result = isEven(10) + "#; + assert_eq!(eval(source).unwrap(), "true"); + } + // Pipe with stdlib tests #[test] fn test_pipe_with_list() { @@ -2948,7 +2988,6 @@ c")"#; mod example_tests { use super::*; use std::fs; - use std::path::Path; fn check_file(path: &str) -> Result<(), String> { let source = fs::read_to_string(path) diff --git a/src/parser.rs b/src/parser.rs index cfbe373..b5acf21 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -478,11 +478,11 @@ impl Parser { self.advance(); self.skip_newlines(); - if self.check(TokenKind::Pipe) || self.peek_is_variant() { - // Enum type + if self.check(TokenKind::Pipe) { + // Enum type - requires leading | for variants (TypeDef::Enum(self.parse_variants()?), Vec::new()) } else { - // Type alias + // Type alias - any type expression (TypeDef::Alias(self.parse_type()?), Vec::new()) } } else { diff --git a/src/typechecker.rs b/src/typechecker.rs index b569589..dc421fb 100644 --- a/src/typechecker.rs +++ b/src/typechecker.rs @@ -532,6 +532,9 @@ impl TypeChecker { self.collect_declaration(decl); } + // Check for circular type definitions + self.check_type_cycles(program); + // Second pass: type check all declarations for decl in &program.declarations { self.check_declaration(decl); @@ -544,6 +547,130 @@ impl TypeChecker { } } + /// Check for circular type alias definitions + fn check_type_cycles(&mut self, program: &Program) { + use std::collections::HashSet; + + // Build a map of type alias dependencies + let mut alias_deps: HashMap> = HashMap::new(); + + for decl in &program.declarations { + if let Declaration::Type(type_decl) = decl { + if let ast::TypeDef::Alias(type_expr) = &type_decl.definition { + let deps = self.collect_type_references(type_expr); + alias_deps.insert(type_decl.name.name.clone(), deps); + } + } + } + + // Check for cycles using DFS + fn has_cycle( + name: &str, + alias_deps: &HashMap>, + visiting: &mut HashSet, + visited: &mut HashSet, + cycle_path: &mut Vec, + ) -> bool { + if visiting.contains(name) { + cycle_path.push(name.to_string()); + return true; + } + if visited.contains(name) { + return false; + } + + visiting.insert(name.to_string()); + cycle_path.push(name.to_string()); + + if let Some(deps) = alias_deps.get(name) { + for dep in deps { + if alias_deps.contains_key(dep) { + if has_cycle(dep, alias_deps, visiting, visited, cycle_path) { + return true; + } + } + } + } + + visiting.remove(name); + cycle_path.pop(); + visited.insert(name.to_string()); + false + } + + let mut visited = HashSet::new(); + + for type_name in alias_deps.keys() { + let mut visiting = HashSet::new(); + let mut cycle_path = Vec::new(); + + if has_cycle(type_name, &alias_deps, &mut visiting, &mut visited, &mut cycle_path) { + // Find the span for this type declaration + let span = program.declarations.iter().find_map(|d| { + if let Declaration::Type(t) = d { + if t.name.name == *type_name { + return Some(t.span); + } + } + None + }).unwrap_or(Span::default()); + + self.errors.push(TypeError { + message: format!( + "Circular type definition detected: {}", + cycle_path.join(" -> ") + ), + span, + }); + } + } + } + + /// Collect all type names referenced in a type expression + fn collect_type_references(&self, type_expr: &TypeExpr) -> Vec { + let mut refs = Vec::new(); + self.collect_type_refs_helper(type_expr, &mut refs); + refs + } + + fn collect_type_refs_helper(&self, type_expr: &TypeExpr, refs: &mut Vec) { + match type_expr { + TypeExpr::Named(ident) => { + // Only include user-defined types, not builtins + match ident.name.as_str() { + "Int" | "Float" | "Bool" | "String" | "Char" | "Unit" | "_" => {} + name => refs.push(name.to_string()), + } + } + TypeExpr::App(constructor, args) => { + self.collect_type_refs_helper(constructor, refs); + for arg in args { + self.collect_type_refs_helper(arg, refs); + } + } + TypeExpr::Function { params, return_type, .. } => { + for p in params { + self.collect_type_refs_helper(p, refs); + } + self.collect_type_refs_helper(return_type, refs); + } + TypeExpr::Tuple(elements) => { + for e in elements { + self.collect_type_refs_helper(e, refs); + } + } + TypeExpr::Record(fields) => { + for f in fields { + self.collect_type_refs_helper(&f.typ, refs); + } + } + TypeExpr::Unit => {} + TypeExpr::Versioned { base, .. } => { + self.collect_type_refs_helper(base, refs); + } + } + } + /// Type check a program with module support pub fn check_program_with_modules( &mut self, @@ -790,6 +917,28 @@ impl TypeChecker { } fn check_function(&mut self, func: &FunctionDecl) { + // Validate that all declared effects exist + let builtin_effects = ["Console", "Fail", "State", "Reader", "Random", "Time", "File", "Process", "Http", "HttpServer", "Test"]; + for effect in &func.effects { + let is_builtin = builtin_effects.contains(&effect.name.as_str()); + let is_defined = self.env.lookup_effect(&effect.name).is_some(); + if !is_builtin && !is_defined { + // Find similar effect names for suggestion + let mut available: Vec<&str> = builtin_effects.to_vec(); + available.extend(self.env.effects.keys().map(|s| s.as_str())); + let suggestions = find_similar_names(&effect.name, available, 2); + + let mut message = format!("Unknown effect: {}", effect.name); + if let Some(hint) = format_did_you_mean(&suggestions) { + message.push_str(&format!(". {}", hint)); + } + self.errors.push(TypeError { + message, + span: effect.span, + }); + } + } + // Set up the environment with parameters let mut local_env = self.env.clone(); for param in &func.params {