Skip to content

Commit 595b2ae

Browse files
aaronvgaaronvg
committed
fix duplicate diagnostics
1 parent 011ee89 commit 595b2ae

16 files changed

+495
-84
lines changed

‎baml_language/crates/baml_compiler_hir/src/lib.rs‎

Lines changed: 173 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,9 @@ use rustc_hash::FxHashMap;
989989
struct ItemInfo {
990990
span: Span,
991991
path: String,
992+
kind: &'static str,
993+
/// Whether we've already emitted an error for this item as the "first definition".
994+
first_error_emitted: bool,
992995
}
993996

994997
/// Result of HIR validation.
@@ -1031,8 +1034,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
10311034
ItemId::Function(loc) => {
10321035
let file = loc.file(db);
10331036
let item_tree = file_item_tree(db, file);
1034-
let func = &item_tree[loc.id(db)];
1035-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1037+
let local_id = loc.id(db);
1038+
let func = &item_tree[local_id];
1039+
let span = get_item_name_span(db, file, "function", func.name.as_str(), local_id.index())
1040+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
10361041
let path = file.path(db).display().to_string();
10371042
check_duplicate(
10381043
&mut seen,
@@ -1046,8 +1051,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
10461051
ItemId::Class(loc) => {
10471052
let file = loc.file(db);
10481053
let item_tree = file_item_tree(db, file);
1049-
let class = &item_tree[loc.id(db)];
1050-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1054+
let local_id = loc.id(db);
1055+
let class = &item_tree[local_id];
1056+
let span = get_item_name_span(db, file, "class", class.name.as_str(), local_id.index())
1057+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
10511058
let path = file.path(db).display().to_string();
10521059
check_duplicate(
10531060
&mut seen,
@@ -1061,8 +1068,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
10611068
ItemId::Enum(loc) => {
10621069
let file = loc.file(db);
10631070
let item_tree = file_item_tree(db, file);
1064-
let enum_def = &item_tree[loc.id(db)];
1065-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1071+
let local_id = loc.id(db);
1072+
let enum_def = &item_tree[local_id];
1073+
let span = get_item_name_span(db, file, "enum", enum_def.name.as_str(), local_id.index())
1074+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
10661075
let path = file.path(db).display().to_string();
10671076
check_duplicate(
10681077
&mut seen,
@@ -1076,8 +1085,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
10761085
ItemId::TypeAlias(loc) => {
10771086
let file = loc.file(db);
10781087
let item_tree = file_item_tree(db, file);
1079-
let alias = &item_tree[loc.id(db)];
1080-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1088+
let local_id = loc.id(db);
1089+
let alias = &item_tree[local_id];
1090+
let span = get_item_name_span(db, file, "type alias", alias.name.as_str(), local_id.index())
1091+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
10811092
let path = file.path(db).display().to_string();
10821093
check_duplicate(
10831094
&mut seen,
@@ -1091,8 +1102,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
10911102
ItemId::Client(loc) => {
10921103
let file = loc.file(db);
10931104
let item_tree = file_item_tree(db, file);
1094-
let client = &item_tree[loc.id(db)];
1095-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1105+
let local_id = loc.id(db);
1106+
let client = &item_tree[local_id];
1107+
let span = get_item_name_span(db, file, "client", client.name.as_str(), local_id.index())
1108+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
10961109
let path = file.path(db).display().to_string();
10971110
check_duplicate(
10981111
&mut seen,
@@ -1106,8 +1119,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
11061119
ItemId::Generator(loc) => {
11071120
let file = loc.file(db);
11081121
let item_tree = file_item_tree(db, file);
1109-
let generator = &item_tree[loc.id(db)];
1110-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1122+
let local_id = loc.id(db);
1123+
let generator = &item_tree[local_id];
1124+
let span = get_item_name_span(db, file, "generator", generator.name.as_str(), local_id.index())
1125+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
11111126
let path = file.path(db).display().to_string();
11121127
check_duplicate(
11131128
&mut seen,
@@ -1122,8 +1137,10 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
11221137
// Tests are validated separately: only same name + same function is a duplicate
11231138
let file = loc.file(db);
11241139
let item_tree = file_item_tree(db, file);
1125-
let test = &item_tree[loc.id(db)];
1126-
let span = Span::new(file.file_id(db), TextRange::empty(0.into()));
1140+
let local_id = loc.id(db);
1141+
let test = &item_tree[local_id];
1142+
let span = get_item_name_span(db, file, "test", test.name.as_str(), local_id.index())
1143+
.unwrap_or_else(|| Span::new(file.file_id(db), TextRange::empty(0.into())));
11271144
let path = file.path(db).display().to_string();
11281145

11291146
// Check each function reference in the test
@@ -1144,6 +1161,8 @@ fn validate_duplicate_names(db: &dyn Db, root: baml_workspace::Project) -> Vec<N
11441161
ItemInfo {
11451162
span,
11461163
path: path.clone(),
1164+
kind: "test",
1165+
first_error_emitted: false,
11471166
},
11481167
);
11491168
}
@@ -1199,7 +1218,21 @@ fn check_duplicate(
11991218
span: Span,
12001219
path: String,
12011220
) {
1202-
if let Some(existing) = seen.get(&name) {
1221+
if let Some(existing) = seen.get_mut(&name) {
1222+
// If this is the first duplicate we've seen, also emit an error for the first definition
1223+
if !existing.first_error_emitted {
1224+
errors.push(NameError::DuplicateName {
1225+
name: name.to_string(),
1226+
kind: existing.kind,
1227+
first: span,
1228+
first_path: path.clone(),
1229+
second: existing.span,
1230+
second_path: existing.path.clone(),
1231+
});
1232+
existing.first_error_emitted = true;
1233+
}
1234+
1235+
// Emit error for the current (duplicate) definition
12031236
errors.push(NameError::DuplicateName {
12041237
name: name.to_string(),
12051238
kind,
@@ -1209,7 +1242,15 @@ fn check_duplicate(
12091242
second_path: path,
12101243
});
12111244
} else {
1212-
seen.insert(name, ItemInfo { span, path });
1245+
seen.insert(
1246+
name,
1247+
ItemInfo {
1248+
span,
1249+
path,
1250+
kind,
1251+
first_error_emitted: false,
1252+
},
1253+
);
12131254
}
12141255
}
12151256

@@ -1318,6 +1359,122 @@ fn get_enum_variant_info(
13181359
None
13191360
}
13201361

1362+
/// Look up the span of a top-level item's name from the syntax tree.
1363+
///
1364+
/// This is used to get accurate spans for duplicate name errors, since the
1365+
/// ItemTree is position-independent and doesn't store spans.
1366+
///
1367+
/// The `occurrence` parameter specifies which occurrence to return (0 = first, 1 = second, etc.)
1368+
/// when there are multiple items of the same kind with the same name in the file.
1369+
/// This corresponds to the collision index in `LocalItemId`.
1370+
fn get_item_name_span(
1371+
db: &dyn Db,
1372+
file: baml_base::files::SourceFile,
1373+
kind: &str,
1374+
name: &str,
1375+
occurrence: u16,
1376+
) -> Option<Span> {
1377+
use baml_compiler_syntax::{
1378+
SyntaxKind,
1379+
ast::{ClassDef, ClientDef, EnumDef, FunctionDef, GeneratorDef, TestDef, TypeAliasDef},
1380+
};
1381+
1382+
let tree = baml_compiler_parser::syntax_tree(db, file);
1383+
let file_id = file.file_id(db);
1384+
let mut matches_found: u16 = 0;
1385+
1386+
for node in tree.children() {
1387+
match kind {
1388+
"function" if node.kind() == SyntaxKind::FUNCTION_DEF => {
1389+
if let Some(func) = FunctionDef::cast(node) {
1390+
if let Some(name_token) = func.name() {
1391+
if name_token.text() == name {
1392+
if matches_found == occurrence {
1393+
return Some(Span::new(file_id, name_token.text_range()));
1394+
}
1395+
matches_found += 1;
1396+
}
1397+
}
1398+
}
1399+
}
1400+
"class" if node.kind() == SyntaxKind::CLASS_DEF => {
1401+
if let Some(class) = ClassDef::cast(node) {
1402+
if let Some(name_token) = class.name() {
1403+
if name_token.text() == name {
1404+
if matches_found == occurrence {
1405+
return Some(Span::new(file_id, name_token.text_range()));
1406+
}
1407+
matches_found += 1;
1408+
}
1409+
}
1410+
}
1411+
}
1412+
"enum" if node.kind() == SyntaxKind::ENUM_DEF => {
1413+
if let Some(enum_def) = EnumDef::cast(node) {
1414+
if let Some(name_token) = enum_def.name() {
1415+
if name_token.text() == name {
1416+
if matches_found == occurrence {
1417+
return Some(Span::new(file_id, name_token.text_range()));
1418+
}
1419+
matches_found += 1;
1420+
}
1421+
}
1422+
}
1423+
}
1424+
"type alias" if node.kind() == SyntaxKind::TYPE_ALIAS_DEF => {
1425+
if let Some(alias) = TypeAliasDef::cast(node) {
1426+
if let Some(name_token) = alias.name() {
1427+
if name_token.text() == name {
1428+
if matches_found == occurrence {
1429+
return Some(Span::new(file_id, name_token.text_range()));
1430+
}
1431+
matches_found += 1;
1432+
}
1433+
}
1434+
}
1435+
}
1436+
"client" if node.kind() == SyntaxKind::CLIENT_DEF => {
1437+
if let Some(client) = ClientDef::cast(node) {
1438+
if let Some(name_token) = client.name() {
1439+
if name_token.text() == name {
1440+
if matches_found == occurrence {
1441+
return Some(Span::new(file_id, name_token.text_range()));
1442+
}
1443+
matches_found += 1;
1444+
}
1445+
}
1446+
}
1447+
}
1448+
"generator" if node.kind() == SyntaxKind::GENERATOR_DEF => {
1449+
if let Some(generator) = GeneratorDef::cast(node) {
1450+
if let Some(name_token) = generator.name() {
1451+
if name_token.text() == name {
1452+
if matches_found == occurrence {
1453+
return Some(Span::new(file_id, name_token.text_range()));
1454+
}
1455+
matches_found += 1;
1456+
}
1457+
}
1458+
}
1459+
}
1460+
"test" if node.kind() == SyntaxKind::TEST_DEF => {
1461+
if let Some(test) = TestDef::cast(node) {
1462+
if let Some(name_token) = test.name() {
1463+
if name_token.text() == name {
1464+
if matches_found == occurrence {
1465+
return Some(Span::new(file_id, name_token.text_range()));
1466+
}
1467+
matches_found += 1;
1468+
}
1469+
}
1470+
}
1471+
}
1472+
_ => {}
1473+
}
1474+
}
1475+
None
1476+
}
1477+
13211478
/// Validate that field names and function parameters don't use reserved keywords.
13221479
///
13231480
/// This checks:

‎baml_language/crates/baml_ide_tests/test_files/syntax/class/duplicate_definitions.baml‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,24 +60,45 @@ type A = int
6060
//----
6161
//- diagnostics
6262
// Error: Duplicate class `A`
63-
// ╭─[ class_duplicate_definitions.baml:1:1 ]
63+
// ╭─[ class_duplicate_definitions.baml:1:7 ]
6464
// │
6565
// 1 │ class A {
66-
// │ │
67-
// │ ╰─ class `A` redefined here
68-
// │ │
69-
// │ ╰─ `A` first defined in class_duplicate_definitions.baml
66+
// │ ┬
67+
// │ ╰── class `A` redefined here
68+
// │
69+
// 5 │ class A {
70+
// │ ┬
71+
// │ ╰── `A` first defined in class_duplicate_definitions.baml
72+
// │
73+
// │ Note: Error code: E0011
74+
// ───╯
75+
// Error: Duplicate class `A`
76+
// ╭─[ class_duplicate_definitions.baml:5:7 ]
77+
// │
78+
// 5 │ class A {
79+
// │ ┬
80+
// │ ╰── class `A` redefined here
81+
// │
82+
// ├─[ class_duplicate_definitions.baml:5:7 ]
83+
// │
84+
// 1 │ class A {
85+
// │ ┬
86+
// │ ╰── `A` first defined in class_duplicate_definitions.baml
7087
// │
7188
// │ Note: Error code: E0011
7289
// ───╯
7390
// Error: Duplicate type alias `A`
74-
// ╭─[ class_duplicate_definitions.baml:1:1 ]
91+
// ╭─[ class_duplicate_definitions.baml:9:6 ]
92+
// │
93+
// 9 │ type A = int
94+
// │ ┬
95+
// │ ╰── type alias `A` redefined here
96+
// │
97+
// ├─[ class_duplicate_definitions.baml:9:6 ]
7598
// │
7699
// 1 │ class A {
77-
// │ │
78-
// │ ╰─ type alias `A` redefined here
79-
// │ │
80-
// │ ╰─ `A` first defined in class_duplicate_definitions.baml
100+
// │ ┬
101+
// │ ╰── `A` first defined in class_duplicate_definitions.baml
81102
// │
82103
// │ Note: Error code: E0011
83104
// ───╯

‎baml_language/crates/baml_ide_tests/test_files/syntax/class/invalid_type_aliases.baml‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,31 @@ type Four = int | string | b
9393
// │
9494
// │ Note: Error code: E0010
9595
// ───╯
96+
// Error: Duplicate class `One`
97+
// ╭─[ class_invalid_type_aliases.baml:1:7 ]
98+
// │
99+
// 1 │ class One {
100+
// │ ─┬─
101+
// │ ╰─── class `One` redefined here
102+
// │
103+
// 6 │ type One = int
104+
// │ ─┬─
105+
// │ ╰─── `One` first defined in class_invalid_type_aliases.baml
106+
// │
107+
// │ Note: Error code: E0011
108+
// ───╯
96109
// Error: Duplicate type alias `One`
97-
// ╭─[ class_invalid_type_aliases.baml:1:1 ]
110+
// ╭─[ class_invalid_type_aliases.baml:6:6 ]
111+
// │
112+
// 6 │ type One = int
113+
// │ ─┬─
114+
// │ ╰─── type alias `One` redefined here
115+
// │
116+
// ├─[ class_invalid_type_aliases.baml:6:6 ]
98117
// │
99118
// 1 │ class One {
100-
// │ │
101-
// │ ╰─ type alias `One` redefined here
102-
// │ │
103-
// │ ╰─ `One` first defined in class_invalid_type_aliases.baml
119+
// │ ─┬─
120+
// │ ╰─── `One` first defined in class_invalid_type_aliases.baml
104121
// │
105122
// │ Note: Error code: E0011
106123
// ───╯

0 commit comments

Comments
 (0)