Merge #247: fix: realising of built-in types, hard-coded aliases

887c681 fix: add whitespace after comma in `Either` display (Volodymyr Herashchenko)
0f54d67 feat: add check for redefining built-in types (Volodymyr Herashchenko)
ec45a2d fix: remove hardcoded built-ins (Volodymyr Herashchenko)

Pull request description:

  Closes #235 and partially closes #202.

  There are 3 small and separate fixes:
  1) Remove hard-coded built-in aliases from the parser, so all alias checking is in `types.rs`
  2) Adds check for redefining built-in aliases and types in `TypeAlias` parsing function. It uses the same logic as the first fix, so I included it here.
  3) Fix small typo within `TypeInner` display function -- there are missing whitespace after comma inside `Either` type. You can see this issue in [this PR](BlockstreamResearch/simplicity-lang-org@ab0365c#diff-45c93d101716efcb5e6e71fbdca58e55488f9b0c15f1ac2f01c9af1b1b39f44eR2936) for `simplicitiy-lang-org`. It's a small enough change that I thought I could include it here.

  Would be happy to split this up if needed.

ACKs for top commit:
  apoelstra:
    ACK 887c681; successfully ran local tests
  delta1:
    ACK 887c681; tested locally

Tree-SHA512: c30a6be4221727d0518e1d0b9559b83783b1834c9b002cfc51c81414209fac924f4fb9786c1ad4ec3057a2572c42ce9b88ac0b2f238b614beec5be68142c28f1

Author: delta1

src/error.rs

@@ -429,6 +429,7 @@ pub enum Error {
     IntegerOutOfBounds(UIntType),
     UndefinedVariable(Identifier),
     RedefinedAlias(AliasName),
+    RedefinedAliasAsBuiltin(AliasName),
     UndefinedAlias(AliasName),
     VariableReuseInPattern(Identifier),
     WitnessReused(WitnessName),
@@ -551,6 +552,10 @@ impl fmt::Display for Error {
                 f,
                 "Type alias `{identifier}` was defined multiple times"
             ),
+            Error::RedefinedAliasAsBuiltin(identifier) => write!(
+                f,
+                "Type alias `{identifier}` is already exists as built-in alias"
+            ),
             Error::UndefinedAlias(identifier) => write!(
                 f,
                 "Type alias `{identifier}` is not defined"

src/parse.rs

@@ -26,7 +26,7 @@ use crate::str::{
     AliasName, Binary, Decimal, FunctionName, Hexadecimal, Identifier, JetName, ModuleName,
     WitnessName,
 };
-use crate::types::{AliasedType, BuiltinAlias, TypeConstructible};
+use crate::types::{AliasedType, BuiltinAlias, TypeConstructible, UIntType};
 
 /// A program is a sequence of items.
 #[derive(Clone, Debug)]
@@ -1002,26 +1002,17 @@ impl ChumskyParse for AliasedType {
         I: ValueInput<'tokens, Token = Token<'src>, Span = Span>,
     {
         let atom = select! {
-            Token::Ident(ident) => {
-                match ident
-                {
-                    "u1" => AliasedType::u1(),
-                    "u2" =>  AliasedType::u2(),
-                    "u4" =>  AliasedType::u4(),
-                    "u8" => AliasedType::u8(),
-                    "u16" => AliasedType::u16(),
-                    "u32" => AliasedType::u32(),
-                    "u64" => AliasedType::u64(),
-                    "u128" => AliasedType::u128(),
-                    "u256" => AliasedType::u256(),
-                    "Ctx8" | "Pubkey" | "Message64" | "Message" | "Signature" | "Scalar" | "Fe" | "Gej"
-                    | "Ge" | "Point" | "Height" | "Time" | "Distance" | "Duration" | "Lock" | "Outpoint"
-                    | "Confidential1" | "ExplicitAsset" | "Asset1" | "ExplicitAmount" | "Amount1"
-                    | "ExplicitNonce" | "Nonce" | "TokenAmount1" => AliasedType::builtin(BuiltinAlias::from_str(ident).unwrap()),
-                    "bool" => AliasedType::boolean(),
-                    _ => AliasedType::alias(AliasName::from_str_unchecked(ident)),
+                Token::Ident(ident) => {
+                    if ident == "bool" {
+                        AliasedType::boolean()
+                    } else if let Ok(uint_type) = UIntType::from_str(ident) {
+                        AliasedType::from(uint_type)
+                    } else if let Ok(builtin) = BuiltinAlias::from_str(ident) {
+                        AliasedType::builtin(builtin)
+                    } else {
+                        AliasedType::alias(AliasName::from_str_unchecked(ident))
+                    }
                 }
-            },
         };
 
         let num = select! {
@@ -1461,7 +1452,25 @@ impl ChumskyParse for TypeAlias {
     where
         I: ValueInput<'tokens, Token = Token<'src>, Span = Span>,
     {
-        let name = AliasName::parser().map_with(|name, e| (name, e.span()));
+        let name = AliasName::parser()
+            .validate(|name, e, emit| {
+                let ident = name.as_inner();
+                let known_type = if ident == "bool" {
+                    Some(AliasedType::boolean())
+                } else if let Ok(uint_type) = UIntType::from_str(ident) {
+                    Some(AliasedType::from(uint_type))
+                } else if let Ok(builtin) = BuiltinAlias::from_str(ident) {
+                    Some(AliasedType::builtin(builtin))
+                } else {
+                    None
+                };
+
+                if known_type.is_some() {
+                    emit.emit(Error::RedefinedAliasAsBuiltin(name.clone()).with_span(e.span()));
+                }
+                name
+            })
+            .map_with(|name, e| (name, e.span()));
 
         just(Token::Type)
             .ignore_then(name)
@@ -2163,3 +2172,19 @@ impl crate::ArbitraryRec for Match {
         })
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_reject_redefined_builtin_type() {
+        let ty = TypeAlias::parse_from_str("type Ctx8 = u32")
+            .expect_err("Redifining built-in alias should be rejected");
+
+        assert_eq!(
+            ty.error(),
+            &Error::RedefinedAliasAsBuiltin(AliasName::from_str_unchecked("Ctx8"))
+        );
+    }
+}

src/types.rs

@@ -37,7 +37,7 @@ impl<A> TypeInner<A> {
         match self {
             TypeInner::Either(_, _) => match n_children_yielded {
                 0 => f.write_str("Either<"),
-                1 => f.write_str(","),
+                1 => f.write_str(", "),
                 n => {
                     debug_assert_eq!(n, 2);
                     f.write_str(">")
@@ -193,6 +193,25 @@ impl fmt::Display for UIntType {
     }
 }
 
+impl FromStr for UIntType {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "u1" => Ok(UIntType::U1),
+            "u2" => Ok(UIntType::U2),
+            "u4" => Ok(UIntType::U4),
+            "u8" => Ok(UIntType::U8),
+            "u16" => Ok(UIntType::U16),
+            "u32" => Ok(UIntType::U32),
+            "u64" => Ok(UIntType::U64),
+            "u128" => Ok(UIntType::U128),
+            "u256" => Ok(UIntType::U256),
+            _ => Err("Unknown integer type".to_string()),
+        }
+    }
+}
+
 impl TryFrom<&StructuralType> for UIntType {
     type Error = ();
 
@@ -1094,5 +1113,7 @@ mod tests {
         assert_eq!("[(); 3]", &array.to_string());
         let list = ResolvedType::list(ResolvedType::unit(), NonZeroPow2Usize::TWO);
         assert_eq!("List<(), 2>", &list.to_string());
+        let either = ResolvedType::either(ResolvedType::unit(), ResolvedType::u32());
+        assert_eq!("Either<(), u32>", &either.to_string());
     }
 }