Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
36 changes: 36 additions & 0 deletions crates/monty/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,13 @@ impl<'i> Prepare<'i> {
// Extract param names from the parsed signature for scope analysis
let param_names: Vec<StringId> = parsed_sig.param_names().collect();

// Reject duplicate parameter names. CPython raises `SyntaxError` at compile
// time; accepting them here desynchronizes the namespace layout built by
// `new_function` (`name_map` is deduplicated by HashMap semantics but each
// `NamespaceId` comes from the positional index), which later panics
// `load_local` at runtime.
reject_duplicate_params(&param_names, self.interner, name.position)?;

// Pass 1: Collect scope information from the function body
let scope_info = collect_function_scope_info(&body, &param_names, self.interner);

Expand Down Expand Up @@ -1486,6 +1493,9 @@ impl<'i> Prepare<'i> {
// Extract param names from the parsed signature for scope analysis
let param_names: Vec<StringId> = parsed_sig.param_names().collect();

// Reject duplicate parameter names (see `prepare_function_def` for the rationale).
reject_duplicate_params(&param_names, self.interner, position)?;

// Pass 1: Collect scope information from the lambda body
// (Lambdas can't have global/nonlocal declarations, but can have nested functions)
let scope_info = collect_function_scope_info(&body_nodes, &param_names, self.interner);
Expand Down Expand Up @@ -1920,6 +1930,32 @@ struct FunctionScopeInfo {
potential_captures: AHashSet<String>,
}

/// Validates that a function's parameter list has no duplicate names.
///
/// Ruff's parser accepts duplicate parameter names (e.g. `def f(x, x)`) that CPython
/// rejects at compile time. Accepting them here would desynchronize the namespace
/// layout: `name_map.len()` counts unique names while each `NamespaceId` comes
/// from the positional index, so the duplicate resolves to a slot past the
/// allocated stack region and panics `load_local` at runtime. Rejecting at the
/// prepare phase matches CPython's compile-time check.
fn reject_duplicate_params(
param_names: &[StringId],
interner: &InternerBuilder,
position: CodeRange,
) -> Result<(), ParseError> {
let mut seen = AHashSet::with_capacity(param_names.len());
for string_id in param_names {
if !seen.insert(*string_id) {
let name_str = interner.get_str(*string_id);
return Err(ParseError::syntax(
format!("duplicate argument '{name_str}' in function definition"),
position,
));
}
}
Ok(())
}

/// Scans a function body to collect scope information (first phase of preparation).
///
/// This function performs three passes over the AST:
Expand Down
8 changes: 8 additions & 0 deletions crates/monty/test_cases/function__err_duplicate_param.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Duplicate parameter names in a function definition are a SyntaxError.
# Previously this panicked the VM at call time because `prepare.rs` sized
# the frame's namespace from the unique name count (HashMap::len) but
# resolved the duplicate to a NamespaceId from the positional index,
# so `load_local` indexed past the allocated stack slots.
def f(x, x):
return x
# Raise=SyntaxError("duplicate argument 'x' in function definition")
39 changes: 39 additions & 0 deletions crates/monty/tests/parse_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,45 @@ fn del_statement_returns_not_implemented_error() {
assert_eq!(get_exc_type(result), ExcType::NotImplementedError);
}

#[test]
fn duplicate_positional_parameter_returns_syntax_error() {
// https://github.com/pydantic/monty/issues/377
//
// Ruff's parser accepts `def f(x, x)` though CPython rejects it at compile time.
// Without an explicit check, `Prepare::new_function` would size the frame from
// the unique-name count (HashMap::len) while resolving the duplicate to a
// positional NamespaceId that points past the allocated stack region, panicking
// `load_local` at call time.
let result = MontyRun::new("def f(x, x): return x\nf(1, 2)".to_owned(), "test.py", vec![]);
let exc = result.expect_err("expected compile error");
assert_eq!(exc.exc_type(), ExcType::SyntaxError);
assert_eq!(exc.message(), Some("duplicate argument 'x' in function definition"));
}

#[test]
fn duplicate_keyword_only_parameter_returns_syntax_error() {
let result = MontyRun::new("def f(*, x, x): return x".to_owned(), "test.py", vec![]);
let exc = result.expect_err("expected compile error");
assert_eq!(exc.exc_type(), ExcType::SyntaxError);
assert_eq!(exc.message(), Some("duplicate argument 'x' in function definition"));
}

#[test]
fn duplicate_mixed_positional_and_keyword_only_parameter_returns_syntax_error() {
let result = MontyRun::new("def f(x, *, x=1): return x".to_owned(), "test.py", vec![]);
let exc = result.expect_err("expected compile error");
assert_eq!(exc.exc_type(), ExcType::SyntaxError);
assert_eq!(exc.message(), Some("duplicate argument 'x' in function definition"));
}

#[test]
fn duplicate_lambda_parameter_returns_syntax_error() {
let result = MontyRun::new("f = lambda x, x: x".to_owned(), "test.py", vec![]);
let exc = result.expect_err("expected compile error");
assert_eq!(exc.exc_type(), ExcType::SyntaxError);
assert_eq!(exc.message(), Some("duplicate argument 'x' in function definition"));
}

#[test]
fn long_source_line_does_not_overflow_column() {
// https://github.com/pydantic/monty/issues/341
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ include = [
]
# Exclude files with intentional syntax errors or special formatting requirements
extend-exclude = [
"crates/monty/test_cases/function__err_duplicate_param.py",
"crates/monty/test_cases/nonlocal__error_module_level.py",
"crates/monty/test_cases/traceback__nonlocal_module_scope.py",
# These files test specific quote styles in error messages and must not be reformatted
Expand Down Expand Up @@ -96,6 +97,8 @@ include = ["scripts", "crates/monty/test_cases", "crates/monty-python", "crates/
exclude = [
"crates/monty-type-checking/tests/bad_types.py",
"crates/monty-type-checking/tests/reveal_types.py",
# duplicate parameter name test intentionally has a syntax error
"crates/monty/test_cases/function__err_duplicate_param.py",
# break/continue outside loop tests intentionally test error handling
"crates/monty/test_cases/loop__break_outside_error.py",
"crates/monty/test_cases/loop__continue_outside_error.py",
Expand Down
Loading