A hook is not a kind of event, so stop saying it is

docs/adr/0017-hook-dispatch-contract.md

@@ -101,9 +101,21 @@ name into the response.
 A `Hook` is now refused where an event listener is expected, and
 `Event::onEvent()`'s default does nothing.
 
-Making `Hook` extend `FOGBase` directly, so the two are genuine peers, is the
-honest modelling change and is **not** taken here: it changes the answer to
-`$obj instanceof Event` for every hook in existence and needs its own issue.
+**Superseded by #1203.** `Hook` extends `FOGBase` directly and the two are
+genuine peers; the boilerplate they actually share — `$name`, `$description`,
+`$active`, the three log settings, `log()` and the constructor — moved to the
+`Listener` trait, which both use. `$obj instanceof Event` now answers what it
+always meant to.
+
+The blast-radius argument that held this back was made on its own evidence
+before the change landed: zero `instanceof Event` in core outside the two
+dispatch classes, zero in `packages/service`, and zero across the 72 hooks and
+15 events in `fog-plugins`; every hook in both trees declares its own `$active`
+and calls `parent::__construct()`; and exactly two files in the estate use the
+log settings, both core and both `$active = false`. `log()` went into the trait
+rather than staying on `Event` because `FOGBase` declares a `log()` with an
+identical signature and a different job, so a hook that lost `Event`'s copy
+would not have failed — it would have quietly called that one.
 
 ### 5. `notify()` on a HookManager is an error
 

docs/refactor-facts.md

