Produce better error message when %XXX hook is used on a unit field.

Closes #1465.
zeek · Jul 6, 2023 · 8568c6b · 8568c6b
1 parent 1244472
commit 8568c6b
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 9 deletions.
diff --git a/spicy/toolchain/src/compiler/visitors/normalizer.cc b/spicy/toolchain/src/compiler/visitors/normalizer.cc
@@ -128,8 +128,18 @@ struct Visitor : public hilti::visitor::PostOrder<void, Visitor> {
         // Link hook to its unit type and field.
 
         NodeRef unit_type_ref = p.findParentRef<type::Unit>();
-        if ( ! unit_type_ref ) {
-            // External hook, do name lookuo.
+        if ( unit_type_ref ) {
+            // Produce a tailored error message if `%XXX` is used on a unit field.
+            if ( auto id = h.id().namespace_(); id && hilti::util::startsWith(h.id().local(), "0x25_") ) {
+                if ( unit_type_ref->as<type::Unit>().itemByName(h.id().namespace_().local()) ) {
+                    p.node.addError(hilti::util::fmt("cannot use hook '%s' with a unit field",
+                                                     hilti::util::replace(h.id().local(), "0x25_", "%")));
+                    return;
+                }
+            }
+        }
+        else {
+            // External hook, do name lookup.
             auto ns = h.id().namespace_();
             if ( ! ns )
                 return;
@@ -144,6 +154,21 @@ struct Visitor : public hilti::visitor::PostOrder<void, Visitor> {
                     modified = true;
                 }
                 else {
+                    // Produce a tailored error message if `%XXX` is used on a unit field.
+                    if ( auto id = ns.namespace_(); id && hilti::util::startsWith(h.id().local(), "0x25_") ) {
+                        if ( auto resolved = hilti::scope::lookupID<hilti::declaration::Type>(id, p, "unit type") ) {
+                            if ( auto utype =
+                                     resolved->first->as<hilti::declaration::Type>().type().tryAs<type::Unit>();
+                                 utype && utype->itemByName(ns.local()) ) {
+                                p.node.addError(hilti::util::fmt("cannot use hook '%s' with a unit field",
+                                                                 hilti::util::replace(h.id().local(), "0x25_", "%")));
+                                // We failed to resolve the ID since it refers to a hook.
+                                // Return early here and do not emit below resolution error.
+                                return;
+                            }
+                        }
+                    }
+
                     p.node.addError(resolved.error());
                     return;
                 }

diff --git a/tests/Baseline/spicy.types.unit.hooks-fail/output b/tests/Baseline/spicy.types.unit.hooks-fail/output
@@ -13,11 +13,13 @@
 [error] <...>/hooks-fail.spicy:17:10: no field 'x1' in unit type
 [error] <...>/hooks-fail.spicy:18:11: unknown hook '%x2'
 [error] <...>/hooks-fail.spicy:19:13: cannot use paths in hooks; trigger on the top-level field instead
-[error] <...>/hooks-fail.spicy:23:15: hook '%init' does not take any parameters
-[error] <...>/hooks-fail.spicy:24:15: signature for hook must be: %gap(seq: uint64, len: uint64)
-[error] <...>/hooks-fail.spicy:24:15: cannot use hook '%gap', unit type does not support sinks because it is not public
-[error] <...>/hooks-fail.spicy:25:12: no field 'y1' in unit type
-[error] <...>/hooks-fail.spicy:26:13: unknown hook '%y2'
-[error] <...>/hooks-fail.spicy:27:15: cannot use paths in hooks; trigger on the top-level field instead
-[error] <...>/hooks-fail.spicy:28:11: unknown ID 'XXXX'
+[error] <...>/hooks-fail.spicy:22:16: cannot use '%done' with a unit field
+[error] <...>/hooks-fail.spicy:25:15: hook '%init' does not take any parameters
+[error] <...>/hooks-fail.spicy:26:15: signature for hook must be: %gap(seq: uint64, len: uint64)
+[error] <...>/hooks-fail.spicy:26:15: cannot use hook '%gap', unit type does not support sinks because it is not public
+[error] <...>/hooks-fail.spicy:27:12: no field 'y1' in unit type
+[error] <...>/hooks-fail.spicy:28:13: unknown hook '%y2'
+[error] <...>/hooks-fail.spicy:29:15: cannot use paths in hooks; trigger on the top-level field instead
+[error] <...>/hooks-fail.spicy:30:11: unknown ID 'XXXX::x'
+[error] <...>/hooks-fail.spicy:31:18: cannot use '%done' with a unit field
 [error] spicyc: aborting after errors
diff --git a/tests/spicy/types/unit/hooks-fail.spicy b/tests/spicy/types/unit/hooks-fail.spicy
@@ -18,6 +18,8 @@ type test = unit {
     on %x2 {}
     on a.b.c {}
 
+    x: uint32;
+    on x::%done {}
     };
 
 on test::%init(i: string) {}
@@ -26,3 +28,4 @@ on test::y1 {}
 on test::%y2 {}
 on test::a.b.c {}
 on XXXX::x {}
+on test::x::%done {}