fix: report duplicate declarations from enum definitions

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

@@ -6,7 +6,7 @@ use crate::pr::{Ty, TyKind, TyTupleField};
 use crate::semantic::ast_expand::expand_expr;
 use crate::semantic::{NS_SELF, STD_LIB_SOURCE_ID};
 use crate::Result;
-use crate::WithErrorInfo;
+use crate::{Error, WithErrorInfo};
 
 impl super::Resolver<'_> {
     // entry point to the resolver
@@ -63,56 +63,64 @@ impl super::Resolver<'_> {
                 // Enums get their own module definition, with NS_SELF pointing to the original type and a
                 // name entry in the module for each alias in the enum's tuple.
 
-                self.current_module_path.push(ident.name);
-
                 let mut module = Module {
                     names: HashMap::new(),
                     redirects: Vec::new(),
                     shadowed: None,
                 };
+                module.names.insert(
+                    NS_SELF.to_string(),
+                    Decl {
+                        declared_at: stmt.id,
+                        kind: DeclKind::Ty(ty.clone()),
+                        annotations: stmt.annotations,
+                        ..Default::default()
+                    },
+                );
 
                 for expr in enum_tuple {
                     let name = expr
                         .alias
                         .clone()
                         .expect("enum literal tuples should always have all fields with aliases");
+                    let member_span = expr.span;
+
+                    // NS_SELF is already in `names`, so a member of that name would
+                    // replace the type entry rather than being reported below.
+                    if name == NS_SELF {
+                        return Err(Error::new_simple(format!(
+                            "`{NS_SELF}` is a reserved name and cannot be an enum member"
+                        ))
+                        .with_span(member_span));
+                    }
+
                     let mut expr = expand_expr(expr)?;
                     expr.ty = Some(ty.clone());
                     // here, the expr needs to have no alias, so that it doesn't clobber an alias
                     // used in a future tuple expression
                     expr.alias = None;
 
-                    module.names.insert(
-                        name,
-                        Decl {
-                            declared_at: stmt.id,
-                            kind: DeclKind::Expr(Box::new(expr)),
-                            ..Default::default()
-                        },
-                    );
-                }
-                module.names.insert(
-                    NS_SELF.to_string(),
-                    Decl {
+                    let member = Decl {
                         declared_at: stmt.id,
-                        kind: DeclKind::Ty(ty),
-                        annotations: stmt.annotations,
+                        kind: DeclKind::Expr(Box::new(expr)),
                         ..Default::default()
-                    },
-                );
-
-                let decl = Decl {
-                    declared_at: stmt.id,
-                    kind: DeclKind::Module(module),
-                    ..Default::default()
-                };
+                    };
+
+                    // `names` is a map, so a repeated member would otherwise keep only
+                    // the last value.
+                    if module.names.insert(name.clone(), member).is_some() {
+                        return Err(Error::new_simple(format!(
+                            "duplicate declarations of {ident}.{name}"
+                        ))
+                        .with_span(member_span));
+                    }
+                }
 
-                let ident = Ident::from_path(self.current_module_path.clone());
+                // `declare` reports a name that's already taken; inserting into
+                // `root_mod.module` directly would overwrite it silently.
                 self.root_mod
-                    .module
-                    .insert(ident, decl)
+                    .declare(ident, DeclKind::Module(module), stmt.id, Vec::new())
                     .with_span(stmt.span)?;
-                self.current_module_path.pop();
             }
         } else {
             let decl = DeclKind::Ty(ty);

prqlc/prqlc/tests/integration/error_messages.rs

@@ -531,6 +531,94 @@ fn enum_type_2() {
     ");
 }
 
+/// An `enum` builds a module and so used to take a different insertion path
+/// than every other statement kind, which meant it replaced a name that was
+/// already declared instead of reporting the collision.
+///
+/// Only the direction where the `enum` comes second is covered — a `module`
+/// declared after an `enum` (or after another `module`) still overwrites
+/// silently, since `fold_module_def_stmt` keeps its own `Module::insert`; #6166
+/// tracks that.
+#[test]
+fn enum_duplicate_of_existing_declaration() {
+    assert_snapshot!(compile(r###"
+    let Status = 5
+    enum Status { Paid = 0 }
+    from invoices
+    "###).unwrap_err(), @"
+    Error:
+       ╭─[ :2:19 ]
+       │
+     2 │ ╭─▶     let Status = 5
+     3 │ ├─▶     enum Status { Paid = 0 }
+       │ │
+       │ ╰────────────────────────────────── duplicate declarations of Status
+    ───╯
+    ");
+
+    assert_snapshot!(compile(r###"
+    module m { let a = 5 }
+    enum m { Paid = 0 }
+    from invoices
+    "###).unwrap_err(), @"
+    Error:
+       ╭─[ :2:27 ]
+       │
+     2 │ ╭─▶     module m { let a = 5 }
+     3 │ ├─▶     enum m { Paid = 0 }
+       │ │
+       │ ╰───────────────────────────── duplicate declarations of m
+    ───╯
+    ");
+
+    assert_snapshot!(compile(r###"
+    enum Status { Paid = 0 }
+    enum Status { Unpaid = 1 }
+    from invoices
+    "###).unwrap_err(), @"
+    Error:
+       ╭─[ :2:29 ]
+       │
+     2 │ ╭─▶     enum Status { Paid = 0 }
+     3 │ ├─▶     enum Status { Unpaid = 1 }
+       │ │
+       │ ╰──────────────────────────────────── duplicate declarations of Status
+    ───╯
+    ");
+}
+
+/// The enum's members become entries in a map, so a repeated name — or one that
+/// collides with the `_self` entry holding the type itself — used to leave only
+/// the last value with no diagnostic.
+#[test]
+fn enum_duplicate_member() {
+    assert_snapshot!(compile(r###"
+    enum Status { Paid = 0, Paid = 1 }
+    from invoices
+    "###).unwrap_err(), @"
+    Error:
+       ╭─[ :2:36 ]
+       │
+     2 │     enum Status { Paid = 0, Paid = 1 }
+       │                                    ┬
+       │                                    ╰── duplicate declarations of Status.Paid
+    ───╯
+    ");
+
+    assert_snapshot!(compile(r###"
+    enum Status { _self = 0, Paid = 1 }
+    from invoices
+    "###).unwrap_err(), @"
+    Error:
+       ╭─[ :2:27 ]
+       │
+     2 │     enum Status { _self = 0, Paid = 1 }
+       │                           ┬
+       │                           ╰── `_self` is a reserved name and cannot be an enum member
+    ───╯
+    ");
+}
+
 #[test]
 fn append_by_wrong() {
     assert_snapshot!(compile(r###"