Repository: kdr-aus/ogma (rust)

Issue:
[BUG] Calling user def with wrong input gives a bad error message
## Current Behaviour

```
def foo Table () { \#t }

>> foo
```

Results in a bad error message, it should point to what implementations are available!

## Expected Behaviour

## Reproduction Steps
1. 

### Additional information


Pull request:
Better not defined error
Fixes #69

Patch:
diff --git a/RELEASE.md b/RELEASE.md
index 3f0b19b..09d8761 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -12,5 +12,6 @@
     (https://github.com/kdr-aus/ogma/pull/77)
 - Flush `Write`r once finished writing. (https://github.com/kdr-aus/ogma/pull/78)
 - Fixes (https://github.com/kdr-aus/ogma/issues/15) with new compilation engine.
+- Output better error messages for unmatched commands (https://github.com/kdr-aus/ogma/pull/86)
 
 **✨ Other Updates**
diff --git a/ogma/src/common/err.rs b/ogma/src/common/err.rs
index e98ed99..21df359 100644
--- a/ogma/src/common/err.rs
+++ b/ogma/src/common/err.rs
@@ -172,19 +172,38 @@ impl Error {
         }
     }
 
-    pub(crate) fn op_not_found(op: &Tag, recursion_detected: bool) -> Self {
+    pub(crate) fn op_not_found(
+        op: &Tag,
+        inty: Option<&Type>,
+        recursion_detected: bool,
+        impls: &Implementations,
+    ) -> Self {
+        fn tystr(x: Option<&Type>) -> String {
+            x.map(|x| x.to_string()).unwrap_or_else(|| "<any>".into())
+        }
+
+        let ty = impls.iter().filter(|x| x.0 == op.str()).collect::<Vec<_>>();
+
         let hlp = if recursion_detected {
             "recursion is not supported.
-          for alternatives, please see <https://daedalus.report/d/docs/ogma.book/11%20(no)%20recursion.md?pwd-raw=docs>"
+          for alternatives, please see <https://daedalus.report/d/docs/ogma.book/11%20(no)%20recursion.md?pwd-raw=docs>".into()
+        } else if ty.is_empty() {
+            "view a list of definitions using `def --list`".into()
         } else {
-            "view a list of definitions using `def --list`"
+            ty.into_iter().fold(
+                format!("`{}` is implemented for the following input types:", op),
+                |s, t| s + " " + &tystr(t.1),
+            )
         };
 
         Error {
             cat: Category::UnknownCommand,
             desc: format!("operation `{}` not defined", op),
-            traces: trace(op, format!("`{}` not found", op)),
-            help_msg: Some(hlp.into()),
+            traces: trace(
+                op,
+                format!("`{}` not defined for input `{}`", op, tystr(inty)),
+            ),
+            help_msg: Some(hlp),
             ..Self::default()
         }
     }
diff --git a/ogma/src/eng/comp/mod.rs b/ogma/src/eng/comp/mod.rs
index 25ac468..2371de7 100644
--- a/ogma/src/eng/comp/mod.rs
+++ b/ogma/src/eng/comp/mod.rs
@@ -425,10 +425,14 @@ impl<'d> Compiler<'d> {
         chgs: Chgs,
         infer_output: &mut bool,
     ) -> Result<Step> {
-        let cmd_node = self
-            .ag
-            .get_impl(opnode, &in_ty)
-            .ok_or_else(|| Error::op_not_found(self.ag[opnode.idx()].tag(), false))?;
+        let cmd_node = self.ag.get_impl(opnode, &in_ty).ok_or_else(|| {
+            Error::op_not_found(
+                self.ag[opnode.idx()].tag(),
+                Some(&in_ty),
+                false,
+                self.defs.impls(),
+            )
+        })?;
 
         match &self.ag[cmd_node.idx()] {
             AstNode::Intrinsic { op: _, intrinsic } => {
diff --git a/ogma/src/eng/graphs/astgraph.rs b/ogma/src/eng/graphs/astgraph.rs
index a8a6c16..fd774a2 100644
--- a/ogma/src/eng/graphs/astgraph.rs
+++ b/ogma/src/eng/graphs/astgraph.rs
@@ -285,7 +285,7 @@ impl AstGraph {
         let impls = defs.impls();
 
         if !impls.contains_op(op.str()) {
-            return Err(Error::op_not_found(&op, false));
+            return Err(Error::op_not_found(&op, None, false, impls));
         }
 
         let op_impls = impls
diff --git a/ogma/src/lang/defs.rs b/ogma/src/lang/defs.rs
index dc57275..756f5f8 100644
--- a/ogma/src/lang/defs.rs
+++ b/ogma/src/lang/defs.rs
@@ -433,7 +433,7 @@ fn assert_all_ops_defined(def: &ast::DefinitionImpl, defs: &Implementations) ->
             let op = block.op();
 
             if !defs.contains_op(op.str()) {
-                return Err(Error::op_not_found(&op, &*op == name));
+                return Err(Error::op_not_found(&op, None, &*op == name, defs));
             }
             check.extend(block.terms().iter().filter_map(|term| match term {
                 ast::Term::Arg(ast::Argument::Expr(expr)) => Some(expr.clone()),
diff --git a/ogma/src/lang/impls/intrinsics/pipeline.rs b/ogma/src/lang/impls/intrinsics/pipeline.rs
index 3d73ac8..d6cfacf 100644
--- a/ogma/src/lang/impls/intrinsics/pipeline.rs
+++ b/ogma/src/lang/impls/intrinsics/pipeline.rs
@@ -706,7 +706,8 @@ fn to_str_intrinsic(mut blk: Block) -> Result<Step> {
                         cat: err::Category::Parsing,
                         desc: format!("invalid format string: {}", e),
                         traces: vec![err::Trace::from_tag(&f.tag, Some("invalid format string".into()))],
-                        help_msg: Some("Number formatting syntax can be found at\n<https://daedalus.report/d/docs/ogma.book/05%20syntax%20and%20semantics/5.4%20number%20formatting.md?pwd-raw=docs>".into()),
+                        help_msg: Some("Number formatting syntax can be found at
+          <https://daedalus.report/d/docs/ogma.book/05%20syntax%20and%20semantics/5.4%20number%20formatting.md?pwd-raw=docs>".into()),
                         hard: true,
                     }
                 })?)
diff --git a/ogma/src/lang/impls/mod.rs b/ogma/src/lang/impls/mod.rs
index 351cd00..30908fe 100644
--- a/ogma/src/lang/impls/mod.rs
+++ b/ogma/src/lang/impls/mod.rs
@@ -80,7 +80,7 @@ impl Implementations {
 
     pub fn get_help(&self, op: &Tag) -> Result<&HelpMessage> {
         if !self.contains_op(op.str()) {
-            return Err(Error::op_not_found(op, false));
+            return Err(Error::op_not_found(op, None, false, self));
         }
 
         Ok(&self.names.get(op.str()).expect("checked was in").1)
@@ -92,7 +92,7 @@ impl Implementations {
 
     pub fn get_impl(&self, op: &Tag, ty: &Type) -> Result<&Implementation> {
         if !self.contains_op(op.str()) {
-            return Err(Error::op_not_found(op, false));
+            return Err(Error::op_not_found(op, Some(ty), false, self));
         }
 
         let mut key = (Str::new(op), Some(ty.clone()));


Test patch:
diff --git a/ogma/tests/commands/definitions.rs b/ogma/tests/commands/definitions.rs
index 58764ac..6bf023a 100644
--- a/ogma/tests/commands/definitions.rs
+++ b/ogma/tests/commands/definitions.rs
@@ -395,3 +395,22 @@ fn passing_invalid_arg_to_def() {
 "#
     );
 }
+
+#[test]
+fn not_defined_err_msg() {
+    let defs = &mut Definitions::new();
+    process_definition("def foo Table () { \\ #t }", Location::Shell, None, defs).unwrap();
+    process_definition("def foo Num () { \\ #t }", Location::Shell, None, defs).unwrap();
+
+    let x = process_w_nil("foo", defs).unwrap_err().to_string();
+    println!("{}", x);
+    assert_eq!(
+        &x,
+        "Unknown Command: operation `foo` not defined
+--> shell:0
+ | foo
+ | ^^^ `foo` not defined for input `Nil`
+--> help: `foo` is implemented for the following input types: Table Number
+"
+    );
+}
diff --git a/ogma/tests/commands/mod.rs b/ogma/tests/commands/mod.rs
index 0c4058b..8327572 100644
--- a/ogma/tests/commands/mod.rs
+++ b/ogma/tests/commands/mod.rs
@@ -394,7 +394,7 @@ fn unused_arg_test() {
         r#"Unknown Command: operation `testing` not defined
 --> shell:6
  | \ 5 { testing 2 { foo  bar } }
- |       ^^^^^^^ `testing` not found
+ |       ^^^^^^^ `testing` not defined for input `<any>`
 --> help: view a list of definitions using `def --list`
 "#
     );
@@ -454,7 +454,7 @@ fn no_cmd_defined() {
         "Unknown Command: operation `undefined` not defined
 --> shell:0
  | undefined
- | ^^^^^^^^^ `undefined` not found
+ | ^^^^^^^^^ `undefined` not defined for input `<any>`
 --> help: view a list of definitions using `def --list`
 "
     );
@@ -468,7 +468,7 @@ fn no_cmd_defined() {
         r#"Unknown Command: operation `undefined` not defined
 --> shell:13
  | \ file.csv | undefined
- |              ^^^^^^^^^ `undefined` not found
+ |              ^^^^^^^^^ `undefined` not defined for input `<any>`
 --> help: view a list of definitions using `def --list`
 "#
     );
@@ -511,7 +511,7 @@ fn forbid_recursion() {
         "Unknown Command: operation `test-recursion` not defined
 --> shell:24
  | def test-recursion () { test-recursion }
- |                         ^^^^^^^^^^^^^^ `test-recursion` not found
+ |                         ^^^^^^^^^^^^^^ `test-recursion` not defined for input `<any>`
 --> help: recursion is not supported.
           for alternatives, please see <https://daedalus.report/d/docs/ogma.book/11%20(no)%20recursion.md?pwd-raw=docs>
 "
diff --git a/ogma/tests/commands/pipeline.rs b/ogma/tests/commands/pipeline.rs
index 22ec616..ea02951 100644
--- a/ogma/tests/commands/pipeline.rs
+++ b/ogma/tests/commands/pipeline.rs
@@ -933,6 +933,6 @@ fn to_str_testing() {
  | \\ 1.234e6 | to-str '[.0n da'
  |                     ^^^^^^^ invalid format string
 --> help: Number formatting syntax can be found at
-<https://daedalus.report/d/docs/ogma.book/05%20syntax%20and%20semantics/5.4%20number%20formatting.md?pwd-raw=docs>
+          <https://daedalus.report/d/docs/ogma.book/05%20syntax%20and%20semantics/5.4%20number%20formatting.md?pwd-raw=docs>
 ");
 }