From cd62792c0aefffe8b0a5c7fc76e95dfa7b86a181 Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Mon, 11 Apr 2022 16:11:16 +0200
Subject: [PATCH] Prevent duplicate named arguments and dictionary keys

---
 src/parse/mod.rs        | 29 ++++++++++++++++++++++++++++-
 src/parse/parser.rs     |  4 ++--
 tests/typ/code/call.typ |  4 ++++
 tests/typ/code/dict.typ |  4 ++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/parse/mod.rs b/src/parse/mod.rs
index 3bf274f2a..6d1985e03 100644
--- a/src/parse/mod.rs
+++ b/src/parse/mod.rs
@@ -12,6 +12,7 @@ pub use resolve::*;
 pub use scanner::*;
 pub use tokens::*;
 
+use std::collections::HashSet;
 use std::sync::Arc;
 
 use crate::syntax::ast::{Associativity, BinOp, UnOp};
@@ -648,9 +649,20 @@ fn array(p: &mut Parser, marker: Marker) {
 /// Convert a collection into a dictionary, producing errors for anything other
 /// than named pairs.
 fn dict(p: &mut Parser, marker: Marker) {
+    let mut used = HashSet::new();
     marker.filter_children(p, |x| match x.kind() {
         kind if kind.is_paren() => Ok(()),
-        NodeKind::Named | NodeKind::Comma | NodeKind::Colon | NodeKind::Spread => Ok(()),
+        NodeKind::Named => {
+            if let Some(NodeKind::Ident(ident)) =
+                x.children().first().map(|child| child.kind())
+            {
+                if !used.insert(ident.clone()) {
+                    return Err("pair has duplicate key");
+                }
+            }
+            Ok(())
+        }
+        NodeKind::Comma | NodeKind::Colon | NodeKind::Spread => Ok(()),
         _ => Err("expected named pair, found expression"),
     });
     marker.end(p, NodeKind::DictExpr);
@@ -729,9 +741,24 @@ fn args(p: &mut Parser, direct: bool, brackets: bool) -> ParseResult {
 
     p.perform(NodeKind::CallArgs, |p| {
         if p.at(&NodeKind::LeftParen) {
+            let marker = p.marker();
             p.start_group(Group::Paren);
             collection(p);
             p.end_group();
+
+            let mut used = HashSet::new();
+            marker.filter_children(p, |x| {
+                if x.kind() == &NodeKind::Named {
+                    if let Some(NodeKind::Ident(ident)) =
+                        x.children().first().map(|child| child.kind())
+                    {
+                        if !used.insert(ident.clone()) {
+                            return Err("duplicate argument");
+                        }
+                    }
+                }
+                Ok(())
+            });
         }
 
         while brackets && p.peek_direct() == Some(&NodeKind::LeftBracket) {
diff --git a/src/parse/parser.rs b/src/parse/parser.rs
index 63ba49187..98adfba26 100644
--- a/src/parse/parser.rs
+++ b/src/parse/parser.rs
@@ -452,9 +452,9 @@ impl Marker {
     }
 
     /// Wrap all children that do not fulfill the predicate in error nodes.
-    pub fn filter_children<F>(self, p: &mut Parser, f: F)
+    pub fn filter_children<F>(self, p: &mut Parser, mut f: F)
     where
-        F: Fn(&Green) -> Result<(), &'static str>,
+        F: FnMut(&Green) -> Result<(), &'static str>,
     {
         for child in &mut p.children[self.0 ..] {
             // Don't expose errors.
diff --git a/tests/typ/code/call.typ b/tests/typ/code/call.typ
index 51ef1df59..55471774d 100644
--- a/tests/typ/code/call.typ
+++ b/tests/typ/code/call.typ
@@ -44,6 +44,10 @@
   test(adder(2)(5), 7)
 }
 
+---
+// Error: 28-47 duplicate argument
+#set text(family: "Arial", family: "Helvetica")
+
 ---
 // Error: 2-6 expected callable or collection, found boolean
 {true()}
diff --git a/tests/typ/code/dict.typ b/tests/typ/code/dict.typ
index b369b8b65..aa840eb35 100644
--- a/tests/typ/code/dict.typ
+++ b/tests/typ/code/dict.typ
@@ -35,6 +35,10 @@
   dict("b") += 1
 }
 
+---
+// Error: 24-32 pair has duplicate key
+{(first: 1, second: 2, first: 3)}
+
 ---
 // Simple expression after already being identified as a dictionary.
 // Error: 9-10 expected named pair, found expression