feat(validator): no EnforceRange for readonly attributes (#740)

* feat(validator): no EnforceRange for readonly attributes

* full sentence

* fmt

* apply feedback

Author: saschanaz

lib/productions/attribute.js

@@ -1,5 +1,8 @@
 import { validationError } from "../error.js";
-import { idlTypeIncludesDictionary } from "../validators/helpers.js";
+import {
+  idlTypeIncludesDictionary,
+  idlTypeIncludesEnforceRange,
+} from "../validators/helpers.js";
 import { Base } from "./base.js";
 import {
   type_with_extended_attributes,
@@ -72,32 +75,32 @@ export class Attribute extends Base {
     yield* this.extAttrs.validate(defs);
     yield* this.idlType.validate(defs);
 
-    switch (this.idlType.generic) {
-      case "sequence":
-      case "record": {
-        const message = `Attributes cannot accept ${this.idlType.generic} types.`;
-        yield validationError(
-          this.tokens.name,
-          this,
-          "attr-invalid-type",
-          message
-        );
-        break;
+    if (["sequence", "record"].includes(this.idlType.generic)) {
+      const message = `Attributes cannot accept ${this.idlType.generic} types.`;
+      yield validationError(
+        this.tokens.name,
+        this,
+        "attr-invalid-type",
+        message
+      );
+    }
+
+    {
+      const { reference } = idlTypeIncludesDictionary(this.idlType, defs) || {};
+      if (reference) {
+        const targetToken = (this.idlType.union ? reference : this.idlType)
+          .tokens.base;
+        const message = "Attributes cannot accept dictionary types.";
+        yield validationError(targetToken, this, "attr-invalid-type", message);
       }
-      default: {
-        const { reference } =
-          idlTypeIncludesDictionary(this.idlType, defs) || {};
-        if (reference) {
-          const targetToken = (this.idlType.union ? reference : this.idlType)
-            .tokens.base;
-          const message = "Attributes cannot accept dictionary types.";
-          yield validationError(
-            targetToken,
-            this,
-            "attr-invalid-type",
-            message
-          );
-        }
+    }
+
+    if (this.readonly) {
+      if (idlTypeIncludesEnforceRange(this.idlType, defs)) {
+        const targetToken = this.idlType.tokens.base;
+        const message =
+          "Readonly attributes cannot accept [EnforceRange] extended attribute.";
+        yield validationError(targetToken, this, "attr-invalid-type", message);
       }
     }
   }

lib/validators/helpers.js

@@ -1,8 +1,10 @@
 /**
+ * @typedef {import("../validator.js").Definitions} Definitions
  * @typedef {import("../productions/dictionary.js").Dictionary} Dictionary
+ * @typedef {import("../../lib/productions/type").Type} Type
  *
- * @param {*} idlType
- * @param {import("../validator.js").Definitions} defs
+ * @param {Type} idlType
+ * @param {Definitions} defs
  * @param {object} [options]
  * @param {boolean} [options.useNullableInner] use when the input idlType is nullable and you want to use its inner type
  * @return {{ reference: *, dictionary: Dictionary }} the type reference that ultimately includes dictionary.
@@ -56,8 +58,8 @@ export function idlTypeIncludesDictionary(
 }
 
 /**
- * @param {*} dict dictionary type
- * @param {import("../validator.js").Definitions} defs
+ * @param {Dictionary} dict dictionary type
+ * @param {Definitions} defs
  * @return {boolean}
  */
 export function dictionaryIncludesRequiredField(dict, defs) {
@@ -80,3 +82,31 @@ export function dictionaryIncludesRequiredField(dict, defs) {
   defs.cache.dictionaryIncludesRequiredField.set(dict, result);
   return result;
 }
+
+/**
+ * For now this only checks the most frequent cases:
+ * 1. direct inclusion of [EnforceRange]
+ * 2. typedef of that
+ *
+ * More complex cases with dictionaries and records are not covered yet.
+ *
+ * @param {Type} idlType
+ * @param {Definitions} defs
+ */
+export function idlTypeIncludesEnforceRange(idlType, defs) {
+  if (idlType.union) {
+    // TODO: This should ideally be checked too
+    return false;
+  }
+
+  if (idlType.extAttrs.some((e) => e.name === "EnforceRange")) {
+    return true;
+  }
+
+  const def = defs.unique.get(idlType.idlType);
+  if (def?.type !== "typedef") {
+    return false;
+  }
+
+  return def.idlType.extAttrs.some((e) => e.name === "EnforceRange");
+}

test/invalid/baseline/invalid-attribute.txt

@@ -10,3 +10,12 @@
 (attr-invalid-type) Validation error at line 18 in invalid-attribute.webidl, inside `interface dictionaryAsAttribute -> attribute dictUnion`:
   attribute (Dict or boolean) dictUnion
              ^ Attributes cannot accept dictionary types.
+(attr-invalid-type) Validation error at line 28 in invalid-attribute.webidl, inside `interface EnforceRangeInReadonlyAttribute -> attribute readOnlyAttr1`:
+  readonly attribute [EnforceRange] long readOnlyAttr1;
+                                    ^ Readonly attributes cannot accept [EnforceRange] extended attribute.
+(attr-invalid-type) Validation error at line 29 in invalid-attribute.webidl, inside `interface EnforceRangeInReadonlyAttribute -> attribute readOnlyAttr2`:
+  readonly attribute [EnforceRange] GPUInt32In readOnlyAttr2;
+                                    ^ Readonly attributes cannot accept [EnforceRange] extended attribute.
+(attr-invalid-type) Validation error at line 30 in invalid-attribute.webidl, inside `interface EnforceRangeInReadonlyAttribute -> attribute readOnlyAttr2`:
+  readonly attribute GPUInt32 readOnlyAttr2;
+                     ^ Readonly attributes cannot accept [EnforceRange] extended attribute.

test/invalid/idl/invalid-attribute.webidl

@@ -17,3 +17,15 @@ interface dictionaryAsAttribute {
   attribute Dict dict;
   attribute (Dict or boolean) dictUnion;
 };
+
+typedef [EnforceRange] long GPUInt32;
+typedef long GPUInt32In;
+
+[Exposed=Window]
+interface EnforceRangeInReadonlyAttribute {
+  attribute [EnforceRange] long noProblem;
+
+  readonly attribute [EnforceRange] long readOnlyAttr1;
+  readonly attribute [EnforceRange] GPUInt32In readOnlyAttr2;
+  readonly attribute GPUInt32 readOnlyAttr2;
+};