From 9773b0fea7ca9140ce30bfadf9a495089a31049c Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Mon, 24 Jun 2019 09:50:51 -0400
Subject: [PATCH] Use generics with range parsing

---
 Cargo.lock                           |  3 --
 members/ranges/Cargo.toml            |  2 +-
 members/ranges/src/lib.rs            | 40 +++++++++++------------
 members/ranges/src/parse.rs          | 17 +++++-----
 members/ranges/src/select.rs         | 15 ++++-----
 src/lib/expansion/methods/arrays.rs  |  4 +--
 src/lib/expansion/methods/strings.rs |  2 +-
 src/lib/expansion/mod.rs             | 48 ++++++++++++++++++++--------
 src/lib/expansion/words/mod.rs       | 16 +++++-----
 src/lib/shell/shell_expand.rs        | 18 +++++++++--
 src/lib/shell/variables.rs           | 18 +++++++++--
 11 files changed, 109 insertions(+), 74 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 6715a047..bc4b4601 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -355,9 +355,6 @@ dependencies = [
 [[package]]
 name = "ion-ranges"
 version = "0.1.0"
-dependencies = [
- "small 0.1.0 (git+https://gitlab.redox-os.org/redox-os/small)",
-]
 
 [[package]]
 name = "ion-shell"
diff --git a/members/ranges/Cargo.toml b/members/ranges/Cargo.toml
index 21c67e08..0e604a71 100644
--- a/members/ranges/Cargo.toml
+++ b/members/ranges/Cargo.toml
@@ -2,6 +2,6 @@
 name = "ion-ranges"
 version = "0.1.0"
 authors = ["Michael Murphy <mmstickman@gmail.com>"]
+edition = "2018"
 
 [dependencies]
-small = { git = "https://gitlab.redox-os.org/redox-os/small", features = ["std"] }
diff --git a/members/ranges/src/lib.rs b/members/ranges/src/lib.rs
index bf6f020d..58d4840a 100644
--- a/members/ranges/src/lib.rs
+++ b/members/ranges/src/lib.rs
@@ -1,5 +1,3 @@
-extern crate small;
-
 mod index;
 mod parse;
 mod range;
@@ -46,24 +44,22 @@ mod tests {
     }
 
     fn test_range<T: Iterator<Item = i8>>(range: &str, expected: T) {
-        let actual: Vec<small::String> = parse_range(range).unwrap().collect();
-        let expected: Vec<_> =
-            expected.map(|i| small::String::from_string(i.to_string())).collect();
+        let actual: Vec<String> = parse_range(range).unwrap().collect();
+        let expected: Vec<_> = expected.map(|i| i.to_string()).collect();
 
         assert_eq!(actual, expected);
     }
 
     fn test_fixed_range<T: Iterator<Item = i8>>(range: &str, expected: T, digits: usize) {
-        let actual: Vec<small::String> = parse_range(range).unwrap().collect();
-        let expected: Vec<_> =
-            expected.map(|i| small::String::from_string(format!("{:01$}", i, digits))).collect();
+        let actual: Vec<String> = parse_range(range).unwrap().collect();
+        let expected: Vec<_> = expected.map(|i| format!("{:01$}", i, digits)).collect();
 
         assert_eq!(actual, expected);
     }
 
     #[test]
     fn range_expand() {
-        if let Some(_) = parse_range("abc") {
+        if let Some(_) = parse_range::<String>("abc") {
             panic!("parse_range() failed");
         }
 
@@ -79,35 +75,35 @@ mod tests {
         test_range("-3...0", -3..=0);
         test_range("-3..0", -3..0);
 
-        let actual: Vec<small::String> = parse_range("a...c").unwrap().collect();
-        let expected: Vec<small::String> = vec!["a".into(), "b".into(), "c".into()];
+        let actual: Vec<String> = parse_range("a...c").unwrap().collect();
+        let expected: Vec<String> = vec!["a".into(), "b".into(), "c".into()];
 
         assert_eq!(actual, expected);
 
-        let actual: Vec<small::String> = parse_range("c...a").unwrap().collect();
-        let expected: Vec<small::String> = vec!["c".into(), "b".into(), "a".into()];
+        let actual: Vec<String> = parse_range("c...a").unwrap().collect();
+        let expected: Vec<String> = vec!["c".into(), "b".into(), "a".into()];
 
         assert_eq!(actual, expected);
 
-        let actual: Vec<small::String> = parse_range("A...C").unwrap().collect();
-        let expected: Vec<small::String> = vec!["A".into(), "B".into(), "C".into()];
+        let actual: Vec<String> = parse_range("A...C").unwrap().collect();
+        let expected: Vec<String> = vec!["A".into(), "B".into(), "C".into()];
 
         assert_eq!(actual, expected);
 
-        let actual: Vec<small::String> = parse_range("C...A").unwrap().collect();
-        let also: Vec<small::String> = parse_range("C..=A").unwrap().collect();
-        let expected: Vec<small::String> = vec!["C".into(), "B".into(), "A".into()];
+        let actual: Vec<String> = parse_range("C...A").unwrap().collect();
+        let also: Vec<String> = parse_range("C..=A").unwrap().collect();
+        let expected: Vec<String> = vec!["C".into(), "B".into(), "A".into()];
 
         assert_eq!(actual, expected);
         assert_eq!(also, expected);
 
-        let actual: Vec<small::String> = parse_range("C..A").unwrap().collect();
-        let expected: Vec<small::String> = vec!["C".into(), "B".into()];
+        let actual: Vec<String> = parse_range("C..A").unwrap().collect();
+        let expected: Vec<String> = vec!["C".into(), "B".into()];
 
         assert_eq!(actual, expected);
 
-        let actual: Vec<small::String> = parse_range("c..a").unwrap().collect();
-        let expected: Vec<small::String> = vec!["c".into(), "b".into()];
+        let actual: Vec<String> = parse_range("c..a").unwrap().collect();
+        let expected: Vec<String> = vec!["c".into(), "b".into()];
 
         assert_eq!(actual, expected);
     }
diff --git a/members/ranges/src/parse.rs b/members/ranges/src/parse.rs
index 92a60e36..0537eb4d 100644
--- a/members/ranges/src/parse.rs
+++ b/members/ranges/src/parse.rs
@@ -1,14 +1,13 @@
 use super::{Index, Range};
-use small;
 use std::{cmp::Ordering, u8};
 
-fn numeric_range<'a>(
+fn numeric_range<'a, K: From<String>>(
     start: isize,
     end: isize,
     step: isize,
     inclusive: bool,
     nb_digits: usize,
-) -> Option<Box<Iterator<Item = small::String> + 'a>> {
+) -> Option<Box<dyn Iterator<Item = K> + 'a>> {
     let end = if start < end && inclusive {
         end + 1
     } else if start > end && inclusive {
@@ -40,12 +39,12 @@ fn numeric_range<'a>(
     }
 }
 
