[doclint] Move doclint under utils/

This patch:
- moves doclint under utils/ folder
- adds tests to verify doclint basic functionality

This patch also drops the jasmine as a spec runner for the doclint
checks. It turned out it's hard to customize jasmine's behavior,
so instead this patch implements a dummy spec runner.

The dummy spec runner allows us:
- to format messages however we want (the custom jasmine reporter would
  also allow us to do this)
- to avoid `beforeAll` functions which pollute global to pass
  initialized variables over to specs

References #14

.eslintignore

@@ -1,2 +1,3 @@
 third_party/*
 examples/*
+utils/doclint/test/

package.json

@@ -10,11 +10,12 @@
     "unit": "jasmine test/test.js",
     "debug-unit": "DEBUG_TEST=true node --inspect-brk ./node_modules/.bin/jasmine test/test.js",
     "test-phantom": "python third_party/phantomjs/test/run-tests.py",
-    "test": "npm run lint --silent && npm run unit && npm run test-phantom",
+    "test-doclint": "jasmine utils/doclint/test/test.js",
+    "test": "npm run lint --silent && npm run unit && npm run test-phantom && npm run test-doclint",
     "install": "node install.js",
     "lint": "([ \"$CI\" = true ] && eslint --quiet -f codeframe . || eslint .) && npm run doc",
     "generate-toc": "markdown-toc -i docs/api.md",
-    "doc": "jasmine test/doclint/lint.js"
+    "doc": "node utils/doclint/lint.js"
   },
   "author": "The Chromium Authors",
   "license": "SEE LICENSE IN LICENSE",

test/doclint/lint.js

@@ -1,151 +0,0 @@
-const path = require('path');
-const jsBuilder = require('./JSBuilder');
-const mdBuilder = require('./MDBuilder');
-const Documentation = require('./Documentation');
-
-const PROJECT_DIR = path.join(__dirname, '..', '..');
-
-let EXCLUDE_CLASSES = new Set([
-  'Connection',
-  'FrameManager',
-  'Helper',
-  'Navigator',
-  'NetworkManager',
-  'ProxyStream'
-]);
-
-let EXCLUDE_METHODS = new Set([
-  'Body.constructor',
-  'Dialog.constructor',
-  'Frame.constructor',
-  'Headers.constructor',
-  'Headers.fromPayload',
-  'InterceptedRequest.constructor',
-  'Page.constructor',
-  'Page.create',
-  'Request.constructor',
-  'Response.constructor',
-]);
-
-/**
- * @param {!Documentation} jsDocumentation
- * @return {!Documentation}
- */
-function filterJSDocumentation(jsDocumentation) {
-  // Filter classes and methods.
-  let classes = [];
-  for (let cls of jsDocumentation.classesArray) {
-    if (EXCLUDE_CLASSES.has(cls.name))
-      continue;
-    let methods = cls.methodsArray.filter(method => {
-      if (method.name.startsWith('_'))
-        return false;
-      return !EXCLUDE_METHODS.has(`${cls.name}.${method.name}`);
-    });
-    let properties = cls.propertiesArray.filter(property => !property.startsWith('_'));
-    classes.push(new Documentation.Class(cls.name, methods, properties));
-  }
-  return new Documentation(classes);
-}
-
-let jsDocumentation;
-let mdDocumentation;
-let mdParseErrors;
-let diff;
-
-beforeAll(SX(async function() {
-  jsDocumentation = filterJSDocumentation(await jsBuilder(path.join(PROJECT_DIR, 'lib')));
-  let mdDoc = await mdBuilder(path.join(PROJECT_DIR, 'docs'));
-  mdDocumentation = mdDoc.documentation;
-  mdParseErrors = mdDoc.errors;
-  diff = Documentation.diff(mdDocumentation, jsDocumentation);
-}));
-
-describe('JavaScript documentation parser', function() {
-  it('should not contain any duplicate classes (probably error in parsing!)', () => {
-    let jsClasses = new Map();
-    for (let jsClass of jsDocumentation.classesArray) {
-      if (jsClasses.has(jsClass.name))
-        fail(`JavaScript has duplicate declaration of ${jsClass.name}. (This probably means that this linter has an error)`);
-      jsClasses.set(jsClass.name, jsClass);
-    }
-  });
-});
-
-describe('Markdown Documentation', function() {
-  it('should not have any parse errors', () => {
-    for (let error of mdParseErrors)
-      fail(error);
-  });
-  it('should not contain any duplicate classes', () => {
-    let mdClasses = new Map();
-    for (let mdClass of mdDocumentation.classesArray) {
-      if (mdClasses.has(mdClass.name))
-        fail(`Documentation has duplicate declaration of class ${mdClass.name}`);
-      mdClasses.set(mdClass.name, mdClass);
-    }
-  });
-  it('class constructors should be defined before other methods', () => {
-    for (let mdClass of mdDocumentation.classesArray) {
-      let constructorMethod = mdClass.methods.get('constructor');
-      if (!constructorMethod)
-        continue;
-      if (mdClass.methodsArray[0] !== constructorMethod)
-        fail(`Method 'new ${mdClass.name}' should go before other methods of class ${mdClass.name}`);
-    }
-  });
-  it('methods should be sorted alphabetically', () => {
-    for (let mdClass of mdDocumentation.classesArray) {
-      for (let i = 0; i < mdClass.methodsArray.length - 1; ++i) {
-        // Constructor should always go first.
-        if (mdClass.methodsArray[i].name === 'constructor')
-          continue;
-        let method1 = mdClass.methodsArray[i];
-        let method2 = mdClass.methodsArray[i + 1];
-        if (method1.name > method2.name)
-          fail(`${mdClass.name}.${method1.name} breaks alphabetic sorting inside class ${mdClass.name}`);
-      }
-    }
-  });
-  it('should not contain any non-existing class', () => {
-    for (let className of diff.extraClasses)
-      fail(`Documentation describes non-existing class ${className}`);
-  });
-  it('should describe all existing classes', () => {
-    for (let className of diff.missingClasses)
-      fail(`Documentation lacks description of class ${className}`);
-  });
-  it('should not contain any non-existing methods', () => {
-    for (let methodName of diff.extraMethods)
-      fail(`Documentation describes non-existing method: ${methodName}`);
-  });
-  it('should describe all existing methods', () => {
-    for (let methodName of diff.missingMethods)
-      fail(`Documentation lacks method ${methodName}`);
-  });
-  it('should describe all arguments propertly', () => {
-    for (let badArgument of diff.badArguments) {
-      let text = [`Method ${badArgument.method} fails to describe its parameters:`];
-      for (let missing of badArgument.missingArgs)
-        text.push(`- Missing description for "${missing}"`);
-      for (let extra of badArgument.extraArgs)
-        text.push(`- Described non-existing parameter "${extra}"`);
-      fail(text.join('\n'));
-    }
-  });
-  it('should not contain any non-existing properties', () => {
-    for (let propertyName of diff.extraProperties)
-      fail(`Documentation describes non-existing property: ${propertyName}`);
-  });
-  it('should describe all existing properties', () => {
-    for (let propertyName of diff.missingProperties)
-      fail(`Documentation lacks property ${propertyName}`);
-  });
-});
-
-// Since Jasmine doesn't like async functions, they should be wrapped
-// in a SX function.
-function SX(fun) {
-  return done => Promise.resolve(fun()).then(done).catch(done.fail);
-}
-

