Merge pull request #12 from elferherrera/similar-name

Similar name check for signature

crates/nu-parser/tests/test_signature.rs

@@ -0,0 +1,119 @@
+use nu_parser::{Flag, PositionalArg, Signature, SyntaxShape};
+
+#[test]
+fn test_signature() {
+    let signature = Signature::new("new_signature");
+    let from_build = Signature::build("new_signature");
+
+    // asserting partial eq implementation
+    assert_eq!(signature, from_build);
+
+    // constructing signature with description
+    let signature = Signature::new("signature").desc("example usage");
+    assert_eq!(signature.usage, "example usage".to_string())
+}
+
+#[test]
+fn test_signature_chained() {
+    let signature = Signature::new("new_signature")
+        .desc("description")
+        .required("required", SyntaxShape::String, "required description")
+        .optional("optional", SyntaxShape::String, "optional description")
+        .required_named(
+            "req_named",
+            SyntaxShape::String,
+            "required named description",
+            Some('r'),
+        )
+        .named("named", SyntaxShape::String, "named description", Some('n'))
+        .switch("switch", "switch description", None)
+        .rest(SyntaxShape::String, "rest description");
+
+    assert_eq!(signature.required_positional.len(), 1);
+    assert_eq!(signature.optional_positional.len(), 1);
+    assert_eq!(signature.named.len(), 3);
+    assert!(signature.rest_positional.is_some());
+    assert_eq!(signature.get_shorts(), vec!['r', 'n']);
+    assert_eq!(signature.get_names(), vec!["req_named", "named", "switch"]);
+    assert_eq!(signature.num_positionals(), 2);
+
+    assert_eq!(
+        signature.get_positional(0),
+        Some(PositionalArg {
+            name: "required".to_string(),
+            desc: "required description".to_string(),
+            shape: SyntaxShape::String,
+            var_id: None
+        })
+    );
+    assert_eq!(
+        signature.get_positional(1),
+        Some(PositionalArg {
+            name: "optional".to_string(),
+            desc: "optional description".to_string(),
+            shape: SyntaxShape::String,
+            var_id: None
+        })
+    );
+    assert_eq!(
+        signature.get_positional(2),
+        Some(PositionalArg {
+            name: "rest".to_string(),
+            desc: "rest description".to_string(),
+            shape: SyntaxShape::String,
+            var_id: None
+        })
+    );
+
+    assert_eq!(
+        signature.get_long_flag("req_named"),
+        Some(Flag {
+            long: "req_named".to_string(),
+            short: Some('r'),
+            arg: Some(SyntaxShape::String),
+            required: true,
+            desc: "required named description".to_string(),
+            var_id: None
+        })
+    );
+
+    assert_eq!(
+        signature.get_short_flag('r'),
+        Some(Flag {
+            long: "req_named".to_string(),
+            short: Some('r'),
+            arg: Some(SyntaxShape::String),
+            required: true,
+            desc: "required named description".to_string(),
+            var_id: None
+        })
+    );
+}
+
+#[test]
+#[should_panic(expected = "There may be duplicate short flags, such as -h")]
+fn test_signature_same_short() {
+    // Creating signature with same short name should panic
+    Signature::new("new_signature")
+        .required_named(
+            "required_named",
+            SyntaxShape::String,
+            "required named description",
+            Some('n'),
+        )
+        .named("named", SyntaxShape::String, "named description", Some('n'));
+}
+
+#[test]
+#[should_panic(expected = "There may be duplicate name flags, such as --help")]
+fn test_signature_same_name() {
+    // Creating signature with same short name should panic
+    Signature::new("new_signature")
+        .required_named(
+            "name",
+            SyntaxShape::String,
+            "required named description",
+            Some('r'),
+        )
+        .named("name", SyntaxShape::String, "named description", Some('n'));
+}

crates/nu-protocol/src/signature.rs

@@ -6,7 +6,7 @@ use crate::SyntaxShape;
 use crate::Value;
 use crate::VarId;
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Flag {
     pub long: String,
     pub short: Option<char>,
@@ -127,10 +127,8 @@ impl Signature {
         desc: impl Into<String>,
         short: Option<char>,
     ) -> Signature {
-        let s = short.map(|c| {
-            debug_assert!(!self.get_shorts().contains(&c));
-            c
-        });
+        let (name, s) = self.check_names(name, short);
+
         self.named.push(Flag {
             long: name.into(),
             short: s,
@@ -151,10 +149,8 @@ impl Signature {
         desc: impl Into<String>,
         short: Option<char>,
     ) -> Signature {
-        let s = short.map(|c| {
-            debug_assert!(!self.get_shorts().contains(&c));
-            c
-        });
+        let (name, s) = self.check_names(name, short);
+
         self.named.push(Flag {
             long: name.into(),
             short: s,
@@ -174,13 +170,7 @@ impl Signature {
         desc: impl Into<String>,
         short: Option<char>,
     ) -> Signature {
-        let s = short.map(|c| {
-            debug_assert!(
-                !self.get_shorts().contains(&c),
-                "There may be duplicate short flags, such as -h"
-            );
-            c
-        });
+        let (name, s) = self.check_names(name, short);
 
         self.named.push(Flag {
             long: name.into(),
@@ -190,18 +180,41 @@ impl Signature {
             desc: desc.into(),
             var_id: None,
         });
+
         self
     }
 
     /// Get list of the short-hand flags
     pub fn get_shorts(&self) -> Vec<char> {
-        let mut shorts = Vec::new();
-        for Flag { short, .. } in &self.named {
-            if let Some(c) = short {
-                shorts.push(*c);
-            }
-        }
-        shorts
+        self.named.iter().filter_map(|f| f.short).collect()
+    }
+
+    /// Get list of the long-hand flags
+    pub fn get_names(&self) -> Vec<String> {
+        self.named.iter().map(|f| f.long.clone()).collect()
+    }
+
+    /// Checks if short or long are already present
+    /// Panics if one of them is found
+    fn check_names(&self, name: impl Into<String>, short: Option<char>) -> (String, Option<char>) {
+        let s = short.map(|c| {
+            debug_assert!(
+                !self.get_shorts().contains(&c),
+                "There may be duplicate short flags, such as -h"
+            );
+            c
+        });
+
+        let name = {
+            let name = name.into();
+            debug_assert!(
+                !self.get_names().contains(&name),
+                "There may be duplicate name flags, such as --help"
+            );
+            name
+        };
+
+        (name, s)
     }
 
     pub fn get_positional(&self, position: usize) -> Option<PositionalArg> {