chore(types): lint the api docs with typescript (#3577)

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 `<link rel="stylesheet">` tag into the page with the desired url or a `<s
 #### frame.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`.
@@ -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.

utils/doclint/check_public_api/Documentation.js

@@ -20,6 +20,7 @@ class Documentation {
    */
   constructor(classesArray) {
     this.classesArray = classesArray;
+    /** @type {!Map<string, !Documentation.Class>} */
     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<string, !Documentation.Member>} */
     this.members = new Map();
+    /** @type {!Map<string, !Documentation.Member>} */
     this.properties = new Map();
+    /** @type {!Map<string, !Documentation.Member>} */
     this.methods = new Map();
+    /** @type {!Map<string, !Documentation.Member>} */
     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<!Documentation.Argument>} argsArray
+   * @param {!Documentation.Type} type
-   * @param {boolean} hasReturn
+   * @param {!Array<!Documentation.Member>} argsArray
-   * @param {boolean} async
    */
-  constructor(type, name, argsArray, hasReturn, async) {
+  constructor(kind, name, type, argsArray) {
-    this.type = type;
+    this.kind = kind;
     this.name = name;
+    this.type = type;
     this.argsArray = argsArray;
+    /** @type {!Map<string, !Documentation.Member>} */
     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<!Documentation.Argument>} argsArray
+   * @param {!Array<!Documentation.Member>} argsArray
-   * @param {boolean} hasReturn
+   * @param {?Documentation.Type} returnType
    * @return {!Documentation.Member}
    */
-  static createMethod(name, argsArray, hasReturn, async) {
+  static createMethod(name, argsArray, returnType) {
-    return new Documentation.Member('method', name, argsArray, hasReturn, async);
+    return new Documentation.Member('method', name, returnType, argsArray,);
   }
 
   /**
    * @param {string} name
+   * @param {!Documentation.Type}
    * @return {!Documentation.Member}
    */
-  static createProperty(name) {
+  static createProperty(name, type) {
-    return new Documentation.Member('property', name, [], false, false);
+    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<!Documentation.Member>=} properties
    */
-  constructor(name) {
+  constructor(name, properties = []) {
     this.name = name;
+    this.properties = properties;
   }
 };
 

utils/doclint/check_public_api/JSBuilder.js

@@ -1,157 +1,30 @@
-/**
+const ts = require('typescript');
- * Copyright 2017 Google Inc. All rights reserved.
+const path = require('path');
- *
- * 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 Documentation = require('./Documentation');
+module.exports = checkSources;
 
-class JSOutline {
 /**
-   * @param {string} text
+ * @param {!Array<!import('../Source')>} sources
-   * @param {string} name
  */
-  constructor(text, name) {
+function checkSources(sources) {
-    this.classes = [];
+  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<!Documentation.Class>} */
+  const classes = [];
   /** @type {!Map<string, string>} */
-    this.inheritance = new Map();
+  const inheritance = new Map();
-    this.errors = [];
+  sourceFiles.filter(x => !x.fileName.includes('node_modules')).map(x => visit(x));
-    this._eventsByClassName = new Map();
+  const errors = [];
-    this._currentClassName = null;
+  const documentation = new Documentation(recreateClassesWithInheritance(classes, inheritance));
-    this._currentClassMembers = [];
+  return {errors, documentation};
-    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;
-  }
-}
 
   /**
    * @param {!Array<!Documentation.Class>} classes
@@ -165,12 +38,9 @@ function recreateClassesWithInheritance(classes, inheritance) {
       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;
+          const memberId = member.kind + ':' + 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);
         }
       }
@@ -178,22 +48,157 @@ function recreateClassesWithInheritance(classes, inheritance) {
     });
   }
 
-/**
- * @param {!Array<!Source>} sources
- * @return {!Promise<{documentation: !Documentation, errors: !Array<string>}>}
- */
-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]);
-  }
-  const documentation = new Documentation(recreateClassesWithInheritance(classes, inheritance));
-  return { documentation, errors };
-};
 
+  /**
+   * @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);
+      }
+    }
+    ts.forEachChild(node, visit);
+  }
+
+  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<!Documentation.Member>} */
+    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));
+  }
+}

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(', '))
+      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.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(arg => new Documentation.Argument(arg));
+      const args = member.args.map(createPropertyFromJSON);
-      const method = Documentation.Member.createMethod(methodName, args, member.hasReturn, false);
+      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}`);

utils/doclint/check_public_api/index.js

@@ -49,7 +49,6 @@ const EXCLUDE_PROPERTIES = new Set([
 /**
  * @param {!Page} page
  * @param {!Array<!Source>} mdSources
- * @param {!Array<!Source>} jsSources
  * @return {!Promise<!Array<!Message>>}
  */
 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].kind === 'event'; ++eventIndex);
-    for (; eventIndex < members.length && members[eventIndex].type !== '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');
+    const constructorIndex = members.findIndex(member => member.kind === 'method' && member.name === 'constructor');
-    if (constructorIndex > 0 && members[constructorIndex - 1].type !== 'event')
+    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 => {
+    const members = cls.membersArray.filter(member => !EXCLUDE_PROPERTIES.has(`${cls.name}.${member.name}`));
-      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}`);
-    });
     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))
