From a0cbaf39abde91891d8594c37f4083255f1d49f5 Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Wed, 21 Nov 2018 14:49:08 -0800 Subject: [PATCH] chore(types): lint the api docs with typescript (#3577) --- docs/api.md | 12 +- .../doclint/check_public_api/Documentation.js | 47 ++- utils/doclint/check_public_api/JSBuilder.js | 387 +++++++++--------- utils/doclint/check_public_api/MDBuilder.js | 55 ++- utils/doclint/check_public_api/index.js | 92 +++-- .../test/check-duplicates/doc.md | 1 + .../test/check-duplicates/foo.js | 3 + .../test/check-returns/doc.md | 3 + .../test/check-returns/result.txt | 2 +- .../test/diff-arguments/doc.md | 7 +- .../test/diff-arguments/foo.js | 11 +- .../test/js-builder-common/result.txt | 66 ++- .../test/js-builder-inheritance/result.txt | 59 ++- .../test/md-builder-common/doc.md | 7 +- .../test/md-builder-common/result.txt | 25 +- utils/doclint/check_public_api/test/test.js | 47 ++- utils/doclint/cli.js | 3 +- 17 files changed, 472 insertions(+), 355 deletions(-) diff --git a/docs/api.md b/docs/api.md index 3679ff25273..3ac25899c69 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1102,7 +1102,7 @@ Get the browser the page belongs to. #### page.click(selector[, options]) - `selector` <[string]> A [selector] to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - `delay` <[number]> Time to wait between `mousedown` and `mouseup` in milliseconds. Defaults to 0. - returns: <[Promise]> Promise which resolves when the element matching `selector` is successfully clicked. The Promise will be rejected if there is no element matching `selector`. @@ -2183,7 +2183,7 @@ await page.mouse.up(); - `x` <[number]> - `y` <[number]> - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - `delay` <[number]> Time to wait between `mousedown` and `mouseup` in milliseconds. Defaults to 0. - returns: <[Promise]> @@ -2192,7 +2192,7 @@ Shortcut for [`mouse.move`](#mousemovex-y-options), [`mouse.down`](#mousedownopt #### mouse.down([options]) - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - returns: <[Promise]> @@ -2209,7 +2209,7 @@ Dispatches a `mousemove` event. #### mouse.up([options]) - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - returns: <[Promise]> @@ -2407,7 +2407,7 @@ Adds a `` tag into the page with the desired url or a ` A [selector] to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - `delay` <[number]> Time to wait between `mousedown` and `mouseup` in milliseconds. Defaults to 0. - returns: <[Promise]> Promise which resolves when the element matching `selector` is successfully clicked. The Promise will be rejected if there is no element matching `selector`. @@ -3000,7 +3000,7 @@ This method returns boxes of the element, or `null` if the element is not visibl #### elementHandle.click([options]) - `options` <[Object]> - - `button` <[string]> `left`, `right`, or `middle`, defaults to `left`. + - `button` <"left"|"right"|"middle"> Defaults to `left`. - `clickCount` <[number]> defaults to 1. See [UIEvent.detail]. - `delay` <[number]> Time to wait between `mousedown` and `mouseup` in milliseconds. Defaults to 0. - returns: <[Promise]> Promise which resolves when the element is successfully clicked. Promise gets rejected if the element is detached from DOM. diff --git a/utils/doclint/check_public_api/Documentation.js b/utils/doclint/check_public_api/Documentation.js index 4033802e9c0..5a47b221f0c 100644 --- a/utils/doclint/check_public_api/Documentation.js +++ b/utils/doclint/check_public_api/Documentation.js @@ -20,6 +20,7 @@ class Documentation { */ constructor(classesArray) { this.classesArray = classesArray; + /** @type {!Map} */ this.classes = new Map(); for (const cls of classesArray) this.classes.set(cls.name, cls); @@ -34,17 +35,21 @@ Documentation.Class = class { constructor(name, membersArray) { this.name = name; this.membersArray = membersArray; + /** @type {!Map} */ this.members = new Map(); + /** @type {!Map} */ this.properties = new Map(); + /** @type {!Map} */ this.methods = new Map(); + /** @type {!Map} */ this.events = new Map(); for (const member of membersArray) { this.members.set(member.name, member); - if (member.type === 'method') + if (member.kind === 'method') this.methods.set(member.name, member); - else if (member.type === 'property') + else if (member.kind === 'property') this.properties.set(member.name, member); - else if (member.type === 'event') + else if (member.kind === 'event') this.events.set(member.name, member); } } @@ -52,39 +57,39 @@ Documentation.Class = class { Documentation.Member = class { /** - * @param {string} type + * @param {string} kind * @param {string} name - * @param {!Array} argsArray - * @param {boolean} hasReturn - * @param {boolean} async + * @param {!Documentation.Type} type + * @param {!Array} argsArray */ - constructor(type, name, argsArray, hasReturn, async) { - this.type = type; + constructor(kind, name, type, argsArray) { + this.kind = kind; this.name = name; + this.type = type; this.argsArray = argsArray; + /** @type {!Map} */ this.args = new Map(); - this.hasReturn = hasReturn; - this.async = async; for (const arg of argsArray) this.args.set(arg.name, arg); } /** * @param {string} name - * @param {!Array} argsArray - * @param {boolean} hasReturn + * @param {!Array} argsArray + * @param {?Documentation.Type} returnType * @return {!Documentation.Member} */ - static createMethod(name, argsArray, hasReturn, async) { - return new Documentation.Member('method', name, argsArray, hasReturn, async); + static createMethod(name, argsArray, returnType) { + return new Documentation.Member('method', name, returnType, argsArray,); } /** * @param {string} name + * @param {!Documentation.Type} * @return {!Documentation.Member} */ - static createProperty(name) { - return new Documentation.Member('property', name, [], false, false); + static createProperty(name, type) { + return new Documentation.Member('property', name, type, []); } /** @@ -92,16 +97,18 @@ Documentation.Member = class { * @return {!Documentation.Member} */ static createEvent(name) { - return new Documentation.Member('event', name, [], false, false); + return new Documentation.Member('event', name, null, []); } }; -Documentation.Argument = class { +Documentation.Type = class { /** * @param {string} name + * @param {!Array=} properties */ - constructor(name) { + constructor(name, properties = []) { this.name = name; + this.properties = properties; } }; diff --git a/utils/doclint/check_public_api/JSBuilder.js b/utils/doclint/check_public_api/JSBuilder.js index c77e6f0b38d..837600fd61b 100644 --- a/utils/doclint/check_public_api/JSBuilder.js +++ b/utils/doclint/check_public_api/JSBuilder.js @@ -1,199 +1,204 @@ -/** - * Copyright 2017 Google Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -const assert = require('assert'); -const esprima = require('esprima'); -const ESTreeWalker = require('../../ESTreeWalker'); +const ts = require('typescript'); +const path = require('path'); const Documentation = require('./Documentation'); - -class JSOutline { - /** - * @param {string} text - * @param {string} name - */ - constructor(text, name) { - this.classes = []; - /** @type {!Map} */ - this.inheritance = new Map(); - this.errors = []; - this._eventsByClassName = new Map(); - this._currentClassName = null; - this._currentClassMembers = []; - this._name = name; - - this._text = text; - const ast = esprima.parseScript(this._text, {loc: true, range: true}); - const walker = new ESTreeWalker(node => { - if (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') - this._onClassDeclaration(node); - else if (node.type === 'MethodDefinition') - this._onMethodDefinition(node); - else if (node.type === 'AssignmentExpression') - this._onAssignmentExpression(node); - }); - walker.walk(ast); - this._flushClassIfNeeded(); - this._recreateClassesWithEvents(); - } - - _onClassDeclaration(node) { - this._flushClassIfNeeded(); - this._currentClassName = this._extractText(node.id); - if (!this._currentClassName) - this._currentClassName = this._name.substring(0, this._name.indexOf('.')); - const superClass = this._extractText(node.superClass); - if (superClass) - this.inheritance.set(this._currentClassName, superClass); - } - - _onMethodDefinition(node) { - assert(this._currentClassName !== null); - assert(node.value.type === 'FunctionExpression'); - const methodName = this._extractText(node.key); - if (node.kind === 'get') { - const property = Documentation.Member.createProperty(methodName); - this._currentClassMembers.push(property); - return; - } - // Async functions have return value. - let hasReturn = node.value.async; - // Extract properties from constructor. - if (node.kind === 'constructor') { - // Extract properties from constructor. - const walker = new ESTreeWalker(node => { - if (node.type !== 'AssignmentExpression') - return; - node = node.left; - if (node.type === 'MemberExpression' && node.object && - node.object.type === 'ThisExpression' && node.property && - node.property.type === 'Identifier') - this._currentClassMembers.push(Documentation.Member.createProperty(node.property.name)); - }); - walker.walk(node); - } else if (!hasReturn) { - const walker = new ESTreeWalker(node => { - if (node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration' || node.type === 'ArrowFunctionExpression') - return ESTreeWalker.SkipSubtree; - if (node.type === 'ReturnStatement') - hasReturn = hasReturn || !!node.argument; - }); - walker.walk(node.value.body); - } - const args = []; - for (const param of node.value.params) { - if (param.type === 'AssignmentPattern' && param.left.name) - args.push(new Documentation.Argument(param.left.name)); - else if (param.type === 'RestElement') - args.push(new Documentation.Argument('...' + param.argument.name)); - else if (param.type === 'Identifier') - args.push(new Documentation.Argument(param.name)); - else if (param.type === 'ObjectPattern' || param.type === 'AssignmentPattern') - args.push(new Documentation.Argument('options')); - else - this.errors.push(`JS Parsing issue: unsupported syntax to define parameter in ${this._currentClassName}.${methodName}(): ${this._extractText(param)}`); - } - const method = Documentation.Member.createMethod(methodName, args, hasReturn, node.value.async); - this._currentClassMembers.push(method); - return ESTreeWalker.SkipSubtree; - } - - _onAssignmentExpression(node) { - if (node.left.type !== 'MemberExpression' || node.right.type !== 'ObjectExpression') - return; - if (node.left.object.type !== 'Identifier' || node.left.property.type !== 'Identifier' || node.left.property.name !== 'Events') - return; - const className = node.left.object.name; - let events = this._eventsByClassName.get(className); - if (!events) { - events = []; - this._eventsByClassName.set(className, events); - } - for (const property of node.right.properties) { - if (property.type !== 'Property' || property.key.type !== 'Identifier' || property.value.type !== 'Literal') - continue; - events.push(Documentation.Member.createEvent(property.value.value)); - } - } - - _flushClassIfNeeded() { - if (this._currentClassName === null) - return; - const jsClass = new Documentation.Class(this._currentClassName, this._currentClassMembers); - this.classes.push(jsClass); - this._currentClassName = null; - this._currentClassMembers = []; - } - - _recreateClassesWithEvents() { - this.classes = this.classes.map(cls => { - const events = this._eventsByClassName.get(cls.name) || []; - const members = cls.membersArray.concat(events); - return new Documentation.Class(cls.name, members); - }); - } - - _extractText(node) { - if (!node) - return null; - const text = this._text.substring(node.range[0], node.range[1]).trim(); - return text; - } -} +module.exports = checkSources; /** - * @param {!Array} classes - * @param {!Map} inheritance - * @return {!Array} + * @param {!Array} sources */ -function recreateClassesWithInheritance(classes, inheritance) { - const classesByName = new Map(classes.map(cls => [cls.name, cls])); - return classes.map(cls => { - const membersMap = new Map(); - for (let wp = cls; wp; wp = classesByName.get(inheritance.get(wp.name))) { - for (const member of wp.membersArray) { - // Member was overridden. - const memberId = member.type + ':' + member.name; - if (membersMap.has(memberId)) - continue; - // Do not inherit constructors - if (wp !== cls && member.name === 'constructor' && member.type === 'method') - continue; - membersMap.set(memberId, member); +function checkSources(sources) { + const excludeClasses = new Set([]); + const program = ts.createProgram({ + options: { + allowJs: true, + target: ts.ScriptTarget.ES2017 + }, + rootNames: sources.map(source => source.filePath()) + }); + const checker = program.getTypeChecker(); + const sourceFiles = program.getSourceFiles(); + /** @type {!Array} */ + const classes = []; + /** @type {!Map} */ + const inheritance = new Map(); + sourceFiles.filter(x => !x.fileName.includes('node_modules')).map(x => visit(x)); + const errors = []; + const documentation = new Documentation(recreateClassesWithInheritance(classes, inheritance)); + return {errors, documentation}; + + /** + * @param {!Array} classes + * @param {!Map} inheritance + * @return {!Array} + */ + function recreateClassesWithInheritance(classes, inheritance) { + const classesByName = new Map(classes.map(cls => [cls.name, cls])); + return classes.map(cls => { + const membersMap = new Map(); + for (let wp = cls; wp; wp = classesByName.get(inheritance.get(wp.name))) { + for (const member of wp.membersArray) { + // Member was overridden. + const memberId = member.kind + ':' + member.name; + if (membersMap.has(memberId)) + continue; + membersMap.set(memberId, member); + } + } + return new Documentation.Class(cls.name, Array.from(membersMap.values())); + }); + } + + + /** + * @param {!ts.Node} node + */ + function visit(node) { + if (ts.isClassDeclaration(node) || ts.isClassExpression(node)) { + const symbol = node.name ? checker.getSymbolAtLocation(node.name) : node.symbol; + let className = symbol.getName(); + + if (className === '__class') { + let parent = node; + while (parent.parent) + parent = parent.parent; + className = path.basename(parent.fileName, '.js'); + } + if (className && !excludeClasses.has(className)) { + classes.push(serializeClass(className, symbol, node)); + const parentClassName = parentClass(node); + if (parentClassName) + inheritance.set(className, parentClassName); + excludeClasses.add(className); } } - return new Documentation.Class(cls.name, Array.from(membersMap.values())); - }); -} - -/** - * @param {!Array} sources - * @return {!Promise<{documentation: !Documentation, errors: !Array}>} - */ -module.exports = async function(sources) { - const classes = []; - const errors = []; - const inheritance = new Map(); - for (const source of sources) { - const outline = new JSOutline(source.text(), source.name()); - classes.push(...outline.classes); - errors.push(...outline.errors); - for (const entry of outline.inheritance) - inheritance.set(entry[0], entry[1]); + ts.forEachChild(node, visit); } - const documentation = new Documentation(recreateClassesWithInheritance(classes, inheritance)); - return { documentation, errors }; -}; + function parentClass(classNode) { + for (const herigateClause of classNode.heritageClauses || []) { + for (const heritageType of herigateClause.types) { + const parentClassName = heritageType.expression.escapedText; + return parentClassName; + } + } + return null; + + } + + function serializeSymbol(symbol, circular = []) { + const type = checker.getTypeOfSymbolAtLocation(symbol, symbol.valueDeclaration); + const name = symbol.getName(); + if (symbol.valueDeclaration.dotDotDotToken) { + const innerType = serializeType(type.typeArguments[0], circular); + innerType.name = '...' + innerType.name; + return Documentation.Member.createProperty('...' + name, innerType); + } + return Documentation.Member.createProperty(name, serializeType(type, circular)); + } + + /** + * @param {!ts.ObjectType} type + */ + function isRegularObject(type) { + if (type.isIntersection()) + return true; + if (!type.objectFlags) + return false; + if (!('aliasSymbol' in type)) + return false; + if (type.getConstructSignatures().length) + return false; + if (type.getCallSignatures().length) + return false; + return true; + } + + /** + * @param {!ts.Type} type + * @return {!Documentation.Type} + */ + function serializeType(type, circular = []) { + let typeName = checker.typeToString(type); + if (typeName === 'any' || typeName === '{ [x: string]: string; }') + typeName = 'Object'; + const nextCircular = [typeName].concat(circular); + + if (isRegularObject(type)) { + let properties = undefined; + if (!circular.includes(typeName)) + properties = type.getProperties().map(property => serializeSymbol(property, nextCircular)); + return new Documentation.Type('Object', properties); + } + if (type.isUnion() && typeName.includes('|')) { + const types = type.types.map(type => serializeType(type, circular)); + const name = types.map(type => type.name).join('|'); + const properties = [].concat(...types.map(type => type.properties)); + return new Documentation.Type(name.replace(/false\|true/g, 'boolean'), properties); + } + if (type.typeArguments) { + const properties = []; + const innerTypeNames = []; + for (const typeArgument of type.typeArguments) { + const innerType = serializeType(typeArgument, nextCircular); + if (innerType.properties) + properties.push(...innerType.properties); + innerTypeNames.push(innerType.name); + } + if (innerTypeNames.length === 1 && innerTypeNames[0] === 'void') + return new Documentation.Type(type.symbol.name); + return new Documentation.Type(`${type.symbol.name}<${innerTypeNames.join(', ')}>`, properties); + } + return new Documentation.Type(typeName, []); + } + + /** + * @param {string} className + * @param {!ts.Symbol} symbol + * @return {} + */ + function serializeClass(className, symbol, node) { + /** @type {!Array} */ + const members = []; + + const type = checker.getTypeOfSymbolAtLocation(symbol, node); + const events = type.getProperty('Events'); + if (events) { + const eventType = checker.getTypeAtLocation(events.valueDeclaration); + for (const property of eventType.getProperties()) { + if (property.valueDeclaration.initializer.text) + members.push(Documentation.Member.createEvent(property.valueDeclaration.initializer.text)); + } + } + + for (const [name, member] of symbol.members || []) { + if (name.startsWith('_')) + continue; + const memberType = checker.getTypeOfSymbolAtLocation(member, member.valueDeclaration); + const signature = memberType.getCallSignatures()[0]; + if (signature) + members.push(serializeSignature(name, signature)); + else + members.push(serializeProperty(name, memberType)); + } + + return new Documentation.Class(className, members); + } + + /** + * @param {string} name + * @param {!ts.Signature} signature + */ + function serializeSignature(name, signature) { + const parameters = signature.parameters.map(s => serializeSymbol(s)); + const returnType = serializeType(signature.getReturnType()); + return Documentation.Member.createMethod(name, parameters, returnType.name !== 'void' ? returnType : null); + } + + /** + * @param {string} name + * @param {!ts.Type} type + */ + function serializeProperty(name, type) { + return Documentation.Member.createProperty(name, serializeType(type)); + } +} diff --git a/utils/doclint/check_public_api/MDBuilder.js b/utils/doclint/check_public_api/MDBuilder.js index eda479e2a58..375d45fc742 100644 --- a/utils/doclint/check_public_api/MDBuilder.js +++ b/utils/doclint/check_public_api/MDBuilder.js @@ -48,13 +48,13 @@ class MDOutline { member = { name: element.textContent, args: [], - hasReturn: false + returnType: null }; currentClass.members.push(member); } else if (element.matches('li') && element.firstChild.matches && element.firstChild.matches('code')) { - member.args.push(element.firstChild.textContent); + member.args.push(parseProperty(element)); } else if (element.matches('li') && element.firstChild.nodeType === Element.TEXT_NODE && element.firstChild.textContent.toLowerCase().startsWith('return')) { - member.hasReturn = true; + member.returnType = parseProperty(element); const expectedText = 'returns: '; let actualText = element.firstChild.textContent; let angleIndex = actualText.indexOf('<'); @@ -67,6 +67,39 @@ class MDOutline { } } return {classes, errors}; + + function parseProperty(element) { + const str = element.textContent; + const name = str.substring(0, str.indexOf('<')).trim(); + const type = findType(str); + const properties = []; + // Strings have enum values instead of properties + if (!type.includes('string')) { + for (const childElement of element.querySelectorAll(':scope > ul > li')) + properties.push(parseProperty(childElement)); + } + return { + name, + type, + properties + }; + } + + /** + * @param {string} str + * @return {string} + */ + function findType(str) { + const start = str.indexOf('<') + 1; + let count = 1; + for (let i = start; i < str.length; i++) { + if (str[i] === '<') count++; + if (str[i] === '>') count--; + if (!count) + return str.substring(start, i); + } + return 'unknown'; + } }); return new MDOutline(classes, errors); } @@ -110,13 +143,21 @@ class MDOutline { return; } parameters = parameters.trim().replace(/[\[\]]/g, ''); - if (parameters !== member.args.join(', ')) - this.errors.push(`Heading arguments for "${member.name}" do not match described ones, i.e. "${parameters}" != "${member.args.join(', ')}"`); - const args = member.args.map(arg => new Documentation.Argument(arg)); - const method = Documentation.Member.createMethod(methodName, args, member.hasReturn, false); + if (parameters !== member.args.map(arg => arg.name).join(', ')) + this.errors.push(`Heading arguments for "${member.name}" do not match described ones, i.e. "${parameters}" != "${member.args.map(a => a.name).join(', ')}"`); + const args = member.args.map(createPropertyFromJSON); + let returnType = null; + if (member.returnType) + returnType = createPropertyFromJSON(member.returnType).type; + const method = Documentation.Member.createMethod(methodName, args, returnType); currentClassMembers.push(method); } + function createPropertyFromJSON(payload) { + const type = new Documentation.Type(payload.type, payload.properties.map(createPropertyFromJSON)); + return Documentation.Member.createProperty(payload.name, type); + } + function handleProperty(member, className, propertyName) { if (!currentClassName || !className || !propertyName || className.toLowerCase() !== currentClassName.toLowerCase()) { this.errors.push(`Failed to process header as property: ${member.name}`); diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 73864f37011..a80f6e9de31 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -49,7 +49,6 @@ const EXCLUDE_PROPERTIES = new Set([ /** * @param {!Page} page * @param {!Array} mdSources - * @param {!Array} jsSources * @return {!Promise>} */ module.exports = async function lint(page, mdSources, jsSources) { @@ -83,21 +82,21 @@ function checkSorting(doc) { // Events should go first. let eventIndex = 0; - for (; eventIndex < members.length && members[eventIndex].type === 'event'; ++eventIndex); - for (; eventIndex < members.length && members[eventIndex].type !== 'event'; ++eventIndex); + for (; eventIndex < members.length && members[eventIndex].kind === 'event'; ++eventIndex); + for (; eventIndex < members.length && members[eventIndex].kind !== 'event'; ++eventIndex); if (eventIndex < members.length) errors.push(`Events should go first. Event '${members[eventIndex].name}' in class ${cls.name} breaks order`); // Constructor should be right after events and before all other members. - const constructorIndex = members.findIndex(member => member.type === 'method' && member.name === 'constructor'); - if (constructorIndex > 0 && members[constructorIndex - 1].type !== 'event') + const constructorIndex = members.findIndex(member => member.kind === 'method' && member.name === 'constructor'); + if (constructorIndex > 0 && members[constructorIndex - 1].kind !== 'event') errors.push(`Constructor of ${cls.name} should go before other methods`); // Events should be sorted alphabetically. for (let i = 0; i < members.length - 1; ++i) { const member1 = cls.membersArray[i]; const member2 = cls.membersArray[i + 1]; - if (member1.type !== 'event' || member2.type !== 'event') + if (member1.kind !== 'event' || member2.kind !== 'event') continue; if (member1.name > member2.name) errors.push(`Event '${member1.name}' in class ${cls.name} breaks alphabetic ordering of events`); @@ -107,16 +106,16 @@ function checkSorting(doc) { for (let i = 0; i < members.length - 1; ++i) { const member1 = cls.membersArray[i]; const member2 = cls.membersArray[i + 1]; - if (member1.type === 'event' || member2.type === 'event') + if (member1.kind === 'event' || member2.kind === 'event') continue; - if (member1.type === 'method' && member1.name === 'constructor') + if (member1.kind === 'method' && member1.name === 'constructor') continue; if (member1.name > member2.name) { let memberName1 = `${cls.name}.${member1.name}`; - if (member1.type === 'method') + if (member1.kind === 'method') memberName1 += '()'; let memberName2 = `${cls.name}.${member2.name}`; - if (member2.type === 'method') + if (member2.kind === 'method') memberName2 += '()'; errors.push(`Bad alphabetic ordering of ${cls.name} members: ${memberName1} should go after ${memberName2}`); } @@ -135,14 +134,7 @@ function filterJSDocumentation(jsDocumentation) { for (const cls of jsDocumentation.classesArray) { if (EXCLUDE_CLASSES.has(cls.name)) continue; - const members = cls.membersArray.filter(member => { - if (member.name.startsWith('_')) - return false; - // Exclude all constructors by default. - if (member.name === 'constructor' && member.type === 'method') - return false; - return !EXCLUDE_PROPERTIES.has(`${cls.name}.${member.name}`); - }); + const members = cls.membersArray.filter(member => !EXCLUDE_PROPERTIES.has(`${cls.name}.${member.name}`)); classes.push(new Documentation.Class(cls.name, members)); } return new Documentation(classes); @@ -162,9 +154,9 @@ function checkDuplicates(doc) { classes.add(cls.name); const members = new Set(); for (const member of cls.membersArray) { - if (members.has(member.type + ' ' + member.name)) - errors.push(`Duplicate declaration of ${member.type} ${cls.name}.${member.name}()`); - members.add(member.type + ' ' + member.name); + if (members.has(member.kind + ' ' + member.name)) + errors.push(`Duplicate declaration of ${member.kind} ${cls.name}.${member.name}()`); + members.add(member.kind + ' ' + member.name); const args = new Set(); for (const arg of member.argsArray) { if (args.has(arg.name)) @@ -206,25 +198,28 @@ function compareDocumentations(actual, expected) { for (const methodName of methodDiff.equal) { const actualMethod = actualClass.methods.get(methodName); const expectedMethod = expectedClass.methods.get(methodName); - if (actualMethod.hasReturn !== expectedMethod.hasReturn) { - if (actualMethod.hasReturn) + if (!actualMethod.type !== !expectedMethod.type) { + if (actualMethod.type) errors.push(`Method ${className}.${methodName} has unneeded description of return type`); - else if (!expectedMethod.async) - errors.push(`Method ${className}.${methodName} is missing return type description`); else - errors.push(`Async method ${className}.${methodName} should describe return type Promise`); + errors.push(`Method ${className}.${methodName} is missing return type description`); + } else if (actualMethod.hasReturn) { + checkType(`Method ${className}.${methodName} has the wrong return type: `, actualMethod.type, expectedMethod.type); } const actualArgs = Array.from(actualMethod.args.keys()); const expectedArgs = Array.from(expectedMethod.args.keys()); - const argDiff = diff(actualArgs, expectedArgs); - if (argDiff.extra.length || argDiff.missing.length) { + const argsDiff = diff(actualArgs, expectedArgs); + if (argsDiff.extra.length || argsDiff.missing.length) { const text = [`Method ${className}.${methodName}() fails to describe its parameters:`]; - for (const arg of argDiff.missing) + for (const arg of argsDiff.missing) text.push(`- Argument not found: ${arg}`); - for (const arg of argDiff.extra) + for (const arg of argsDiff.extra) text.push(`- Non-existing argument found: ${arg}`); errors.push(text.join('\n')); } + + for (const arg of argsDiff.equal) + checkProperty(`Method ${className}.${methodName}()`, actualMethod.args.get(arg), expectedMethod.args.get(arg)); } const actualProperties = Array.from(actualClass.properties.keys()).sort(); const expectedProperties = Array.from(expectedClass.properties.keys()).sort(); @@ -242,6 +237,43 @@ function compareDocumentations(actual, expected) { for (const eventName of eventsDiff.missing) errors.push(`Event not found in class ${className}: '${eventName}'`); } + + + /** + * @param {string} source + * @param {!Documentation.Member} actual + * @param {!Documentation.Member} expected + */ + function checkProperty(source, actual, expected) { + checkType(source + ' ' + actual.name, actual.type, expected.type); + } + + /** + * @param {string} source + * @param {!Documentation.Type} actual + * @param {!Documentation.Type} expected + */ + function checkType(source, actual, expected) { + // TODO(@JoelEinbinder): check functions and Serializable + if (actual.name.includes('unction') || actual.name.includes('Serializable')) + return; + // We don't have nullchecks on for TypeScript + const actualName = actual.name.replace(/[\? ]/g, ''); + // TypeScript likes to add some spaces + const expectedName = expected.name.replace(/\ /g, ''); + if (expectedName !== actualName) + errors.push(`${source} ${actualName} != ${expectedName}`); + const actualPropertiesMap = new Map(actual.properties.map(property => [property.name, property.type])); + const expectedPropertiesMap = new Map(expected.properties.map(property => [property.name, property.type])); + const propertiesDiff = diff(Array.from(actualPropertiesMap.keys()).sort(), Array.from(expectedPropertiesMap.keys()).sort()); + for (const propertyName of propertiesDiff.extra) + errors.push(`${source} has unexpected property ${propertyName}`); + for (const propertyName of propertiesDiff.missing) + errors.push(`${source} is missing property ${propertyName}`); + for (const propertyName of propertiesDiff.equal) + checkType(source + '.' + propertyName, actualPropertiesMap.get(propertyName), expectedPropertiesMap.get(propertyName)); + } + return errors; } diff --git a/utils/doclint/check_public_api/test/check-duplicates/doc.md b/utils/doclint/check_public_api/test/check-duplicates/doc.md index 1e3b621c618..ea743ecd1e3 100644 --- a/utils/doclint/check_public_api/test/check-duplicates/doc.md +++ b/utils/doclint/check_public_api/test/check-duplicates/doc.md @@ -12,3 +12,4 @@ ### class: Bar +[number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type "Number" diff --git a/utils/doclint/check_public_api/test/check-duplicates/foo.js b/utils/doclint/check_public_api/test/check-duplicates/foo.js index b8dcec5dc4c..640271ae492 100644 --- a/utils/doclint/check_public_api/test/check-duplicates/foo.js +++ b/utils/doclint/check_public_api/test/check-duplicates/foo.js @@ -2,6 +2,9 @@ class Foo { test() { } + /** + * @param {number} arg + */ title(arg) { } } diff --git a/utils/doclint/check_public_api/test/check-returns/doc.md b/utils/doclint/check_public_api/test/check-returns/doc.md index faecedeeca5..1dffb8b6f0d 100644 --- a/utils/doclint/check_public_api/test/check-returns/doc.md +++ b/utils/doclint/check_public_api/test/check-returns/doc.md @@ -9,3 +9,6 @@ #### foo.www() - returns <[string]> + +[string]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type "String" +[number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type "Number" diff --git a/utils/doclint/check_public_api/test/check-returns/result.txt b/utils/doclint/check_public_api/test/check-returns/result.txt index 6f685d3321e..ceb202b25f5 100644 --- a/utils/doclint/check_public_api/test/check-returns/result.txt +++ b/utils/doclint/check_public_api/test/check-returns/result.txt @@ -1,4 +1,4 @@ [MarkDown] foo.www() has mistyped 'return' type declaration: expected exactly 'returns: ', found 'returns '. -[MarkDown] Async method Foo.asyncFunction should describe return type Promise +[MarkDown] Method Foo.asyncFunction is missing return type description [MarkDown] Method Foo.return42 is missing return type description [MarkDown] Method Foo.returnNothing has unneeded description of return type \ No newline at end of file diff --git a/utils/doclint/check_public_api/test/diff-arguments/doc.md b/utils/doclint/check_public_api/test/diff-arguments/doc.md index 618febfdc98..a991bfa66b9 100644 --- a/utils/doclint/check_public_api/test/diff-arguments/doc.md +++ b/utils/doclint/check_public_api/test/diff-arguments/doc.md @@ -1,11 +1,14 @@ ### class: Foo #### foo.bar(options) - `options` <[Object]> - + - `visibility` <[boolean]> #### foo.foo(arg1, arg2) - `arg1` <[string]> - `arg2` <[string]> #### foo.test(...files) -- `...filePaths` <[string]> +- `...filePaths` <...[string]> +[Object]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object "Object" +[string]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type "String" +[boolean]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type "Boolean" diff --git a/utils/doclint/check_public_api/test/diff-arguments/foo.js b/utils/doclint/check_public_api/test/diff-arguments/foo.js index 84e64504660..d102f6f7ae1 100644 --- a/utils/doclint/check_public_api/test/diff-arguments/foo.js +++ b/utils/doclint/check_public_api/test/diff-arguments/foo.js @@ -1,10 +1,19 @@ class Foo { + /** + * @param {string} arg1 + */ foo(arg1, arg3 = {}) { } + /** + * @param {...string} filePaths + */ test(...filePaths) { } - bar({visibility}) { + /** + * @param {{visibility?: boolean}} options + */ + bar(options) { } } diff --git a/utils/doclint/check_public_api/test/js-builder-common/result.txt b/utils/doclint/check_public_api/test/js-builder-common/result.txt index 0327dd4e3f2..40e9475b020 100644 --- a/utils/doclint/check_public_api/test/js-builder-common/result.txt +++ b/utils/doclint/check_public_api/test/js-builder-common/result.txt @@ -3,52 +3,46 @@ { "name": "A", "members": [ + { + "name": "anevent", + "kind": "event" + }, { "name": "property1", - "type": "property", - "hasReturn": false, - "async": false, - "args": [] - }, - { - "name": "_property2", - "type": "property", - "hasReturn": false, - "async": false, - "args": [] - }, - { - "name": "constructor", - "type": "method", - "hasReturn": false, - "async": false, - "args": [ - "delegate" - ] + "type": { + "name": "number" + }, + "kind": "property" }, { "name": "getter", - "type": "property", - "hasReturn": false, - "async": false, - "args": [] + "type": { + "name": "Object" + }, + "kind": "property" }, { "name": "method", - "type": "method", - "hasReturn": true, - "async": true, + "type": { + "name": "Promise" + }, + "kind": "method", "args": [ - "foo", - "bar" + { + "name": "foo", + "type": { + "name": "Object" + }, + "kind": "property" + }, + { + "name": "bar", + "type": { + "name": "Object" + }, + "kind": "property" + } ] - }, - { - "name": "anevent", - "type": "event", - "hasReturn": false, - "async": false, - "args": [] } ] } diff --git a/utils/doclint/check_public_api/test/js-builder-inheritance/result.txt b/utils/doclint/check_public_api/test/js-builder-inheritance/result.txt index 1abadad93ff..0975d03ab39 100644 --- a/utils/doclint/check_public_api/test/js-builder-inheritance/result.txt +++ b/utils/doclint/check_public_api/test/js-builder-inheritance/result.txt @@ -3,57 +3,56 @@ { "name": "A", "members": [ - { - "name": "constructor", - "type": "method", - "hasReturn": false, - "async": false, - "args": [] - }, { "name": "foo", - "type": "method", - "hasReturn": false, - "async": false, + "kind": "method", "args": [ - "a" + { + "name": "a", + "type": { + "name": "Object" + }, + "kind": "property" + } ] }, { "name": "bar", - "type": "method", - "hasReturn": false, - "async": false, - "args": [] + "kind": "method" } ] }, { "name": "B", "members": [ + { + "name": "foo", + "kind": "event" + }, { "name": "bar", - "type": "method", - "hasReturn": false, - "async": false, + "kind": "method", "args": [ - "override" + { + "name": "override", + "type": { + "name": "Object" + }, + "kind": "property" + } ] }, { "name": "foo", - "type": "event", - "hasReturn": false, - "async": false, - "args": [] - }, - { - "name": "foo", - "type": "method", - "hasReturn": false, - "async": false, + "kind": "method", "args": [ - "a" + { + "name": "a", + "type": { + "name": "Object" + }, + "kind": "property" + } ] } ] diff --git a/utils/doclint/check_public_api/test/md-builder-common/doc.md b/utils/doclint/check_public_api/test/md-builder-common/doc.md index f38c4bf96d2..42695c4cd42 100644 --- a/utils/doclint/check_public_api/test/md-builder-common/doc.md +++ b/utils/doclint/check_public_api/test/md-builder-common/doc.md @@ -3,7 +3,7 @@ This is a class. #### event: 'frame' -- <[Frame]> +- <[Frame]> This event is dispatched. @@ -17,3 +17,8 @@ The method runs document.querySelector. - <[string]> Contains the URL of the request. + +[string]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type "String" +[Promise]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise "Promise" +[ElementHandle]: # "ElementHandle" +[ElementHandle]: # "Frame" diff --git a/utils/doclint/check_public_api/test/md-builder-common/result.txt b/utils/doclint/check_public_api/test/md-builder-common/result.txt index d8f18c82e74..0265d3571b5 100644 --- a/utils/doclint/check_public_api/test/md-builder-common/result.txt +++ b/utils/doclint/check_public_api/test/md-builder-common/result.txt @@ -5,26 +5,27 @@ "members": [ { "name": "frame", - "type": "event", - "hasReturn": false, - "async": false, - "args": [] + "kind": "event" }, { "name": "$", - "type": "method", - "hasReturn": true, - "async": false, + "type": { + "name": "Promise" + }, + "kind": "method", "args": [ - "selector" + { + "name": "selector", + "type": { + "name": "string" + }, + "kind": "property" + } ] }, { "name": "url", - "type": "property", - "hasReturn": false, - "async": false, - "args": [] + "kind": "property" } ] } diff --git a/utils/doclint/check_public_api/test/test.js b/utils/doclint/check_public_api/test/test.js index 503a9fab677..0f35ef362d4 100644 --- a/utils/doclint/check_public_api/test/test.js +++ b/utils/doclint/check_public_api/test/test.js @@ -91,24 +91,37 @@ async function testJSBuilder(state, test) { expect(serialize(documentation)).toBeGolden('result.txt'); } +/** + * @param {import('../Documentation')} doc + */ function serialize(doc) { - const result = {classes: []}; - for (let cls of doc.classesArray) { - const classJSON = { + const result = { + classes: doc.classesArray.map(cls => ({ name: cls.name, - members: [] - }; - result.classes.push(classJSON); - for (let member of cls.membersArray) { - classJSON.members.push({ - name: member.name, - type: member.type, - hasReturn: member.hasReturn, - async: member.async, - args: member.argsArray.map(arg => arg.name) - }); - } - } + members: cls.membersArray.map(serializeMember) + })) + }; return JSON.stringify(result, null, 2); } - +/** + * @param {import('../Documentation').Member} member + */ +function serializeMember(member) { + return { + name: member.name, + type: serializeType(member.type), + kind: member.kind, + args: member.argsArray.length ? member.argsArray.map(serializeMember) : undefined + } +} +/** + * @param {import('../Documentation').Type} type + */ +function serializeType(type) { + if (!type) + return undefined; + return { + name: type.name, + properties: type.properties.length ? type.properties.map(serializeMember) : undefined + } +} \ No newline at end of file diff --git a/utils/doclint/cli.js b/utils/doclint/cli.js index 57fe39a5bbc..95b18e8fb70 100755 --- a/utils/doclint/cli.js +++ b/utils/doclint/cli.js @@ -48,8 +48,9 @@ async function run() { const browser = await puppeteer.launch(); const page = await browser.newPage(); const checkPublicAPI = require('./check_public_api'); - const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib'), '.js'); + const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib')); messages.push(...await checkPublicAPI(page, mdSources, jsSources)); + await browser.close(); for (const source of mdSources) {