Skip to content

Commit b602e7d

Browse files
authored
baml_language: add duplicate parameter error (#3518)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Compiler now detects and reports duplicate parameter names in both functions and lambda expressions. This improved validation catches parameter naming conflicts early, preventing potential runtime issues. * **Tests** * Added comprehensive test coverage for duplicate parameter detection to ensure the validation works correctly for both duplicate and distinct parameter names. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/BoundaryML/baml/pull/3518) <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 790a8bd commit b602e7d

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

baml_language/crates/baml_compiler2_hir/src/builder.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,24 @@ impl<'db> SemanticIndexBuilder<'db> {
246246
}
247247
}
248248

249+
/// Emit a `DuplicateDefinition` diagnostic for any parameter name that
250+
/// appears more than once in a function or lambda signature. Applies
251+
/// uniformly to positional and defaulted parameters — both share the
252+
/// same `params` vector, and any name collision among them would make
253+
/// later references to the name ambiguous within the body.
254+
fn emit_duplicate_param_diagnostics(&mut self, params: &[ast::Param]) {
255+
let mut seen: FxHashMap<Name, Vec<MemberSite>> = FxHashMap::default();
256+
for param in params {
257+
seen.entry(param.name.clone())
258+
.or_default()
259+
.push(MemberSite {
260+
range: param.name_span,
261+
kind: DefinitionKind::Parameter,
262+
});
263+
}
264+
self.emit_duplicate_diagnostics(seen);
265+
}
266+
249267
/// Emit `DuplicateDefinition` diagnostics for any name with more than one site.
250268
fn emit_duplicate_diagnostics(&mut self, seen: FxHashMap<Name, Vec<MemberSite>>) {
251269
let scope = self.current_scope_path();
@@ -798,6 +816,7 @@ impl<'db> SemanticIndexBuilder<'db> {
798816
.params
799817
.push((param.name.clone(), idx));
800818
}
819+
self.emit_duplicate_param_diagnostics(&func_def.params);
801820
self.lambda_stack.push(scope_id);
802821
for param in &func_def.params {
803822
if let Some(default) = param.default {
@@ -955,6 +974,7 @@ impl<'db> SemanticIndexBuilder<'db> {
955974
.params
956975
.push((param.name.clone(), idx));
957976
}
977+
self.emit_duplicate_param_diagnostics(&f.params);
958978
for param in &f.params {
959979
if let Some(default) = param.default {
960980
self.walk_expr(

baml_language/crates/baml_tests/src/compiler2_hir.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,77 @@ mod tests {
542542
assert!(sites.iter().all(|s| s.kind == DefinitionKind::Variant));
543543
}
544544

545+
/// Duplicate parameter names within a function signature — across any
546+
/// combination of positional and defaulted parameters — produce a
547+
/// `DuplicateDefinition` diagnostic.
548+
#[test]
549+
fn duplicate_function_param_produces_diagnostic() {
550+
use baml_compiler2_hir::{contributions::DefinitionKind, diagnostic::Hir2Diagnostic};
551+
552+
let cases: &[(&str, &str, &str)] = &[
553+
(
554+
"positional/positional",
555+
"dup_param_pos_pos.baml",
556+
"function Foo(x: int, x: string) -> string { x }",
557+
),
558+
(
559+
"positional/defaulted",
560+
"dup_param_pos_opt.baml",
561+
"function Foo(x: int, x: string = \"hi\") -> string { x }",
562+
),
563+
(
564+
"defaulted/defaulted",
565+
"dup_param_opt_opt.baml",
566+
"function Foo(x: int = 0, x: string = \"hi\") -> string { x }",
567+
),
568+
];
569+
570+
for (label, file_name, source) in cases {
571+
let mut db = make_db();
572+
let file = db.add_file(file_name, source);
573+
let index = file_semantic_index(&db, file);
574+
let diags = index.diagnostics();
575+
let dups: Vec<_> = diags
576+
.iter()
577+
.filter(|d| matches!(d, Hir2Diagnostic::DuplicateDefinition { name, .. } if name == &Name::new("x")))
578+
.collect();
579+
assert_eq!(
580+
dups.len(),
581+
1,
582+
"{label}: expected 1 duplicate diagnostic for 'x'"
583+
);
584+
let Hir2Diagnostic::DuplicateDefinition { scope, sites, .. } = dups[0] else {
585+
panic!("{label}: expected DuplicateDefinition diagnostic");
586+
};
587+
assert_eq!(scope.as_ref().unwrap(), &Name::new("Foo"), "{label}");
588+
assert_eq!(sites.len(), 2, "{label}");
589+
assert!(
590+
sites.iter().all(|s| s.kind == DefinitionKind::Parameter),
591+
"{label}: all sites should be Parameter kind"
592+
);
593+
}
594+
}
595+
596+
/// Distinct parameter names in a function signature do not produce a
597+
/// duplicate diagnostic — regression guard against over-firing.
598+
#[test]
599+
fn distinct_function_params_have_no_duplicate_diagnostic() {
600+
use baml_compiler2_hir::diagnostic::Hir2Diagnostic;
601+
602+
let mut db = make_db();
603+
let file = db.add_file(
604+
"distinct_params.baml",
605+
"function Foo(x: int, y: string = \"hi\") -> string { y }",
606+
);
607+
let index = file_semantic_index(&db, file);
608+
let diags = index.diagnostics();
609+
assert!(
610+
!diags
611+
.iter()
612+
.any(|d| matches!(d, Hir2Diagnostic::DuplicateDefinition { .. }))
613+
);
614+
}
615+
545616
/// Same-scope let shadowing is legal and does not produce duplicate diagnostics.
546617
#[test]
547618
fn same_scope_let_shadowing_has_no_duplicate_diagnostic() {

0 commit comments

Comments
 (0)