Improve macro call arguments mismatch errors

rinja_derive/src/generator/node.rs

@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 use std::collections::hash_map::{Entry, HashMap};
-use std::fmt::Write;
+use std::fmt::{self, Write};
 use std::mem;
 
 use parser::node::{
@@ -1421,36 +1421,91 @@ fn macro_call_ensure_arg_count(
     def: &Macro<'_>,
     ctx: &Context<'_>,
 ) -> Result<(), CompileError> {
-    if call.args.len() == def.args.len() {
-        // exactly enough arguments were provided
-        return Ok(());
+    if call.args.len() > def.args.len() {
+        return Err(ctx.generate_error(
+            format_args!(
+                "macro `{}` expected {} argument{}, found {}",
+                def.name,
+                def.args.len(),
+                if def.args.len() > 1 { "s" } else { "" },
+                call.args.len(),
+            ),
+            call.span(),
+        ));
+    }
+
+    // First we list of arguments position, then we remove every argument with a value.
+    let mut args: Vec<_> = def.args.iter().map(|&(name, _)| Some(name)).collect();
+    for (pos, arg) in call.args.iter().enumerate() {
+        let pos = match **arg {
+            Expr::NamedArgument(name, ..) => {
+                def.args.iter().position(|(arg_name, _)| *arg_name == name)
+            }
+            _ => Some(pos),
+        };
+        if let Some(pos) = pos {
+            if mem::take(&mut args[pos]).is_none() {
+                // This argument was already passed, so error.
+                return Err(ctx.generate_error(
+                    format_args!(
+                        "argument `{}` was passed more than once when calling macro `{}`",
+                        def.args[pos].0, def.name,
+                    ),
+                    call.span(),
+                ));
+            }
+        }
+    }
+
+    // Now we can check off arguments with a default value, too.
+    for (pos, (_, dflt)) in def.args.iter().enumerate() {
+        if dflt.is_some() {
+            args[pos] = None;
+        }
     }
 
-    let nb_default_args = def
-        .args
-        .iter()
-        .rev()
-        .take_while(|(_, default_value)| default_value.is_some())
-        .count();
-    if call.args.len() < def.args.len() && call.args.len() >= def.args.len() - nb_default_args {
-        // all missing arguments have a default value, and there weren't too many args
-        return Ok(());
+    // Now that we have a needed information, we can print an error message (if needed).
+    struct FmtMissing<'a, I> {
+        count: usize,
+        missing: I,
+        name: &'a str,
     }
 
-    // either too many or not enough arguments were provided
-    let (expected_args, extra) = match nb_default_args {
-        0 => (def.args.len(), ""),
-        _ => (nb_default_args, "at least "),
+    impl<'a, I: Iterator<Item = &'a str> + Clone> fmt::Display for FmtMissing<'a, I> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            if self.count == 1 {
+                let a = self.missing.clone().next().unwrap();
+                write!(
+                    f,
+                    "missing argument when calling macro `{}`: `{a}`",
+                    self.name
+                )
+            } else {
+                write!(f, "missing arguments when calling macro `{}`: ", self.name)?;
+                for (idx, a) in self.missing.clone().enumerate() {
+                    if idx == self.count - 1 {
+                        write!(f, " and ")?;
+                    } else if idx > 0 {
+                        write!(f, ", ")?;
+                    }
+                    write!(f, "`{a}`")?;
+                }
+                Ok(())
+            }
+        }
+    }
+
+    let missing = args.iter().filter_map(Option::as_deref);
+    let fmt_missing = FmtMissing {
+        count: missing.clone().count(),
+        missing,
+        name: def.name,
     };
-    Err(ctx.generate_error(
-        format_args!(
-            "macro {:?} expected {extra}{expected_args} argument{}, found {}",
-            def.name,
-            if expected_args != 1 { "s" } else { "" },
-            call.args.len(),
-        ),
-        call.span(),
-    ))
+    if fmt_missing.count == 0 {
+        Ok(())
+    } else {
+        Err(ctx.generate_error(fmt_missing, call.span()))
+    }
 }
 
 #[derive(Clone, Copy, PartialEq)]

testing/tests/ui/macro.rs

@@ -39,5 +39,27 @@ struct NoClosingParen3;
 #[template(source = "{% macro thrice(a, b, c = %}{% endmacro %}", ext = "html")]
 struct NoClosingParen4;
 
+#[derive(rinja::Template)]
+#[template(
+    source = r#"
+        {% macro example(name, value, current, label="", id="") %}
+        {% endmacro %}
+        {% call example(name="name", value="") %}
+    "#,
+    ext = "txt"
+)]
+struct WrongNumberOfParams;
+
+#[derive(rinja::Template)]
+#[template(
+    source = r#"
+        {% macro example(name, value, arg=12) %}
+        {% endmacro %}
+        {% call example(0, name="name", value="") %}
+    "#,
+    ext = "txt"
+)]
+struct DuplicatedArg;
+
 fn main() {
 }

testing/tests/ui/macro.stderr

@@ -1,4 +1,4 @@
-error: macro "thrice" expected 1 argument, found 2
+error: macro `thrice` expected 1 argument, found 2
  --> InvalidNumberOfArgs.html:5:2
        "- call thrice(2, 3) -%}"
  --> tests/ui/macro.rs:4:21
@@ -11,7 +11,7 @@ error: macro "thrice" expected 1 argument, found 2
 8 | | {%- call thrice(2, 3) -%}", ext = "html")]
   | |__________________________^
 
