chore(remap): improve type error message (#6483)

Author: JeanMertz

lib/vrl/compiler/src/expression/function_argument.rs

@@ -1,6 +1,7 @@
 use crate::expression::Expr;
 use crate::parser::{Ident, Node};
 use crate::{Parameter, Span};
+use std::fmt;
 use std::ops::Deref;
 
 #[derive(Debug, PartialEq)]
@@ -36,6 +37,12 @@ impl FunctionArgument {
     }
 }
 
+impl fmt::Display for FunctionArgument {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.expr.fmt(f)
+    }
+}
+
 impl Deref for FunctionArgument {
     type Target = Node<Expr>;
 

lib/vrl/compiler/src/expression/function_call.rs

@@ -1,5 +1,5 @@
 use crate::expression::{FunctionArgument, Noop};
-use crate::function::ArgumentList;
+use crate::function::{ArgumentList, Parameter};
 use crate::parser::{Ident, Node};
 use crate::{value::Kind, Context, Expression, Function, Resolved, Span, State, TypeDef};
 use diagnostic::{DiagnosticError, Label, Note};
@@ -120,10 +120,12 @@ impl FunctionCall {
             let expr_kind = argument.type_def(state).kind();
             if !parameter.kind().contains(expr_kind) {
                 return Err(Error::InvalidArgumentKind {
-                    keyword: parameter.keyword,
+                    function_ident: function.identifier(),
+                    abort_on_error,
+                    arguments_fmt,
+                    parameter: *parameter,
                     got: expr_kind,
-                    expected: parameter.kind(),
-                    expr_span: argument.span(),
+                    argument,
                     argument_span,
                 });
             }
@@ -266,6 +268,7 @@ impl PartialEq for FunctionCall {
 // -----------------------------------------------------------------------------
 
 #[derive(thiserror::Error, Debug)]
+#[allow(clippy::large_enum_variant)]
 pub enum Error {
     #[error("call to undefined function")]
     Undefined {
@@ -302,10 +305,12 @@ pub enum Error {
 
     #[error("invalid argument type")]
     InvalidArgumentKind {
-        keyword: &'static str,
+        function_ident: &'static str,
+        abort_on_error: bool,
+        arguments_fmt: Vec<String>,
+        parameter: Parameter,
         got: Kind,
-        expected: Kind,
-        expr_span: Span,
+        argument: FunctionArgument,
         argument_span: Span,
     },
 
@@ -427,12 +432,16 @@ impl DiagnosticError for Error {
             }
 
             InvalidArgumentKind {
-                expr_span,
-                argument_span,
-                keyword,
+                parameter,
                 got,
-                expected,
+                argument,
+                argument_span,
+                ..
             } => {
+                let keyword = parameter.keyword;
+                let expected = parameter.kind();
+                let expr_span = argument.span();
+
                 // TODO: extract this out into a helper
                 let kind_str = |kind: &Kind| {
                     if kind.is_any() {
@@ -453,7 +462,7 @@ impl DiagnosticError for Error {
                         format!(
                             r#"but the parameter "{}" expects {}"#,
                             keyword,
-                            kind_str(expected)
+                            kind_str(&expected)
                         ),
                         argument_span,
                     ),
@@ -475,6 +484,70 @@ impl DiagnosticError for Error {
 
         match self {
             AbortInfallible { .. } | FallibleArgument { .. } => vec![Note::SeeErrorDocs],
+            InvalidArgumentKind {
+                function_ident,
+                abort_on_error,
+                arguments_fmt,
+                parameter,
+                argument,
+                ..
+            } => {
+                // TODO: move this into a generic helper function
+                let guard = match parameter.kind() {
+                    Kind::Bytes => format!("string!({})", argument),
+                    Kind::Integer => format!("int!({})", argument),
+                    Kind::Float => format!("float!({})", argument),
+                    Kind::Boolean => format!("bool!({})", argument),
+                    Kind::Object => format!("object!({})", argument),
+                    Kind::Array => format!("array!({})", argument),
+                    Kind::Timestamp => format!("timestamp!({})", argument),
+                    _ => return vec![],
+                };
+
+                let coerce = match parameter.kind() {
+                    Kind::Bytes => Some(format!(r#"to_string({}) ?? "default""#, argument)),
+                    Kind::Integer => Some(format!("to_int({}) ?? 0", argument)),
+                    Kind::Float => Some(format!("to_float({}) ?? 0", argument)),
+                    Kind::Boolean => Some(format!("to_bool({}) ?? false", argument)),
+                    Kind::Timestamp => Some(format!("to_timestamp({}) ?? now()", argument)),
+                    _ => None,
+                };
+
+                let args = {
+                    let mut args = String::new();
+                    let mut iter = arguments_fmt.iter().peekable();
+                    while let Some(arg) = iter.next() {
+                        args.push_str(arg);
+                        if iter.peek().is_some() {
+                            args.push_str(", ");
+                        }
+                    }
+
+                    args
+                };
+
+                let abort = if *abort_on_error { "!" } else { "" };
+
+                let mut notes = vec![];
+
+                let call = format!("{}{}({}))", function_ident, abort, args);
+
+                notes.append(&mut Note::solution(
+                    "guard against invalid type at runtime",
+                    vec![format!("{} = {}", argument, guard), call.clone()],
+                ));
+
+                if let Some(coerce) = coerce {
+                    notes.append(&mut Note::solution(
+                        "coerce with default value",
+                        vec![format!("{} = {}", argument, coerce), call],
+                    ))
+                }
+
+                notes.push(Note::SeeErrorDocs);
+
+                notes
+            }
             _ => vec![],
         }
     }

lib/vrl/diagnostic/src/diagnostic.rs

@@ -118,13 +118,14 @@ impl From<Box<dyn DiagnosticError>> for Diagnostic {
 impl From<Diagnostic> for diagnostic::Diagnostic<()> {
     fn from(diag: Diagnostic) -> Self {
         let mut notes = diag.notes.to_vec();
-        notes.push(Note::SeeLangDocs);
 
         // not all codes have a page on the site yet
         if diag.code >= 100 && diag.code <= 110 {
             notes.push(Note::SeeCodeDocs(diag.code));
         }
 
+        notes.push(Note::SeeLangDocs);
+
         diagnostic::Diagnostic {
             severity: diag.severity.into(),
             code: Some(format!("E{:03}", diag.code)),

lib/vrl/diagnostic/src/note.rs

@@ -11,6 +11,21 @@ pub enum Note {
 
     #[doc(hidden)]
     SeeDocs(String, String),
+    #[doc(hidden)]
+    Basic(String),
+}
+
+impl Note {
+    pub fn solution(title: impl Into<String>, content: Vec<impl Into<String>>) -> Vec<Self> {
+        let mut notes = vec![Self::Basic(format!("try: {}", title.into()))];
+
+        notes.push(Self::Basic(" ".to_owned()));
+        for line in content {
+            notes.push(Self::Basic(format!("    {}", line.into())));
+        }
+        notes.push(Self::Basic(" ".to_owned()));
+        notes
+    }
 }
 
 impl fmt::Display for Note {
@@ -35,6 +50,7 @@ impl fmt::Display for Note {
                     kind, path
                 )
             }
+            Basic(string) => write!(f, "{}", string),
         }
     }
 }

lib/vrl/tests/tests/diagnostics/call_to_undefined_function.vrl

@@ -9,7 +9,7 @@
 #   │ undefined function
 #   │ did you mean "parse_duration"?
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/105
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 unknown_function(.foo, .bar)

lib/vrl/tests/tests/diagnostics/function_argument_arity_mismatch.vrl

@@ -9,7 +9,7 @@
 #   │           too many function arguments
 #   │           this function takes a maximum of 1 argument
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/106
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 to_string("foo", "bar")

lib/vrl/tests/tests/diagnostics/function_argument_missing.vrl

@@ -6,7 +6,7 @@
 # 2 │ to_string()
 #   │ ^^^^^^^^^^^ required argument missing: "value" (position 0)
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/107
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 to_string()

lib/vrl/tests/tests/diagnostics/invalid_if_condition_type.vrl

@@ -10,8 +10,8 @@
 #   │    instead it resolves to "string"
 #   │
 #   = hint: coerce the value using one of the coercion functions
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/102
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 if "nope" {
     true

lib/vrl/tests/tests/diagnostics/invalid_regex.vrl

@@ -6,7 +6,7 @@
 # 2 │ r'['
 #   │ ^^^^ regex parse error: unclosed character class: [
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/101
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 r'['

lib/vrl/tests/tests/diagnostics/regex_parsing_unsuccessful.vrl

@@ -6,7 +6,7 @@
 # 2 │ r'['
 #   │ ^^^^ regex parse error: unclosed character class: [
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/101
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 r'['

lib/vrl/tests/tests/diagnostics/unhandled_expression_error.vrl

@@ -10,7 +10,7 @@
 #   │ handle the error case to ensure runtime success
 #   │
 #   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/100
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 1 / 0

lib/vrl/tests/tests/diagnostics/unhandled_function_error.vrl

@@ -10,7 +10,7 @@
 #   │ handle the error case to ensure runtime success
 #   │
 #   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/100
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 parse_json("true")

lib/vrl/tests/tests/diagnostics/unhandled_parse_regex_all_type.vrl

@@ -10,8 +10,19 @@
 #   │           this expression resolves to one of "string" or "null"
 #   │           but the parameter "value" expects the exact type "string"
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
+#   = try: guard against invalid type at runtime
+#   =
+#   =     .result[0].an = string!(.result[0].an)
+#   =     sha3(.result[0].an))
+#   =
+#   = try: coerce with default value
+#   =
+#   =     .result[0].an = to_string(.result[0].an) ?? "default"
+#   =     sha3(.result[0].an))
+#   =
+#   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/110
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 .result = parse_regex_all!(.message, r'(?P<an>an.)')
 .a = sha3(.result[0].an)

lib/vrl/tests/tests/diagnostics/unknown_function_argument_keyword.vrl

@@ -8,7 +8,7 @@
 #   │ │
 #   │ this function accepts the following keywords: "value"
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/108
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 to_string(foo: "foo")

lib/vrl/tests/tests/diagnostics/unneeded_error_assignment.vrl

@@ -10,7 +10,7 @@
 #   │ use: ok = 5
 #   │
 #   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/104
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 ok, err = 5;

lib/vrl/tests/tests/diagnostics/unsuccessful_parse_json_type.vrl

@@ -10,8 +10,19 @@
 #   │      this expression resolves to "unknown type"
 #   │      but the parameter "value" expects the exact type "string"
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
+#   = try: guard against invalid type at runtime
+#   =
+#   =     result.message = string!(result.message)
+#   =     sha3(result.message))
+#   =
+#   = try: coerce with default value
+#   =
+#   =     result.message = to_string(result.message) ?? "default"
+#   =     sha3(result.message))
+#   =
+#   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/110
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 .message = to_string!(.message)
 result = parse_json!(.message)

lib/vrl/tests/tests/internal/infallible_ok_maybe_null.vrl

@@ -10,8 +10,19 @@
 #   │               this expression resolves to one of "string" or "null"
 #   │               but the parameter "value" expects the exact type "string"
 #   │
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
+#   = try: guard against invalid type at runtime
+#   =
+#   =     .foo = string!(.foo)
+#   =     upcase(.foo))
+#   =
+#   = try: coerce with default value
+#   =
+#   =     .foo = to_string(.foo) ?? "default"
+#   =     upcase(.foo))
+#   =
+#   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/110
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 .foo # any
 .foo, err = to_string(.foo) # string or null

lib/vrl/tests/tests/issues/6469_fallible_operations_marked_as_infallible.vrl

@@ -10,7 +10,7 @@
 #   │ handle the error case to ensure runtime success
 #   │
 #   = see error handling documentation at: https://vector.dev/docs/reference/vrl/
-#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 #   = learn more at: https://errors.vrl.dev/100
+#   = see language documentation at: https://vector.dev/docs/reference/vrl/
 
 5 + to_int(.foo)