Skip to content

Validate that all IDL types are defined somewhere #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2021
Merged

Validate that all IDL types are defined somewhere #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/idl/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ for (const [shortname, ast] of Object.entries(parsedFiles)) {

The following guarantees are provided by this package:
- All IDL files can be parsed by the version of [webidl2.js](https://github.com/w3c/webidl2.js/) used in `peerDependencies` in `package.json`.
- All types except `void` are defined by some specification.
- No duplicate top-level definitions or members.
- No missing or mismatched types in inheritance chains.
- No conflicts when applying mixins and partials.

The following guarantees are *not* provided:
- `WebIDL2.validate` rules are not guaranteed to pass.
- Extended attributes are not validated at all.
- Some types referenced may not be defined.
- Generally no other guarantees except the ones enforced by tests.
73 changes: 73 additions & 0 deletions test/idl/consistency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,79 @@ describe('Web IDL consistency', () => {
}
});

// Validate that there are no unknown types.
it('all used types are defined', () => {
// There are types in lots of places in the AST (interface members,
// arguments, return types) and rather than trying to cover them all, walk
// the whole AST looking for "idlType".
const usedTypes = new Set();

// Serialize and reparse the ast to not have to worry about own properties
// vs enumerable properties on the prototypes, etc.
const pending = [JSON.parse(JSON.stringify([...dfns, ...partials]))];
while (pending.length) {
const node = pending.pop();
for (const [key, value] of Object.entries(node)) {
if (key === 'idlType' && typeof value === 'string') {
usedTypes.add(value);
} else if (typeof value === 'object' && value !== null) {
pending.push(value);
}
}
}

const knownTypes = new Set([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dontcallmedom I note that the WebIDL parser in Reffy has a similar list to avoid flagging well-known types as dependencies:
https://github.com/w3c/reffy/blob/master/src/cli/parse-webidl.js#L352

Comparing the two lists (leaving aside void and types defined by other specs), I note the following differences:

  • ArrayBufferView in Reffy's WebIDL parser list but not here
  • BufferSource in Reffy's WebIDL parser list but not here
  • DOMException in Reffy's WebIDL parser list but not here
  • DOMTimeStamp in Reffy's WebIDL parser list but not here
  • Error in Reffy's WebIDL parser list but not here
  • Function in Reffy's WebIDL parser list but not here
  • RegExp in Reffy's WebIDL parser list but not here
  • VoidFunction in Reffy's WebIDL parser list but not here
  • symbol here but not in Reffy's WebIDL parser list

Most of them are from section 4 in WebIDL and can probably be removed from Reffy's WebIDL parser list because they get defined as "real" WebIDL structures.

I'm not sure why Error and RegExp are in Reffy's WebIDL parser list.

Last one is symbol, which seems correct but does not appear in any spec that defines some WebIDL.

I don't think that any change is needed here. Obviously, "DRY" could be an idea but then lists should not have to change often so probably not worth the hassle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created w3c/reffy#516 to possibly refresh the list in Reffy.

// Types defined by Web IDL itself:
'any', // https://heycam.github.io/webidl/#idl-any
'ArrayBuffer', // https://heycam.github.io/webidl/#idl-ArrayBuffer
'boolean', // https://heycam.github.io/webidl/#idl-boolean
'byte', // https://heycam.github.io/webidl/#idl-byte
'ByteString', // https://heycam.github.io/webidl/#idl-ByteString
'DataView', // https://heycam.github.io/webidl/#idl-DataView
'DOMString', // https://heycam.github.io/webidl/#idl-DOMString
'double', // https://heycam.github.io/webidl/#idl-double
'float', // https://heycam.github.io/webidl/#idl-float
'Float32Array', // https://heycam.github.io/webidl/#idl-Float32Array
'Float64Array', // https://heycam.github.io/webidl/#idl-Float64Array
'Int16Array', // https://heycam.github.io/webidl/#idl-Int16Array
'Int32Array', // https://heycam.github.io/webidl/#idl-Int32Array
'Int8Array', // https://heycam.github.io/webidl/#idl-Int8Array
'long long', // https://heycam.github.io/webidl/#idl-long-long
'long', // https://heycam.github.io/webidl/#idl-long
'object', // https://heycam.github.io/webidl/#idl-object
'octet', // https://heycam.github.io/webidl/#idl-octet
'short', // https://heycam.github.io/webidl/#idl-short
'symbol', // https://heycam.github.io/webidl/#idl-symbol
'Uint16Array', // https://heycam.github.io/webidl/#idl-Uint16Array
'Uint32Array', // https://heycam.github.io/webidl/#idl-Uint32Array
'Uint8Array', // https://heycam.github.io/webidl/#idl-Uint8Array
'Uint8ClampedArray', // https://heycam.github.io/webidl/#idl-Uint8ClampedArray
'unrestricted double', // https://heycam.github.io/webidl/#idl-unrestricted-double
'unrestricted float', // https://heycam.github.io/webidl/#idl-unrestricted-float
'unsigned long long', // https://heycam.github.io/webidl/#idl-unsigned-long-long
'unsigned long', // https://heycam.github.io/webidl/#idl-unsigned-long
'unsigned short', // https://heycam.github.io/webidl/#idl-unsigned-short
'USVString', // https://heycam.github.io/webidl/#idl-USVString
'undefined', // https://heycam.github.io/webidl/#idl-undefined

// TODO: drop void when it has been removed from all specs
'void',

// Types defined by other specs:
'CSSOMString', // https://drafts.csswg.org/cssom/#cssomstring-type
'WindowProxy' // https://html.spec.whatwg.org/multipage/window-object.html#windowproxy
]);

// Add any types defined by the parsed IDL.
for (const dfn of dfns) {
knownTypes.add(dfn.name);
}

for (const usedType of usedTypes) {
assert(knownTypes.has(usedType), `type ${usedType} is used but never defined`);
}
});

// This test should remain the last one as it slightly modifies objects in dfns in place.
it('merging in partials/mixins', () => {
const merged = merge(dfns, partials, includes);
Expand Down