add trait NodeClass, struct Class

minor fix

src/ast/json.rs

@@ -1,7 +1,7 @@
 //! A hard-coded specification of JSON ASTs in a format editable by Sapling
 
 use super::display_token::{syntax_category, DisplayToken, RecTok};
-use super::{Ast, DeleteError, InsertError};
+use super::{Ast, AstClass, DeleteError, InsertError};
 use crate::arena::Arena;
 use crate::core::Size;
 
@@ -22,6 +22,55 @@ const CHAR_ARRAY: char = 'a';
 const CHAR_OBJECT: char = 'o';
 const CHAR_STRING: char = 's';
 
+/// The sapling representation of the AST for a subset of JSON (where all values are either 'true'
+/// or 'false', and keys only contain ASCII).
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[allow(missing_docs)]
+pub enum Class {
+    True,
+    False,
+    Null,
+    Str,
+    Array,
+    Object,
+}
+
+impl<'arena> AstClass<'arena> for Class {
+    fn to_char(self) -> char {
+        match self {
+            Class::True => CHAR_TRUE,
+            Class::False => CHAR_FALSE,
+            Class::Null => CHAR_NULL,
+            Class::Array => CHAR_ARRAY,
+            Class::Object => CHAR_OBJECT,
+            Class::Str => CHAR_STRING,
+        }
+    }
+
+    fn name(self) -> &'static str {
+        match self {
+            Class::True => "True",
+            Class::False => "False",
+            Class::Null => "Null",
+            Class::Array => "Array",
+            Class::Object => "Object",
+            Class::Str => "String",
+        }
+    }
+
+    fn from_char(c: char) -> Option<Self> {
+        match c {
+            CHAR_TRUE => Some(Class::True),
+            CHAR_FALSE => Some(Class::False),
+            CHAR_NULL => Some(Class::Null),
+            CHAR_ARRAY => Some(Class::Array),
+            CHAR_OBJECT => Some(Class::Object),
+            CHAR_STRING => Some(Class::Str),
+            _ => None,
+        }
+    }
+}
+
 /// The sapling representation of the AST for a subset of JSON (where all values are either 'true'
 /// or 'false', and keys only contain ASCII).
 #[derive(Debug, Eq, PartialEq, Clone, Hash)]
@@ -47,24 +96,6 @@ pub enum Json<'arena> {
     Str(String),
 }
 
-impl Json<'_> {
-    /// Return an iterator over all the possible chars that could represent JSON nodes
-    fn all_object_chars() -> Box<dyn Iterator<Item = char>> {
-        Box::new(
-            [
-                CHAR_TRUE,
-                CHAR_FALSE,
-                CHAR_NULL,
-                CHAR_ARRAY,
-                CHAR_OBJECT,
-                CHAR_STRING,
-            ]
-            .iter()
-            .copied(),
-        )
-    }
-}
-
 impl Default for Json<'_> {
     fn default() -> Json<'static> {
         Json::Object(vec![])
@@ -73,6 +104,7 @@ impl Default for Json<'_> {
 
 impl<'arena> Ast<'arena> for Json<'arena> {
     type FormatStyle = JsonFormat;
+    type Class = Class;
 
     /* FORMATTING FUNCTIONS */
 
@@ -376,23 +408,18 @@ impl<'arena> Ast<'arena> for Json<'arena> {
 
     /* AST EDITING FUNCTIONS */
 
-    fn from_char(&self, c: char) -> Option<Self> {
-        match c {
-            CHAR_TRUE => Some(Json::True),
-            CHAR_FALSE => Some(Json::False),
-            CHAR_NULL => Some(Json::Null),
-            CHAR_ARRAY => Some(Json::Array(vec![])),
-            CHAR_OBJECT => Some(Json::Object(vec![])),
-            CHAR_STRING => Some(Json::Str("".to_string())),
-            _ => None,
+    fn from_class(node_type: Self::Class) -> Self {
+        match node_type {
+            Class::True => Json::True,
+            Class::False => Json::False,
+            Class::Null => Json::Null,
+            Class::Array => Json::Array(vec![]),
+            Class::Object => Json::Object(vec![]),
+            Class::Str => Json::Str("".to_string()),
         }
     }
 
-    fn valid_chars() -> Box<dyn Iterator<Item = char>> {
-        Self::all_object_chars()
-    }
-
-    fn is_valid_child(&self, index: usize, c: char) -> bool {
+    fn is_valid_child(&self, index: usize, node_type: Self::Class) -> bool {
         match self {
             // values like 'true' and 'false' can never have children
             Json::True | Json::False | Json::Str(_) | Json::Null => false,
@@ -401,16 +428,18 @@ impl<'arena> Ast<'arena> for Json<'arena> {
             // fields must have their left hand side be a string
             Json::Field(_) => {
                 if index == 0 {
-                    c == CHAR_STRING
+                    node_type == Class::Str
                 } else {
                     true
                 }
             }
         }
     }
 
-    fn is_valid_root(&self, c: char) -> bool {
-        Self::valid_chars().any(|x| x == c)
+    fn is_valid_root(&self, node_type: Class) -> bool {
+        match node_type {
+            _ => true,
+        }
     }
 }
 

src/ast/mod.rs

@@ -102,10 +102,24 @@ fn write_tree_view_recursive<'arena, Node>(
     }
 }
 
+/// All the possible types which [`Ast`] nodes can take.
+pub trait AstClass<'arena>: Copy + std::fmt::Debug + Eq + std::hash::Hash {
+    /// Gets the [`char`] that would have been used to create this value
+    fn to_char(self) -> char;
+
+    /// Returns the name of this value
+    fn name(self) -> &'static str;
+
+    /// Creates a `AstClass` from a [`char`], returning [`None`] if invalid.
+    fn from_char(c: char) -> Option<Self>;
+}
+
 /// The specification of an AST that sapling can edit
 pub trait Ast<'arena>: std::fmt::Debug + Clone + Eq + Default + std::hash::Hash {
     /// A type parameter that will represent the different ways this AST can be rendered
     type FormatStyle;
