Web IDL study: detect event handlers with no matching event

src/lib/study-webidl.js

@@ -20,6 +20,7 @@
  */
 
 import * as WebIDL2 from 'webidl2';
+import { getInterfaceTreeInfo } from 'reffy';
 
 const getSpecs = list => [...new Set(list.map(({ spec }) => spec))];
 const specName = spec => spec.shortname ?? spec.url;
@@ -174,6 +175,15 @@ function studyWebIdl (specs, { crawledResults = [], curatedResults = [] } = {})
   const usedTypes = {}; // Index of types used in the IDL
   const usedExtAttrs = {}; // Index of extended attributes
 
+  // We need to run the analysis on all specs, even if caller is only
+  // interested in a few of them, because types may be defined in specs that
+  // the caller is not interested in.
+  const allSpecs = (crawledResults.length > 0) ? crawledResults : specs;
+  const allEvents = allSpecs
+    .filter(spec => Array.isArray(spec.events))
+    .map(spec => spec.events.map(e => Object.assign({ spec }, e)))
+    .flat();
+
   // Record an anomaly for the given spec(s),
   // provided we are indeed interested in the results
   function recordAnomaly (spec, name, message) {
@@ -193,17 +203,54 @@ function studyWebIdl (specs, { crawledResults = [], curatedResults = [] } = {})
   function inheritsFrom (iface, ancestor) {
     if (!iface.inheritance) return false;
     if (iface.inheritance === ancestor) return true;
+    if (!dfns[iface.inheritance]) return false;
     const parentInterface = dfns[iface.inheritance].find(({ idl }) => !idl.partial)?.idl;
     if (!parentInterface) return false;
     return inheritsFrom(parentInterface, ancestor);
   }
 
+  function getBubblingPath(iface) {
+    const interfaces = Object.values(dfns)
+      .map(dfn => dfn.find(d => d.idl.type === 'interface'))
+      .filter(dfn => !!dfn)
+      .map(dfn => dfn.idl);
+    const { bubblingPath } = getInterfaceTreeInfo(iface, interfaces);
+    return bubblingPath;
+  }
+
   // TODO: a full check of event handlers would require:
   // - checking if the interface doesn't include a mixin with eventhandlers
   function checkEventHandlers (spec, memberHolder, iface = memberHolder) {
-    const eventHandler = memberHolder.members.find(m => m?.name?.startsWith('on') && m.type === 'attribute' && m.idlType?.idlType === 'EventHandler');
-    if (eventHandler && !inheritsFrom(iface, 'EventTarget')) {
-      recordAnomaly(spec, 'unexpectedEventHandler', `The interface \`${iface.name}\` defines an event handler \`${eventHandler.name}\` but does not inherit from \`EventTarget\``);
+    const eventHandlers = memberHolder.members.filter(m => m?.name?.startsWith('on') && m.type === 'attribute' && m.idlType?.idlType === 'EventHandler');
+    if (eventHandlers.length > 0 && !inheritsFrom(iface, 'EventTarget')) {
+      recordAnomaly(spec, 'unexpectedEventHandler', `The interface \`${iface.name}\` defines an event handler \`${eventHandlers[0].name}\` but does not inherit from \`EventTarget\``);
+    }
+
+    for (const eventHandler of eventHandlers) {
+      const eventType = eventHandler.name.substring(2);
+      const events = allEvents.filter(e => e.type === eventType);
+      let event = events.find(e => e.targets?.find(target =>
+        target === iface.name));
+      if (!event) {
+        // No event found with the same type that targets the interface
+        // directly, but one the events may target an interface that
+        // inherits from the interface, or may bubble on the interface.
+        event = events.find(e => e.targets?.find(target => {
+          const inheritanceFound = dfns[target]?.find(dfn =>
+            dfn.idl.type === 'interface' &&
+            inheritsFrom(dfn.idl, iface.name));
+          if (inheritanceFound) return true;
+          if (!e.bubbles) return false;
+          const bubblingPath = getBubblingPath(target);
+          if (!bubblingPath) return false;
+          return bubblingPath.find(bubblingIface =>
+            iface.name === bubblingIface ||
+            inheritsFrom(iface, bubblingIface));
+        }));
+      }
+      if (!event) {
+        recordAnomaly(spec, 'noEvent', `The interface \`${iface.name}\` defines an event handler \`${eventHandler.name}\` but no event named "${eventType}" targets it`);
+      }
     }
   }
 
@@ -384,10 +431,6 @@ function studyWebIdl (specs, { crawledResults = [], curatedResults = [] } = {})
     }
   }
 
