From 4f0f22ec786f7d209cd40aa144dca95d5f9757b5 Mon Sep 17 00:00:00 2001
From: Benjamin Bannier <benjamin.bannier@corelight.com>
Date: Wed, 13 Sep 2023 15:40:58 +0200
Subject: [PATCH] Fix handling of module scope when checking exported Spicy
 types for collisions

When checking exported Spicy types for collisions with existing Zeek
types we previously would also check whether they collide with names in
global scope, i.e., we didn't provide a `no_global` arg to
`detail::lookup_ID` which defaulted to false (since we also provided a
module name I'd argue that the behavior of that function is confusing
and probably error-prone -- like seen here).

This meant that e.g., a Spicy enum `foo::Direction` (automatically in
implicit Spicy module scope) would be detected to collide with the
existing Zeek `Direction` enum.

With this patch we use the `lookup_ID` API correctly and do not check
against potential collisions with globals anymore since it is not
needed.

Closes #3279.
---
 src/spicy/manager.cc                          |  2 +-
 .../export-type-collision-with-global.spicy   | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 testing/btest/spicy/export-type-collision-with-global.spicy

diff --git a/src/spicy/manager.cc b/src/spicy/manager.cc
index 3a8373959e..5968cc6d4a 100644
--- a/src/spicy/manager.cc
+++ b/src/spicy/manager.cc
@@ -215,7 +215,7 @@ void Manager::registerPacketAnalyzer(const std::string& name, const std::string&
 void Manager::registerType(const std::string& id, const TypePtr& type) {
     auto [ns, local] = parseID(id);
 
-    if ( const auto& old = detail::lookup_ID(local.c_str(), ns.c_str()) ) {
+    if ( const auto& old = detail::lookup_ID(local.c_str(), ns.c_str(), true) ) {
         // This is most likely to trigger for IDs that other Spicy modules
         // register. If we two Spicy modules need the same type, that's ok as
         // long as they match.
diff --git a/testing/btest/spicy/export-type-collision-with-global.spicy b/testing/btest/spicy/export-type-collision-with-global.spicy
new file mode 100644
index 0000000000..dbdffa7879
--- /dev/null
+++ b/testing/btest/spicy/export-type-collision-with-global.spicy
@@ -0,0 +1,20 @@
+# @TEST-REQUIRES: have-spicy
+#
+# @TEST-EXEC: spicyz -d -o foo.hlto foo.spicy foo.evt
+# @TEST-EXEC: zeek -NN Zeek::Spicy foo.hlto >output1 2>&1
+# @TEST-EXEC: ZEEK_SPICY_MODULE_PATH=$PWD zeek -B all -NN Zeek::Spicy >output2 2>&1
+# @TEST-EXEC: diff output1 output2 1>&2
+#
+# @TEST-DOC: This validates that an exported Spicy name with local part identical with a Zeek name in global scope does not clash. Regression test for #3279.
+
+# @TEST-START-FILE foo.spicy
+module foo;
+
+# `foo::Direction` has the same local part as Zeek's `::Direction`.
+public type Direction = enum { a };
+public type Other = enum { a };
+# @TEST-END-FILE
+
+# @TEST-START-FILE foo.evt
+import foo;
+# @TEST-END-FILE