Add detection of IDL attributes for URL with wrong type (#836)

close #830

.github/workflows/file-issue-for-review.yml

@@ -73,7 +73,8 @@ jobs:
           --what incompatiblePartialIdlExposure \
           --what unexpectedEventHandler \
           --what wrongCaseEnumValue \
-          --what noEvent
+          --what noEvent \
+          --what urlType
     - name: Run issue filer script
       working-directory: strudy
       run: node src/reporting/file-issue-for-review.js --max 10

src/lib/study-webidl.js

@@ -134,6 +134,19 @@ function isOverloadedOperation (a, b) {
   return true;
 }
 
+function isDOMStringUrlMember(member) {
+  if (member.type !== 'attribute' && member.type !== 'field') {
+    return false;
+  }
+  if (!member.name.match(/urls?$/i)) {
+    return false;
+  }
+  if (member.idlType?.idlType !== "DOMString" && (member.idlType?.generic !== "sequence" && member.idlType.idlType?.[0]?.idlType !== "DOMString")) {
+    return false;
+  }
+  return true;
+}
+
 // Helper to test if two members define an operation with the same identifier,
 // one of them being a static operation and the other a regular one. This is
 // allowed in Web IDL, see https://github.com/whatwg/webidl/issues/1097
@@ -366,11 +379,16 @@ function studyWebIdl (specs, { crawledResults = [], curatedResults = [] } = {})
     if (!target.idl.members || !source.idl.members) {
       return;
     }
+    let targetName = dfnName(target);
     for (const targetMember of target.idl.members) {
       if (!targetMember.name) {
         continue;
       }
 
+      if (isDOMStringUrlMember(targetMember)) {
+        recordAnomaly(target.spec, 'urlType', `\`${describeMember(targetMember)}\` in ${targetName} uses \`DOMString\` instead of recommended \`USVString\` for URLs`);
+      }
+
       for (const sourceMember of source.idl.members) {
         if (!sourceMember.name) {
           continue;
@@ -384,7 +402,6 @@ function studyWebIdl (specs, { crawledResults = [], curatedResults = [] } = {})
         }
 
         // Prepare anomaly parameters
-        let targetName = dfnName(target);
         let sourceName = dfnName(source);
         const specs = [target.spec];
         if (sourceName === targetName) {

src/lib/study.js

@@ -174,6 +174,7 @@ const anomalyGroups = [
       { name: 'unknownExposure', title: 'Unknown globals in `[Exposed]` attribute' },
       { name: 'unknownExtAttr', title: 'Unknown extended attributes' },
       { name: 'unknownType', title: 'Unknown Web IDL type' },
+      { name: 'urlType', title: 'Wrong Web IDL type for URLs' },
       {
         name: 'wrongCaseEnumValue',
         title: 'Enum values that ignore naming conventions',
@@ -701,4 +702,4 @@ export default async function study(specs, options = {}) {
 
   // Return the structured report
   return result;
-}
+}

test/study-webidl.js

@@ -533,6 +533,36 @@ interface mixin MyNamespaceMixin {};
     });
   });
 
+  it('reports dictionary members and interface URL attributes that use DOMString instead of USVString', () => {
+    const report = analyzeIdl(`
+[Exposed=*] interface MyLink {
+  attribute DOMString url;
+  attribute DOMString alternateUrl;
+};
+dictionary MyLinkOptions {
+  DOMString fallbackUrl;
+  sequence<DOMString> urls;
+};
+`);
+    assertNbAnomalies(report, 4);
+    assertAnomaly(report, 0, {
+      name: 'urlType',
+      message: '`attribute url` in interface `MyLink` uses `DOMString` instead of recommended `USVString` for URLs'
+    });
+    assertAnomaly(report, 1, {
+      name: 'urlType',
+      message: '`attribute alternateUrl` in interface `MyLink` uses `DOMString` instead of recommended `USVString` for URLs'
+    });
+    assertAnomaly(report, 2, {
+      name: 'urlType',
+      message: '`field fallbackUrl` in dictionary `MyLinkOptions` uses `DOMString` instead of recommended `USVString` for URLs'
+    });
+    assertAnomaly(report, 3, {
+      name: 'urlType',
+      message: '`field urls` in dictionary `MyLinkOptions` uses `DOMString` instead of recommended `USVString` for URLs'
+    });
+  });
+
   it('reports overloads across definitions (partial, same spec)', () => {
     const report = analyzeIdl(`
 [Global=Home,Exposed=*]