Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,8 @@ pub struct Parameter {
pub name: String,
pub type_annotation: Option<TypeExpr>,
pub default: Option<Expression>,
#[serde(default)]
pub pattern: Option<Pattern>,
}

/// Import item for selective imports: `import { foo as bar } from "module"`
Expand Down
72 changes: 71 additions & 1 deletion src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6339,7 +6339,12 @@ impl Interpreter {
// Should not reach here due to arity check above
Value::Unit
};
func_env.borrow_mut().define(param.name.clone(), value);
func_env
.borrow_mut()
.define(param.name.clone(), value.clone());
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function-call parameter binding defines the synthetic "_destructure_N" parameter in the function environment even when the parameter is a destructuring pattern. This makes an internal implementation detail user-visible (it can be referenced from the function body) and can also collide with user-chosen parameter names (e.g., a user explicitly naming a param _destructure_0). Consider skipping define(param.name, ...) when param.pattern.is_some() (bind only the pattern vars), or use a non-lexable internal name and ensure it’s never bound in the runtime environment.

Suggested change
func_env
.borrow_mut()
.define(param.name.clone(), value.clone());
// Only bind the named parameter when there is no destructuring pattern.
// For destructuring params, bind only the pattern variables to avoid
// exposing synthetic names (e.g., "_destructure_0") in the environment.
if param.pattern.is_none() {
func_env
.borrow_mut()
.define(param.name.clone(), value.clone());
}

Copilot uses AI. Check for mistakes.
if let Some(ref pat) = param.pattern {
self.bind_pattern(pat, &value)?;
}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call_user_function now clones every argument value to bind it into the function environment (define(..., value.clone())). Since Value::clone() is deep for arrays/maps/structs, this adds potentially significant overhead to every function call even when the parameter is not destructured. You can avoid the clone by binding the destructuring pattern before moving value into the environment, or only cloning when param.pattern.is_some().

Suggested change
func_env
.borrow_mut()
.define(param.name.clone(), value.clone());
if let Some(ref pat) = param.pattern {
self.bind_pattern(pat, &value)?;
}
// Bind any destructuring pattern before moving the value into the environment
if let Some(ref pat) = param.pattern {
self.bind_pattern(pat, &value)?;
}
func_env
.borrow_mut()
.define(param.name.clone(), value);

Copilot uses AI. Check for mistakes.
}

// Environment is already set to func_env for contract checking and body execution
Expand Down Expand Up @@ -12017,4 +12022,69 @@ page
other => panic!("expected String('ab'), got {:?}", other),
}
}

#[test]
fn test_destructured_map_param() {
let result = eval(
r#"
fn greet({ name, email }) {
return name + " <" + email + ">"
}
greet(map { "name": "Alice", "email": "a@b.com" })
"#,
)
.unwrap();
match result {
Value::String(s) => assert_eq!(s, "Alice <a@b.com>"),
other => panic!("expected String, got {:?}", other),
}
}

#[test]
fn test_destructured_map_param_with_type() {
let result = eval(
r#"
fn greet({ name, email }: Map) -> String {
return name + " <" + email + ">"
}
greet(map { "name": "Bob", "email": "b@b.com" })
"#,
)
.unwrap();
match result {
Value::String(s) => assert_eq!(s, "Bob <b@b.com>"),
other => panic!("expected String, got {:?}", other),
}
}

#[test]
fn test_destructured_array_param() {
let result = eval(
r#"
fn first_two([a, b, ...rest]) {
return a + b
}
first_two([10, 20, 30])
"#,
)
.unwrap();
assert!(matches!(result, Value::Int(30)));
}

#[test]
fn test_destructured_param_with_regular_params() {
let result = eval(
r#"
fn process(id, { name }) {
return str(id) + ": " + name
}
process(42, map { "name": "Alice" })
"#,
)
.unwrap();
match result {
Value::String(s) => assert_eq!(s, "42: Alice"),
other => panic!("expected String, got {:?}", other),
}
}
}
12 changes: 11 additions & 1 deletion src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,15 @@ impl Parser {

if !self.check(&TokenKind::RightParen) {
loop {
let name = self.consume_identifier("Expected parameter name")?;
let (name, pattern) =
if self.check(&TokenKind::LeftBrace) || self.check(&TokenKind::LeftBracket) {
let pat = self.parse_pattern()?;
let synth_name = format!("_destructure_{}", params.len());
(synth_name, Some(pat))
} else {
let name = self.consume_identifier("Expected parameter name")?;
(name, None)
Comment on lines +311 to +317
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic parameter name (_destructure_{n}) is used as Parameter.name for destructured parameters. This can leak into user-facing diagnostics (e.g., the “required parameter cannot follow a parameter with a default” error will report _destructure_1, which doesn't exist in source code). Consider storing a separate display name (derived from the pattern) for diagnostics, or enhancing error formatting to refer to the pattern/first bound identifier instead of the synthetic name.

Copilot uses AI. Check for mistakes.
};

let type_annotation = if self.match_token(&[TokenKind::Colon]) {
Some(self.parse_type()?)
Expand All @@ -325,6 +333,7 @@ impl Parser {
name,
type_annotation,
default,
pattern,
});

if !self.match_token(&[TokenKind::Comma]) {
Expand Down Expand Up @@ -522,6 +531,7 @@ impl Parser {
name: param_name,
type_annotation,
default: None,
pattern: None,
});
}

Expand Down
Loading
Loading