(core) Configure more comprehensive eslint rules for Typescript

Summary:
- Update rules to be more like we've had with tslint
- Switch tsserver plugin to eslint (tsserver makes for a much faster way to lint in editors)
- Apply suggested auto-fixes
- Fix all lint errors and warnings in core/, app/, test/

Test Plan: Some behavior may change subtly (e.g. added missing awaits), relying on existing tests to catch problems.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2785

app/server/lib/ActiveDoc.ts

@@ -715,7 +715,7 @@ export class ActiveDoc extends EventEmitter {
    */
   public async getFormulaError(docSession: DocSession, tableId: string, colId: string,
                                rowId: number): Promise<CellValue> {
-    if (!this._granularAccess.hasTableAccess(docSession, tableId)) { return null; }
+    if (!await this._granularAccess.hasTableAccess(docSession, tableId)) { return null; }
     this.logInfo(docSession, "getFormulaError(%s, %s, %s, %s)",
       docSession, tableId, colId, rowId);
     await this.waitForInitialization();
@@ -1094,7 +1094,7 @@ export class ActiveDoc extends EventEmitter {
     ]);
 
     // Migrate the document if needed.
-    const values = marshal.loads(docInfoData!);
+    const values = marshal.loads(docInfoData);
     const versionCol = values.schemaVersion;
     const docSchemaVersion = (versionCol && versionCol.length === 1 ? versionCol[0] : 0);
     if (docSchemaVersion < schemaVersion) {
@@ -1126,7 +1126,7 @@ export class ActiveDoc extends EventEmitter {
     const tableNames: string[] = await this._rawPyCall('load_meta_tables', tablesData, columnsData);
 
     // Figure out which tables are on-demand.
-    const tablesParsed: BulkColValues = marshal.loads(tablesData!);
+    const tablesParsed: BulkColValues = marshal.loads(tablesData);
     const onDemandMap = zipObject(tablesParsed.tableId as string[], tablesParsed.onDemand);
     const onDemandNames = remove(tableNames, (t) => onDemandMap[t]);
 
@@ -1183,7 +1183,7 @@ export class ActiveDoc extends EventEmitter {
 
     const result: ApplyUAResult = await new Promise<ApplyUAResult>(
       (resolve, reject) =>
-        this._sharing!.addUserAction({action, docSession, resolve, reject}));
+        this._sharing.addUserAction({action, docSession, resolve, reject}));
     this.logDebug(docSession, "_applyUserActions returning %s", shortDesc(result));
 
     if (result.isModification) {

app/server/lib/AppEndpoint.ts

@@ -4,7 +4,7 @@
  * of the client-side code.
  */
 import * as express from 'express';
-import fetch, {RequestInit, Response as FetchResponse} from 'node-fetch';
+import fetch, {Response as FetchResponse, RequestInit} from 'node-fetch';
 
 import {ApiError} from 'app/common/ApiError';
 import {getSlugIfNeeded, parseSubdomainStrictly} from 'app/common/gristUrls';

app/server/lib/DocApi.ts

@@ -215,10 +215,9 @@ export class DocWorkerApi {
 
     // Initiate a fork.  Used internally to implement ActiveDoc.fork.  Only usable via a Permit.
     this._app.post('/api/docs/:docId/create-fork', canEdit, throttled(async (req, res) => {
-      const mreq = req as RequestWithLogin;
       const docId = stringParam(req.params.docId);
       const srcDocId = stringParam(req.body.srcDocId);
-      if (srcDocId !== mreq.specialPermit?.otherDocId) { throw new Error('access denied'); }
+      if (srcDocId !== req.specialPermit?.otherDocId) { throw new Error('access denied'); }
       await this._docManager.storageManager.prepareFork(srcDocId, docId);
       res.json({srcDocId, docId});
     }));
@@ -249,7 +248,7 @@ export class DocWorkerApi {
     this._app.post('/api/docs/:docId/recover', canEdit, throttled(async (req, res) => {
       const recoveryModeRaw = req.body.recoveryMode;
       const recoveryMode = (typeof recoveryModeRaw === 'boolean') ? recoveryModeRaw : undefined;
-      if (!this._isOwner(req)) { throw new Error('Only owners can control recovery mode'); }
+      if (!await this._isOwner(req)) { throw new Error('Only owners can control recovery mode'); }
       const activeDoc = await this._docManager.fetchDoc(docSessionFromRequest(req), getDocId(req), recoveryMode);
       res.json({
         recoveryMode: activeDoc.recoveryMode

app/server/lib/DocManager.ts

@@ -16,7 +16,8 @@ import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager';
 import {assertAccess, Authorizer, DocAuthorizer, DummyAuthorizer,
         isSingleUserMode} from 'app/server/lib/Authorizer';
 import {Client} from 'app/server/lib/Client';
-import {getDocSessionCachedDoc, makeExceptionalDocSession, makeOptDocSession, OptDocSession} from 'app/server/lib/DocSession';
+import {getDocSessionCachedDoc, makeExceptionalDocSession, makeOptDocSession} from 'app/server/lib/DocSession';
+import {OptDocSession} from 'app/server/lib/DocSession';
 import * as docUtils from 'app/server/lib/docUtils';
 import {GristServer} from 'app/server/lib/GristServer';
 import {IDocStorageManager} from 'app/server/lib/IDocStorageManager';
@@ -498,7 +499,7 @@ export class DocManager extends EventEmitter {
         // TODO: We should be skeptical of the upload file to close a possible
         // security vulnerability. See https://phab.getgrist.com/T457.
         const docName = await this._createNewDoc(id);
-        const docPath = await this.storageManager.getPath(docName);
+        const docPath: string = this.storageManager.getPath(docName);
         await docUtils.copyFile(uploadInfo.files[0].absPath, docPath);
         await this.storageManager.addToStorage(docName);
         return {title: basename, id: docName};

app/server/lib/DocPluginManager.ts

@@ -65,7 +65,12 @@ export class DocPluginManager {
   private _pluginInstances: PluginInstance[];
 
 
-  constructor(private _localPlugins: LocalPlugin[], private _appRoot: string, private _activeDoc: ActiveDoc, private _server: GristServer) {
+  constructor(
+    private _localPlugins: LocalPlugin[],
+    private _appRoot: string,
+    private _activeDoc: ActiveDoc,
+    private _server: GristServer
+  ) {
     this.gristDocAPI = new GristDocAPIImpl(_activeDoc);
     this._pluginInstances = [];
     this.ready = this._initialize();

app/server/lib/DocStorage.ts

@@ -439,7 +439,9 @@ export class DocStorage implements ISQLiteDB {
    * Note that SQLite may contain tables that aren't used for Grist data (e.g. attachments), for
    * which such encoding/marshalling is not used, and e.g. binary data is stored to BLOBs directly.
    */
-  private static _encodeValue(marshaller: marshal.Marshaller, sqlType: string, val: any): Uint8Array|string|number|boolean {
+  private static _encodeValue(
+    marshaller: marshal.Marshaller, sqlType: string, val: any
+  ): Uint8Array|string|number|boolean {
     const marshalled = () => {
       marshaller.marshal(val);
       return marshaller.dump();
@@ -1376,7 +1378,7 @@ export class DocStorage implements ISQLiteDB {
     // columns, or adding or removing or changing default values on a column."
     const row = await this.get("PRAGMA schema_version");
     assert(row && row.schema_version, "Could not retrieve schema_version.");
-    const newSchemaVersion = row!.schema_version + 1;
+    const newSchemaVersion = row.schema_version + 1;
     const tmpTableId = DocStorage._makeTmpTableId(tableId);
     await this._getDB().runEach(
       "PRAGMA writable_schema=ON",
@@ -1462,7 +1464,7 @@ export class DocStorage implements ISQLiteDB {
    * should be reasonably fast:
    *   https://sqlite.org/tempfiles.html#temp_databases
    */
-  public async _fetchQueryWithManyParameters(query: ExpandedQuery): Promise<Buffer> {
+  private async _fetchQueryWithManyParameters(query: ExpandedQuery): Promise<Buffer> {
     const db = this._getDB();
     return db.execTransaction(async () => {
       const tableNames: string[] = [];
@@ -1490,7 +1492,7 @@ export class DocStorage implements ISQLiteDB {
    * Construct SQL for an ExpandedQuery.  Expects that filters have been converted into
    * a set of WHERE terms that should be ANDed.
    */
-  public _getSqlForQuery(query: ExpandedQuery, whereParts: string[]) {
+  private _getSqlForQuery(query: ExpandedQuery, whereParts: string[]) {
     const whereClause = whereParts.length > 0 ? `WHERE ${whereParts.join(' AND ')}` : '';
     const limitClause = (typeof query.limit === 'number') ? `LIMIT ${query.limit}` : '';
     const joinClauses = query.joins ? query.joins.join(' ') : '';

app/server/lib/DocStorageManager.ts

@@ -196,7 +196,7 @@ export class DocStorageManager implements IDocStorageManager {
    * Electron version only. Shows the given doc in the file explorer.
    */
   public async showItemInFolder(docName: string): Promise<void> {
-    this._shell.showItemInFolder(await this.getPath(docName));
+    this._shell.showItemInFolder(this.getPath(docName));
   }
 
   public async closeStorage() {

app/server/lib/ExternalStorage.ts

@@ -133,7 +133,7 @@ export class KeyMappedExternalStorage implements ExternalStorage {
 export class ChecksummedExternalStorage implements ExternalStorage {
   private _closed: boolean = false;
 
-  constructor(readonly label: string, private _ext: ExternalStorage, private _options: {
+  constructor(public readonly label: string, private _ext: ExternalStorage, private _options: {
     maxRetries: number,         // how many time to retry inconsistent downloads
     initialDelayMs: number,     // how long to wait before retrying
     localHash: PropStorage,     // key/value store for hashes of downloaded content

app/server/lib/FlexServer.ts

@@ -145,7 +145,7 @@ export class FlexServer implements GristServer {
   private _sendAppPage: (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
 
   constructor(public port: number, public name: string = 'flexServer',
-              readonly options: FlexServerOptions = {}) {
+              public readonly options: FlexServerOptions = {}) {
     this.app = express();
     this.app.set('port', port);
     this.appRoot = getAppRoot();

app/server/lib/GranularAccess.ts

@@ -724,7 +724,9 @@ export class GranularAccess implements GranularAccessForBundle {
     const ruler = await this._getRuler(cursor);
     const tableId = getTableId(action);
     const ruleSets = ruler.ruleCollection.getAllColumnRuleSets(tableId);
-    const colIds = new Set(([] as string[]).concat(...ruleSets.map(ruleSet => ruleSet.colIds === '*' ? [] : ruleSet.colIds)));
+    const colIds = new Set(([] as string[]).concat(
+      ...ruleSets.map(ruleSet => ruleSet.colIds === '*' ? [] : ruleSet.colIds)
+    ));
     const access = await ruler.getAccess(cursor.docSession);
     // Check columns in a consistent order, for determinism (easier testing).
     // TODO: could pool some work between columns by doing them together rather than one by one.
@@ -1164,7 +1166,9 @@ export class GranularAccess implements GranularAccessForBundle {
       const rowsBefore = cloneDeep(tableData?.getTableDataAction() || ['TableData', '', [], {}] as TableDataAction);
       docData.receiveAction(docAction);
       // If table is deleted, state afterwards doesn't matter.
-      const rowsAfter = docData.getTable(tableId) ? cloneDeep(tableData?.getTableDataAction() || ['TableData', '', [], {}] as TableDataAction) : rowsBefore;
+      const rowsAfter = docData.getTable(tableId) ?
+        cloneDeep(tableData?.getTableDataAction() || ['TableData', '', [], {}] as TableDataAction) :
+        rowsBefore;
       const step: ActionStep = {action: docAction, rowsBefore, rowsAfter};
       steps.push(step);
     }
@@ -1208,7 +1212,7 @@ export class GranularAccess implements GranularAccessForBundle {
     if (applied) {
       // Rules may have changed - back them off to a copy of their original state.
       ruler = new Ruler(this);
-      ruler.update(metaDocData);
+      await ruler.update(metaDocData);
     }
     let replaceRuler = false;
     for (const docAction of docActions) {
@@ -1228,7 +1232,7 @@ export class GranularAccess implements GranularAccessForBundle {
         replaceRuler = true;
       } else if (replaceRuler) {
         ruler = new Ruler(this);
-        ruler.update(metaDocData);
+        await ruler.update(metaDocData);
         replaceRuler = false;
       }
       step.ruler = ruler;
@@ -1625,7 +1629,8 @@ export class CensorshipInfo {
       const tableId = tableRefToTableId.get(tableRef);
       if (!tableId) { throw new Error('table not found'); }
       const colId = rec.get('colId') as string;
-      if (this.censoredTables.has(tableRef) || (colId !== 'manualSort' && permInfo.getColumnAccess(tableId, colId).perms.read === 'deny')) {
+      if (this.censoredTables.has(tableRef) ||
+          (colId !== 'manualSort' && permInfo.getColumnAccess(tableId, colId).perms.read === 'deny')) {
         censoredColumnCodes.add(columnCode(tableRef, colId));
       }
     }

app/server/lib/HostedStorageManager.ts

@@ -239,7 +239,7 @@ export class HostedStorageManager implements IDocStorageManager {
     await this.prepareLocalDoc(docName, 'new');
     if (this._inventory) {
       await this._inventory.create(docName);
-      this._onInventoryChange(docName);
+      await this._onInventoryChange(docName);
     }
     this.markAsChanged(docName);
   }

app/server/lib/ITestingHooks-ti.ts

@@ -10,18 +10,14 @@ export const ITestingHooks = t.iface([], {
   "updateAuthToken": t.func("void", t.param("instId", "string"), t.param("authToken", "string")),
   "getAuthToken": t.func(t.union("string", "null"), t.param("instId", "string")),
   "useTestToken": t.func("void", t.param("instId", "string"), t.param("token", "string")),
-  "setLoginSessionProfile": t.func("void", t.param("gristSidCookie", "string"),
-                                   t.param("profile", t.union("UserProfile", "null")), t.param("org", "string", true)),
+  "setLoginSessionProfile": t.func("void", t.param("gristSidCookie", "string"), t.param("profile", t.union("UserProfile", "null")), t.param("org", "string", true)),
   "setServerVersion": t.func("void", t.param("version", t.union("string", "null"))),
   "disconnectClients": t.func("void"),
   "commShutdown": t.func("void"),
   "commRestart": t.func("void"),
   "commSetClientPersistence": t.func("void", t.param("ttlMs", "number")),
   "closeDocs": t.func("void"),
-  "setDocWorkerActivation": t.func("void", t.param("workerId", "string"),
-                                   t.param("active", t.union(t.lit('active'),
-                                                             t.lit('inactive'),
-                                                             t.lit('crash')))),
+  "setDocWorkerActivation": t.func("void", t.param("workerId", "string"), t.param("active", t.union(t.lit('active'), t.lit('inactive'), t.lit('crash')))),
   "flushAuthorizerCache": t.func("void"),
   "getDocClientCounts": t.func(t.array(t.tuple("string", "number"))),
   "setActiveDocTimeout": t.func("number", t.param("seconds", "number")),
@@ -29,6 +25,5 @@ export const ITestingHooks = t.iface([], {
 
 const exportedTypeSuite: t.ITypeSuite = {
   ITestingHooks,
-  UserProfile: t.name("object"),
 };
 export default exportedTypeSuite;

app/server/lib/ITestingHooks.ts

@@ -1,8 +1,8 @@
 import {UserProfile} from 'app/common/LoginSessionAPI';
 
 export interface ITestingHooks {
-  getOwnPort(): number;
-  getPort(): number;
+  getOwnPort(): Promise<number>;
+  getPort(): Promise<number>;
   updateAuthToken(instId: string, authToken: string): Promise<void>;
   getAuthToken(instId: string): Promise<string|null>;
   useTestToken(instId: string, token: string): Promise<void>;

app/server/lib/OnDemandActions.ts

@@ -62,7 +62,7 @@ export class OnDemandActions {
       // Check that the actionType can be applied without the sandbox and also that the action
       // is on a data table.
       const isOnDemandAction = ACTION_TYPES.has(a[0] as string);
-      const isDataTableAction = typeof a[1] === 'string' && !(a[1] as string).startsWith('_grist_');
+      const isDataTableAction = typeof a[1] === 'string' && !a[1].startsWith('_grist_');
       if (a[0] === 'ApplyUndoActions') {
         // Split actions inside the undo action array.
         const [undoNormal, undoOnDemand] = this.splitByOnDemand(a[1] as UserAction[]);

app/server/lib/Sharing.ts

@@ -155,7 +155,7 @@ export class Sharing {
   private async _rebaseLocalActions(): Promise<void> {
     const rebaseQueue: Deque<UserActionBundle> = new Deque<UserActionBundle>();
     try {
-      await this.createCheckpoint();
+      this.createCheckpoint();
       const actions: LocalActionBundle[] = await this._actionHistory.fetchAllLocal();
       assert(actions.length > 0);
       await this.doApplyUserActionBundle(this._createUndo(actions), null);
@@ -163,7 +163,7 @@ export class Sharing {
       await this._actionHistory.clearLocalActions();
     } catch (e) {
       log.error("Can't undo local actions; sharing is off");
-      await this.rollbackToCheckpoint();
+      this.rollbackToCheckpoint();
       // TODO this.disconnect();
       // TODO errorState = true;
       return;
@@ -185,11 +185,11 @@ export class Sharing {
       }
     }
     if (rebaseFailures.length > 0) {
-      await this.createBackupAtCheckpoint();
+      this.createBackupAtCheckpoint();
       // TODO we should notify the user too.
       log.error('Rebase failed to reapply some of your actions, backup of local at...');
     }
-    await this.releaseCheckpoint();
+    this.releaseCheckpoint();
   }
 
   // ======================================================================
@@ -374,7 +374,9 @@ export class Sharing {
     const docActions = getEnvContent(sandboxActionBundle.stored).concat(
       getEnvContent(sandboxActionBundle.calc));
 
-    const accessControl = this._activeDoc.getGranularAccessForBundle(docSession || makeExceptionalDocSession('share'), docActions, undo, userActions);
+    const accessControl = this._activeDoc.getGranularAccessForBundle(
+      docSession || makeExceptionalDocSession('share'), docActions, undo, userActions
+    );
     try {
       // TODO: see if any of the code paths that have no docSession are relevant outside
       // of tests.

app/server/lib/TestingHooks.ts

@@ -6,13 +6,15 @@ import * as Comm from 'app/server/lib/Comm';
 import {ILoginSession} from 'app/server/lib/ILoginSession';
 import * as log from 'app/server/lib/log';
 import {IMessage, Rpc} from 'grain-rpc';
-import {createCheckers} from 'ts-interface-checker';
+import * as t from 'ts-interface-checker';
 import {FlexServer} from './FlexServer';
 import {IInstanceManager} from './IInstanceManager';
 import {ITestingHooks} from './ITestingHooks';
 import ITestingHooksTI from './ITestingHooks-ti';
 import {connect, fromCallback} from './serverUtils';
 
+const tiCheckers = t.createCheckers(ITestingHooksTI, {UserProfile: t.name("object")});
+
 export function startTestingHooks(socketPath: string, port: number, instanceManager: IInstanceManager,
                                   comm: Comm, flexServer: FlexServer,
                                   workerServers: FlexServer[]): Promise<net.Server> {
@@ -27,7 +29,7 @@ export function startTestingHooks(socketPath: string, port: number, instanceMana
       // Register the testing implementation.
       rpc.registerImpl('testing',
                        new TestingHooks(port, instanceManager, comm, flexServer, workerServers),
-                       createCheckers(ITestingHooksTI).ITestingHooks);
+                       tiCheckers.ITestingHooks);
     });
     server.listen(socketPath);
   });
@@ -47,7 +49,7 @@ export interface TestingHooksClient extends ITestingHooks {
 export async function connectTestingHooks(socketPath: string): Promise<TestingHooksClient> {
   const socket = await connect(socketPath);
   const rpc = connectToSocket(new Rpc({logger: {}}), socket);
-  return Object.assign(rpc.getStub<TestingHooks>('testing', createCheckers(ITestingHooksTI).ITestingHooks), {
+  return Object.assign(rpc.getStub<TestingHooks>('testing', tiCheckers.ITestingHooks), {
     close: () => socket.end(),
   });
 }
@@ -61,12 +63,12 @@ export class TestingHooks implements ITestingHooks {
     private _workerServers: FlexServer[]
   ) {}
 
-  public getOwnPort(): number {
+  public async getOwnPort(): Promise<number> {
     log.info("TestingHooks.getOwnPort called");
     return this._server.getOwnPort();
   }
 
-  public getPort(): number {
+  public async getPort(): Promise<number> {
     log.info("TestingHooks.getPort called");
     return this._port;
   }
@@ -100,7 +102,7 @@ export class TestingHooks implements ITestingHooks {
     log.info("TestingHooks.setServerVersion called with", version);
     this._comm.setServerVersion(version);
     for (const server of this._workerServers) {
-      await server.comm.setServerVersion(version);
+      server.comm.setServerVersion(version);
     }
   }
 
@@ -108,7 +110,7 @@ export class TestingHooks implements ITestingHooks {
     log.info("TestingHooks.disconnectClients called");
     this._comm.destroyAllClients();
     for (const server of this._workerServers) {
-      await server.comm.destroyAllClients();
+      server.comm.destroyAllClients();
     }
   }
 
@@ -134,7 +136,7 @@ export class TestingHooks implements ITestingHooks {
     log.info("TestingHooks.setClientPersistence called with", ttlMs);
     this._comm.testSetClientPersistence(ttlMs);
     for (const server of this._workerServers) {
-      await server.comm.testSetClientPersistence(ttlMs);
+      server.comm.testSetClientPersistence(ttlMs);
     }
   }
 
@@ -174,7 +176,7 @@ export class TestingHooks implements ITestingHooks {
     log.info("TestingHooks.flushAuthorizerCache called");
     this._server.dbManager.flushDocAuthCache();
     for (const server of this._workerServers) {
-      await server.dbManager.flushDocAuthCache();
+      server.dbManager.flushDocAuthCache();
     }
   }
 

app/server/lib/uploads.ts

@@ -316,7 +316,7 @@ export async function createTmpDir(options: tmp.Options): Promise<TmpDirResult>
     try {
       // Still call the original callback, so that `tmp` module doesn't keep remembering about
       // this directory and doesn't try to delete it again on exit.
-      tmpCleanup();
+      await tmpCleanup();
     } catch (err) {
       // OK if it fails because the dir is already removed.
     }