From 717135099205a482a85eaafceaa1e4edf67ab125 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 1 Apr 2025 14:59:40 -0700 Subject: [PATCH] fix!: Tighten and correct typings on ASTNode (#8835) * fix!: Tighten typings on ASTNode.create*Node() methods. * fix: Restore missing condition. * fix: Fix unsafe casts, non-null assertions and incorrect types. * refactor: Simplify parent input checks. --- core/keyboard_nav/ast_node.ts | 103 ++++++++++------------------ core/renderers/common/marker_svg.ts | 4 +- 2 files changed, 38 insertions(+), 69 deletions(-) diff --git a/core/keyboard_nav/ast_node.ts b/core/keyboard_nav/ast_node.ts index ec46a4d3f..0de0ffb57 100644 --- a/core/keyboard_nav/ast_node.ts +++ b/core/keyboard_nav/ast_node.ts @@ -47,9 +47,7 @@ export class ASTNode { private readonly location: IASTNodeLocation; /** The coordinate on the workspace. */ - // AnyDuringMigration because: Type 'null' is not assignable to type - // 'Coordinate'. - private wsCoordinate: Coordinate = null as AnyDuringMigration; + private wsCoordinate: Coordinate | null = null; /** * @param type The type of the location. @@ -119,7 +117,7 @@ export class ASTNode { * @returns The workspace coordinate or null if the location is not a * workspace. */ - getWsCoordinate(): Coordinate { + getWsCoordinate(): Coordinate | null { return this.wsCoordinate; } @@ -144,12 +142,12 @@ export class ASTNode { private findNextForInput(): ASTNode | null { const location = this.location as Connection; const parentInput = location.getParentInput(); - const block = parentInput!.getSourceBlock(); - // AnyDuringMigration because: Argument of type 'Input | null' is not - // assignable to parameter of type 'Input'. - const curIdx = block!.inputList.indexOf(parentInput as AnyDuringMigration); - for (let i = curIdx + 1; i < block!.inputList.length; i++) { - const input = block!.inputList[i]; + const block = parentInput?.getSourceBlock(); + if (!block || !parentInput) return null; + + const curIdx = block.inputList.indexOf(parentInput); + for (let i = curIdx + 1; i < block.inputList.length; i++) { + const input = block.inputList[i]; const fieldRow = input.fieldRow; for (let j = 0; j < fieldRow.length; j++) { const field = fieldRow[j]; @@ -209,12 +207,12 @@ export class ASTNode { private findPrevForInput(): ASTNode | null { const location = this.location as Connection; const parentInput = location.getParentInput(); - const block = parentInput!.getSourceBlock(); - // AnyDuringMigration because: Argument of type 'Input | null' is not - // assignable to parameter of type 'Input'. - const curIdx = block!.inputList.indexOf(parentInput as AnyDuringMigration); + const block = parentInput?.getSourceBlock(); + if (!block || !parentInput) return null; + + const curIdx = block.inputList.indexOf(parentInput); for (let i = curIdx; i >= 0; i--) { - const input = block!.inputList[i]; + const input = block.inputList[i]; if (input.connection && input !== parentInput) { return ASTNode.createInputNode(input); } @@ -440,18 +438,11 @@ export class ASTNode { // substack. const topBlock = block.getTopStackBlock(); const topConnection = getParentConnection(topBlock); + const parentInput = topConnection?.targetConnection?.getParentInput(); // If the top connection has a parentInput, create an AST node pointing to // that input. - if ( - topConnection && - topConnection.targetConnection && - topConnection.targetConnection.getParentInput() - ) { - // AnyDuringMigration because: Argument of type 'Input | null' is not - // assignable to parameter of type 'Input'. - return ASTNode.createInputNode( - topConnection.targetConnection.getParentInput() as AnyDuringMigration, - ); + if (parentInput) { + return ASTNode.createInputNode(parentInput); } else { // Go to stack level if you are not underneath an input. return ASTNode.createStackNode(topBlock); @@ -538,7 +529,9 @@ export class ASTNode { case ASTNode.types.NEXT: { const connection = this.location as Connection; const targetConnection = connection.targetConnection; - return ASTNode.createConnectionNode(targetConnection!); + return targetConnection + ? ASTNode.createConnectionNode(targetConnection) + : null; } case ASTNode.types.BUTTON: return this.navigateFlyoutContents(true); @@ -575,7 +568,9 @@ export class ASTNode { case ASTNode.types.INPUT: { const connection = this.location as Connection; const targetConnection = connection.targetConnection; - return ASTNode.createConnectionNode(targetConnection!); + return targetConnection + ? ASTNode.createConnectionNode(targetConnection) + : null; } } @@ -708,10 +703,7 @@ export class ASTNode { * @param field The location of the AST node. * @returns An AST node pointing to a field. */ - static createFieldNode(field: Field): ASTNode | null { - if (!field) { - return null; - } + static createFieldNode(field: Field): ASTNode { return new ASTNode(ASTNode.types.FIELD, field); } @@ -724,25 +716,14 @@ export class ASTNode { * @returns An AST node pointing to a connection. */ static createConnectionNode(connection: Connection): ASTNode | null { - if (!connection) { - return null; - } const type = connection.type; - if (type === ConnectionType.INPUT_VALUE) { - // AnyDuringMigration because: Argument of type 'Input | null' is not - // assignable to parameter of type 'Input'. - return ASTNode.createInputNode( - connection.getParentInput() as AnyDuringMigration, - ); - } else if ( - type === ConnectionType.NEXT_STATEMENT && - connection.getParentInput() + const parentInput = connection.getParentInput(); + if ( + (type === ConnectionType.INPUT_VALUE || + type === ConnectionType.NEXT_STATEMENT) && + parentInput ) { - // AnyDuringMigration because: Argument of type 'Input | null' is not - // assignable to parameter of type 'Input'. - return ASTNode.createInputNode( - connection.getParentInput() as AnyDuringMigration, - ); + return ASTNode.createInputNode(parentInput); } else if (type === ConnectionType.NEXT_STATEMENT) { return new ASTNode(ASTNode.types.NEXT, connection); } else if (type === ConnectionType.OUTPUT_VALUE) { @@ -761,7 +742,7 @@ export class ASTNode { * @returns An AST node pointing to a input. */ static createInputNode(input: Input): ASTNode | null { - if (!input || !input.connection) { + if (!input.connection) { return null; } return new ASTNode(ASTNode.types.INPUT, input.connection); @@ -773,10 +754,7 @@ export class ASTNode { * @param block The block used to create an AST node. * @returns An AST node pointing to a block. */ - static createBlockNode(block: Block): ASTNode | null { - if (!block) { - return null; - } + static createBlockNode(block: Block): ASTNode { return new ASTNode(ASTNode.types.BLOCK, block); } @@ -790,10 +768,7 @@ export class ASTNode { * @returns An AST node of type stack that points to the top block on the * stack. */ - static createStackNode(topBlock: Block): ASTNode | null { - if (!topBlock) { - return null; - } + static createStackNode(topBlock: Block): ASTNode { return new ASTNode(ASTNode.types.STACK, topBlock); } @@ -806,10 +781,7 @@ export class ASTNode { * @returns An AST node of type stack that points to the top block on the * stack. */ - static createButtonNode(button: FlyoutButton): ASTNode | null { - if (!button) { - return null; - } + static createButtonNode(button: FlyoutButton): ASTNode { return new ASTNode(ASTNode.types.BUTTON, button); } @@ -822,12 +794,9 @@ export class ASTNode { * workspace. */ static createWorkspaceNode( - workspace: Workspace | null, - wsCoordinate: Coordinate | null, - ): ASTNode | null { - if (!wsCoordinate || !workspace) { - return null; - } + workspace: Workspace, + wsCoordinate: Coordinate, + ): ASTNode { const params = {wsCoordinate}; return new ASTNode(ASTNode.types.WORKSPACE, workspace, params); } diff --git a/core/renderers/common/marker_svg.ts b/core/renderers/common/marker_svg.ts index 77d35883c..4805e7040 100644 --- a/core/renderers/common/marker_svg.ts +++ b/core/renderers/common/marker_svg.ts @@ -287,8 +287,8 @@ export class MarkerSvg { */ protected showWithCoordinates_(curNode: ASTNode) { const wsCoordinate = curNode.getWsCoordinate(); - let x = wsCoordinate.x; - const y = wsCoordinate.y; + let x = wsCoordinate?.x ?? 0; + const y = wsCoordinate?.y ?? 0; if (this.workspace.RTL) { x -= this.constants_.CURSOR_WS_WIDTH;