chore: fix lint errors and run lint in CI (#348)

* chore: fix lint errors except non-null createXXXNode

* chrore: suppress lint errors for createXXXNode

* chore: add CI that runs the regular build and lint

test would be nice too but to do separately

* chore: remove unneeded non-null assertions

Now `setCurNode` can take null we don't need them.

* fix: use parameter not hardcoded value

Author: microbit-matt-hillsdon

.github/workflows/build.yml

@@ -0,0 +1,22 @@
+# Build workflow
+name: Build
+
+on: [pull_request, workflow_dispatch]
+
+permissions:
+  contents: read
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: latest
+          cache: 'npm'
+      - run: npm ci
+      - run: npm run build
+      - run: npm run lint

eslint.config.js

@@ -129,7 +129,7 @@ module.exports = [
     },
   },
   {
-    files: ['**/*.mocha.js'],
+    files: ['**/*.mocha.js', 'test/webdriverio/test/*_test.mjs'],
     languageOptions: {
       globals: {
         ...globals.mocha,

src/actions/action_menu.ts

@@ -10,7 +10,6 @@ import {
   ContextMenu,
   ContextMenuRegistry,
   ShortcutRegistry,
-  comments,
   utils as BlocklyUtils,
   WidgetDiv,
 } from 'blockly';
@@ -23,10 +22,7 @@ const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind(
   ShortcutRegistry.registry,
 );
 
-export interface Scope {
-  block?: BlockSvg;
-  workspace?: WorkspaceSvg;
-  comment?: comments.RenderedWorkspaceComment;
+export interface ScopeWithConnection extends ContextMenuRegistry.Scope {
   connection?: Connection;
 }
 
@@ -100,6 +96,8 @@ export class ActionMenu {
    * Returns true if it is possible to open the action menu in the
    * current location, even if the menu was not opened due there being
    * no applicable menu items.
+   *
+   * @param workspace The workspace.
    */
   private openActionMenu(workspace: WorkspaceSvg): boolean {
     let menuOptions: Array<
@@ -114,7 +112,7 @@ export class ActionMenu {
     if (!node) return false;
     const nodeType = node.getType();
     switch (nodeType) {
-      case ASTNode.types.BLOCK:
+      case ASTNode.types.BLOCK: {
         const block = node.getLocation() as BlockSvg;
         rtl = block.RTL;
         // Reimplement BlockSvg.prototype.generateContextMenu as that
@@ -130,11 +128,12 @@ export class ActionMenu {
         }
         // End reimplement.
         break;
+      }
 
       // case Blockly.ASTNode.types.INPUT:
       case ASTNode.types.NEXT:
       case ASTNode.types.PREVIOUS:
-      case ASTNode.types.INPUT:
+      case ASTNode.types.INPUT: {
         const connection = node.getLocation() as Connection;
         rtl = connection.getSourceBlock().RTL;
 
@@ -143,6 +142,7 @@ export class ActionMenu {
         // a possible kind of scope.
         this.addConnectionItems(connection, menuOptions);
         break;
+      }
 
       default:
         console.info(`No action menu for ASTNode of type ${nodeType}`);
@@ -177,24 +177,21 @@ export class ActionMenu {
    */
   private addConnectionItems(
     connection: Connection,
-    menuOptions: (
+    menuOptions: Array<
       | ContextMenuRegistry.ContextMenuOption
       | ContextMenuRegistry.LegacyContextMenuOption
-    )[],
+    >,
   ) {
-    const insertAction = ContextMenuRegistry.registry.getItem('insert');
-    if (!insertAction) throw new Error("can't find insert action");
-
-    const pasteAction = ContextMenuRegistry.registry.getItem(
-      'blockPasteFromContextMenu',
-    );
-    if (!pasteAction) throw new Error("can't find paste action");
-    const possibleOptions = [insertAction, pasteAction /* etc.*/];
+    const possibleOptions = [
+      this.getContextMenuAction('insert'),
+      this.getContextMenuAction('blockPasteFromContextMenu'),
+    ];
 
     // Check preconditions and get menu texts.
     const scope = {
       connection,
     } as unknown as ContextMenuRegistry.Scope;
+
     for (const option of possibleOptions) {
       const precondition = option.preconditionFn?.(scope);
       if (precondition === 'hidden') continue;
@@ -205,14 +202,33 @@ export class ActionMenu {
       menuOptions.push({
         text: displayText,
         enabled: precondition === 'enabled',
-        callback: option.callback!,
+        callback: option.callback,
         scope,
         weight: option.weight,
       });
     }
     return menuOptions;
   }
 
+  /**
+   * Find a context menu action, throwing an `Error` if it is not present or
+   * not an action. This usefully narrows the type to `ActionRegistryItem`
+   * which is not exported from Blockly.
+   *
+   * @param id The id of the action.
+   * @returns the action.
+   */
+  private getContextMenuAction(id: string) {
+    const item = ContextMenuRegistry.registry.getItem(id);
+    if (!item) {
+      throw new Error(`can't find context menu item ${id}`);
+    }
+    if (!item?.callback) {
+      throw new Error(`context menu item unexpectedly not action ${id}`);
+    }
+    return item;
+  }
+
   /**
    * Create a fake PointerEvent for opening the action menu for the
    * given ASTNode.

src/actions/arrow_navigation.ts

@@ -57,7 +57,7 @@ export class ArrowNavigation {
       right: {
         name: Constants.SHORTCUT_NAMES.RIGHT,
         preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
-        callback: (workspace, _, shortcut) => {
+        callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           let isHandled = false;
           switch (this.navigation.getState(workspace)) {
@@ -94,7 +94,7 @@ export class ArrowNavigation {
       left: {
         name: Constants.SHORTCUT_NAMES.LEFT,
         preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
-        callback: (workspace, _, shortcut) => {
+        callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           let isHandled = false;
           switch (this.navigation.getState(workspace)) {
@@ -129,7 +129,7 @@ export class ArrowNavigation {
       down: {
         name: Constants.SHORTCUT_NAMES.DOWN,
         preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
-        callback: (workspace, _, shortcut) => {
+        callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           const flyout = workspace.getFlyout();
           let isHandled = false;
@@ -170,7 +170,7 @@ export class ArrowNavigation {
       up: {
         name: Constants.SHORTCUT_NAMES.UP,
         preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
-        callback: (workspace, _, shortcut) => {
+        callback: (workspace, e, shortcut) => {
           const flyout = workspace.getFlyout();
           const toolbox = workspace.getToolbox() as Toolbox;
           let isHandled = false;

src/actions/clipboard.ts

@@ -18,6 +18,7 @@ import * as Constants from '../constants';
 import type {BlockSvg, WorkspaceSvg} from 'blockly';
 import {LineCursor} from '../line_cursor';
 import {Navigation} from '../navigation';
+import {ScopeWithConnection} from './action_menu';
 
 const KeyCodes = blocklyUtils.KeyCodes;
 const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind(
@@ -238,21 +239,23 @@ export class Clipboard {
   private copyPrecondition(workspace: WorkspaceSvg) {
     if (!this.canCurrentlyEdit(workspace)) return false;
     switch (this.navigation.getState(workspace)) {
-      case Constants.STATE.WORKSPACE:
+      case Constants.STATE.WORKSPACE: {
         const curNode = workspace?.getCursor()?.getCurNode();
         const source = curNode?.getSourceBlock();
         return !!(
           source?.isDeletable() &&
           source?.isMovable() &&
           !Gesture.inProgress()
         );
-      case Constants.STATE.FLYOUT:
+      }
+      case Constants.STATE.FLYOUT: {
         const flyoutWorkspace = workspace.getFlyout()?.getWorkspace();
         const sourceBlock = flyoutWorkspace
           ?.getCursor()
           ?.getCurNode()
           ?.getSourceBlock();
         return !!(sourceBlock && !Gesture.inProgress());
+      }
       default:
         return false;
     }
@@ -314,18 +317,15 @@ export class Clipboard {
   private registerPasteContextMenuAction() {
     const pasteAction: ContextMenuRegistry.RegistryItem = {
       displayText: (scope) => `Paste (${this.getPlatformPrefix()}V)`,
-      preconditionFn: (scope) => {
-        const ws =
-          scope.block?.workspace ??
-          (scope as any).connection?.getSourceBlock().workspace;
+      preconditionFn: (scope: ScopeWithConnection) => {
+        const block = scope.block ?? scope.connection?.getSourceBlock();
+        const ws = block?.workspace as WorkspaceSvg | null;
         if (!ws) return 'hidden';
-
         return this.pastePrecondition(ws) ? 'enabled' : 'disabled';
       },
-      callback: (scope) => {
-        const ws =
-          scope.block?.workspace ??
-          (scope as any).connection?.getSourceBlock().workspace;
+      callback: (scope: ScopeWithConnection) => {
+        const block = scope.block ?? scope.connection?.getSourceBlock();
+        const ws = block?.workspace as WorkspaceSvg | null;
         if (!ws) return;
         return this.pasteCallback(ws);
       },
@@ -379,6 +379,7 @@ export class Clipboard {
         this.navigation.tryToConnectNodes(
           pasteWorkspace,
           targetNode,
+          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
           ASTNode.createBlockNode(block)!,
         );
       }

src/actions/delete.ts

@@ -113,7 +113,7 @@ export class DeleteAction {
         // Run the original precondition code, from the context menu option.
         // If the item would be hidden or disabled, respect it.
         const originalPreconditionResult =
-          this.oldContextMenuItem!.preconditionFn?.(scope) ?? 'enabled';
+          this.oldContextMenuItem?.preconditionFn?.(scope) ?? 'enabled';
         if (!ws || originalPreconditionResult !== 'enabled') {
           return originalPreconditionResult;
         }

src/actions/disconnect.ts

@@ -117,12 +117,17 @@ export class DisconnectAction {
     if (!curConnection.isConnected()) {
       return;
     }
+    const targetConnection = curConnection.targetConnection;
+    if (!targetConnection) {
+      throw new Error('Must have target if connected');
+    }
+
     const superiorConnection = curConnection.isSuperior()
       ? curConnection
-      : curConnection.targetConnection!;
+      : targetConnection;
 
     const inferiorConnection = curConnection.isSuperior()
-      ? curConnection.targetConnection!
+      ? targetConnection
       : curConnection;
 
     if (inferiorConnection.getSourceBlock().isShadow()) {
@@ -141,7 +146,7 @@ export class DisconnectAction {
 
     if (wasVisitingConnection) {
       const connectionNode = ASTNode.createConnectionNode(superiorConnection);
-      workspace.getCursor()!.setCurNode(connectionNode!);
+      workspace.getCursor()?.setCurNode(connectionNode);
     }
   }
 }

src/actions/edit.ts

@@ -4,19 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {
-  Connection,
-  ContextMenuRegistry,
-  ShortcutRegistry,
-  comments,
-  utils as BlocklyUtils,
-} from 'blockly';
-import * as Constants from '../constants';
-import type {BlockSvg, WorkspaceSvg} from 'blockly';
+import {ContextMenuRegistry} from 'blockly';
+import type {WorkspaceSvg} from 'blockly';
 import {LineCursor} from '../line_cursor';
-import {NavigationController} from '../navigation_controller';
-
-const KeyCodes = BlocklyUtils.KeyCodes;
 
 /**
  * Action to edit a block.  This just moves the cursor to the first

src/actions/enter.ts

@@ -134,6 +134,7 @@ export class EnterAction {
         !this.navigation.tryToConnectNodes(
           workspace,
           stationaryNode,
+          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
           ASTNode.createBlockNode(newBlock)!,
         )
       ) {
@@ -144,7 +145,7 @@ export class EnterAction {
     }
 
     this.navigation.focusWorkspace(workspace);
-    workspace.getCursor()!.setCurNode(ASTNode.createBlockNode(newBlock)!);
+    workspace.getCursor()?.setCurNode(ASTNode.createBlockNode(newBlock));
   }
 
   /**
@@ -155,17 +156,22 @@ export class EnterAction {
    */
   private triggerButtonCallback(workspace: WorkspaceSvg) {
     const button = this.navigation
-      .getFlyoutCursor(workspace)!
-      .getCurNode()
+      .getFlyoutCursor(workspace)
+      ?.getCurNode()
       ?.getLocation() as FlyoutButton | undefined;
     if (!button) return;
-    const buttonCallback = (workspace as any).flyoutButtonCallbacks.get(
-      (button as any).callbackKey,
-    );
-    if (typeof buttonCallback === 'function') {
+
+    const flyoutButtonCallbacks: Map<string, (p1: FlyoutButton) => void> =
+      // @ts-expect-error private field access
+      workspace.flyoutButtonCallbacks;
+
+    const info = button.info;
+    if ('callbackkey' in info) {
+      const buttonCallback = flyoutButtonCallbacks.get(info.callbackkey);
+      if (!buttonCallback) {
+        throw new Error('No callback function found for flyout button.');
+      }
       buttonCallback(button);
-    } else if (!button.isLabel()) {
-      throw new Error('No callback function found for flyout button.');
     }
   }
 
@@ -208,8 +214,8 @@ export class EnterAction {
     }
 
     const curBlock = this.navigation
-      .getFlyoutCursor(workspace)!
-      .getCurNode()
+      .getFlyoutCursor(workspace)
+      ?.getCurNode()
       ?.getLocation() as BlockSvg | undefined;
     if (!curBlock?.isEnabled()) {
       console.warn("Can't insert a disabled block.");