feat(rpc): run doclint against rpc client (microsoft#3260)

dgozman · web-flow · commit 126b1f79d43e · 2020-07-31T17:00:36.000-07:00
diff --git a/package.json b/package.json
@@ -17,8 +17,9 @@
     "tsc": "tsc -p .",
     "tsc-installer": "tsc -p ./src/install/tsconfig.json",
     "doc": "node utils/doclint/cli.js",
+    "doc-channel": "node utils/doclint/cli.js --channel",
     "test-infra": "node utils/doclint/check_public_api/test/test.js && node utils/doclint/preprocessor/test.js && node utils/testrunner/test/test.js",
-    "lint": "npm run eslint && npm run tsc && npm run doc && npm run check-deps && npm run generate-channels && npm run test-types && npm run test-infra",
+    "lint": "npm run eslint && npm run tsc && npm run doc && npm run doc-channel && npm run check-deps && npm run generate-channels && npm run test-types && npm run test-infra",
     "debug-test": "node --inspect-brk test/test.js",
     "clean": "rimraf lib && rimraf types",
     "prepare": "node install-from-github.js",
diff --git a/src/rpc/client/api.ts b/src/rpc/client/api.ts
@@ -0,0 +1,44 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * 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.
+ */
+
+export { Accessibility } from './accessibility';
+export { Browser } from './browser';
+export { BrowserContext } from './browserContext';
+export { BrowserServer } from './browserServer';
+export { BrowserType } from './browserType';
+export { ConsoleMessage } from './consoleMessage';
+export { Dialog } from './dialog';
+export { Download } from './download';
+export { ElementHandle } from './elementHandle';
+export { FileChooser } from './fileChooser';
+export { Logger } from '../../loggerSink';
+export { TimeoutError } from '../../errors';
+export { Frame } from './frame';
+export { Keyboard, Mouse } from './input';
+export { JSHandle } from './jsHandle';
+export { Request, Response, Route } from './network';
+export { Page } from './page';
+export { Selectors } from './selectors';
+export { Worker } from './worker';
+
+export { ChromiumBrowser } from './chromiumBrowser';
+export { ChromiumBrowserContext } from './chromiumBrowserContext';
+export { ChromiumCoverage } from './chromiumCoverage';
+export { CDPSession } from './cdpSession';
+
+export { WebKitBrowser } from './webkitBrowser';
+
+export { FirefoxBrowser } from './firefoxBrowser';
diff --git a/src/rpc/client/browserContext.ts b/src/rpc/client/browserContext.ts
@@ -169,15 +169,15 @@ export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserC
     });
   }
 
-  async exposeBinding(name: string, binding: frames.FunctionWithSource): Promise<void> {
+  async exposeBinding(name: string, playwrightBinding: frames.FunctionWithSource): Promise<void> {
     return this._wrapApiCall('browserContext.exposeBinding', async () => {
       for (const page of this.pages()) {
         if (page._bindings.has(name))
           throw new Error(`Function "${name}" has been already registered in one of the pages`);
       }
       if (this._bindings.has(name))
         throw new Error(`Function "${name}" has been already registered`);
-      this._bindings.set(name, binding);
+      this._bindings.set(name, playwrightBinding);
       await this._channel.exposeBinding({ name });
     });
   }
diff --git a/src/rpc/client/browserType.ts b/src/rpc/client/browserType.ts
@@ -22,7 +22,7 @@ import { BrowserServer } from './browserServer';
 import { headersObjectToArray, envObjectToArray } from '../../converters';
 import { serializeArgument } from './jsHandle';
 import { assert } from '../../helper';
-import { LaunchOptions, LaunchServerOptions, BrowserContextOptions, ConnectOptions } from './types';
+import { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions } from './types';
 
 export class BrowserType extends ChannelOwner<BrowserTypeChannel, BrowserTypeInitializer> {
 
@@ -76,7 +76,7 @@ export class BrowserType extends ChannelOwner<BrowserTypeChannel, BrowserTypeIni
     }, logger);
   }
 
