(core) Add rules to eslint to better match our coding conventions.

Summary:
We used tslint earlier, and on switching to eslint, some rules were not
transfered. This moves more rules over, for consistent conventions or helpful
warnings.

- Name private members with a leading underscore.
- Prefer interface over a type alias.
- Use consistent spacing around ':' in type annotations.
- Use consistent spacing around braces of code blocks.
- Use semicolons consistently at the ends of statements.
- Use braces around even one-liner blocks, like conditionals and loops.
- Warn about shadowed variables.

Test Plan: Fixed all new warnings. Should be no behavior changes in code.

Reviewers: paulfitz

Reviewed By: paulfitz

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

app/client/components/ActionLog.ts

@@ -440,7 +440,7 @@ export class ActionLog extends dispose.Disposable implements IDomComponent {
     const fieldIndex = viewSection.viewFields().peek().findIndex((f: any) => f.colId.peek() === colId);
 
     // Finally, move cursor position to the section, column (if we found it), and row.
-    this._gristDoc.moveToCursorPos({rowId, sectionId, fieldIndex}).catch(() => { /* do nothing */});
+    this._gristDoc.moveToCursorPos({rowId, sectionId, fieldIndex}).catch(() => { /* do nothing */ });
   }
 
 }

app/client/components/CellPosition.ts

