Improve error message

golemcloud · Oct 2, 2024 · 4fe3c6c · 4fe3c6c
1 parent 126e900
commit 4fe3c6c
Show file tree

Hide file tree

Showing 4 changed files with 364 additions and 246 deletions.
diff --git a/golem-rib/src/compiler/byte_code.rs b/golem-rib/src/compiler/byte_code.rs
@@ -984,6 +984,47 @@ mod compiler_tests {
         assert_eq!(instructions, expected_instructions);
     }
 
+    #[cfg(test)]
+    mod invalid_function_invoke_tests {
+        use crate::{compiler, Expr};
+        use crate::compiler::byte_code::compiler_tests::internal;
+
+        #[test]
+        fn test_invalid_function_call() {
+            let expr = r#"
+               foo(request);
+               "success"
+            "#;
+
+            let expr = Expr::from_text(expr).unwrap();
+            let compiler_error = compiler::compile(&expr, &vec![]).unwrap_err();
+
+            assert_eq!(
+                compiler_error,
+                "Unknown function call foo"
+            );
+        }
+
+        #[test]
+        fn test_invalid_resource_method_call() {
+            let metadata = internal::metadata_with_resource_methods();
+            let expr = r#"
+               let user_id = "user";
+               golem:it/api.{cart(user_id).add-item}("apple");
+               golem:it/api.{cart(user_id).foo}("apple");
+                "success"
+            "#;
+
+            let expr = Expr::from_text(expr).unwrap();
+            let compiler_error = compiler::compile(&expr, &metadata).unwrap_err();
+            assert_eq!(
+                compiler_error,
+                "Invalid resource method call golem:it/api.{cart(user_id).foo}. `foo` doesn't exist in resource `cart`"
+            );
+
+        }
+    }
+
     #[cfg(test)]
     mod global_input_tests {
         use crate::compiler::byte_code::compiler_tests::internal;
@@ -993,6 +1034,64 @@ mod compiler_tests {
             TypeRecord, TypeResult, TypeStr, TypeTuple, TypeU32, TypeU64, TypeVariant,
         };
 
+        #[tokio::test]
+        async fn test_str_global_input() {
+            let request_value_type = AnalysedType::Str(TypeStr);
+
+            let output_analysed_type = AnalysedType::Str(TypeStr);
+
+            let analysed_exports = internal::get_component_metadata(
+                "my-worker-function",
+                vec![request_value_type.clone()],
+                output_analysed_type,
+            );
+
+            let expr = r#"
+               let x = request;
+               my-worker-function(x);
+               match x {
+                "foo"  => "success",
+                 _ => "fallback"
+               }
+            "#;
+
+            let expr = Expr::from_text(expr).unwrap();
+            let compiled = compiler::compile(&expr, &analysed_exports).unwrap();
+            let expected_type_info =
+                internal::rib_input_type_info(vec![("request", request_value_type)]);
+
+            assert_eq!(compiled.global_input_type_info, expected_type_info);
+        }
+
+        #[tokio::test]
+        async fn test_number_global_input() {
+            let request_value_type = AnalysedType::U32(TypeU32);
+
+            let output_analysed_type = AnalysedType::Str(TypeStr);
+
+            let analysed_exports = internal::get_component_metadata(
+                "my-worker-function",
+                vec![request_value_type.clone()],
+                output_analysed_type,
+            );
+
+            let expr = r#"
+               let x = request;
+               my-worker-function(x);
+               match x {
+                1  => "success",
+                0 => "failure"
+               }
+            "#;
+
+            let expr = Expr::from_text(expr).unwrap();
+            let compiled = compiler::compile(&expr, &analysed_exports).unwrap();
+            let expected_type_info =
+                internal::rib_input_type_info(vec![("request", request_value_type)]);
+
+            assert_eq!(compiled.global_input_type_info, expected_type_info);
+        }
+
         #[tokio::test]
         async fn test_variant_type_info() {
             let request_value_type = AnalysedType::Variant(TypeVariant {
@@ -1280,72 +1379,58 @@ mod compiler_tests {
         }
     }
 
-    #[tokio::test]
-    async fn test_str_global_input() {
-        let request_value_type = AnalysedType::Str(TypeStr);
-
-        let output_analysed_type = AnalysedType::Str(TypeStr);
-
-        let analysed_exports = internal::get_component_metadata(
-            "my-worker-function",
-            vec![request_value_type.clone()],
-            output_analysed_type,
-        );
-
-        let expr = r#"
-               let x = request;
-               my-worker-function(x);
-               match x {
-                "foo"  => "success",
-                 _ => "fallback"
-               }
-            "#;
-
-        let expr = Expr::from_text(expr).unwrap();
-        let compiled = compiler::compile(&expr, &analysed_exports).unwrap();
-        let expected_type_info =
-            internal::rib_input_type_info(vec![("request", request_value_type)]);
-
-        assert_eq!(compiled.global_input_type_info, expected_type_info);
-    }
-
-    #[tokio::test]
-    async fn test_number_global_input() {
-        let request_value_type = AnalysedType::U32(TypeU32);
-
-        let output_analysed_type = AnalysedType::Str(TypeStr);
-
-        let analysed_exports = internal::get_component_metadata(
-            "my-worker-function",
-            vec![request_value_type.clone()],
-            output_analysed_type,
-        );
-
-        let expr = r#"
-               let x = request;
-               my-worker-function(x);
-               match x {
-                1  => "success",
-                0 => "failure"
-               }
-            "#;
-
-        let expr = Expr::from_text(expr).unwrap();
-        let compiled = compiler::compile(&expr, &analysed_exports).unwrap();
-        let expected_type_info =
-            internal::rib_input_type_info(vec![("request", request_value_type)]);
-
-        assert_eq!(compiled.global_input_type_info, expected_type_info);
-    }
-
     mod internal {
         use crate::RibInputTypeInfo;
-        use golem_wasm_ast::analysis::{
-            AnalysedExport, AnalysedFunction, AnalysedFunctionParameter, AnalysedFunctionResult,
-            AnalysedType,
-        };
+        use golem_wasm_ast::analysis::{AnalysedExport, AnalysedFunction, AnalysedFunctionParameter, AnalysedFunctionResult, AnalysedInstance, AnalysedResourceId, AnalysedResourceMode, AnalysedType, NameOptionTypePair, NameTypePair, TypeF32, TypeHandle, TypeList, TypeRecord, TypeStr, TypeU32, TypeVariant};
         use std::collections::HashMap;
 
+        pub(crate) fn metadata_with_resource_methods() -> Vec<AnalysedExport> {
+            let instance = AnalysedExport::Instance(AnalysedInstance {
+                name: "golem:it/api".to_string(),
+                functions: vec![
+                    AnalysedFunction {
+                        name: "[constructor]cart".to_string(),
+                        parameters: vec![AnalysedFunctionParameter {
+                            name: "param1".to_string(),
+                            typ: AnalysedType::Str(TypeStr),
+                        }],
+                        results: vec![AnalysedFunctionResult {
+                            name: None,
+                            typ: AnalysedType::Handle(TypeHandle {
+                                resource_id: AnalysedResourceId(0),
+                                mode: AnalysedResourceMode::Owned,
+                            }),
+                        }],
+                    },
+                    AnalysedFunction {
+                        name: "[method]cart.add-item".to_string(),
+                        parameters: vec![
+                            AnalysedFunctionParameter {
+                                name: "self".to_string(),
+                                typ: AnalysedType::Handle(TypeHandle {
+                                    resource_id: AnalysedResourceId(0),
+                                    mode: AnalysedResourceMode::Borrowed,
+                                }),
+                            },
+                            AnalysedFunctionParameter {
+                                name: "item".to_string(),
+                                typ: AnalysedType::Record(TypeRecord {
+                                    fields: vec![
+                                        NameTypePair {
+                                            name: "name".to_string(),
+                                            typ: AnalysedType::Str(TypeStr),
+                                        },
+                                    ],
+                                }),
+                            },
+                        ],
+                        results: vec![],
+                    }
+                ],
+            });
+
+            vec![instance]
+        }
         pub(crate) fn get_component_metadata(
             function_name: &str,
             input_types: Vec<AnalysedType>,

diff --git a/golem-rib/src/expr.rs b/golem-rib/src/expr.rs
@@ -132,6 +132,10 @@ impl Expr {
         matches!(self, Expr::Cond(_, _, _, _))
     }
 
+    pub fn is_function_call(&self) -> bool {
+        matches!(self, Expr::Call(_, _, _))
+    }
+
     pub fn is_match_expr(&self) -> bool {
         matches!(self, Expr::PatternMatch(_, _, _))
     }
@@ -379,6 +383,46 @@ impl Expr {
         Expr::Sequence(expressions, inferred_type)
     }
 
+    // Useful in various scenarios such as error messages, where we need to specify the
+    // kind of expression even before identifying the types fully
+    pub fn expr_kind(&self) -> &'static str {
+        match self  {
+            Expr::Let(_, _, _, _) => "let binding",
+            Expr::SelectField(_, _, _) => "select field",
+            Expr::SelectIndex(_, _, _) => "select index",
+            Expr::Sequence(_, _) => "list",
+            Expr::Record(_, _) => "record",
+            Expr::Tuple(_, _) => "tuple",
+            Expr::Literal(_, _) => "literal",
+            Expr::Number(_, _, _) => "number",
+            Expr::Flags(_, _) => "flags",
+            Expr::Identifier(_, _) => "identifer",
+            Expr::Boolean(_, _) => "boolean",
+            Expr::Concat(_, _) => "string",
+            Expr::Multiple(_, _) => "multline code block",
+            Expr::Not(_, _) => "not",
+            Expr::GreaterThan(_, _, _) => "boolean",
+            Expr::And(_, _, _) => "boolean",
+            Expr::GreaterThanOrEqualTo(_, _, _) => "boolean",
+            Expr::LessThanOrEqualTo(_, _, _) => "boolean",
+            Expr::EqualTo(_, _, _) => "boolean",
+            Expr::LessThan(_, _, _) => "boolean",
+            Expr::Cond(_, _, _, _) => "if else",
+            Expr::PatternMatch(_, _, _) => "pattern match",
+            Expr::Option(_, _) => "option",
+            Expr::Result(expr, _) =>  {
+               match expr {
+                    Ok(_) => "result::ok",
+                    Err(_) => "result::err",
+                }
+            },
+            Expr::Call(_, _, _) => "function call",
+            Expr::Unwrap(_, _) => "unwrap",
+            Expr::Throw(_, _) => "throw",
+            Expr::GetTag(_, _) => "get tag"
+        }
+    }
+
     pub fn inferred_type(&self) -> InferredType {
         match self {
             Expr::Let(_, _, _, inferred_type)

diff --git a/golem-rib/src/function_name.rs b/golem-rib/src/function_name.rs
@@ -495,6 +495,36 @@ impl ParsedFunctionReference {
         }
     }
 
+    pub fn resource_method_name(&self) -> Option<String> {
+        match self {
+            Self::Function {  .. } => None,
+            Self::RawResourceConstructor { resource, .. } =>  None,
+            Self::RawResourceDrop { resource, .. } => None,
+            Self::RawResourceMethod {
+                method, ..
+            } => Some(method.clone()),
+            Self::RawResourceStaticMethod {
+               method, ..
+            } => Some(method.clone()),
+            ParsedFunctionReference::IndexedResourceConstructor {  .. } => {
+                None
+            }
+            ParsedFunctionReference::IndexedResourceMethod {
+                method, ..
+            } => {
+                Some(method.clone())
+            }
+            ParsedFunctionReference::IndexedResourceStaticMethod {
+               method, ..
+            } => {
+                Some(method.clone())
+            }
+            ParsedFunctionReference::IndexedResourceDrop { .. } => {
+                None
+            }
+        }
+    }
+
     pub fn method_as_static(&self) -> Option<ParsedFunctionReference> {
         match self {
             Self::RawResourceMethod { resource, method } => Some(Self::RawResourceStaticMethod {