@@ -355,6 +355,11 @@ third-party code only.
 ```
 
 ### F-18 — `Hook extends Event` defeats the only type check separating the two
+**Closed by #1203**: `Hook` extends `FOGBase` and both use the `Listener`
+trait, so `instanceof Event` is false for a hook. The refusal below stays as
+the specific diagnostic, and runs *before* the `instanceof Event` arm so a
+plugin author is told what they actually did. Recorded as found:
+
 `EventManager::register()` guards with `$listener instanceof Event`, which a
 `Hook` satisfies. A hook object can therefore be registered as an event
 listener, and `notify()` then calls `Event::onEvent()` — whose default body

packages/web/lib/fog/event.class.php

@@ -30,140 +30,8 @@
  */
 abstract class Event extends FOGBase
 {
-    /**
-     * The name.
-     *
-     * @var string
-     */
-    protected $name;
-    /**
-     * A description.
-     *
-     * @var string
-     */
-    protected $description;
-    /**
-     * The active flag of this.
-     *
-     * @var bool
-     */
-    public $active = true;
-    /**
-     * The log level.
-     *
-     * @var int
-     */
-    public $logLevel = 0;
-    /**
-     * Whether to store log to file
-     *
-     * @var bool
-     */
-    public $logToFile = false;
-    /**
-     * Whether to show log in browser
-     *
-     * @var bool
-     */
-    public $logToBrowser = true;
-    /**
-     * Initializes the base elements of the event or hook
-     *
-     * @return void
-     */
-    public function __construct()
-    {
-        parent::__construct();
-        if (!self::$FOGUser->isValid()) {
-            self::$FOGUser =& $GLOBALS['currentUser'];
-        }
-    }
-    /**
-     * How to log this file.
-     *
-     * @param string $txt     The text to log.
-     * @param int    $curlog  The logLevel setting.
-     * @param int    $logfile The logToFile setting.
-     * @param int    $logbrow The logToBrowser setting.
-     * @param object $obj     The object.
-     * @param int    $level   The basic log level.
-     *
-     * @return void
-     */
-    public static function log(
-        $txt,
-        $curlog,
-        $logfile,
-        $logbrow,
-        $obj,
-        $level = 1
-    ) {
-        if (self::$ajax) {
-            return;
-        }
-        $findArr = [
-            "#\r#",
-            "#\n#",
-            '#\s+#',
-            '# ,#',
-        ];
-        $repArr = [
-            '',
-            ' ',
-            ' ',
-            ','
-        ];
-        $txt = preg_replace($findArr, $repArr, $txt);
-        $txt = trim($txt);
-        if (empty($txt)) {
-            return;
-        }
-        $txt = sprintf(
-            '[%s] %s',
-            self::niceDate()->format('Y-m-d H:i:s'),
-            $txt
-        );
-        $msg = '%s<div class='
-            . '"alert alert-info alert-dismissible fade show">'
-            . '<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>'
-            . '%s</div>%s';
-        if (!self::$post && $logbrow) {
-            if ($curlog >= $level) {
-                printf(
-                    $msg,
-                    "\n",
-                    $txt,
-                    "\n"
-                );
-            }
-        }
-        $typePath = 'events';
-        if ($obj instanceof Hook) {
-            $typePath = 'hooks';
-        }
-        if ($logfile) {
-            $log = sprintf(
-                '%s%slib%s%s%s%s.log',
-                BASEPATH,
-                DS,
-                DS,
-                $typePath,
-                DS,
-                // Short name: this becomes a log filename.
-                self::shortName($obj)
-            );
-            $logtxt = sprintf(
-                "[%s] %s\r\n",
-                self::niceDate()->format('d-m-Y H:i:s'),
-                $txt
-            );
-            file_put_contents(
-                $log,
-                $logtxt,
-                FILE_APPEND | LOCK_EX
-            );
-        }
-    }
+    use Listener;
+
     /**
      * Simply adds the run method, though should be more defined.
      *

packages/web/lib/fog/eventmanager.class.php

@@ -173,23 +173,25 @@ private static function _recordEventName($event)
      */
     protected function acceptListener($listener)
     {
-        if (!($listener instanceof Event)) {
-            throw new \Exception(_('Class must extend event'));
-        }
-        // Hook extends Event, so the guard above accepts a hook -- the only
-        // type check separating the two, and it does not. What follows from
-        // that is not theoretical: notify() would then call Event::onEvent()
-        // on it, and hooks do not implement onEvent(), so the inherited
-        // default ran. That default used to print the event name into the
-        // response, which on a client protocol endpoint is arbitrary text in
-        // front of a ##@GO reply.
+        // Hooks first, and it has to stay first. Both arms refuse a hook now
+        // that Hook no longer extends Event (#1203), so whichever runs first
+        // decides the message -- and "a hook is not an event listener" is the
+        // one that tells a plugin author what they did. Falling through to
+        // "Class must extend event" would send them off to add an `extends`
+        // that is exactly the thing #1203 removed.
         //
-        // The two have genuinely different dispatch contracts -- see
-        // HookManager::notify() -- so a hook is refused here rather than
-        // silently dispatched as something it is not.
+        // The refusal itself predates the hierarchy change and is not made
+        // redundant by it: the two have genuinely different dispatch
+        // contracts -- see HookManager::notify() -- and before #1194 a hook
+        // registered here would be handed to Event::onEvent(), whose default
+        // printed the event name into the response, which on a client
+        // protocol endpoint is arbitrary text in front of a ##@GO reply.
         if ($listener instanceof Hook) {
             throw new \Exception(_('A hook is not an event listener'));
         }
+        if (!($listener instanceof Event)) {
+            throw new \Exception(_('Class must extend event'));
+        }
     }
     /**
      * Renders an event name for a log line.

packages/web/lib/fog/hook.class.php

@@ -30,8 +30,26 @@
  * @license  http://opensource.org/licenses/gpl-3.0 GPLv3
  * @link     https://fogproject.org
  */
-abstract class Hook extends Event
+abstract class Hook extends FOGBase
 {
+    /*
+     * NOT `extends Event`. A hook exists to change something in flight and is
+     * dispatched by HookManager::processEvent(), which calls a method the
+     * listener named and hands it a payload by reference; an event exists to
+     * be told something happened and is dispatched by EventManager::notify(),
+     * which calls a fixed onEvent() and discards the result. Neither is a kind
+     * of the other, and while the inheritance stood, `instanceof Event` -- the
+     * one type check separating them -- said they were. See #1203 and
+     * docs/adr/0017-hook-dispatch-contract.md.
+     *
+     * The boilerplate the two genuinely share ($name, $active, the log
+     * settings, log() itself) comes from the Listener trait, so nothing a hook
+     * relied on went away with the parent. What did go away is run() and
+     * onEvent(): both are the EVENT dispatch surface, both were empty, and no
+     * hook in core or in fog-plugins calls either.
+     */
+    use Listener;
+
     /**
      * Function enables reportTypes
      * to allow plugins, and all hooks really, to tie into