-  // We need to run the analysis on all specs, even if caller is only
-  // interested in a few of them, because types may be defined in specs that
-  // the caller is not interested in.
-  const allSpecs = (crawledResults.length > 0) ? crawledResults : specs;
   allSpecs
     // We're only interested in specs that define Web IDL content
     .filter(spec => !!spec.idl)

src/lib/study.js

@@ -149,6 +149,11 @@ const anomalyGroups = [
         guidance: 'See the [`[Exposed]`](https://webidl.spec.whatwg.org/#Exposed) extended attribute section in Web IDL for requirements.'
       },
       { name: 'invalid', title: 'Invalid Web IDL' },
+      {
+        name: 'noEvent',
+        title: 'Suspicious event handlers',
+        description: 'The following suspicious event handlers were found'
+      },
       { name: 'noExposure', title: 'Missing `[Exposed]` attributes' },
       { name: 'noOriginalDefinition', title: 'Missing base interfaces' },
       { name: 'overloaded', title: 'Invalid overloaded operations' },

test/study-webidl.js

@@ -148,7 +148,7 @@ interface WhereIAm {};
     });
   });
 
-  it('reports no anomaly for valid EventHandler attributes definitions', () => {
+  it('reports EventHandler attributes with no matching events', () => {
     const report = analyzeIdl(`
 [Exposed=*]
 interface Event {};
@@ -163,6 +163,34 @@ interface Carlos : EventTarget {
 [Exposed=*]
 interface EventTarget {};
 `);
+    assertNbAnomalies(report, 1);
+    assertAnomaly(report, 0, {
+      name: 'noEvent',
+      message: 'The interface `Carlos` defines an event handler `onbigbisous` but no event named "bigbisous" targets it'
+    });
+  });
+
+  it('reports no anomaly for valid EventHandler attributes definitions', () => {
+    const crawlResult = toCrawlResult(`
+[Exposed=*]
+interface Event {};
+[LegacyTreatNonObjectAsNull]
+callback EventHandlerNonNull = any (Event event);
+typedef EventHandlerNonNull? EventHandler;
+
+[Global=Window,Exposed=*]
+interface Carlos : EventTarget {
+  attribute EventHandler onbigbisous;
+};
+[Exposed=*]
+interface EventTarget {};
+`);
+    crawlResult[0].events = [{
+      type: 'bigbisous',
+      interface: 'Event',
+      targets: ['Carlos']
+    }];
+    const report = study(crawlResult);
     assertNbAnomalies(report, 0);
   });
 
@@ -179,11 +207,73 @@ interface Carlos {
   attribute EventHandler onbigbisous;
 };
 `);
-    assertNbAnomalies(report, 1);
+    assertNbAnomalies(report, 2);
     assertAnomaly(report, 0, {
       name: 'unexpectedEventHandler',
       message: 'The interface `Carlos` defines an event handler `onbigbisous` but does not inherit from `EventTarget`'
     });
+    assertAnomaly(report, 1, { name: 'noEvent' });
+  });
+
+  it('follows the inheritance chain to assess event targets', () => {
+    const crawlResult = toCrawlResult(`
+[Exposed=*]
+interface Event {};
+[LegacyTreatNonObjectAsNull]
+callback EventHandlerNonNull = any (Event event);
+typedef EventHandlerNonNull? EventHandler;
+
+[Global=Window,Exposed=*]
+interface Singer : EventTarget {
+  attribute EventHandler onbigbisous;
+};
+
+[Global=Window,Exposed=*]
+interface Carlos : Singer {};
+[Exposed=*]
+interface EventTarget {};
+`);
+    crawlResult[0].events = [{
+      type: 'bigbisous',
+      interface: 'Event',
+      targets: [
+        'Carlos'
+      ]
+    }];
+    const report = study(crawlResult);
+    assertNbAnomalies(report, 0);
+  });
+
+  it('follows the bubbling path to assess event targets', () => {
+    const crawlResult = toCrawlResult(`
+[Exposed=*]
+interface Event {};
+[LegacyTreatNonObjectAsNull]
+callback EventHandlerNonNull = any (Event event);
+typedef EventHandlerNonNull? EventHandler;
+
+[Global=Window,Exposed=*]
+interface IDBDatabase : EventTarget {
+  attribute EventHandler onabort;
+};
+[Global=Window,Exposed=*]
+interface IDBTransaction : EventTarget {
+  attribute EventHandler onabort;
+};
+[Global=Window,Exposed=*]
+interface MyIndexedDB : IDBDatabase {
+};
+
+[Exposed=*]
+interface EventTarget {};`);
+    crawlResult[0].events = [{
+      type: 'abort',
+      interface: 'Event',
+      targets: ['IDBTransaction'],
+      bubbles: true
+    }];
+    const report = study(crawlResult);
+    assertNbAnomalies(report, 0);
   });
 
   it('detects incompatible partial exposure issues', () => {