Skip to content

Commit a2cf1fe

Browse files
prql-botkgutwin
andauthored
fix: report duplicate import definitions as an error (#6150)
Co-authored-by: Karl Gutwin <karl@gutwin.org>
1 parent 193d3b2 commit a2cf1fe

2 files changed

Lines changed: 58 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44

55
**Language**:
66

7+
- Report duplicate `import` definitions as an error rather than silently keeping
8+
the last one. `import a.b` followed by `import c.b` now reports
9+
`duplicate declarations of b`, matching `let` and `type`; use `import d = c.b`
10+
to bring in both. The same applies to a multi-file project, where `import a.b`
11+
previously discarded a sibling `b.prql`. This is a small breaking change —
12+
programs that relied on the last declaration winning now fail to compile.
13+
(@prql-bot, #6150)
714
- Move all standard types into a dedicated `std.types` submodule in order to
815
disambiguate between `type text` - `module text` and `type date` -
916
`module date`. (@kgutwin, #6155)

prqlc/prqlc/src/semantic/resolver/stmt.rs

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,14 @@ impl super::Resolver<'_> {
150150

151151
fn fold_import_def_stmt(&mut self, stmt: Stmt, ident: Ident) -> Result<()> {
152152
let target = stmt.kind.into_import_def().unwrap();
153-
let decl = Decl {
154-
declared_at: stmt.id,
155-
kind: DeclKind::Import(target.name),
156-
annotations: stmt.annotations,
157-
..Default::default()
158-
};
153+
let decl = DeclKind::Import(target.name);
159154

155+
// `declare` rather than a direct `Module::insert`, so that a name
156+
// already taken is reported as a duplicate instead of being overwritten,
157+
// matching `let` and `type`.
160158
self.root_mod
161-
.module
162-
.insert(ident, decl)
159+
.declare(ident, decl, stmt.id, stmt.annotations)
160+
.push_hint("to import both, alias one of them: `import alias = path`")
163161
.with_span(stmt.span)?;
164162
Ok(())
165163
}
@@ -238,3 +236,48 @@ fn prepare_expr_decl(value: Box<Expr>) -> DeclKind {
238236
_ => DeclKind::Expr(value),
239237
}
240238
}
239+
240+
#[cfg(test)]
241+
mod test {
242+
use insta::assert_snapshot;
243+
244+
use crate::tests::compile;
245+
246+
#[test]
247+
fn duplicate_import_is_an_error() {
248+
// Import defs bypassed the duplicate check that `let` and `type` get, so
249+
// the second `b` used to silently win.
250+
assert_snapshot!(compile(r"
251+
import a.b
252+
import c.b
253+
from t
254+
").unwrap_err(), @"
255+
Error:
256+
╭─[ :2:19 ]
257+
258+
2 │ ╭─▶ import a.b
259+
3 │ ├─▶ import c.b
260+
│ │
261+
│ ╰──────────────────────── duplicate declarations of b
262+
263+
│ Help: to import both, alias one of them: `import alias = path`
264+
───╯
265+
");
266+
}
267+
268+
#[test]
269+
fn aliased_import_avoids_duplicate() {
270+
// `import d = …` is the documented way to bring in two targets that
271+
// would otherwise land on the same name.
272+
assert_snapshot!(compile(r"
273+
import a.b
274+
import d = c.b
275+
from t
276+
").unwrap(), @"
277+
SELECT
278+
*
279+
FROM
280+
t
281+
");
282+
}
283+
}

0 commit comments

Comments
 (0)