Skip to content

fix!: Tighten and correct typings on ASTNode #8835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonfunko merged 4 commits into from
Apr 1, 2025
Merged

fix!: Tighten and correct typings on ASTNode #8835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eab10e0
fix!: Tighten typings on ASTNode.create*Node() methods.
gonfunko Mar 31, 2025
709d742
fix: Restore missing condition.
gonfunko Mar 31, 2025
b21e77e
fix: Fix unsafe casts, non-null assertions and incorrect types.
gonfunko Mar 31, 2025
df82018
refactor: Simplify parent input checks.
gonfunko Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 36 additions & 67 deletions core/keyboard_nav/ast_node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand All @@ -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];
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions core/renderers/common/marker_svg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down