-fn char_range<'a>(
+fn char_range<'a, K: From<String>>(
     start: u8,
     mut end: u8,
     step: isize,
     inclusive: bool,
-) -> Option<Box<Iterator<Item = small::String> + 'a>> {
+) -> Option<Box<dyn Iterator<Item = K> + 'a>> {
     if !start.is_ascii_alphabetic() || !end.is_ascii_alphabetic() || step == 0 {
         return None;
     }
@@ -67,18 +66,18 @@ fn char_range<'a>(
 fn count_minimum_digits(a: &str) -> usize {
     match a.bytes().find(|&c| c != b'-') {
         Some(b'0') => a.len(),
-        Some(b'1'...b'9') => 0,
+        Some(b'1'..=b'9') => 0,
         Some(_) => panic!("count_minimum_digits should only be called for a valid number."),
         None => 0,
     }
 }
 
-fn finish(
+fn finish<K: From<String>>(
     inclusive: bool,
     start_str: &str,
     end_str: &str,
     step: isize,
-) -> Option<Box<Iterator<Item = small::String>>> {
+) -> Option<Box<dyn Iterator<Item = K>>> {
     if let (Ok(start), Ok(end)) = (start_str.parse::<isize>(), end_str.parse::<isize>()) {
         let step = if step == 1 && start >= end { -step } else { step };
         let nb_digits = usize::max(count_minimum_digits(start_str), count_minimum_digits(end_str));
@@ -96,7 +95,7 @@ fn finish(
 //      Inclusive nonstepped: {start...end}
 //      Exclusive stepped: {start..step..end}
 //      Inclusive stepped: {start..step...end}
-pub fn parse_range(input: &str) -> Option<Box<Iterator<Item = small::String>>> {
+pub fn parse_range<K: From<String>>(input: &str) -> Option<Box<dyn Iterator<Item = K>>> {
     let mut parts = input.split("..").collect::<Vec<_>>();
     let len = parts.len();
 
diff --git a/members/ranges/src/select.rs b/members/ranges/src/select.rs
index c39d4799..29326dfe 100644
--- a/members/ranges/src/select.rs
+++ b/members/ranges/src/select.rs
@@ -1,5 +1,4 @@
 use super::{parse_index_range, Index, Range};
-use small;
 use std::{
     iter::{empty, FromIterator},
     str::FromStr,
@@ -7,7 +6,7 @@ use std::{
 
 /// Represents a filter on a vector-like object
 #[derive(Debug, PartialEq, Clone)]
-pub enum Select {
+pub enum Select<K> {
     /// Select all elements
     All,
     /// Select a single element based on its index
@@ -15,12 +14,12 @@ pub enum Select {
     /// Select a range of elements
     Range(Range),
     /// Select an element by mapped key
-    Key(small::String),
+    Key(K),
 }
 
 pub trait SelectWithSize {
     type Item;
-    fn select<O>(&mut self, &Select, usize) -> O
+    fn select<O, K>(&mut self, selection: &Select<K>, len: usize) -> O
     where
         O: FromIterator<Self::Item>;
 }
@@ -31,7 +30,7 @@ where
 {
     type Item = T;
 
-    fn select<O>(&mut self, s: &Select, size: usize) -> O
+    fn select<O, K>(&mut self, s: &Select<K>, size: usize) -> O
     where
         O: FromIterator<Self::Item>,
     {
@@ -48,10 +47,10 @@ where
     }
 }
 
-impl FromStr for Select {
+impl<K: FromStr> FromStr for Select<K> {
     type Err = ();
 
-    fn from_str(data: &str) -> Result<Select, ()> {
+    fn from_str(data: &str) -> Result<Self, ()> {
         if data == ".." {
             Ok(Select::All)
         } else if let Ok(index) = data.parse::<isize>() {
@@ -59,7 +58,7 @@ impl FromStr for Select {
         } else if let Some(range) = parse_index_range(data) {
             Ok(Select::Range(range))
         } else {
-            Ok(Select::Key(data.into()))
+            Ok(Select::Key(K::from_str(data).map_err(|_| ())?))
         }
     }
 }
diff --git a/src/lib/expansion/methods/arrays.rs b/src/lib/expansion/methods/arrays.rs
index 15b979ae..c6d5978c 100644
--- a/src/lib/expansion/methods/arrays.rs
+++ b/src/lib/expansion/methods/arrays.rs
@@ -17,7 +17,7 @@ pub struct ArrayMethod<'a> {
     method:    &'a str,
     variable:  &'a str,
     pattern:   Pattern<'a>,
-    selection: Select,
+    selection: Select<types::Str>,
 }
 
 impl<'a> ArrayMethod<'a> {
@@ -25,7 +25,7 @@ impl<'a> ArrayMethod<'a> {
         method: &'a str,
         variable: &'a str,
         pattern: Pattern<'a>,
-        selection: Select,
+        selection: Select<types::Str>,
     ) -> Self {
         ArrayMethod { method, variable, pattern, selection }
     }
diff --git a/src/lib/expansion/methods/strings.rs b/src/lib/expansion/methods/strings.rs
index f1c3e9a9..70fd3f06 100644
--- a/src/lib/expansion/methods/strings.rs
+++ b/src/lib/expansion/methods/strings.rs
@@ -87,7 +87,7 @@ pub struct StringMethod<'a> {
     /// Pattern to use for certain methods
     pub pattern: &'a str,
     /// Selection to use to control the output of this method
-    pub selection: Select,
+    pub selection: Select<types::Str>,
 }
 
 impl<'a> StringMethod<'a> {
diff --git a/src/lib/expansion/mod.rs b/src/lib/expansion/mod.rs
index 4a930ba8..d3860aca 100644
--- a/src/lib/expansion/mod.rs
+++ b/src/lib/expansion/mod.rs
@@ -84,7 +84,7 @@ pub enum ExpansionError<T: fmt::Debug + error::Error + fmt::Display + 'static> {
 
     /// A wrong index was given for indexing variable
     #[error(display = "index '{:?}' is not valid for {} variable '{}'", _0, _1, _2)]
-    InvalidIndex(Select, &'static str, String),
+    InvalidIndex(Select<types::Str>, &'static str, String),
 
     /// Mixed types between maps and scalar/array value
     #[error(display = "variable '{}' is not a map-like value", _0)]
@@ -123,21 +123,25 @@ pub trait Expander: Sized {
     /// Expand a tilde form to the correct directory.
     fn tilde(&self, _input: &str) -> Result<types::Str, Self::Error>;
     /// Expand an array variable with some selection.
-    fn array(&self, _name: &str, _selection: &Select) -> Result<Args, Self::Error>;
+    fn array(&self, _name: &str, _selection: &Select<types::Str>) -> Result<Args, Self::Error>;
     /// Expand a string variable given if it's quoted / unquoted
     fn string(&self, _name: &str) -> Result<types::Str, Self::Error>;
     /// Expand a subshell expression.
     fn command(&self, _command: &str) -> Result<types::Str, Self::Error>;
     /// Iterating upon key-value maps.
-    fn map_keys(&self, _name: &str, _select: &Select) -> Result<Args, Self::Error>;
+    fn map_keys(&self, _name: &str, _select: &Select<types::Str>) -> Result<Args, Self::Error>;
     /// Iterating upon key-value maps.
-    fn map_values(&self, _name: &str, _select: &Select) -> Result<Args, Self::Error>;
+    fn map_values(&self, _name: &str, _select: &Select<types::Str>) -> Result<Args, Self::Error>;
     /// Get a string that exists in the shell.
     fn get_string(&self, value: &str) -> Result<types::Str, Self::Error> {
         Ok(self.expand_string(value)?.join(" ").into())
     }
     /// Select the proper values from an iterator
-    fn select<I: Iterator<Item = types::Str>>(vals: I, select: &Select, n: usize) -> Option<Args> {
+    fn select<I: Iterator<Item = types::Str>>(
+        vals: I,
+        select: &Select<types::Str>,
+        n: usize,
+    ) -> Option<Args> {
         match select {
             Select::All => Some(vals.collect()),
             Select::Range(range) => range
@@ -170,7 +174,7 @@ pub trait Expander: Sized {
                             token_buffer.reserve(2 * keys.size_hint().0);
                             for index in keys {
                                 let select = index
-                                    .parse::<Select>()
+                                    .parse::<Select<types::Str>>()
                                     .map_err(|_| ExpansionError::IndexParsingError(index.into()))?;
                                 token_buffer.push(WordToken::ArrayVariable(
                                     data,
@@ -213,7 +217,7 @@ trait ExpanderInternal: Expander {
         &self,
         current: &mut types::Str,
         command: &str,
-        selection: &Select,
+        selection: &Select<types::Str>,
     ) -> Result<(), Self::Error> {
         self.command(command)
             .map(|result| Self::slice(current, result.trim_end_matches('\n'), &selection))
@@ -249,7 +253,11 @@ trait ExpanderInternal: Expander {
         Ok(())
     }
 
-    fn array_expand(&self, elements: &[&str], selection: &Select) -> Result<Args, Self::Error> {
+    fn array_expand(
+        &self,
+        elements: &[&str],
+        selection: &Select<types::Str>,
+    ) -> Result<Args, Self::Error> {
         match selection {
             Select::All => {
                 let mut collected = Args::new();
@@ -302,7 +310,7 @@ trait ExpanderInternal: Expander {
         }
     }
 
-    fn slice<S: AsRef<str>>(output: &mut types::Str, expanded: S, selection: &Select) {
+    fn slice<S: AsRef<str>>(output: &mut types::Str, expanded: S, selection: &Select<types::Str>) {
         match selection {
             Select::All => output.push_str(expanded.as_ref()),
             Select::Index(Index::Forward(id)) => {
@@ -371,7 +379,7 @@ trait ExpanderInternal: Expander {
                             "{}",
                             output
                                 .split_whitespace()
-                                .select::<Vec<_>>(index, output.split_whitespace().count())
+                                .select::<Vec<_>, _>(index, output.split_whitespace().count())
                                 .into_iter()
                                 .format(" ")
                         )))
@@ -379,7 +387,7 @@ trait ExpanderInternal: Expander {
                         Ok(output
                             .split_whitespace()
                             .map(From::from)
-                            .select::<Args>(index, output.split_whitespace().count()))
+                            .select::<Args, _>(index, output.split_whitespace().count()))
                     }
                 })
             }
@@ -647,7 +655,11 @@ pub(crate) mod test {
             }
         }
 
-        fn array(&self, variable: &str, _selection: &Select) -> Result<types::Args, Self::Error> {
+        fn array(
+            &self,
+            variable: &str,
+            _selection: &Select<types::Str>,
+        ) -> Result<types::Args, Self::Error> {
             match variable {
                 "ARRAY" => Ok(args!["a", "b", "c"].to_owned()),
                 _ => Err(ExpansionError::VarNotFound),
@@ -658,11 +670,19 @@ pub(crate) mod test {
 
         fn tilde(&self, input: &str) -> Result<types::Str, Self::Error> { Ok(input.into()) }
 
-        fn map_keys<'a>(&'a self, _name: &str, _select: &Select) -> Result<Args, Self::Error> {
+        fn map_keys<'a>(
+            &'a self,
+            _name: &str,
+            _select: &Select<types::Str>,
+        ) -> Result<Args, Self::Error> {
             Err(ExpansionError::VarNotFound)
         }
 
-        fn map_values<'a>(&'a self, _name: &str, _select: &Select) -> Result<Args, Self::Error> {
+        fn map_values<'a>(
+            &'a self,
+            _name: &str,
+            _select: &Select<types::Str>,
+        ) -> Result<Args, Self::Error> {
             Err(ExpansionError::VarNotFound)
         }
     }
diff --git a/src/lib/expansion/words/mod.rs b/src/lib/expansion/words/mod.rs
index 9eff3f88..a13e49e4 100644
--- a/src/lib/expansion/words/mod.rs
+++ b/src/lib/expansion/words/mod.rs
@@ -5,8 +5,8 @@ use super::{
     methods::{ArrayMethod, Pattern, StringMethod},
     Expander, ExpansionError, Result,
 };
-use crate::parser::lexers::ArgumentSplitter;
 pub use crate::ranges::{Select, SelectWithSize};
+use crate::{parser::lexers::ArgumentSplitter, types};
 use std::borrow::Cow;
 
 #[derive(Debug, PartialEq, Eq, Hash)]
@@ -36,11 +36,11 @@ pub enum WordToken<'a> {
     Normal(Cow<'a, str>, bool, bool),
     Whitespace(&'a str),
     Brace(Vec<&'a str>),
-    Array(Vec<&'a str>, Select),
-    Variable(&'a str, Select),
-    ArrayVariable(&'a str, bool, Select),
-    ArrayProcess(&'a str, bool, Select),
-    Process(&'a str, Select),
+    Array(Vec<&'a str>, Select<types::Str>),
+    Variable(&'a str, Select<types::Str>),
+    ArrayVariable(&'a str, bool, Select<types::Str>),
+    ArrayProcess(&'a str, bool, Select<types::Str>),
+    Process(&'a str, Select<types::Str>),
     StringMethod(StringMethod<'a>),
     ArrayMethod(ArrayMethod<'a>, bool),
     Arithmetic(&'a str),
@@ -484,7 +484,7 @@ impl<'a, E: Expander + 'a> WordIterator<'a, E> {
         ))
     }
 
-    fn read_selection<I>(&mut self, iterator: &mut I) -> Result<Select, E::Error>
+    fn read_selection<I>(&mut self, iterator: &mut I) -> Result<Select<types::Str>, E::Error>
     where
         I: Iterator<Item = u8>,
     {
@@ -495,7 +495,7 @@ impl<'a, E: Expander + 'a> WordIterator<'a, E> {
                 let value = self.expanders.expand_string(&self.data[start..self.read])?.join(" ");
                 self.read += 1;
                 return value
-                    .parse::<Select>()
+                    .parse::<Select<types::Str>>()
                     .map_err(|_| ExpansionError::IndexParsingError(value.into()));
             }
             self.read += 1;
diff --git a/src/lib/shell/shell_expand.rs b/src/lib/shell/shell_expand.rs
index 1ca87c15..8c9f7d43 100644
--- a/src/lib/shell/shell_expand.rs
+++ b/src/lib/shell/shell_expand.rs
@@ -40,7 +40,11 @@ impl<'a, 'b> Expander for Shell<'b> {
     }
 
     /// Expand an array variable with some selection
-    fn array(&self, name: &str, selection: &Select) -> expansion::Result<types::Args, Self::Error> {
+    fn array(
+        &self,
+        name: &str,
+        selection: &Select<types::Str>,
+    ) -> expansion::Result<types::Args, Self::Error> {
         match self.variables.get(name) {
             Some(Value::Array(array)) => match selection {
                 Select::All => {
@@ -155,7 +159,11 @@ impl<'a, 'b> Expander for Shell<'b> {
         }
     }
 
-    fn map_keys(&self, name: &str, sel: &Select) -> expansion::Result<types::Args, Self::Error> {
+    fn map_keys(
+        &self,
+        name: &str,
+        sel: &Select<types::Str>,
+    ) -> expansion::Result<types::Args, Self::Error> {
         match self.variables.get(name) {
             Some(&Value::HashMap(ref map)) => {
                 Self::select(map.keys().map(|x| format!("{}", x).into()), sel, map.len())
@@ -170,7 +178,11 @@ impl<'a, 'b> Expander for Shell<'b> {
         }
     }
 
-    fn map_values(&self, name: &str, sel: &Select) -> expansion::Result<types::Args, Self::Error> {
+    fn map_values(
+        &self,
+        name: &str,
+        sel: &Select<types::Str>,
+    ) -> expansion::Result<types::Args, Self::Error> {
         match self.variables.get(name) {
             Some(&Value::HashMap(ref map)) => {
                 Self::select(map.values().map(|x| format!("{}", x).into()), sel, map.len())
diff --git a/src/lib/shell/variables.rs b/src/lib/shell/variables.rs
index 356e1b93..54970194 100644
--- a/src/lib/shell/variables.rs
+++ b/src/lib/shell/variables.rs
@@ -292,7 +292,11 @@ pub(crate) mod tests {
 
         fn string(&self, var: &str) -> Result<types::Str, IonError> { self.0.get_str(var) }
 
-        fn array(&self, _variable: &str, _selection: &Select) -> Result<types::Args, Self::Error> {
+        fn array(
+            &self,
+            _variable: &str,
+            _selection: &Select<types::Str>,
+        ) -> Result<types::Args, Self::Error> {
             Err(ExpansionError::VarNotFound)
         }
 
@@ -300,11 +304,19 @@ pub(crate) mod tests {
 
         fn tilde(&self, input: &str) -> Result<types::Str, Self::Error> { Ok(input.into()) }
 
-        fn map_keys(&self, _name: &str, _select: &Select) -> Result<types::Args, Self::Error> {
+        fn map_keys(
+            &self,
+            _name: &str,
+            _select: &Select<types::Str>,
+        ) -> Result<types::Args, Self::Error> {
             Err(ExpansionError::VarNotFound)
         }
 
-        fn map_values(&self, _name: &str, _select: &Select) -> Result<types::Args, Self::Error> {
+        fn map_values(
+            &self,
+            _name: &str,
+            _select: &Select<types::Str>,
+        ) -> Result<types::Args, Self::Error> {
             Err(ExpansionError::VarNotFound)
         }
     }
-- 
GitLab