@@ -28,8 +28,9 @@ export function samePosition(a: CellPosition, b: CellPosition) {
  * @param docModel Document model
  */
 export function fromCursor(position: CursorPos, docModel: DocModel): CellPosition | null {
-  if (!position.sectionId || !position.rowId || position.fieldIndex == null)
+  if (!position.sectionId || !position.rowId || position.fieldIndex == null) {
     return null;
+  }
 
   const section = docModel.viewSections.getRowModel(position.sectionId);
   const colRef = section.viewFields().peek()[position.fieldIndex]?.colRef.peek();

app/client/components/CodeEditorPanel.js

@@ -31,6 +31,7 @@ CodeEditorPanel.prototype.buildDom = function() {
     kd.scope(this._schema, function(schema) {
       // The reason to scope and rebuild instead of using `kd.text(schema)` is because
       // hljs.highlightBlock(elem) replaces `elem` with a whole new dom tree.
+      if (!schema) { return null; }
       return dom(
         'code.g-code-viewer.python',
         schema,

app/client/components/Comm.ts

@@ -204,15 +204,16 @@ export class Comm extends dispose.Disposable implements GristServerAPI, DocListA
   public pendingRequests: Map<number, CommRequestInFlight>;
   public nextRequestNumber: number = 0;
 
+  protected listenTo: BackboneEvents["listenTo"];            // set by Backbone
+  protected trigger: BackboneEvents["trigger"];              // set by Backbone
+  protected stopListening: BackboneEvents["stopListening"];  // set by Backbone
+
   // This is a map from docId to the connection for the server that manages
   // that docId.  In classic Grist, which doesn't have fixed docIds or multiple
   // servers, the key is always "null".
   private _connections: Map<string|null, GristWSConnection> = new Map();
   private _collectedUserActions: UserAction[] | null;
   private _singleWorkerMode: boolean = getInitialDocAssignment() === null;  // is this classic Grist?
-  private listenTo: BackboneEvents["listenTo"];            // set by Backbone
-  private trigger: BackboneEvents["trigger"];              // set by Backbone
-  private stopListening: BackboneEvents["stopListening"];  // set by Backbone
 
   public create() {
     this.autoDisposeCallback(() => {

app/client/components/Cursor.ts

@@ -79,7 +79,7 @@ export class Cursor extends Disposable {
     optCursorPos = optCursorPos || {};
     this.viewData = baseView.viewData;
 
-    this._sectionId = this.autoDispose(ko.computed(() => baseView.viewSection.id()))
+    this._sectionId = this.autoDispose(ko.computed(() => baseView.viewSection.id()));
     this._rowId = ko.observable(optCursorPos.rowId || 0);
     this.rowIndex = this.autoDispose(ko.computed({
       read: () => {

app/client/components/CursorMonitor.ts

@@ -48,33 +48,33 @@ export class CursorMonitor extends Disposable {
     // whenever current position changes, store it in the memory
     this.autoDispose(doc.cursorPosition.addListener(pos => {
       // if current position is not restored yet, don't change it
-      if (!this._restored) return;
-      if (pos) this.storePosition(pos);
-    }))
+      if (!this._restored) { return; }
+      if (pos) { this._storePosition(pos); }
+    }));
   }
 
   private _whenDocumentLoadsRestorePosition(doc: GristDoc) {
     // on view shown
     this.autoDispose(doc.currentView.addListener(async view => {
       // if the position was restored for this document do nothing
-      if (this._restored) return;
+      if (this._restored) { return; }
       // set that we already restored the position, as some view is shown to the user
       this._restored = true;
       // if view wasn't rendered (page is displaying history or code view) do nothing
-      if (!view) return;
+      if (!view) { return; }
       const viewId = doc.activeViewId.get();
-      const position = this.restoreLastPosition(viewId);
+      const position = this._restoreLastPosition(viewId);
       if (position) {
         await doc.recursiveMoveToCursorPos(position, true);
       }
     }));
   }
 
-  private storePosition(pos: ViewCursorPos) {
+  private _storePosition(pos: ViewCursorPos) {
     this._store.update(this._docId, pos);
   }
 
-  private restoreLastPosition(view: IDocPage) {
+  private _restoreLastPosition(view: IDocPage) {
     const lastPosition = this._store.read(this._docId);
     this._store.clear(this._docId);
     if (lastPosition && lastPosition.position.viewId == view) {
@@ -87,13 +87,13 @@ export class CursorMonitor extends Disposable {
 // Internal implementations for working with local storage
 class StorageWrapper {
 
-  constructor(private storage = getStorage()) {
+  constructor(private _storage = getStorage()) {
 
   }
 
   public update(docId: string, position: ViewCursorPos): void {
     try {
-      const storage = this.storage;
+      const storage = this._storage;
       const data = { docId, position, timestamp: Date.now() };
       storage.setItem(this._key(docId), JSON.stringify(data));
     } catch (e) {
@@ -102,14 +102,14 @@ class StorageWrapper {
   }
 
   public clear(docId: string,): void {
-    const storage = this.storage;
+    const storage = this._storage;
     storage.removeItem(this._key(docId));
   }
 
   public read(docId: string): { docId: string; position: ViewCursorPos; } | undefined {
-    const storage = this.storage;
+    const storage = this._storage;
     const result = storage.getItem(this._key(docId));
-    if (!result) return undefined;
+    if (!result) { return undefined; }
     return JSON.parse(result);
   }
 

app/client/components/EditorMonitor.ts

@@ -25,7 +25,7 @@ export class EditorMonitor extends Disposable {
     this._store = new EditMemoryStorage(doc.docId(), store);
 
     // listen to document events to handle view load event
-    this._listenToReload(doc)
+    this._listenToReload(doc);
   }
 
   /**
@@ -67,7 +67,7 @@ export class EditorMonitor extends Disposable {
       }
       executed = true;
       // if view wasn't rendered (page is displaying history or code view) do nothing
-      if (!view) return;
+      if (!view) { return; }
       const lastEdit = this._restorePosition();
       if (lastEdit) {
         // set the cursor at right cell
@@ -98,11 +98,11 @@ function typedListener(owner: IDisposableOwner) {
 type EditorState = any;
 
 // Schema for value stored in the local storage
-type LastEditData = {
+interface LastEditData {
   // absolute position for a cell
-  position: CellPosition,
+  position: CellPosition;
   // editor's state
-  value: EditorState
+  value: EditorState;
 }
 
 // Abstraction for working with local storage
@@ -111,7 +111,7 @@ class EditMemoryStorage {
   private _entry: LastEditData | null = null;
   private _timestamp = 0;
 
-  constructor(private _docId: string, private storage = getStorage()) {
+  constructor(private _docId: string, private _storage = getStorage()) {
   }
 
   public updateValue(pos: CellPosition, value: EditorState): void {
@@ -138,7 +138,7 @@ class EditMemoryStorage {
   }
 
   protected load() {
-    const storage = this.storage;
+    const storage = this._storage;
     const data = storage.getItem(this._key());
     this._entry = null;
     this._timestamp = 0;
@@ -150,7 +150,7 @@ class EditMemoryStorage {
           console.error("[EditMemory] Data in local storage has a different structure");
           return;
         }
-        this._entry = entry
+        this._entry = entry;
         this._timestamp = timestamp;
       } catch (e) {
         console.error("[EditMemory] Can't deserialize date from local storage");
@@ -159,7 +159,7 @@ class EditMemoryStorage {
   }
 
   protected save(): void {
-    const storage = this.storage;
+    const storage = this._storage;
 
     // if entry was removed - clear the storage
     if (!this._entry) {

app/client/components/GristDoc.ts

@@ -111,10 +111,10 @@ export class GristDoc extends DisposableWithEvents {
   public readonly fieldEditorHolder = Holder.create(this);
 
   // Holds current view that is currently rendered
-  public currentView : Observable<BaseView | null>;
+  public currentView: Observable<BaseView | null>;
 
   // Holds current cursor position with a view id
-  public cursorPosition : Computed<ViewCursorPos | undefined>;
+  public cursorPosition: Computed<ViewCursorPos | undefined>;
 
   private _actionLog: ActionLog;
   private _undoStack: UndoStack;
@@ -250,22 +250,22 @@ export class GristDoc extends DisposableWithEvents {
     });
     // then listen if the view is present, because we still need to wait for it load properly
     this.autoDispose(viewInstance.addListener(async (view) => {
-      if (!view) return;
+      if (!view) { return; }
       await view.getLoadingDonePromise();
       this.currentView.set(view);
-    }))
+    }));
 
     // create observable for current cursor position
     this.cursorPosition = Computed.create<ViewCursorPos | undefined>(this, use => {
       // get the BaseView
       const view = use(viewInstance);
-      if (!view) return undefined;
+      if (!view) { return undefined; }
       // get current viewId
       const viewId = use(this.activeViewId);
-      if (typeof viewId != 'number') return undefined;
+      if (typeof viewId != 'number') { return undefined; }
       // read latest position
       const currentPosition = use(view.cursor.currentPosition);
-      if (currentPosition) return { ...currentPosition, viewId }
+      if (currentPosition) { return { ...currentPosition, viewId }; }
       return undefined;
     });
 
@@ -333,7 +333,7 @@ export class GristDoc extends DisposableWithEvents {
       return;
     }
     try {
-      const viewInstance = await this._switchToSectionId(cursorPos.sectionId)
+      const viewInstance = await this._switchToSectionId(cursorPos.sectionId);
       if (viewInstance) {
         viewInstance.setCursorPos(cursorPos);
       }
@@ -643,7 +643,7 @@ export class GristDoc extends DisposableWithEvents {
       }
       const view: ViewRec = section.view.peek();
       const viewId = view.getRowId();
-      if (viewId != this.activeViewId.get()) await this.openDocPage(view.getRowId());
+      if (viewId != this.activeViewId.get()) { await this.openDocPage(view.getRowId()); }
       if (setAsActiveSection) { view.activeSectionId(cursorPos.sectionId); }
       const fieldIndex = cursorPos.fieldIndex;
       const viewInstance = await waitObs(section.viewInstance);
@@ -669,14 +669,14 @@ export class GristDoc extends DisposableWithEvents {
    * @param input Optional. Cell's initial value
    */
   public async activateEditorAtCursor(options: { init?: string, state?: any}) {
-    const view = await this.waitForView();
+    const view = await this._waitForView();
     view?.activateEditorAtCursor(options);
   }
 
   /**
    * Waits for a view to be ready
    */
-  private async waitForView() {
+  private async _waitForView() {
     const view = await waitObs(this.viewModel.activeSection.peek().viewInstance);
     await view?.getLoadingDonePromise();
     return view;
@@ -788,13 +788,13 @@ export class GristDoc extends DisposableWithEvents {
    * Convert a url hash to a cursor position.
    */
   private _getCursorPosFromHash(hash: HashLink): CursorPos {
-    const cursorPos : CursorPos = { rowId: hash.rowId, sectionId: hash.sectionId };
+    const cursorPos: CursorPos = { rowId: hash.rowId, sectionId: hash.sectionId };
     if (cursorPos.sectionId != undefined && hash.colRef !== undefined){
       // translate colRef to a fieldIndex
       const section = this.docModel.viewSections.getRowModel(cursorPos.sectionId);
       const fieldIndex = section.viewFields.peek().all()
           .findIndex(x=> x.colRef.peek() == hash.colRef);
-      if (fieldIndex >= 0) cursorPos.fieldIndex = fieldIndex;
+      if (fieldIndex >= 0) { cursorPos.fieldIndex = fieldIndex; }
     }
     return cursorPos;
   }

app/client/components/GristWSConnection.ts

@@ -106,6 +106,8 @@ export class GristWSConnection extends Disposable {
   public useCount: number = 0;
   public on: BackboneEvents['on'];    // set by Backbone
 
+  protected trigger: BackboneEvents['trigger']; // set by Backbone
+
   private _clientId: string|null;
   private _clientCounter: string;     // Identifier of this GristWSConnection object in this browser tab session
   private _assignmentId: string|null;
@@ -118,7 +120,6 @@ export class GristWSConnection extends Disposable {
   private _reconnectAttempts: number = 0;
   private _wantReconnect: boolean = true;
   private _ws: WebSocket|null = null;
-  private trigger: BackboneEvents['trigger']; // set by Backbone
 
   constructor(private _settings: GristWSSettings = new GristWSSettingsBrowser()) {
     super();

app/client/components/TypeTransform.ts

@@ -28,8 +28,8 @@ import pickBy = require('lodash/pickBy');
  * Creates an instance of TypeTransform for a single field. Extends ColumnTransform.
  */
 export class TypeTransform extends ColumnTransform {
-  private reviseTypeChange = Observable.create(this, false);
-  private transformWidget: Computed<NewAbstractWidget|null>;
+  private _reviseTypeChange = Observable.create(this, false);
+  private _transformWidget: Computed<NewAbstractWidget|null>;
 
   constructor(gristDoc: GristDoc, fieldBuilder: FieldBuilder) {
     super(gristDoc, fieldBuilder);
@@ -37,7 +37,7 @@ export class TypeTransform extends ColumnTransform {
 
     // The display widget of the new transform column. Used to build the transform config menu.
     // Only set while transforming.
-    this.transformWidget = Computed.create(this, fromKo(fieldBuilder.widgetImpl), (use, widget) => {
+    this._transformWidget = Computed.create(this, fromKo(fieldBuilder.widgetImpl), (use, widget) => {
       return use(this.origColumn.isTransforming) ? widget : null;
     });
   }
@@ -49,12 +49,12 @@ export class TypeTransform extends ColumnTransform {
     // An observable to disable all buttons before the dom get removed.
     const disableButtons = Observable.create(null, false);
 
-    this.reviseTypeChange.set(false);
+    this._reviseTypeChange.set(false);
     this.editor = this.autoDispose(AceEditor.create({ observable: this.transformColumn.formula }));
     return dom('div',
       testId('type-transform-top'),
-      dom.maybe(this.transformWidget, transformWidget => transformWidget.buildTransformConfigDom()),
-      dom.maybe(this.reviseTypeChange, () =>
+      dom.maybe(this._transformWidget, transformWidget => transformWidget.buildTransformConfigDom()),
+      dom.maybe(this._reviseTypeChange, () =>
         dom('div.transform_editor', this.buildEditorDom(),
           testId("type-transform-formula")
         )
@@ -64,7 +64,7 @@ export class TypeTransform extends ColumnTransform {
           'Cancel', testId("type-transform-cancel"),
           dom.cls('disabled', disableButtons)
         ),
-        dom.domComputed(this.reviseTypeChange, revising => {
+        dom.domComputed(this._reviseTypeChange, revising => {
           if (revising) {
             return basicButton(dom.on('click', () => this.editor.writeObservable()),
               'Preview', testId("type-transform-update"),
@@ -72,7 +72,7 @@ export class TypeTransform extends ColumnTransform {
               { title: 'Update formula (Shift+Enter)' }
             );
           } else {
-            return basicButton(dom.on('click', () => { this.reviseTypeChange.set(true); }),
+            return basicButton(dom.on('click', () => { this._reviseTypeChange.set(true); }),
               'Revise', testId("type-transform-revise"),
               dom.cls('disabled', disableButtons)
             );

app/client/components/UndoStack.ts

@@ -117,13 +117,13 @@ export class UndoStack extends dispose.Disposable {
       // context where the change was originally made. We jump first immediately to feel more
       // responsive, then again when the action is done. The second jump matters more for most
       // changes, but the first is the important one when Undoing an AddRecord.
-      this._gristDoc.moveToCursorPos(ag.cursorPos, ag).catch(() => {/* do nothing */})
+      this._gristDoc.moveToCursorPos(ag.cursorPos, ag).catch(() => { /* do nothing */ });
       await this._gristDoc.docComm.applyUserActionsById(
         actionGroups.map(a => a.actionNum),
         actionGroups.map(a => a.actionHash),
         isUndo,
         { otherId: ag.actionNum });
-      this._gristDoc.moveToCursorPos(ag.cursorPos, ag).catch(() => {/* do nothing */})
+      this._gristDoc.moveToCursorPos(ag.cursorPos, ag).catch(() => { /* do nothing */ });
     } catch (err) {
       err.message = `Failed to apply ${isUndo ? 'undo' : 'redo'} action: ${err.message}`;
       throw err;

app/client/components/duplicatePage.ts

@@ -18,7 +18,7 @@ export async function duplicatePage(gristDoc: GristDoc, pageId: number) {
   const pagesTable = gristDoc.docModel.pages;
   const pageName = pagesTable.rowModels[pageId].view.peek().name.peek();
   let inputEl: HTMLInputElement;
-  setTimeout(() => {inputEl.focus(); inputEl.select(); }, 100);
+  setTimeout(() => { inputEl.focus(); inputEl.select(); }, 100);
 
   confirmModal('Duplicate page', 'Save', () => makeDuplicate(gristDoc, pageId, inputEl.value), (
     dom('div', [