[doclint] add linting for class properties

This patch: - adds linting for class properties - adds documentation for the missing class properties References #14.
2017-07-12 08:01:21 -07:00 · 2017-07-12 08:01:21 -07:00 · 38e6f53cc7
commit 38e6f53cc7
parent 60621b8815
6 changed files with 179 additions and 37 deletions
--- a/docs/api.md
+++ b/docs/api.md
@ -10,6 +10,8 @@
  * [browser.closePage(page)](#browserclosepagepage)
  * [browser.newPage()](#browsernewpage)
  * [browser.version()](#browserversion)
+  * [browser.stderr](#browserstderr)
+  * [browser.stdout](#browserstdout)
 - [class: Page](#class-page)
  * [page.addScriptTag(url)](#pageaddscripttagurl)
  * [page.click(selector)](#pageclickselector)
@ -42,6 +44,7 @@
  * [dialog.accept([promptText])](#dialogacceptprompttext)
  * [dialog.dismiss()](#dialogdismiss)
  * [dialog.message()](#dialogmessage)
+  * [dialog.type](#dialogtype)
 - [class: Frame](#class-frame)
  * [frame.childFrames()](#framechildframes)
  * [frame.evaluate(fun, ...args)](#frameevaluatefun-args)
@ -54,9 +57,21 @@
  * [frame.waitFor(selector)](#framewaitforselector)
 - [class: Request](#class-request)
  * [request.response()](#requestresponse)
+  * [request.headers](#requestheaders)
+  * [request.method](#requestmethod)
+  * [request.url](#requesturl)
 - [class: Response](#class-response)
+  * [response.headers](#responseheaders)
+  * [response.ok](#responseok)
+  * [response.status](#responsestatus)
+  * [response.statusText](#responsestatustext)
+  * [response.url](#responseurl)
  * [response.request()](#responserequest)
 - [class: InterceptedRequest](#class-interceptedrequest)
+  * [interceptedRequest.headers](#interceptedrequestheaders)
+  * [interceptedRequest.method](#interceptedrequestmethod)
+  * [interceptedRequest.url](#interceptedrequesturl)
+  * [interceptedRequest.postData](#interceptedrequestpostdata)
  * [interceptedRequest.abort()](#interceptedrequestabort)
  * [interceptedRequest.continue()](#interceptedrequestcontinue)
  * [interceptedRequest.isHandled()](#interceptedrequestishandled)
@ -108,9 +123,19 @@ Closes chromium application with all the pages (if any were opened). The browser
 Create a new page in browser and returns a promise which gets resolved with a Page object.

 #### browser.version()
-
 - returns: <[Promise]<[string]>>

+
+#### browser.stderr
+- <[stream.Readable]>
+
+A Readable Stream that represents the browser process's stderr.
+
+#### browser.stdout
+- <[stream.Readable]>
+
+A Readable Stream that represents the browser process's stdout.
+
 ### class: Page

 Page provides interface to interact with a tab in a browser. Pages are created by browser:
@ -285,8 +310,14 @@ Shortcut for [page.mainFrame().waitFor(selector)](#framewaitforselector).
 - `promptText` <[string]> A text to enter in prompt. Does not cause any effects if the dialog type is not prompt.

 #### dialog.dismiss()
+
 #### dialog.message()

+#### dialog.type
+- <[string]>
+
+Dialog's type, could be one of the `alert`, `beforeunload`, `confirm` and `prompt`.
+
 ### class: Frame
 #### frame.childFrames()
 #### frame.evaluate(fun, ...args)
@ -309,11 +340,74 @@ Shortcut for [page.mainFrame().waitFor(selector)](#framewaitforselector).

 ### class: Request
 #### request.response()
+#### request.headers
+- <[Headers]>
+
+Contains the associated [Headers] object of the request.
+
+#### request.method
+- <[string]>
+
+Contains the request's method (GET, POST, etc.)
+
+#### request.url
+- <[string]>
+
+Contains the URL of the request.

 ### class: Response
+
+#### response.headers
+- <[Headers]>
+
+Contains the [Headers] object associated with the response.
+
+#### response.ok
+- <[boolean]>
+
+Contains a boolean stating whether the response was successful (status in the range 200-299) or not.
+
+#### response.status
+- <[number]>
+
+Contains the status code of the response (e.g., 200 for a success).
+
+
+#### response.statusText
+- <[string]>
+
+Contains the status message corresponding to the status code (e.g., OK for 200).
+
+#### response.url
+- <[string]>
+
+Contains the URL of the response.
+
 #### response.request()

 ### class: InterceptedRequest
+
+#### interceptedRequest.headers
+- <[Headers]>
+
+Contains the [Headers] object associated with the request.
+
+#### interceptedRequest.method
+- <[string]>
+
+Contains the request's method (GET, POST, etc.)
+
+#### interceptedRequest.url
+- <[string]>
+
+Contains the URL of the request.
+
+
+#### interceptedRequest.postData
+- <[string]>
+
+In case of a `POST` request, contains `POST` data.
+
 #### interceptedRequest.abort()
 #### interceptedRequest.continue()
 #### interceptedRequest.isHandled()
@ -360,6 +454,8 @@ If there's already a header with name `name`, the header gets overwritten.
 [number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type "Number"
 [Object]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object "Object"
 [Page]: https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#class-page "Page"
+[Headers]: https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#class-headers "Headers"
 [InterceptedRequest]: https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#class-interceptedrequest "Page"
 [Promise]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise "Promise"
 [string]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type "String"
+[stream.Readable]: https://nodejs.org/api/stream.html#stream_class_stream_readable
--- a/lib/NetworkManager.js
+++ b/lib/NetworkManager.js
@ -291,7 +291,6 @@ class Request extends Body {
    this.url = payload.url;
    this.method = payload.method;
    this.headers = Headers.fromPayload(payload.headers);
-    this.postData = payload.postData;
  }

  /**
--- a/test/doclint/Documentation.js
+++ b/test/doclint/Documentation.js
@ -25,6 +25,8 @@ class Documentation {
    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);
@ -47,6 +49,11 @@ class Documentation {
          });
        }
      }
+      const actualProperties = actualClass.propertiesArray.slice().sort();
+      const expectedProperties = expectedClass.propertiesArray.slice().sort();
+      const propertyDiff = diff(actualProperties, expectedProperties);
+      result.extraProperties.push(...(propertyDiff.extra.map(propertyName => className + '.' + propertyName)));
+      result.missingProperties.push(...(propertyDiff.missing.map(propertyName => className + '.' + propertyName)));
    }
    return result;
  }
@ -56,13 +63,16 @@ Documentation.Class = class {
  /**
   * @param {string} name
   * @param {!Array<!Documentation.Method>} methodsArray
+   * @param {!Array<string>} propertiesArray
   */
-  constructor(name, methodsArray) {
+  constructor(name, methodsArray, propertiesArray) {
    this.name = name;
    this.methodsArray = methodsArray;
    this.methods = new Map();
    for (let method of methodsArray)
      this.methods.set(method.name, method);
+    this.propertiesArray = propertiesArray;
+    this.properties = new Set(propertiesArray);
  }
 };

--- a/test/doclint/JSBuilder.js
+++ b/test/doclint/JSBuilder.js
@ -9,6 +9,7 @@ class JSOutline {
    this.classes = [];
    this._currentClassName = null;
    this._currentClassMethods = [];
+    this._currentClassProperties = [];

    this._text = text;
    let ast = esprima.parseScript(this._text, {loc: true, range: true});
@ -36,15 +37,34 @@ class JSOutline {
    let methodName = this._extractText(node.key);
    let method = new Documentation.Method(methodName, args);
    this._currentClassMethods.push(method);
+    // Extract properties from constructor.
+    if (node.kind === 'constructor') {
+      let 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._currentClassProperties.push(node.property.name);
+      });
+      walker.walk(node);
+    }
+    return ESTreeWalker.SkipSubtree;
+  }
+
+  _onMemberExpression(node) {
+
  }

  _flushClassIfNeeded() {
    if (this._currentClassName === null)
      return;
-    let jsClass = new Documentation.Class(this._currentClassName, this._currentClassMethods);
+    let jsClass = new Documentation.Class(this._currentClassName, this._currentClassMethods, this._currentClassProperties);
    this.classes.push(jsClass);
    this._currentClassName = null;
    this._currentClassMethods = [];
+    this._currentClassProperties = [];
  }

  _extractText(node) {
--- a/test/doclint/MDBuilder.js
+++ b/test/doclint/MDBuilder.js
@ -24,22 +24,22 @@ class MDOutline {
    const classes = await page.evaluate(() => {
      let classes = [];
      let currentClass = {};
-      let method = {};
+      let member = {};
      for (let element of document.body.querySelectorAll('h3, h4, h4 + ul > li')) {
        if (element.matches('h3')) {
          currentClass = {
            name: element.textContent,
-            methods: [],
+            members: [],
          };
          classes.push(currentClass);
        } else if (element.matches('h4')) {
-          method = {
+          member = {
            name: element.textContent,
            args: []
          };
-          currentClass.methods.push(method);
+          currentClass.members.push(member);
        } else if (element.matches('li') && element.firstChild.matches && element.firstChild.matches('code')) {
-          method.args.push(element.firstChild.textContent);
+          member.args.push(element.firstChild.textContent);
        }
      }
      return classes;
@ -53,48 +53,56 @@ class MDOutline {
    const classHeading = /^class: (\w+)$/;
    const constructorRegex = /^new (\w+)\((.*)\)$/;
    const methodRegex = /^(\w+)\.(\w+)\((.*)\)$/;
+    const propertyRegex = /^(\w+)\.(\w+)$/;
    let currentClassName = null;
    let currentClassMethods = [];
+    let currentClassProperties = [];
    for (const cls of classes) {
      let match = cls.name.match(classHeading);
      currentClassName = match[1];
-      for (let mdMethod of cls.methods) {
-        let className = null;
-        let methodName = null;
-        let parameters = null;
-        let title = mdMethod.name;
-        if (constructorRegex.test(title)) {
-          let match = title.match(constructorRegex);
-          className = match[1];
-          parameters = match[2];
-          methodName = 'constructor';
-        } else if (methodRegex.test(title)) {
-          let match = title.match(methodRegex);
-          className = match[1];
-          methodName = match[2];
-          parameters = match[3];
+      for (let member of cls.members) {
+        if (constructorRegex.test(member.name)) {
+          let match = member.name.match(constructorRegex);
+          handleMethod.call(this, member, match[1], 'constructor', match[2]);
+        } else if (methodRegex.test(member.name)) {
+          let match = member.name.match(methodRegex);
+          handleMethod.call(this, member, match[1], match[2], match[3]);
+        } else if (propertyRegex.test(member.name)) {
+          let match = member.name.match(propertyRegex);
+          handleProperty.call(this, member, match[1], match[2]);
        }
-
-        if (!currentClassName || !className || !methodName || className.toLowerCase() !== currentClassName.toLowerCase()) {
-          console.warn('failed to process header as method: ' + mdMethod.name);
-          continue;
-        }
-        parameters = parameters.trim().replace(/[\[\]]/g, '');
-        if (parameters !== mdMethod.args.join(', '))
-          this.errors.push(`Heading arguments for "${mdMethod.name}" do not match described ones, i.e. "${parameters}" != "${mdMethod.args.join(', ')}"`);
-        let args = mdMethod.args.map(arg => new Documentation.Argument(arg));
-        let method = new Documentation.Method(methodName, args);
-        currentClassMethods.push(method);
      }
      flushClassIfNeeded.call(this);
    }

+    function handleMethod(member, className, methodName, parameters) {
+      if (!currentClassName || !className || !methodName || className.toLowerCase() !== currentClassName.toLowerCase()) {
+        this.errors.push(`Failed to process header as method: ${member.name}`);
+        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(', ')}"`);
+      let args = member.args.map(arg => new Documentation.Argument(arg));
+      let method = new Documentation.Method(methodName, args);
+      currentClassMethods.push(method);
+    }
+
+    function handleProperty(member, className, propertyName) {
+      if (!currentClassName || !className || !propertyName || className.toLowerCase() !== currentClassName.toLowerCase()) {
+        this.errors.push(`Failed to process header as property: ${member.name}`);
+        return;
+      }
+      currentClassProperties.push(propertyName);
+    }
+
    function flushClassIfNeeded() {
      if (currentClassName === null)
        return;
-      this.classes.push(new Documentation.Class(currentClassName, currentClassMethods));
+      this.classes.push(new Documentation.Class(currentClassName, currentClassMethods, currentClassProperties));
      currentClassName = null;
      currentClassMethods = [];
+      currentClassProperties = [];
    }
  }
 }
--- a/test/doclint/lint.js
+++ b/test/doclint/lint.js
@ -42,7 +42,8 @@ function filterJSDocumentation(jsDocumentation) {
        return false;
      return !EXCLUDE_METHODS.has(`${cls.name}.${method.name}`);
    });
-    classes.push(new Documentation.Class(cls.name, methods));
+    let properties = cls.propertiesArray.filter(property => !property.startsWith('_'));
+    classes.push(new Documentation.Class(cls.name, methods, properties));
  }
  return new Documentation(classes);
 }
@ -132,6 +133,14 @@ describe('Markdown Documentation', function() {
      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