#485 Improve the error message when redefining a builtin function

src/fun/builtins.rs

@@ -25,6 +25,23 @@ pub const NAT_SUCC: &str = "Nat/Succ";
 pub const NAT_ZERO: &str = "Nat/Zero";
 pub const NAT_SUCC_TAG: u32 = 0;
 
+pub const TREE: &str = "Tree";
+pub const TREE_NODE: &str = "Tree/Node";
+pub const TREE_LEAF: &str = "Tree/Leaf";
+
+pub const MAP: &str = "Map";
+pub const MAP_NODE: &str = "Map/Node";
+pub const MAP_LEAF: &str = "Map/Leaf";
+
+pub const IO: &str = "IO";
+pub const IO_DONE: &str = "IO/Done";
+pub const IO_CALL: &str = "IO/Call";
+
+pub const BUILTIN_CTRS: &[&str] =
+  &[LCONS, LNIL, SCONS, SNIL, NAT_SUCC, NAT_ZERO, TREE_NODE, TREE_LEAF, MAP_NODE, MAP_LEAF, IO_DONE, IO_CALL];
+
+pub const BUILTIN_TYPES: &[&str] = &[LIST, STRING, NAT, TREE, MAP, IO];
+
 impl Book {
   pub fn builtins() -> Book {
     TermParser::new(BUILTINS)

src/fun/parser.rs

@@ -133,12 +133,12 @@ impl<'a> TermParser<'a> {
             def.rules.push(rule);
           } else {
             // Trying to add a new rule to a previous definition, coming from a different rule.
-            let msg = format!("Redefinition of function '{name}'");
+            let msg = Self::redefinition_of_function_msg(builtin, &name);
             return self.with_ctx(Err(msg), ini_idx, end_idx);
           }
         } else {
           // Trying to add a new rule to a previous definition, coming from another kind of top-level.
-          let msg = format!("Redefinition of function '{name}'");
+          let msg = Self::redefinition_of_function_msg(builtin, &name);
           return self.with_ctx(Err(msg), ini_idx, end_idx);
         }
       } else {
@@ -801,23 +801,13 @@ impl Indent {
 
 impl Book {
   fn add_adt(&mut self, nam: Name, adt: Adt) -> ParseResult<()> {
-    if let Some(adt) = self.adts.get(&nam) {
-      if adt.builtin {
-        return Err(format!("{} is a built-in datatype and should not be overridden.", nam));
-      } else {
-        return Err(format!("Repeated datatype '{}'", nam));
-      }
+    if self.adts.contains_key(&nam) {
+      Err(TermParser::redefinition_of_type_msg(&nam))?
     } else {
       for ctr in adt.ctrs.keys() {
         match self.ctrs.entry(ctr.clone()) {
           indexmap::map::Entry::Vacant(e) => _ = e.insert(nam.clone()),
-          indexmap::map::Entry::Occupied(e) => {
-            if self.adts.get(e.get()).is_some_and(|adt| adt.builtin) {
-              return Err(format!("{} is a built-in constructor and should not be overridden.", e.key()));
-            } else {
-              return Err(format!("Repeated constructor '{}'", e.key()));
-            }
-          }
+          indexmap::map::Entry::Occupied(e) => Err(TermParser::redefinition_of_constructor_msg(e.key()))?,
         }
       }
       self.adts.insert(nam.clone(), adt);
@@ -1234,4 +1224,28 @@ pub trait ParserCommons<'a>: Parser<'a> {
     self.consume_exactly("`")?;
     Ok(result)
   }
+
+  fn redefinition_of_function_msg(builtin: bool, function_name: &str) -> String {
+    if builtin {
+      format!("Redefinition of builtin (function) '{function_name}'.")
+    } else {
+      format!("Redefinition of function '{function_name}'.")
+    }
+  }
+
+  fn redefinition_of_constructor_msg(constructor_name: &str) -> String {
+    if crate::fun::builtins::BUILTIN_CTRS.contains(&constructor_name) {
+      format!("Redefinition of builtin (constructor) '{constructor_name}'.")
+    } else {
+      format!("Redefinition of constructor '{constructor_name}'.")
+    }
+  }
+
+  fn redefinition_of_type_msg(type_name: &str) -> String {
+    if crate::fun::builtins::BUILTIN_TYPES.contains(&type_name) {
+      format!("Redefinition of builtin (type) '{type_name}'.")
+    } else {
+      format!("Redefinition of type '{type_name}'.")
+    }
+  }
 }

src/imp/parser.rs

@@ -1069,12 +1069,12 @@ impl<'a> PyParser<'a> {
     end_idx: usize,
     builtin: bool,
   ) -> ParseResult<()> {
-    if book.defs.contains_key(&def.name) {
-      let msg = format!("Redefinition of function '{}'.", def.name);
+    if let Some(def) = book.defs.get(&def.name) {
+      let msg = Self::redefinition_of_function_msg(def.builtin, &def.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     if book.ctrs.contains_key(&def.name) {
-      let msg = format!("Redefinition of constructor '{}'.", def.name);
+      let msg = Self::redefinition_of_constructor_msg(&def.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     def.order_kwargs(book)?;
@@ -1093,17 +1093,17 @@ impl<'a> PyParser<'a> {
     builtin: bool,
   ) -> ParseResult<()> {
     if book.adts.contains_key(&r#enum.name) {
-      let msg = format!("Redefinition of type '{}'.", r#enum.name);
+      let msg = PyParser::redefinition_of_type_msg(&r#enum.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     let mut adt = Adt { ctrs: Default::default(), builtin };
     for variant in r#enum.variants {
-      if book.defs.contains_key(&variant.name) {
-        let msg = format!("Redefinition of function '{}'.", variant.name);
+      if let Some(def) = book.defs.get(&variant.name) {
+        let msg = PyParser::redefinition_of_function_msg(def.builtin, &variant.name);
         return self.with_ctx(Err(msg), ini_idx, end_idx);
       }
       if book.ctrs.contains_key(&variant.name) {
-        let msg = format!("Redefinition of constructor '{}'.", variant.name);
+        let msg = PyParser::redefinition_of_constructor_msg(&variant.name);
         return self.with_ctx(Err(msg), ini_idx, end_idx);
       }
       book.ctrs.insert(variant.name.clone(), r#enum.name.clone());
@@ -1122,16 +1122,16 @@ impl<'a> PyParser<'a> {
     builtin: bool,
   ) -> ParseResult<()> {
     if book.adts.contains_key(&obj.name) {
-      let msg = format!("Redefinition of type '{}'.", obj.name);
+      let msg = PyParser::redefinition_of_type_msg(&obj.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     let mut adt = Adt { ctrs: Default::default(), builtin };
-    if book.defs.contains_key(&obj.name) {
-      let msg = format!("Redefinition of function '{}'.", obj.name);
+    if let Some(def) = book.defs.get(&obj.name) {
+      let msg = PyParser::redefinition_of_function_msg(def.builtin, &obj.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     if book.ctrs.contains_key(&obj.name) {
-      let msg = format!("Redefinition of constructor '{}'.", obj.name);
+      let msg = PyParser::redefinition_of_constructor_msg(&obj.name);
       return self.with_ctx(Err(msg), ini_idx, end_idx);
     }
     book.ctrs.insert(obj.name.clone(), obj.name.clone());

tests/golden_tests/parse_file/redefinition_builtin.bend

@@ -0,0 +1,2 @@
+def Map/get(m):
+  return m

tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend

@@ -0,0 +1,2 @@
+def String/Cons(x):
+  return x

tests/golden_tests/parse_file/redefinition_type_with_object.bend

@@ -0,0 +1 @@
+object IO { run }

tests/snapshots/compile_file_o_all__adt_string.bend.snap

@@ -4,7 +4,7 @@ input_file: tests/golden_tests/compile_file_o_all/adt_string.bend
 ---
 Errors:
 In tests/golden_tests/compile_file_o_all/adt_string.bend :
-String is a built-in datatype and should not be overridden.
+Redefinition of builtin (type) 'String'.
   1 | type String = S
   2 | 
   3 | main = String/S

tests/snapshots/parse_file__redefinition_builtin.bend.snap

@@ -0,0 +1,10 @@
+---
+source: tests/golden_tests.rs
+input_file: tests/golden_tests/parse_file/redefinition_builtin.bend
+---
+Errors:
+In tests/golden_tests/parse_file/redefinition_builtin.bend :
+Redefinition of builtin (function) 'Map/get'.
+  1 | def Map/get(m):
+  2 |   return m
+

tests/snapshots/parse_file__redefinition_ctr_with_fun.bend.snap

@@ -0,0 +1,10 @@
+---
+source: tests/golden_tests.rs
+input_file: tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend
+---
+Errors:
+In tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend :
+Redefinition of builtin (constructor) 'String/Cons'.
+  1 | def String/Cons(x):
+  2 |   return x
+

tests/snapshots/parse_file__redefinition_imp_fun.bend.snap

@@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_imp_fun.bend
 ---
 Errors:
 In tests/golden_tests/parse_file/redefinition_imp_fun.bend :
-Redefinition of function 'A'
+Redefinition of function 'A'.
   5 | (A) = 1

tests/snapshots/parse_file__redefinition_type_with_object.bend.snap

@@ -0,0 +1,9 @@
+---
+source: tests/golden_tests.rs
+input_file: tests/golden_tests/parse_file/redefinition_type_with_object.bend
+---
+Errors:
+In tests/golden_tests/parse_file/redefinition_type_with_object.bend :
+Redefinition of builtin (type) 'IO'.
+  1 | object IO { run }
+

tests/snapshots/parse_file__redefinition_with_def_between.bend.snap

@@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_def_between.bend
 ---
 Errors:
 In tests/golden_tests/parse_file/redefinition_with_def_between.bend :
-Redefinition of function 'A'
+Redefinition of function 'A'.
    4 | (A) = @x x

tests/snapshots/parse_file__redefinition_with_object_between.bend.snap

@@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_object_between.bend
 ---
 Errors:
 In tests/golden_tests/parse_file/redefinition_with_object_between.bend :
-Redefinition of function 'A'
+Redefinition of function 'A'.
   3 | A = 1

tests/snapshots/parse_file__redefinition_with_type_between.bend.snap

@@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_type_between.bend
 ---
 Errors:
 In tests/golden_tests/parse_file/redefinition_with_type_between.bend :
-Redefinition of function 'A'
+Redefinition of function 'A'.
   3 | A = 1

tests/snapshots/parse_file__repeated_adt_name.bend.snap

@@ -4,7 +4,7 @@ input_file: tests/golden_tests/parse_file/repeated_adt_name.bend
 ---
 Errors:
 In tests/golden_tests/parse_file/repeated_adt_name.bend :
-Repeated datatype 'Foo'
+Redefinition of type 'Foo'.
   2 | type Foo = B
   3 | 
   4 | main = *