utils/doclint/Documentation.js

@@ -14,19 +14,22 @@ class Documentation {
    * @param {!Documentation} expected
    */
   static diff(actual, expected) {
-    const result = {};
     // Diff classes.
+    const result = {
+      extraClasses: [],
+      missingClasses: [],
+      extraMethods: [],
+      missingMethods: [],
+      extraProperties: [],
+      missingProperties: [],
+      badArguments: [],
+    };
     const actualClasses = Array.from(actual.classes.keys()).sort();
     const expectedClasses = Array.from(expected.classes.keys()).sort();
     let classesDiff = diff(actualClasses, expectedClasses);
-    result.extraClasses = classesDiff.extra;
-    result.missingClasses = classesDiff.missing;
+    result.extraClasses.push(...classesDiff.extra);
+    result.missingClasses.push(...classesDiff.missing);
 
-    result.extraMethods = [];
-    result.missingMethods = [];
-    result.badArguments = [];
-    result.extraProperties = [];
-    result.missingProperties = [];
     for (let className of classesDiff.equal) {
       const actualClass = actual.classes.get(className);
       const expectedClass = expected.classes.get(className);

utils/doclint/MDBuilder.js

@@ -3,15 +3,14 @@ const markdownToc = require('markdown-toc');
 const path = require('path');
 const Documentation = require('./Documentation');
 const commonmark = require('commonmark');
-const Browser = require('../../lib/Browser');
 
 class MDOutline {
   /**
-   * @param {!Browser} browser
+   * @param {!Page} page
    * @param {string} text
    * @return {!MDOutline}
    */
-  static async create(browser, text) {
+  static async create(page, text) {
     // Render markdown as HTML.
     const reader = new commonmark.Parser();
     const parsed = reader.parse(text);
@@ -19,7 +18,6 @@ class MDOutline {
     const html = writer.render(parsed);
 
     // Extract headings.
-    const page = await browser.newPage();
     await page.setContent(html);
     const classes = await page.evaluate(() => {
       let classes = [];
@@ -59,6 +57,8 @@ class MDOutline {
     let currentClassProperties = [];
     for (const cls of classes) {
       let match = cls.name.match(classHeading);
+      if (!match)
+        continue;
       currentClassName = match[1];
       for (let member of cls.members) {
         if (constructorRegex.test(member.name)) {
@@ -108,26 +108,25 @@ class MDOutline {
 }
 
 /**
+ * @param {!Page} page
  * @param {!Array<string>} dirPath
  * @return {!Promise<{documentation: !Documentation, errors: !Array<string>}>}
  */
-module.exports = async function(dirPath) {
+module.exports = async function(page, dirPath) {
   let filePaths = fs.readdirSync(dirPath)
       .filter(fileName => fileName.endsWith('.md'))
       .map(fileName => path.join(dirPath, fileName));
   let classes = [];
   let errors = [];
-  const browser = new Browser({args: ['--no-sandbox']});
   for (let filePath of filePaths) {
     const markdownText = fs.readFileSync(filePath, 'utf8');
     const newMarkdownText = markdownToc.insert(markdownText);
     if (markdownText !== newMarkdownText)
       errors.push('Markdown TOC is outdated, run `yarn generate-toc`');
-    let outline = await MDOutline.create(browser, markdownText);
+    let outline = await MDOutline.create(page, markdownText);
     classes.push(...outline.classes);
     errors.push(...outline.errors);
   }
-  await browser.close();
   const documentation = new Documentation(classes);
   return { documentation, errors };
 };

utils/doclint/README.md

@@ -0,0 +1,30 @@
+# DocLint
+
+**Doclint** is a small program that lints Puppeteer's documentation against
+Puppeteer's source code.
+
+Doclint works in a few steps:
+
+1. Read sources in `lib/` folder, parse AST trees and extract public API
+2. Read sources in `docs/` folder, render markdown to HTML, use puppeteer to traverse the HTML
+  and extract described API
+3. Compare one API to another
+
+Doclint is also responsible for general markdown checks, most notably for the table of contents
+relevancy.
+
+## Running
+
+```bash
+npm run doc
+```
+
+## Tests
+
+Doclint has its own set of jasmine tests, located at `utils/doclint/test` folder.
+
+To execute tests, run:
+
+```bash
+npm run test-doclint
+```

utils/doclint/lint.js

@@ -0,0 +1,148 @@
+const {describe, it, fail, runSpecs} = require('./specRunner');
+const path = require('path');
+const jsBuilder = require('./JSBuilder');
+const mdBuilder = require('./MDBuilder');
+const Documentation = require('./Documentation');
+const Browser = require('../../lib/Browser');
+
+const PROJECT_DIR = path.join(__dirname, '..', '..');
+
+let EXCLUDE_CLASSES = new Set([
+  'Connection',
+  'FrameManager',
+  'Helper',
+  'Navigator',
+  'NetworkManager',
+  'ProxyStream'
+]);
+
+let EXCLUDE_METHODS = new Set([
+  'Body.constructor',
+  'Dialog.constructor',
+  'Frame.constructor',
+  'Headers.constructor',
+  'Headers.fromPayload',
+  'InterceptedRequest.constructor',
+  'Page.constructor',
+  'Page.create',
+  'Request.constructor',
+  'Response.constructor',
+]);
+
+const browser = new Browser({args: ['--no-sandbox']});
+browser.newPage()
+    .then(initializeSpecs)
+    .then(runSpecs)
+    .catch(console.error)
+    .then(() => browser.close());
+
+async function initializeSpecs(page) {
+  let jsResult = await jsBuilder(path.join(PROJECT_DIR, 'lib'));
+  let mdResult = await mdBuilder(page, path.join(PROJECT_DIR, 'docs'));
+  let jsDocumentation = filterJSDocumentation(jsResult);
+  let mdDocumentation = mdResult.documentation;
+  let diff = Documentation.diff(mdDocumentation, jsDocumentation);
+
+  describe('JavaScript documentation parser', function() {
+    it('should not contain any duplicate classes (probably error in parsing!)', () => {
+      let jsClasses = new Map();
+      for (let jsClass of jsDocumentation.classesArray) {
+        if (jsClasses.has(jsClass.name))
+          fail(`JavaScript has duplicate declaration of ${jsClass.name}. (This probably means that this linter has an error)`);
+        jsClasses.set(jsClass.name, jsClass);
+      }
+    });
+  });
+
+  describe('Markdown Documentation', function() {
+    it('should not have any parse errors', () => {
+      for (let error of mdResult.errors)
+        fail(error);
+    });
+    it('should not contain any duplicate classes', () => {
+      let mdClasses = new Map();
+      for (let mdClass of mdDocumentation.classesArray) {
+        if (mdClasses.has(mdClass.name))
+          fail(`Documentation has duplicate declaration of class ${mdClass.name}`);
+        mdClasses.set(mdClass.name, mdClass);
+      }
+    });
+    it('class constructors should be defined before other methods', () => {
+      for (let mdClass of mdDocumentation.classesArray) {
+        let constructorMethod = mdClass.methods.get('constructor');
+        if (!constructorMethod)
+          continue;
+        if (mdClass.methodsArray[0] !== constructorMethod)
+          fail(`Method 'new ${mdClass.name}' should go before other methods of class ${mdClass.name}`);
+      }
+    });
+    it('methods should be sorted alphabetically', () => {
+      for (let mdClass of mdDocumentation.classesArray) {
+        for (let i = 0; i < mdClass.methodsArray.length - 1; ++i) {
+          // Constructor should always go first.
+          if (mdClass.methodsArray[i].name === 'constructor')
+            continue;
+          let method1 = mdClass.methodsArray[i];
+          let method2 = mdClass.methodsArray[i + 1];
+          if (method1.name > method2.name)
+            fail(`${mdClass.name}.${method1.name} breaks alphabetic sorting inside class ${mdClass.name}`);
+        }
+      }
+    });
+    it('should not contain any non-existing class', () => {
+      for (let className of diff.extraClasses)
+        fail(`Documentation describes non-existing class ${className}`);
+    });
+    it('should describe all existing classes', () => {
+      for (let className of diff.missingClasses)
+        fail(`Documentation lacks description of class ${className}`);
+    });
+    it('should not contain any non-existing methods', () => {
+      for (let methodName of diff.extraMethods)
+        fail(`Documentation describes non-existing method: ${methodName}`);
+    });
+    it('should describe all existing methods', () => {
+      for (let methodName of diff.missingMethods)
+        fail(`Documentation lacks method ${methodName}`);
+    });
+    it('should describe all arguments propertly', () => {
+      for (let badArgument of diff.badArguments) {
+        let text = [`Method ${badArgument.method} fails to describe its parameters:`];
+        for (let missing of badArgument.missingArgs)
+          text.push(`- Missing description for "${missing}"`);
+        for (let extra of badArgument.extraArgs)
+          text.push(`- Described non-existing parameter "${extra}"`);
+        fail(text.join('\n'));
+      }
+    });
+    it('should not contain any non-existing properties', () => {
+      for (let propertyName of diff.extraProperties)
+        fail(`Documentation describes non-existing property: ${propertyName}`);
+    });
+    it('should describe all existing properties', () => {
+      for (let propertyName of diff.missingProperties)
+        fail(`Documentation lacks property ${propertyName}`);
+    });
+  });
+}
+
+/**
+ * @param {!Documentation} jsDocumentation
+ * @return {!Documentation}
+ */
+function filterJSDocumentation(jsDocumentation) {
+  // Filter classes and methods.
+  let classes = [];
+  for (let cls of jsDocumentation.classesArray) {
+    if (EXCLUDE_CLASSES.has(cls.name))
+      continue;
+    let methods = cls.methodsArray.filter(method => {
+      if (method.name.startsWith('_'))
+        return false;
+      return !EXCLUDE_METHODS.has(`${cls.name}.${method.name}`);
+    });
+    let properties = cls.propertiesArray.filter(property => !property.startsWith('_'));
+    classes.push(new Documentation.Class(cls.name, methods, properties));
+  }
+  return new Documentation(classes);
+}

utils/doclint/specRunner.js

@@ -0,0 +1,85 @@
+const startTime = Date.now();
+let allTests = [];
+let titles = [];
+let currentTest = null;
+
+const colors = {
+  reset: '\x1b[0m',
+  red: '\x1b[31m',
+  green: '\x1b[32m',
+};
+
+/**
+ * @param {string} title
+ * @param {function()} fun
+ */
+function describe(title, fun) {
+  titles.push(title);
+  fun();
+  titles.pop();
+}
+
+/**
+ * @param {string} title
+ * @param {function()} fun
+ */
+function it(title, fun) {
+  titles.push(title);
+  allTests.push({
+    errors: [],
+    title: titles.join(' '),
+    fun,
+  });
+  titles.pop();
+}
+
+/**
+ * @param {string} msg
+ */
+function fail(msg) {
+  currentTest.errors.push(msg);
+}
+
+function runSpecs() {
+  console.log('Started\n');
+  for (currentTest of allTests) {
+    currentTest.fun();
+    if (currentTest.errors.length)
+      process.stdout.write(`${colors.red}F${colors.reset}`);
+    else
+      process.stdout.write(`${colors.green}.${colors.reset}`);
+  }
+  console.log('\n');
+  reportErrors();
+}
+
+function reportErrors() {
+  let failedTests = allTests.filter(test => !!test.errors.length);
+  if (failedTests.length) {
+    console.log('Failures:');
+    for (let i = 0; i < failedTests.length; ++i) {
+      let test = failedTests[i];
+      console.log(`${i + 1}) ${test.title}`);
+      console.log(`  Messages:`);
+      for (let error of test.errors) {
+        error = error.split('\n').join('\n      ');
+        console.log('    * ' + colors.red + error + colors.reset);
+      }
+    }
+    console.log('');
+  }
+
+  console.log(`Ran ${allTests.length} specs`);
+  console.log(`${allTests.length} specs, ${failedTests.length} failures`);
+  const runningTime = Date.now() - startTime;
+  console.log(`Finished in ${runningTime / 1000} seconds`);
+  process.exit(failedTests.length > 0 ? 1 : 0);
+}
+
+module.exports = {
+  describe,
+  it,
+  fail,
+  runSpecs,
+};
+

utils/doclint/test/01-missing-class/foo.js

@@ -0,0 +1,2 @@
+class Foo {
+}

utils/doclint/test/02-extra-class/doc.md

@@ -0,0 +1,3 @@
+### class: Foo
+
+### class: Bar

utils/doclint/test/02-extra-class/foo.js

@@ -0,0 +1,2 @@
+class Foo {
+}

utils/doclint/test/03-missing-method/doc.md

@@ -0,0 +1,2 @@
+### class: Foo
+

utils/doclint/test/03-missing-method/foo.js

@@ -0,0 +1,4 @@
+class Foo {
+  bar() {
+  }
+}

utils/doclint/test/04-extra-method/doc.md

@@ -0,0 +1,4 @@
+### class: Foo
+
+#### foo.bar()
+

utils/doclint/test/04-extra-method/foo.js

@@ -0,0 +1,2 @@
+class Foo {
+}

utils/doclint/test/05-missing-property/doc.md

@@ -0,0 +1,3 @@
+### class: Foo
+
+#### new Foo()

utils/doclint/test/05-missing-property/foo.js

@@ -0,0 +1,5 @@
+class Foo {
+  constructor() {
+    this.barProperty = 42;
+  }
+}

utils/doclint/test/06-extra-property/doc.md

@@ -0,0 +1,3 @@
+### class: Foo
+
+#### foo.bazProperty

utils/doclint/test/06-extra-property/foo.js

@@ -0,0 +1,4 @@
+class Foo {
+  constructor() {
+  }
+}

utils/doclint/test/07-bad-arguments/doc.md

@@ -0,0 +1,4 @@
+### class: Foo
+
+#### new Foo(arg2)
+- `arg2` <[string]> Some arg.

utils/doclint/test/07-bad-arguments/foo.js

@@ -0,0 +1,4 @@
+class Foo {
+  constructor(arg1) {
+  }
+}

utils/doclint/test/08-outdated-table-of-contents/doc.md

@@ -0,0 +1,8 @@
+<!-- toc -->
+
+- [class: Dialog](#class-dialog)
+
+<!-- tocstop -->
+
+### class: Foo
+

utils/doclint/test/08-outdated-table-of-contents/foo.js

@@ -0,0 +1,2 @@
+class Foo {
+}

utils/doclint/test/test.js

@@ -0,0 +1,73 @@
+const path = require('path');
+const jsBuilder = require('../JSBuilder');
+const mdBuilder = require('../MDBuilder');
+const Documentation = require('../Documentation');
+const Browser = require('../../../lib/Browser');
+
+const browser = new Browser({args: ['--no-sandbox']});
+let page;
+
+beforeAll(SX(async function() {
+  page = await browser.newPage();
+}));
+
+afterAll(SX(async function() {
+  await browser.close();
+}));
+
+describe('doclint', function() {
+  test('01-missing-class', diff => {
+    expect(diff.missingClasses.length).toBe(1);
+    expect(diff.missingClasses[0]).toBe('Foo');
+  });
+  test('02-extra-class', diff => {
+    expect(diff.extraClasses.length).toBe(1);
+    expect(diff.extraClasses[0]).toBe('Bar');
+  });
+  test('03-missing-method', diff => {
+    expect(diff.missingMethods.length).toBe(1);
+    expect(diff.missingMethods[0]).toBe('Foo.bar');
+  });
+  test('04-extra-method', diff => {
+    expect(diff.extraMethods.length).toBe(1);
+    expect(diff.extraMethods[0]).toBe('Foo.bar');
+  });
+  test('05-missing-property', diff => {
+    expect(diff.missingProperties.length).toBe(1);
+    expect(diff.missingProperties[0]).toBe('Foo.barProperty');
+  });
+  test('06-extra-property', diff => {
+    expect(diff.extraProperties.length).toBe(1);
+    expect(diff.extraProperties[0]).toBe('Foo.bazProperty');
+  });
+  test('07-bad-arguments', diff => {
+    expect(diff.badArguments.length).toBe(1);
+    expect(diff.badArguments[0]).toEqual({
+      method: 'Foo.constructor',
+      missingArgs: ['arg1'],
+      extraArgs: ['arg2']
+    });
+  });
+  test('08-outdated-table-of-contents', (diff, mdErrors) => {
+    expect(mdErrors.length).toBe(1);
+    expect(mdErrors[0]).toBe('Markdown TOC is outdated, run `yarn generate-toc`');
+  });
+});
+
+async function test(folderName, func) {
+  it(folderName, SX(async () => {
+    const [jsResult, mdResult] = await Promise.all([
+      jsBuilder(path.join(__dirname, folderName)),
+      mdBuilder(page, path.join(__dirname, folderName))
+    ]);
+    const jsDocumentation = jsResult;
+    const mdDocumentation = mdResult.documentation;
+    func(Documentation.diff(mdDocumentation, jsDocumentation), mdResult.errors);
+  }));
+}
+
+// Since Jasmine doesn't like async functions, they should be wrapped
+// in a SX function.
+function SX(fun) {
+  return done => Promise.resolve(fun()).then(done).catch(done.fail);
+}