+    /// A type parameter that will represent the different node types this AST can use
+    type Class: AstClass<'arena>;
 
     /* FORMATTING FUNCTIONS */
 
@@ -205,17 +219,12 @@ pub trait Ast<'arena>: std::fmt::Debug + Clone + Eq + Default + std::hash::Hash
 
     /* AST EDITING FUNCTIONS */
 
-    /// Generate a new node from a [`char`] that a user typed.  If `c` is an element of
-    /// [`valid_chars`](Ast::valid_chars), this must return [`Some`],
-    /// if it isn't, then this should return [`None`].
-    fn from_char(&self, c: char) -> Option<Self>;
-
-    /// Generate an iterator over the possible shorthand [`char`]s for valid  node
-    fn valid_chars() -> Box<dyn Iterator<Item = char>>;
+    /// Generate a new node from a AstClass.
+    fn from_class(node_type: Self::Class) -> Self;
 
     /// Returns whether or not a given index and [`char`] is a valid child
-    fn is_valid_child(&self, index: usize, c: char) -> bool;
+    fn is_valid_child(&self, index: usize, node_type: Self::Class) -> bool;
 
     /// Returns whether or not a give index and ['char'] is a valid root
-    fn is_valid_root(&self, c: char) -> bool;
+    fn is_valid_root(&self, node_type: Self::Class) -> bool;
 }

src/editor/dag.rs

@@ -2,7 +2,7 @@
 
 use crate::arena::Arena;
 use crate::ast;
-use crate::ast::Ast;
+use crate::ast::{Ast, AstClass};
 use crate::editor::normal_mode::Action;
 
 use crate::core::{Direction, Path, Side};
