diff --git a/golem-rib/src/compiler/byte_code.rs b/golem-rib/src/compiler/byte_code.rs index 94b66cb11..5282dfddc 100644 --- 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 { + 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, diff --git a/golem-rib/src/expr.rs b/golem-rib/src/expr.rs index cbdefb7ab..11d11cc06 100644 --- 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 index 52b277ec6..5a2e49022 100644 --- 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 { + 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 { match self { Self::RawResourceMethod { resource, method } => Some(Self::RawResourceStaticMethod { diff --git a/golem-rib/src/type_inference/call_arguments_inference.rs b/golem-rib/src/type_inference/call_arguments_inference.rs index d35b8fde1..473d557db 100644 --- a/golem-rib/src/type_inference/call_arguments_inference.rs +++ b/golem-rib/src/type_inference/call_arguments_inference.rs @@ -45,7 +45,7 @@ mod internal { Expr, FunctionTypeRegistry, InferredType, ParsedFunctionName, RegistryKey, RegistryValue, }; use golem_wasm_ast::analysis::AnalysedType; - use std::fmt::Display; + use std::fmt::{Display, format}; pub(crate) fn resolve_call_argument_types( call_type: &mut CallType, @@ -57,6 +57,7 @@ mod internal { CallType::Function(dynamic_parsed_function_name) => { let parsed_function_static = dynamic_parsed_function_name.clone().to_static(); let function = parsed_function_static.clone().function; + let resource_name = function.resource_name().ok_or("Resource name not found")?; if function.resource_name().is_some() { let constructor_name = { let raw_str = function.resource_name().ok_or("Resource name not found")?; @@ -84,7 +85,24 @@ mod internal { registry_key, constructor_params, inferred_type, - )?; + ).map_err(|e| match e { + ArgTypesInferenceError::InvalidFunctionCall => { + format!("Invalid resource constructor call: {}", resource_name) + } + ArgTypesInferenceError::ArgumentSizeMisMatch { + expected, + provided, + } => format!( + "Invalid resource constructor call. {} expects {} arguments, but provided {}", + resource_name, expected, provided + ), + ArgTypesInferenceError::TypeMisMatchError { expected, provided } => { + format!( + "Invalid arguments for resource constructor {}. Expected type {:?}, but provided {}", + resource_name, expected, provided + ) + } + })?; // Infer the types of resource method parameters let resource_method_name = function.function_name(); @@ -99,16 +117,50 @@ mod internal { registry_key, args, inferred_type, - ) + ).map_err(|e| match e { + ArgTypesInferenceError::InvalidFunctionCall => { + format!("Invalid resource method call {}. `{}` doesn't exist in resource `{}`", parsed_function_static, parsed_function_static.function.resource_method_name().unwrap(), resource_name) + } + ArgTypesInferenceError::ArgumentSizeMisMatch { + expected, + provided, + } => format!( + "Invalid call: {}. Expected {} arguments, but provided {}", + parsed_function_static, expected, provided + ), + ArgTypesInferenceError::TypeMisMatchError { expected, provided } => { + format!( + "Invalid arguments to resource method {}. Expected type {:?}, but provided {}", + parsed_function_static, expected, provided + ) + } + }) } else { let registry_key = RegistryKey::from_invocation_name(call_type); infer_types( - &FunctionNameInternal::Fqn(parsed_function_static), + &FunctionNameInternal::Fqn(parsed_function_static.clone()), function_type_registry, registry_key, args, inferred_type, - ) + ).map_err(|e| match e { + ArgTypesInferenceError::InvalidFunctionCall => { + format!("Invalid function call: {}", parsed_function_static.function.function_name()) + } + ArgTypesInferenceError::ArgumentSizeMisMatch { + expected, + provided, + } => format!( + "Invalid function call: {}. Expected {} arguments, but provided {}", + parsed_function_static, expected, provided + ), + ArgTypesInferenceError::TypeMisMatchError { expected, provided } => { + format!( + "Invalid argument types in function {}. Expected type {:?}, but provided {}", + parsed_function_static.function.function_name(), expected, provided + ) + } + }) } } @@ -128,7 +180,44 @@ mod internal { registry_key, args, inferred_type, - ) + ).map_err(|e| match e { + ArgTypesInferenceError::InvalidFunctionCall => { + format!("Invalid variant constructor call: {}", variant_name) + } + ArgTypesInferenceError::ArgumentSizeMisMatch { + expected, + provided, + } => format!( + "Invalid variant construction: {}. Expected {} arguments, but provided {}", + variant_name, expected, provided + ), + ArgTypesInferenceError::TypeMisMatchError { expected, provided } => { + format!( + "Invalid type for {} construction arguments. Expected type {:?}, but provided {}", + variant_name, expected, provided + ) + } + }) + } + } + } + enum ArgTypesInferenceError { + InvalidFunctionCall, + ArgumentSizeMisMatch { + expected: usize, + provided: usize, + }, + TypeMisMatchError { + expected: AnalysedType, + provided: String // If only partial information about type is available + }, + } + + impl ArgTypesInferenceError { + fn type_mismatch(expected: AnalysedType, provided: &'static str) -> ArgTypesInferenceError { + ArgTypesInferenceError::TypeMisMatchError { + expected, + provided: provided.to_string(), } } } @@ -139,7 +228,7 @@ mod internal { key: RegistryKey, args: &mut [Expr], inferred_type: &mut InferredType, - ) -> Result<(), String> { + ) -> Result<(), ArgTypesInferenceError> { if let Some(value) = function_type_registry.types.get(&key) { match value { RegistryValue::Value(_) => Ok(()), @@ -155,12 +244,10 @@ mod internal { Ok(()) } else { - Err(format!( - "Variant {} expects {} arguments, but {} were provided", - function_name, - parameter_types.len(), - args.len() - )) + Err(ArgTypesInferenceError::ArgumentSizeMisMatch { + expected: parameter_types.len(), + provided: args.len(), + }) } } RegistryValue::Function { @@ -190,20 +277,19 @@ mod internal { Ok(()) } else { - Err(format!( - "Function {} expects {} arguments, but {} were provided", - function_name, - parameter_types.len(), - args.len() - )) + Err(ArgTypesInferenceError::ArgumentSizeMisMatch { + expected: parameter_types.len(), + provided: args.len(), + }) } } } } else { - Err(format!("Unknown function/variant call {}", function_name)) + Err(ArgTypesInferenceError::InvalidFunctionCall) } } + #[derive(Clone)] enum FunctionNameInternal { ResourceConstructorName(String), ResourceMethodName(String), @@ -231,183 +317,56 @@ mod internal { } // A preliminary check of the arguments passed before typ inference - pub(crate) fn check_function_arguments( + fn check_function_arguments( expected: &AnalysedType, passed: &Expr, - ) -> Result<(), String> { + ) -> Result<(), ArgTypesInferenceError> { + // Valid expressions, whose evaluation/inference may give the right expected type + // We will leave it to the rest of the type inference process to check the validity of the expression let valid_possibilities = passed.is_identifier() || passed.is_select_field() || passed.is_select_index() || passed.is_select_field() || passed.is_match_expr() - || passed.is_if_else(); - - match expected { - AnalysedType::U32(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected U32, but found {:?}", passed)) - } - } - - AnalysedType::U64(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected U64, but found {:?}", passed)) - } - } - - AnalysedType::Variant(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected Variant, but found {:?}", passed)) - } - } - - AnalysedType::Result(_) => { - if valid_possibilities || passed.is_result() { - Ok(()) - } else { - Err(format!("Expected Result, but found {:?}", passed)) - } - } - AnalysedType::Option(_) => { - if valid_possibilities || passed.is_option() { - Ok(()) - } else { - Err(format!("Expected Option, but found {:?}", passed)) - } - } - AnalysedType::Enum(_) => { - if valid_possibilities { - Ok(()) - } else { - Err(format!("Expected Enum, but found {:?}", passed)) - } - } - AnalysedType::Flags(_) => { - if valid_possibilities || passed.is_flags() { - Ok(()) - } else { - Err(format!("Expected Flags, but found {:?}", passed)) - } - } - AnalysedType::Record(_) => { - if valid_possibilities || passed.is_record() { - Ok(()) - } else { - Err(format!("Expected Record, but found {:?}", passed)) - } - } - AnalysedType::Tuple(_) => { - if valid_possibilities || passed.is_tuple() { - Ok(()) - } else { - Err(format!("Expected Tuple, but found {:?}", passed)) - } - } - AnalysedType::List(_) => { - if valid_possibilities || passed.is_list() { - Ok(()) - } else { - Err(format!("Expected List, but found {:?}", passed)) - } - } - AnalysedType::Str(_) => { - if valid_possibilities || passed.is_concat() || passed.is_literal() { - Ok(()) - } else { - Err(format!("Expected Str, but found {:?}", passed)) - } - } - // TODO? - AnalysedType::Chr(_) => { - if valid_possibilities || passed.is_literal() { - Ok(()) - } else { - Err(format!("Expected Chr, but found {:?}", passed)) - } - } - AnalysedType::F64(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected F64, but found {:?}", passed)) - } - } - AnalysedType::F32(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected F32, but found {:?}", passed)) - } - } - AnalysedType::S64(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected S64, but found {:?}", passed)) - } - } - AnalysedType::S32(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected S32, but found {:?}", passed)) - } - } - AnalysedType::U16(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected U16, but found {:?}", passed)) - } - } - AnalysedType::S16(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected S16, but found {:?}", passed)) - } - } - AnalysedType::U8(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected U8, but found {:?}", passed)) - } - } - AnalysedType::S8(_) => { - if valid_possibilities || passed.is_number() { - Ok(()) - } else { - Err(format!("Expected S8, but found {:?}", passed)) - } - } - AnalysedType::Bool(_) => { - if valid_possibilities || passed.is_boolean() || passed.is_comparison() { - Ok(()) - } else { - Err(format!("Expected Bool, but found {:?}", passed)) - } - } - AnalysedType::Handle(_) => { - if valid_possibilities { - Ok(()) - } else { - Err(format!("Expected Handle, but found {:?}", passed)) - } - } + || passed.is_if_else() + || passed.is_function_call(); + + let is_valid = match expected { + AnalysedType::U32(_) => valid_possibilities || passed.is_number(), + AnalysedType::U64(_) => valid_possibilities || passed.is_number(), + AnalysedType::Variant(_) => valid_possibilities, + AnalysedType::Result(_) => valid_possibilities || passed.is_result(), + AnalysedType::Option(_) => valid_possibilities || passed.is_option(), + AnalysedType::Enum(_) => valid_possibilities, + AnalysedType::Flags(_) => valid_possibilities || passed.is_flags(), + AnalysedType::Record(_) => valid_possibilities || passed.is_record(), + AnalysedType::Tuple(_) => valid_possibilities || passed.is_tuple(), + AnalysedType::List(_) => valid_possibilities || passed.is_list(), + AnalysedType::Str(_) => valid_possibilities || passed.is_concat() || passed.is_literal(), + AnalysedType::Chr(_) => valid_possibilities || passed.is_literal(), + AnalysedType::F64(_) => valid_possibilities || passed.is_number(), + AnalysedType::F32(_) => valid_possibilities || passed.is_number(), + AnalysedType::S64(_) => valid_possibilities || passed.is_number(), + AnalysedType::S32(_) => valid_possibilities || passed.is_number(), + AnalysedType::U16(_) => valid_possibilities || passed.is_number(), + AnalysedType::S16(_) => valid_possibilities || passed.is_number(), + AnalysedType::U8(_) => valid_possibilities || passed.is_number(), + AnalysedType::S8(_) => valid_possibilities || passed.is_number(), + AnalysedType::Bool(_) => valid_possibilities || passed.is_boolean() || passed.is_comparison(), + AnalysedType::Handle(_) => valid_possibilities + }; + + if is_valid { + Ok(()) + } else { + Err(ArgTypesInferenceError::type_mismatch(expected.clone(), passed.expr_kind())) } } fn tag_argument_types( args: &mut [Expr], parameter_types: &[AnalysedType], - ) -> Result<(), String> { + ) -> Result<(), ArgTypesInferenceError> { for (arg, param_type) in args.iter_mut().zip(parameter_types) { check_function_arguments(param_type, arg)?; arg.add_infer_type_mut(param_type.clone().into());