-  async launchPersistentContext(userDataDir: string, options: LaunchOptions & BrowserContextOptions = {}): Promise<BrowserContext> {
+  async launchPersistentContext(userDataDir: string, options: LaunchPersistentContextOptions = {}): Promise<BrowserContext> {
     const logger = options.logger;
     options = { ...options, logger: undefined };
     return this._wrapApiCall('browserType.launchPersistentContext', async () => {
diff --git a/src/rpc/client/electron.ts b/src/rpc/client/electron.ts
@@ -25,6 +25,11 @@ import { Events } from './events';
 import { envObjectToArray } from '../../converters';
 import { WaitForEventOptions, Env, LoggerSink } from './types';
 
+type ElectronOptions = Omit<ElectronLaunchOptions, 'env'> & {
+  env?: Env,
+  logger?: LoggerSink,
+};
+
 export class Electron extends ChannelOwner<ElectronChannel, ElectronInitializer> {
   static from(electron: ElectronChannel): Electron {
     return (electron as any)._object;
@@ -34,7 +39,7 @@ export class Electron extends ChannelOwner<ElectronChannel, ElectronInitializer>
     super(parent, type, guid, initializer);
   }
 
-  async launch(executablePath: string, options: ElectronLaunchOptions & { env?: Env, logger?: LoggerSink } = {}): Promise<ElectronApplication> {
+  async launch(executablePath: string, options: ElectronOptions = {}): Promise<ElectronApplication> {
     const logger = options.logger;
     options = { ...options, logger: undefined };
     return this._wrapApiCall('electron.launch', async () => {
diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { ElementHandleChannel, JSHandleInitializer, ElementHandleScrollIntoViewIfNeededOptions, ElementHandleHoverOptions, ElementHandleClickOptions, ElementHandleDblclickOptions, ElementHandleSelectOptionOptions, ElementHandleFillOptions, ElementHandleSetInputFilesOptions, ElementHandlePressOptions, ElementHandleCheckOptions, ElementHandleUncheckOptions, ElementHandleScreenshotOptions, ElementHandleTypeOptions } from '../channels';
+import { ElementHandleChannel, JSHandleInitializer, ElementHandleScrollIntoViewIfNeededOptions, ElementHandleHoverOptions, ElementHandleClickOptions, ElementHandleDblclickOptions, ElementHandleFillOptions, ElementHandleSetInputFilesOptions, ElementHandlePressOptions, ElementHandleCheckOptions, ElementHandleUncheckOptions, ElementHandleScreenshotOptions, ElementHandleTypeOptions, ElementHandleSelectTextOptions } from '../channels';
 import { Frame } from './frame';
 import { FuncOn, JSHandle, serializeArgument, parseResult } from './jsHandle';
 import { ChannelOwner } from './channelOwner';
@@ -123,7 +123,7 @@ export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
     });
   }
 
-  async selectText(options: ElementHandleSelectOptionOptions = {}): Promise<void> {
+  async selectText(options: ElementHandleSelectTextOptions = {}): Promise<void> {
     return this._wrapApiCall('elementHandle.selectText', async () => {
       await this._elementChannel.selectText(options);
     });
diff --git a/src/rpc/client/jsHandle.ts b/src/rpc/client/jsHandle.ts
@@ -62,8 +62,8 @@ export class JSHandle<T = any> extends ChannelOwner<JSHandleChannel, JSHandleIni
     return JSHandle.from(result.handle) as SmartHandle<R>;
   }
 
-  async getProperty(name: string): Promise<JSHandle> {
-    const result = await this._channel.getProperty({ name });
+  async getProperty(propertyName: string): Promise<JSHandle> {
+    const result = await this._channel.getProperty({ name: propertyName });
     return JSHandle.from(result.handle);
   }
 
diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts
@@ -278,13 +278,13 @@ export class Page extends ChannelOwner<PageChannel, PageInitializer> {
     await this.exposeBinding(name, (options, ...args: any) => playwrightFunction(...args));
   }
 
-  async exposeBinding(name: string, binding: FunctionWithSource) {
+  async exposeBinding(name: string, playwrightBinding: FunctionWithSource) {
     return this._wrapApiCall('page.exposeBinding', async () => {
       if (this._bindings.has(name))
         throw new Error(`Function "${name}" has been already registered`);
       if (this._browserContext._bindings.has(name))
         throw new Error(`Function "${name}" has been already registered in the browser context`);
-      this._bindings.set(name, binding);
+      this._bindings.set(name, playwrightBinding);
       await this._channel.exposeBinding({ name });
     });
   }
diff --git a/src/rpc/client/types.ts b/src/rpc/client/types.ts
@@ -49,9 +49,13 @@ export type BrowserContextOptions = Omit<BrowserNewContextOptions, 'viewport' |
 type LaunchOverrides = {
   ignoreDefaultArgs?: boolean | string[],
   env?: Env,
-  firefoxUserPrefs?: { [key: string]: string | number | boolean },
   logger?: LoggerSink,
 };
-export type LaunchOptions = Omit<BrowserTypeLaunchOptions, 'ignoreAllDefaultArgs' | 'ignoreDefaultArgs' | 'env' | 'firefoxUserPrefs'> & LaunchOverrides;
-export type LaunchServerOptions = Omit<BrowserTypeLaunchServerOptions, 'ignoreAllDefaultArgs' | 'ignoreDefaultArgs' | 'env' | 'firefoxUserPrefs'> & LaunchOverrides;
+type FirefoxUserPrefs = {
+  firefoxUserPrefs?: { [key: string]: string | number | boolean },
+};
+type LaunchOptionsBase = Omit<BrowserTypeLaunchOptions, 'ignoreAllDefaultArgs' | 'ignoreDefaultArgs' | 'env' | 'firefoxUserPrefs'> & LaunchOverrides;
+export type LaunchOptions = LaunchOptionsBase & FirefoxUserPrefs;
+export type LaunchServerOptions = Omit<BrowserTypeLaunchServerOptions, 'ignoreAllDefaultArgs' | 'ignoreDefaultArgs' | 'env' | 'firefoxUserPrefs'> & LaunchOverrides & FirefoxUserPrefs;
+export type LaunchPersistentContextOptions = LaunchOptionsBase & BrowserContextOptions;
 export type ConnectOptions = BrowserTypeConnectParams & { logger?: LoggerSink };
diff --git a/utils/doclint/check_public_api/Documentation.js b/utils/doclint/check_public_api/Documentation.js
@@ -132,7 +132,6 @@ Documentation.Member = class {
    * @param {string[]=} templates
    */
   constructor(kind, name, type, argsArray, comment = '', returnComment = '', required = true, templates = []) {
-    if (name === 'code') debugger;
     this.kind = kind;
     this.name = name;
     this.type = type;
diff --git a/utils/doclint/check_public_api/JSBuilder.js b/utils/doclint/check_public_api/JSBuilder.js
@@ -99,7 +99,7 @@ function checkSources(sources) {
           parent = parent.parent;
         className = path.basename(parent.fileName,  '.js');
       }
-      if (className && !excludeClasses.has(className)) {
+      if (className && !excludeClasses.has(className) && !fileName.endsWith('/protocol.ts')) {
         excludeClasses.add(className);
         const renamed = expandPrefix(className);
         classes.push(serializeClass(renamed, symbol, node));
@@ -194,7 +194,7 @@ function checkSources(sources) {
     } else if (isRegularObject(type)) {
       let properties = undefined;
       if (!circular.includes(typeName))
-        properties = type.getProperties().map(property => serializeSymbol(property, nextCircular));
+        properties = getTypeProperties(type).map(property => serializeSymbol(property, nextCircular));
       return new Documentation.Type('Object', properties);
     }
     if (type.isUnion() && (typeName.includes('|') || type.types.every(type => type.isStringLiteral() || type.intrinsicName === 'number'))) {
@@ -284,6 +284,26 @@ function checkSources(sources) {
   function serializeProperty(name, type) {
     return Documentation.Member.createProperty(name, serializeType(type));
   }
+
+  /**
+   * @param {!ts.Type} type
+   */
+  function getTypeProperties(type) {
+    if (type.aliasSymbol && type.aliasSymbol.escapedName === 'Pick') {
+      const props = getTypeProperties(type.aliasTypeArguments[0]);
+      const pickNames = type.aliasTypeArguments[1].types.map(t => t.value);
+      return props.filter(p => pickNames.includes(p.getName()));
+    }
+    if (!type.isIntersection())
+      return type.getProperties();
+    let props = [];
+    for (const innerType of type.types) {
+      let innerProps = getTypeProperties(innerType);
+      props = props.filter(p => !innerProps.find(e => e.getName() === p.getName()));
+      props.push(...innerProps);
+    }
+    return props;
+  }
 }
 
 function expandPrefix(name) {
diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js
@@ -18,14 +18,9 @@ const jsBuilder = require('./JSBuilder');
 const mdBuilder = require('./MDBuilder');
 const Documentation = require('./Documentation');
 const Message = require('../Message');
-const path = require('path');
 
 const EXCLUDE_PROPERTIES = new Set([
-  'Browser.create',
-  'Headers.fromPayload',
-  'Page.create',
   'JSHandle.toString',
-  'TimeoutError.name',
 ]);
 
 /**
diff --git a/utils/doclint/cli.js b/utils/doclint/cli.js
@@ -36,6 +36,9 @@ run();
 async function run() {
   const startTime = Date.now();
   const onlyBrowserVersions = process.argv.includes('--only-browser-versions');
+  const channel = process.argv.includes('--channel');
+  if (channel)
+    console.warn(`${YELLOW_COLOR}NOTE: checking documentation against //src/rpc/client${RESET_COLOR}`);
 
   /** @type {!Array<!Message>} */
   const messages = [];
@@ -66,8 +69,13 @@ async function run() {
       const browser = await playwright.chromium.launch();
       const page = await browser.newPage();
       const checkPublicAPI = require('./check_public_api');
-      const rpcDir = path.join(PROJECT_DIR, 'src', 'rpc');
-      const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'src'), '', [rpcDir]);
+      let jsSources;
+      if (channel) {
+        jsSources = await Source.readdir(path.join(PROJECT_DIR, 'src', 'rpc', 'client'), '', []);
+      } else {
+        const rpcDir = path.join(PROJECT_DIR, 'src', 'rpc');
+        jsSources = await Source.readdir(path.join(PROJECT_DIR, 'src'), '', [rpcDir]);
+      }
       messages.push(...await checkPublicAPI(page, [api], jsSources));
       await browser.close();
     }

Original file line number	Diff line number	Diff line change
`@@ -169,15 +169,15 @@ export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserC`
`169`	`169`	`});`
`170`	`170`	`}`
`171`	`171`
`172`		`- async exposeBinding(name: string, binding: frames.FunctionWithSource): Promise<void> {`
	`172`	`+ async exposeBinding(name: string, playwrightBinding: frames.FunctionWithSource): Promise<void> {`
`173`	`173`	`return this._wrapApiCall('browserContext.exposeBinding', async () => {`
`174`	`174`	`for (const page of this.pages()) {`
`175`	`175`	`if (page._bindings.has(name))`
`176`	`176`	throw new Error(`Function "${name}" has been already registered in one of the pages`);
`177`	`177`	`}`
`178`	`178`	`if (this._bindings.has(name))`
`179`	`179`	throw new Error(`Function "${name}" has been already registered`);
`180`		`- this._bindings.set(name, binding);`
	`180`	`+ this._bindings.set(name, playwrightBinding);`
`181`	`181`	`await this._channel.exposeBinding({ name });`
`182`	`182`	`});`
`183`	`183`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,8 +62,8 @@ export class JSHandle<T = any> extends ChannelOwner<JSHandleChannel, JSHandleIni`
`62`	`62`	`return JSHandle.from(result.handle) as SmartHandle<R>;`
`63`	`63`	`}`
`64`	`64`
`65`		`- async getProperty(name: string): Promise<JSHandle> {`
`66`		`- const result = await this._channel.getProperty({ name });`
	`65`	`+ async getProperty(propertyName: string): Promise<JSHandle> {`
	`66`	`+ const result = await this._channel.getProperty({ name: propertyName });`
`67`	`67`	`return JSHandle.from(result.handle);`
`68`	`68`	`}`
`69`	`69`