@@ -82,10 +82,15 @@ pub enum EditErr {
     NoChangesToRedo,
     /// The user typed a char that doesn't correspond to any node
     CharNotANode(char),
+    /// Trying to insert a node that cannot be root node
+    CannotBeRoot {
+        /// The [`str`] representing the disallowed node type
+        class: &'static str,
+    },
     /// Trying to insert a node that cannot be a child of the cursor
     CannotBeChild {
-        /// The [`char`] representing the disallowed child type
-        c: char,
+        /// The [`str`] representing the disallowed child type
+        class: &'static str,
         /// The [`display_name`](Ast::display_name) of the parent node
         parent_name: String,
     },
@@ -117,8 +122,11 @@ impl EditErr {
             EditErr::InsertError(e) => log::warn!("{}", e),
             EditErr::DeleteError(e) => log::warn!("{}", e),
             EditErr::CharNotANode(c) => log::warn!("'{}' doesn't correspond to any node type.", c),
-            EditErr::CannotBeChild { c, parent_name } => {
-                log::warn!("'{}' cannot be a child of {}", c, parent_name)
+            EditErr::CannotBeRoot { class } => {
+                log::warn!("'{}' cannot be root", class)
+            }
+            EditErr::CannotBeChild { class, parent_name } => {
+                log::warn!("'{}' cannot be a child of {}", class, parent_name)
             }
             EditErr::AddSiblingToRoot => log::warn!("Can't add siblings to the root."),
             EditErr::DeletingRoot => log::warn!("Can't delete the root."),
@@ -365,31 +373,33 @@ impl<'arena, Node: Ast<'arena>> Dag<'arena, Node> {
 
     /// Replaces the current cursor with a node represented by `c`
     fn replace_cursor(&mut self, c: char) -> EditResult {
+        let class = Node::Class::from_char(c).ok_or(EditErr::CharNotANode(c))?;
         self.perform_edit(
             |_this: &mut Self,
              _parent_and_index: Option<(&'arena Node, usize)>,
              cursor: &'arena Node| {
-                // Early return if the `c` can't go in the cursor's location
+                // Early return if the `class` can't go in the cursor's location
                 match _parent_and_index {
                     Some((parent, cursor_index)) => {
-                        if !parent.is_valid_child(cursor_index, c) {
+                        if !parent.is_valid_child(cursor_index, class) {
                             return Err(EditErr::CannotBeChild {
-                                c,
+                                class: class.name(),
                                 parent_name: _parent_and_index
                                     .map_or("<root>".to_owned(), |(p, _)| p.display_name()),
                             });
                         }
                     }
                     None => {
-                        if !cursor.is_valid_root(c) {
-                            return Err(EditErr::CharNotANode(c));
+                        if !cursor.is_valid_root(class) {
+                            return Err(EditErr::CannotBeRoot {
+                                class: class.name(),
+                            });
                         }
                     }
                 }
 
-                // Create the node to replace.  We can use unwrap here, because 'c' is always one
-                // of valid chars.
-                let new_node = cursor.from_char(c).unwrap();
+                // Create the node to replace.
+                let new_node = Node::from_class(class);
                 let new_node_name = new_node.display_name();
                 Ok((
                     new_node,
@@ -406,22 +416,22 @@ impl<'arena, Node: Ast<'arena>> Dag<'arena, Node> {
     /// Updates the internal state so that the tree now contains `new_node` inserted as the last
     /// child of the selected node.  Also moves the cursor so that the new node is selected.
     fn insert_child(&mut self, c: char) -> EditResult {
+        let class = Node::Class::from_char(c).ok_or(EditErr::CharNotANode(c))?;
         self.perform_edit(
             |this: &mut Self,
              _parent_and_index: Option<(&'arena Node, usize)>,
              cursor: &'arena Node| {
                 log::debug!("Inserting '{}' as a new child.", c);
-                if !cursor.is_valid_child(cursor.children().len(), c) {
+                if !cursor.is_valid_child(cursor.children().len(), class) {
                     log::debug!("New node could not be a valid child of the cursor");
                     // Short circuit if `c` couldn't be a valid child of the cursor
                     return Err(EditErr::CannotBeChild {
-                        c,
+                        class: class.name(),
                         parent_name: cursor.display_name(),
                     });
                 }
                 // Create the node to insert
-                // we can use unwrap here, because 'c' is always one of valid chars.
-                let new_node = this.arena.alloc(cursor.from_char(c).unwrap());
+                let new_node = this.arena.alloc(Node::from_class(class));
                 let new_node_name = new_node.display_name();
                 // Clone the node that currently is the cursor, and add the new child to the end of its
                 // children.
@@ -448,6 +458,7 @@ impl<'arena, Node: Ast<'arena>> Dag<'arena, Node> {
     /// Updates the internal state so that the tree now contains `new_node` inserted as the first
     /// child of the selected node.  Also moves the cursor so that the new node is selected.
     fn insert_next_to_cursor(&mut self, c: char, side: Side) -> EditResult {
+        let class = Node::Class::from_char(c).ok_or(EditErr::CharNotANode(c))?;
         self.perform_edit(
             |this: &mut Self,
              parent_and_index: Option<(&'arena Node, usize)>,
@@ -457,9 +468,9 @@ impl<'arena, Node: Ast<'arena>> Dag<'arena, Node> {
                 let (parent, cursor_index) = parent_and_index.ok_or(EditErr::AddSiblingToRoot)?;
 
                 // Short circuit if not an insertable char
-                if !parent.is_valid_child(cursor_index, c) {
+                if !parent.is_valid_child(cursor_index, class) {
                     return Err(EditErr::CannotBeChild {
-                        c,
+                        class: class.name(),
                         parent_name: parent.display_name(),
                     });
                 }
@@ -470,10 +481,8 @@ impl<'arena, Node: Ast<'arena>> Dag<'arena, Node> {
                         Side::Next => 1,
                     };
                 let mut cloned_parent = parent.clone();
-                // Create the new sibling node according to the given char.
-                let new_node = this
-                    .arena
-                    .alloc(cloned_parent.from_char(c).ok_or(EditErr::CharNotANode(c))?);
+                // Create the new sibling node according to the given class.
+                let new_node = this.arena.alloc(Node::from_class(class));
                 // Store the new_node's display name before it's consumed by `insert_child`
                 let new_node_name = new_node.display_name();
                 // Add the new child to the children of the cloned cursor
@@ -1137,7 +1146,7 @@ mod tests {
             Path::from_vec(vec![1, 0]),
             Action::Replace('n'),
             EditErr::CannotBeChild {
-                c: 'n',
+                class: "Null",
                 parent_name: ("field".to_string()),
             },
         );
@@ -1339,7 +1348,6 @@ mod tests {
     /// This is a regression test for issue #68, where Sapling crashes if an invalid char is used
     /// to insert into any node
     #[test]
-    #[ignore]
     fn invalid_insert_crash() {
         // Create and initialise Dag to test (start with JSON `[null]` with the cursor selecting
         // the `null`)
@@ -1369,7 +1377,6 @@ mod tests {
     /// This is a regression test for issue #68, where Sapling crashes if an invalid char is used
     /// to insert into any node
     #[test]
-    #[ignore]
     fn invalid_replace_crash() {
         // Create and initialise Dag to test (start with JSON `[null]` with the cursor selecting
         // the `null`)