From 386fa099204650bfcefeeeb67833849952340492 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 8 May 2023 20:59:38 +0200
Subject: [PATCH] refactor(core): verify there are no ops with duplicate names
 (#19047)

This commit adds a "debug build" only check that verifies on startup
that there are no duplicate ops (ie. the op names are unique).
---
 core/runtime.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/core/runtime.rs b/core/runtime.rs
index fb4716e7ca..bd79ca6cc0 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -769,7 +769,7 @@ impl JsRuntime {
     let macroware = move |d| middleware.iter().fold(d, |d, m| m(d));
 
     // Flatten ops, apply middlware & override disabled ops
-    exts
+    let ops: Vec<_> = exts
       .iter_mut()
       .filter_map(|e| e.init_ops())
       .flatten()
@@ -777,7 +777,37 @@ impl JsRuntime {
         name: d.name,
         ..macroware(d)
       })
-      .collect()
+      .collect();
+
+    // In debug build verify there are no duplicate ops.
+    #[cfg(debug_assertions)]
+    {
+      let mut count_by_name = HashMap::new();
+
+      for op in ops.iter() {
+        count_by_name
+          .entry(&op.name)
+          .or_insert(vec![])
+          .push(op.name.to_string());
+      }
+
+      let mut duplicate_ops = vec![];
+      for (op_name, _count) in
+        count_by_name.iter().filter(|(_k, v)| v.len() > 1)
+      {
+        duplicate_ops.push(op_name.to_string());
+      }
+      if !duplicate_ops.is_empty() {
+        let mut msg = "Found ops with duplicate names:\n".to_string();
+        for op_name in duplicate_ops {
+          msg.push_str(&format!("  - {}\n", op_name));
+        }
+        msg.push_str("Op names need to be unique.");
+        panic!("{}", msg);
+      }
+    }
+
+    ops
   }
 
   /// Initializes ops of provided Extensions
@@ -4787,4 +4817,29 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
       "Cannot load extension module from external code"
     );
   }
+
+  #[cfg(debug_assertions)]
+  #[test]
+  #[should_panic(expected = "Found ops with duplicate names:")]
+  fn duplicate_op_names() {
+    mod a {
+      use super::*;
+
+      #[op]
+      fn op_test() -> Result<String, Error> {
+        Ok(String::from("Test"))
+      }
+    }
+
+    #[op]
+    fn op_test() -> Result<String, Error> {
+      Ok(String::from("Test"))
+    }
+
+    deno_core::extension!(test_ext, ops = [a::op_test, op_test]);
+    JsRuntime::new(RuntimeOptions {
+      extensions: vec![test_ext::init_ops()],
+      ..Default::default()
+    });
+  }
 }