+      if (members.has(member.kind + ' ' + member.name))
-        errors.push(`Duplicate declaration of ${member.type} ${cls.name}.${member.name}()`);
+        errors.push(`Duplicate declaration of ${member.kind} ${cls.name}.${member.name}()`);
-      members.add(member.type + ' ' + 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.type !== !expectedMethod.type) {
-        if (actualMethod.hasReturn)
+        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);
+      const argsDiff = diff(actualArgs, expectedArgs);
-      if (argDiff.extra.length || argDiff.missing.length) {
+      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;
 }
 

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"

utils/doclint/check_public_api/test/check-duplicates/foo.js

@@ -2,6 +2,9 @@ class Foo {
   test() {
   }
 
+  /**
+   * @param {number} arg
+   */
   title(arg) {
   }
 }

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"

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

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"

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) {
   }
 }

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",
+          "type": {
-          "hasReturn": false,
+            "name": "number"
-          "async": false,
-          "args": []
           },
-        {
+          "kind": "property"
-          "name": "_property2",
-          "type": "property",
-          "hasReturn": false,
-          "async": false,
-          "args": []
-        },
-        {
-          "name": "constructor",
-          "type": "method",
-          "hasReturn": false,
-          "async": false,
-          "args": [
-            "delegate"
-          ]
         },
         {
           "name": "getter",
-          "type": "property",
+          "type": {
-          "hasReturn": false,
+            "name": "Object"
-          "async": false,
+          },
-          "args": []
+          "kind": "property"
         },
         {
           "name": "method",
-          "type": "method",
+          "type": {
-          "hasReturn": true,
+            "name": "Promise"
-          "async": true,
+          },
+          "kind": "method",
           "args": [
-            "foo",
+            {
-            "bar"
+              "name": "foo",
-          ]
+              "type": {
+                "name": "Object"
+              },
+              "kind": "property"
             },
             {
-          "name": "anevent",
+              "name": "bar",
-          "type": "event",
+              "type": {
-          "hasReturn": false,
+                "name": "Object"
-          "async": false,
+              },
-          "args": []
+              "kind": "property"
+            }
+          ]
         }
       ]
     }

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",
+          "kind": "method",
-          "hasReturn": false,
-          "async": false,
           "args": [
-            "a"
+            {
+              "name": "a",
+              "type": {
+                "name": "Object"
+              },
+              "kind": "property"
+            }
           ]
         },
         {
           "name": "bar",
-          "type": "method",
+          "kind": "method"
-          "hasReturn": false,
-          "async": false,
-          "args": []
         }
       ]
     },
     {
       "name": "B",
       "members": [
+        {
+          "name": "foo",
+          "kind": "event"
+        },
         {
           "name": "bar",
-          "type": "method",
+          "kind": "method",
-          "hasReturn": false,
-          "async": false,
           "args": [
-            "override"
+            {
+              "name": "override",
+              "type": {
+                "name": "Object"
+              },
+              "kind": "property"
+            }
           ]
         },
         {
           "name": "foo",
-          "type": "event",
+          "kind": "method",
-          "hasReturn": false,
-          "async": false,
-          "args": []
-        },
-        {
-          "name": "foo",
-          "type": "method",
-          "hasReturn": false,
-          "async": false,
           "args": [
-            "a"
+            {
+              "name": "a",
+              "type": {
+                "name": "Object"
+              },
+              "kind": "property"
+            }
           ]
         }
       ]

utils/doclint/check_public_api/test/md-builder-common/doc.md

@@ -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"

utils/doclint/check_public_api/test/md-builder-common/result.txt

@@ -5,26 +5,27 @@
       "members": [
         {
           "name": "frame",
-          "type": "event",
+          "kind": "event"
-          "hasReturn": false,
-          "async": false,
-          "args": []
         },
         {
           "name": "$",
-          "type": "method",
+          "type": {
-          "hasReturn": true,
+            "name": "Promise<ElementHandle>"
-          "async": false,
+          },
+          "kind": "method",
           "args": [
-            "selector"
+            {
+              "name": "selector",
+              "type": {
+                "name": "string"
+              },
+              "kind": "property"
+            }
           ]
         },
         {
           "name": "url",
-          "type": "property",
+          "kind": "property"
-          "hasReturn": false,
-          "async": false,
-          "args": []
         }
       ]
     }

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: []};
+  const result = {
-  for (let cls of doc.classesArray) {
+    classes: doc.classesArray.map(cls => ({
-    const classJSON = {
       name: cls.name,
-      members: []
+      members: cls.membersArray.map(serializeMember)
+    }))
   };
-    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)
-      });
-    }
-  }
   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
+  }
+}

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) {