-error: macro "thrice" expected 2 arguments, found 0
+error: missing arguments when calling macro `thrice`: `param` and `param2`
  --> InvalidNumberOfArgs2.html:5:2
        "- call thrice() -%}"
   --> tests/ui/macro.rs:12:21
@@ -24,7 +24,7 @@ error: macro "thrice" expected 2 arguments, found 0
 16 | | {%- call thrice() -%}", ext = "html")]
    | |______________________^
 
-error: macro "thrice" expected 0 arguments, found 2
+error: macro `thrice` expected 0 argument, found 2
  --> InvalidNumberOfArgs3.html:4:2
        "- call thrice(1, 2) -%}"
   --> tests/ui/macro.rs:20:21
@@ -67,3 +67,29 @@ error: expected `)` to close macro argument list
    |
 39 | #[template(source = "{% macro thrice(a, b, c = %}{% endmacro %}", ext = "html")]
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: missing argument when calling macro `example`: `current`
+ --> WrongNumberOfParams.txt:4:10
+       " call example(name=\"name\", value=\"\") %}\n    "
+  --> tests/ui/macro.rs:44:14
+   |
+44 |       source = r#"
+   |  ______________^
+45 | |         {% macro example(name, value, current, label="", id="") %}
+46 | |         {% endmacro %}
+47 | |         {% call example(name="name", value="") %}
+48 | |     "#,
+   | |______^
+
+error: argument `name` was passed more than once when calling macro `example`
+ --> DuplicatedArg.txt:4:10
+       " call example(0, name=\"name\", value=\"\") %}\n    "
+  --> tests/ui/macro.rs:55:14
+   |
+55 |       source = r#"
+   |  ______________^
+56 | |         {% macro example(name, value, arg=12) %}
+57 | |         {% endmacro %}
+58 | |         {% call example(0, name="name", value="") %}
+59 | |     "#,
+   | |______^

testing/tests/ui/macro_default_value.stderr

@@ -1,4 +1,4 @@
-error: macro "thrice" expected at least 1 argument, found 0
+error: missing argument when calling macro `thrice`: `param1`
  --> InvalidDefault1.html:4:2
        "- call thrice() -%}"
  --> tests/ui/macro_default_value.rs:4:21
@@ -10,7 +10,7 @@ error: macro "thrice" expected at least 1 argument, found 0
 7 | | {%- call thrice() -%}", ext = "html")]
   | |______________________^
 
-error: macro "thrice" expected at least 1 argument, found 3
+error: macro `thrice` expected 2 arguments, found 3
  --> InvalidDefault2.html:4:2
        "- call thrice(1, 2, 3) -%}"
   --> tests/ui/macro_default_value.rs:11:21

testing/tests/ui/macro_named_argument.rs

@@ -41,5 +41,17 @@ struct InvalidNamedArg4;
 {%- call thrice(3, param1=2) -%}", ext = "html")]
 struct InvalidNamedArg5;
 
+#[derive(Template)]
+#[template(source = "{%- macro thrice(param1, param2) -%}
+{%- endmacro -%}
+{%- call thrice() -%}", ext = "html")]
+struct MissingArgs;
+
+#[derive(Template)]
+#[template(source = "{%- macro thrice(param1, param2=1) -%}
+{%- endmacro -%}
+{%- call thrice() -%}", ext = "html")]
+struct MissingArgs2;
+
 fn main() {
 }

testing/tests/ui/macro_named_argument.stderr

@@ -1,4 +1,4 @@
-error: no argument named `param3` in macro "thrice"
+error: missing argument when calling macro `thrice`: `param2`
  --> InvalidNamedArg.html:5:2
        "- call thrice(param1=2, param3=3) -%}"
  --> tests/ui/macro_named_argument.rs:4:21
@@ -49,7 +49,7 @@ error: named arguments must always be passed last
 33 | | {%- call thrice(param1=2, 3) -%}", ext = "html")]
    | |_________________________________^
 
-error: `param1` is passed more than once
+error: argument `param1` was passed more than once when calling macro `thrice`
  --> InvalidNamedArg5.html:4:2
        "- call thrice(3, param1=2) -%}"
   --> tests/ui/macro_named_argument.rs:38:21
@@ -60,3 +60,25 @@ error: `param1` is passed more than once
 40 | | {%- endmacro -%}
 41 | | {%- call thrice(3, param1=2) -%}", ext = "html")]
    | |_________________________________^
+
+error: missing arguments when calling macro `thrice`: `param1` and `param2`
+ --> MissingArgs.html:3:2
+       "- call thrice() -%}"
+  --> tests/ui/macro_named_argument.rs:45:21
+   |
+45 |   #[template(source = "{%- macro thrice(param1, param2) -%}
+   |  _____________________^
+46 | | {%- endmacro -%}
+47 | | {%- call thrice() -%}", ext = "html")]
+   | |______________________^
+
+error: missing argument when calling macro `thrice`: `param1`
+ --> MissingArgs2.html:3:2
+       "- call thrice() -%}"
+  --> tests/ui/macro_named_argument.rs:51:21
+   |
+51 |   #[template(source = "{%- macro thrice(param1, param2=1) -%}
+   |  _____________________^
+52 | | {%- endmacro -%}
+53 | | {%- call thrice() -%}", ext = "html")]
+   | |______________________^