Error on duplicate symbol variant with modifiers in different orders (#5587)

Co-authored-by: Laurenz <laurmaedje@gmail.com>

2 files changed, 84 insertions, 9 deletions

diff --git a/crates/typst-library/src/foundations/symbol.rs b/crates/typst-library/src/foundations/symbol.rs
index 88030528..72800f31 100644
--- a/crates/typst-library/src/foundations/symbol.rs
+++ b/crates/typst-library/src/foundations/symbol.rs
@@ -1,11 +1,12 @@
 use std::cmp::Reverse;
-use std::collections::BTreeSet;
+use std::collections::{BTreeSet, HashMap};
 use std::fmt::{self, Debug, Display, Formatter, Write};
 use std::sync::Arc;
 
 use ecow::{eco_format, EcoString};
 use serde::{Serialize, Serializer};
 use typst_syntax::{is_ident, Span, Spanned};
+use typst_utils::hash128;
 
 use crate::diag::{bail, SourceResult, StrResult};
 use crate::foundations::{cast, func, scope, ty, Array, Func, NativeFunc, Repr as _};
@@ -198,24 +199,62 @@ impl Symbol {
         #[variadic]
         variants: Vec<Spanned<SymbolVariant>>,
     ) -> SourceResult<Symbol> {
-        let mut list = Vec::new();
         if variants.is_empty() {
             bail!(span, "expected at least one variant");
         }
-        for Spanned { v, span } in variants {
-            if list.iter().any(|(prev, _)| &v.0 == prev) {
-                bail!(span, "duplicate variant");
-            }
+
+        // Maps from canonicalized 128-bit hashes to indices of variants we've
+        // seen before.
+        let mut seen = HashMap::<u128, usize>::new();
+
+        // A list of modifiers, cleared & reused in each iteration.
+        let mut modifiers = Vec::new();
+
+        // Validate the variants.
+        for (i, &Spanned { ref v, span }) in variants.iter().enumerate() {
+            modifiers.clear();
+
             if !v.0.is_empty() {
+                // Collect all modifiers.
                 for modifier in v.0.split('.') {
                     if !is_ident(modifier) {
                         bail!(span, "invalid symbol modifier: {}", modifier.repr());
                     }
+                    modifiers.push(modifier);
+                }
+            }
+
+            // Canonicalize the modifier order.
+            modifiers.sort();
+
+            // Ensure that there are no duplicate modifiers.
+            if let Some(ms) = modifiers.windows(2).find(|ms| ms[0] == ms[1]) {
+                bail!(
+                    span, "duplicate modifier within variant: {}", ms[0].repr();
+                    hint: "modifiers are not ordered, so each one may appear only once"
+                )
+            }
+
+            // Check whether we had this set of modifiers before.
+            let hash = hash128(&modifiers);
+            if let Some(&i) = seen.get(&hash) {
+                if v.0.is_empty() {
+                    bail!(span, "duplicate default variant");
+                } else if v.0 == variants[i].v.0 {
+                    bail!(span, "duplicate variant: {}", v.0.repr());
+                } else {
+                    bail!(
+                        span, "duplicate variant: {}", v.0.repr();
+                        hint: "variants with the same modifiers are identical, regardless of their order"
+                    )
                 }
             }
-            list.push((v.0, v.1));
+
+            seen.insert(hash, i);
         }
-        Ok(Symbol::runtime(list.into_boxed_slice()))
+
+        let list = variants.into_iter().map(|s| (s.v.0, s.v.1)).collect();
+        Ok(Symbol::runtime(list))
     }
 }
 
diff --git a/tests/suite/symbols/symbol.typ b/tests/suite/symbols/symbol.typ
index f2f6abf8..4c64700a 100644
--- a/tests/suite/symbols/symbol.typ
+++ b/tests/suite/symbols/symbol.typ
@@ -39,13 +39,49 @@
   ("invalid. id!", "x")
 )
 
+--- symbol-constructor-duplicate-modifier ---
+// Error: 2:3-2:31 duplicate modifier within variant: "duplicate"
+// Hint: 2:3-2:31 modifiers are not ordered, so each one may appear only once
+#symbol(
+  ("duplicate.duplicate", "x"),
+)
+
+--- symbol-constructor-duplicate-default-variant ---
+// Error: 3:3-3:6 duplicate default variant
+#symbol(
+  "x",
+  "y",
+)
+
+--- symbol-constructor-duplicate-empty-variant ---
+// Error: 3:3-3:12 duplicate default variant
+#symbol(
+  ("", "x"),
+  ("", "y"),
+)
+
+--- symbol-constructor-default-and-empty-variants ---
+// Error: 3:3-3:12 duplicate default variant
+#symbol(
+  "x",
+  ("", "y"),
+)
+
 --- symbol-constructor-duplicate-variant ---
-// Error: 3:3-3:29 duplicate variant
+// Error: 3:3-3:29 duplicate variant: "duplicate.variant"
 #symbol(
   ("duplicate.variant", "x"),
   ("duplicate.variant", "y"),
 )
 
+--- symbol-constructor-duplicate-variant-different-order ---
+// Error: 3:3-3:29 duplicate variant: "variant.duplicate"
+// Hint: 3:3-3:29 variants with the same modifiers are identical, regardless of their order
+#symbol(
+  ("duplicate.variant", "x"),
+  ("variant.duplicate", "y"),
+)
+
 --- symbol-unknown-modifier ---
 // Error: 13-20 unknown symbol modifier
 #emoji.face.garbage