fix(validators): ensure cached values of dictionaryIncludesRequiredField are boolean (#546)

The two `return true` statements would leave undefined in the cache, and
looking up the same dictionary again would then return undefined instead
of true. The bug manifests when the same dictionary appears as an
argument twice or more.

Fixes #545.

Author: foolip

lib/validators/helpers.js

@@ -62,17 +62,19 @@ export function dictionaryIncludesRequiredField(dict, defs) {
   if (defs.cache.dictionaryIncludesRequiredField.has(dict)) {
     return defs.cache.dictionaryIncludesRequiredField.get(dict);
   }
-  defs.cache.dictionaryIncludesRequiredField.set(dict, undefined); // indeterminate
-  if (dict.inheritance) {
+  // Set cached result to indeterminate to short-circuit circular definitions.
+  // The final result will be updated to true or false.
+  defs.cache.dictionaryIncludesRequiredField.set(dict, undefined);
+  let result = dict.members.some(field => field.required);
+  if (!result && dict.inheritance) {
     const superdict = defs.unique.get(dict.inheritance);
     if (!superdict) {
-      return true;
-    }
-    if (dictionaryIncludesRequiredField(superdict, defs)) {
-      return true;
+      // Assume required members in the supertype if it is unknown.
+      result = true;
+    } else if (dictionaryIncludesRequiredField(superdict, defs)) {
+      result = true;
     }
   }
-  const result = dict.members.some(field => field.required);
   defs.cache.dictionaryIncludesRequiredField.set(dict, result);
   return result;
 }

test/invalid/baseline/argument-dict-optional.txt

@@ -1,18 +1,18 @@
 (dict-arg-optional) Validation error at line 25 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op1 -> argument shouldBeOptional`:
   undefined op1(Optional shouldBeOptional);
                          ^ Dictionary argument must be optional if it has no required fields
-(dict-arg-optional) Validation error at line 29 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op3 -> argument union`:
+(dict-arg-optional) Validation error at line 31 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op3 -> argument union`:
 (Optional or boolean) union);
                       ^ Dictionary argument must be optional if it has no required fields
-(dict-arg-optional) Validation error at line 30 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op4 -> argument union`:
+(dict-arg-optional) Validation error at line 32 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op4 -> argument union`:
   undefined op4(OptionalUnion union);
                               ^ Dictionary argument must be optional if it has no required fields
-(dict-arg-optional) Validation error at line 33 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op6 -> argument recursive`:
+(dict-arg-optional) Validation error at line 35 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op6 -> argument recursive`:
   undefined op6(Recursive recursive);
                           ^ Dictionary argument must be optional if it has no required fields
-(dict-arg-optional) Validation error at line 36 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op8 -> argument lastRequired`:
+(dict-arg-optional) Validation error at line 38 in argument-dict-optional.webidl, inside `interface mixin Container -> operation op8 -> argument lastRequired`:
   undefined op8(Optional lastRequired, optional DOMString yay
                          ^ Dictionary argument must be optional if it has no required fields
-(dict-arg-optional) Validation error at line 42 in argument-dict-optional.webidl, inside `interface ContainerInterface -> iterable -> argument shouldBeOptional`:
+(dict-arg-optional) Validation error at line 44 in argument-dict-optional.webidl, inside `interface ContainerInterface -> iterable -> argument shouldBeOptional`:
 <DOMString>(Optional shouldBeOptional);
                      ^ Dictionary argument must be optional if it has no required fields

test/invalid/idl/argument-dict-optional.webidl

@@ -25,6 +25,8 @@ interface mixin Container {
   undefined op1(Optional shouldBeOptional);
   undefined op2(Required noNeedToBeOptional);
   undefined op22(Required2 noNeedToBeOptional);
+  // The same again to expose caching bug:
+  undefined op23(Required2 noNeedToBeOptional);
 
   undefined op3((Optional or boolean) union);
   undefined op4(OptionalUnion union);