From 0cfae88e99455d7cf28e9b94e67ec5195f9fd846 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 16:19:02 +0000 Subject: [PATCH 1/5] Initial plan From ce2492fcf77c224e0061bd2ace47fc5b4453ec34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 16:41:20 +0000 Subject: [PATCH 2/5] Implement simplified transpilation for null-coalescing operator Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- source/test-complex-assignment.bs | 5 + source/test-dotted-set.bs | 5 + source/test-indexed-set.bs | 5 + source/test-null-coalescing.bs | 5 + src/astUtils/creators.ts | 2 +- .../transpile/BrsFilePreTranspileProcessor.ts | 191 +++++++++++++++++- .../NullCoalescenceExpression.spec.ts | 183 ++++++++++++++++- 7 files changed, 387 insertions(+), 9 deletions(-) create mode 100644 source/test-complex-assignment.bs create mode 100644 source/test-dotted-set.bs create mode 100644 source/test-indexed-set.bs create mode 100644 source/test-null-coalescing.bs diff --git a/source/test-complex-assignment.bs b/source/test-complex-assignment.bs new file mode 100644 index 000000000..77aa1cb4f --- /dev/null +++ b/source/test-complex-assignment.bs @@ -0,0 +1,5 @@ +sub main() + user = invalid + count += user ?? 0 + print count +end sub \ No newline at end of file diff --git a/source/test-dotted-set.bs b/source/test-dotted-set.bs new file mode 100644 index 000000000..1c955fc61 --- /dev/null +++ b/source/test-dotted-set.bs @@ -0,0 +1,5 @@ +sub main() + user = invalid + m.chosenUser = user ?? {} + print m.chosenUser +end sub \ No newline at end of file diff --git a/source/test-indexed-set.bs b/source/test-indexed-set.bs new file mode 100644 index 000000000..c0ef036f0 --- /dev/null +++ b/source/test-indexed-set.bs @@ -0,0 +1,5 @@ +sub main() + user = invalid + m["chosenUser"] = user ?? {} + print m["chosenUser"] +end sub \ No newline at end of file diff --git a/source/test-null-coalescing.bs b/source/test-null-coalescing.bs new file mode 100644 index 000000000..640495e08 --- /dev/null +++ b/source/test-null-coalescing.bs @@ -0,0 +1,5 @@ +sub main() + user = invalid + chosenUser = user ?? {} + print chosenUser +end sub \ No newline at end of file diff --git a/src/astUtils/creators.ts b/src/astUtils/creators.ts index 97906e2bc..c3b205df6 100644 --- a/src/astUtils/creators.ts +++ b/src/astUtils/creators.ts @@ -203,7 +203,7 @@ export function createIfStatement(options: { { if: options.if ?? createToken(TokenKind.If), then: options.then ?? createToken(TokenKind.Then), - else: options.else ?? createToken(TokenKind.Else), + else: options.elseBranch ? (options.else ?? createToken(TokenKind.Else)) : undefined, endIf: options.endIf ?? createToken(TokenKind.EndIf) }, options.condition, diff --git a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts index d61657ad4..b924b0805 100644 --- a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts +++ b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts @@ -1,13 +1,13 @@ -import { createAssignmentStatement, createBlock, createDottedSetStatement, createIfStatement, createIndexedSetStatement, createToken } from '../../astUtils/creators'; -import { isAssignmentStatement, isBinaryExpression, isBlock, isBody, isBrsFile, isDottedGetExpression, isDottedSetStatement, isGroupingExpression, isIndexedGetExpression, isIndexedSetStatement, isLiteralExpression, isUnaryExpression, isVariableExpression } from '../../astUtils/reflection'; +import { createAssignmentStatement, createBlock, createDottedSetStatement, createIfStatement, createIndexedSetStatement, createToken, createVariableExpression, createInvalidLiteral } from '../../astUtils/creators'; +import { isAssignmentStatement, isBinaryExpression, isBlock, isBody, isBrsFile, isCallExpression, isCallfuncExpression, isDottedGetExpression, isDottedSetStatement, isGroupingExpression, isIndexedGetExpression, isIndexedSetStatement, isLiteralExpression, isUnaryExpression, isVariableExpression } from '../../astUtils/reflection'; import { createVisitor, WalkMode } from '../../astUtils/visitors'; import type { BrsFile } from '../../files/BrsFile'; import type { BeforeFileTranspileEvent } from '../../interfaces'; import type { Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import type { Expression, Statement } from '../../parser/AstNode'; -import type { TernaryExpression } from '../../parser/Expression'; -import { LiteralExpression } from '../../parser/Expression'; +import type { TernaryExpression, NullCoalescingExpression } from '../../parser/Expression'; +import { BinaryExpression, DottedGetExpression, IndexedGetExpression, LiteralExpression } from '../../parser/Expression'; import { ParseMode } from '../../parser/Parser'; import type { IfStatement } from '../../parser/Statement'; import type { Scope } from '../../Scope'; @@ -41,6 +41,9 @@ export class BrsFilePreTranspileProcessor { const visitor = createVisitor({ TernaryExpression: (ternaryExpression) => { this.processTernaryExpression(ternaryExpression, visitor, walkMode); + }, + NullCoalescingExpression: (nullCoalescingExpression) => { + this.processNullCoalescingExpression(nullCoalescingExpression, visitor, walkMode); } }); this.event.file.ast.walk(visitor, { walkMode: walkMode }); @@ -178,6 +181,186 @@ export class BrsFilePreTranspileProcessor { } } + private processNullCoalescingExpression(nullCoalescingExpression: NullCoalescingExpression, visitor: ReturnType, walkMode: WalkMode) { + // Check if this null coalescing expression has mutating expressions that require scope protection + const consequentInfo = util.getExpressionInfo(nullCoalescingExpression.consequent, this.event.file); + const alternateInfo = util.getExpressionInfo(nullCoalescingExpression.alternate, this.event.file); + + let hasMutatingExpression = [ + ...consequentInfo.expressions, + ...alternateInfo.expressions + ].find(e => isCallExpression(e) || isCallfuncExpression(e) || isDottedGetExpression(e)); + + // Only optimize if there are no mutating expressions + if (hasMutatingExpression) { + return; + } + + function getOwnerAndKey(statement: Statement) { + const parent = statement.parent; + if (isBlock(parent) || isBody(parent)) { + let idx = parent.statements.indexOf(statement); + if (idx > -1) { + return { owner: parent.statements, key: idx }; + } + } + } + + //if the null coalescing expression is part of a simple assignment, rewrite it as an `IfStatement` + let parent = nullCoalescingExpression.findAncestor(x => !isGroupingExpression(x)); + let operator: Token; + //operators like `+=` will cause the RHS to be a BinaryExpression due to how the parser handles this. let's do a little magic to detect this situation + if ( + //parent is a binary expression + isBinaryExpression(parent) && + ( + (isAssignmentStatement(parent.parent) && isVariableExpression(parent.left) && parent.left.name === parent.parent.name) || + (isDottedSetStatement(parent.parent) && isDottedGetExpression(parent.left) && parent.left.name === parent.parent.name) || + (isIndexedSetStatement(parent.parent) && isIndexedGetExpression(parent.left) && parent.left.index === parent.parent.index) + ) + ) { + //keep the correct operator (i.e. `+=`) + operator = parent.operator; + //use the outer parent and skip this BinaryExpression + parent = parent.parent; + } + let ifStatement: IfStatement; + + if (isAssignmentStatement(parent)) { + // Create condition: variableName = invalid + const condition = new BinaryExpression( + createVariableExpression(parent.name.text), + createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); + + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createAssignmentStatement({ + name: parent.name, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.alternate + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); + + // First, we need to create the initial assignment statement + const initialAssignment = createAssignmentStatement({ + name: parent.name, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.consequent + }); + + // Replace the parent with a sequence: first the initial assignment, then the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + // Replace with initial assignment first + this.event.editor.setProperty(owner, key, initialAssignment); + // Insert the if statement after + this.event.editor.addToArray(owner, key + 1, ifStatement); + } + } else if (isDottedSetStatement(parent)) { + // Create condition: obj.name = invalid + const condition = new BinaryExpression( + new DottedGetExpression(parent.obj, parent.name, parent.dot ?? createToken(TokenKind.Dot, '.', nullCoalescingExpression.questionQuestionToken.range)), + createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); + + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createDottedSetStatement({ + obj: parent.obj, + name: parent.name, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.alternate + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); + + // First, we need to create the initial dotted set statement + const initialDottedSet = createDottedSetStatement({ + obj: parent.obj, + name: parent.name, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.consequent + }); + + // Replace the parent with a sequence: first the initial assignment, then the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + // Replace with initial assignment first + this.event.editor.setProperty(owner, key, initialDottedSet); + // Insert the if statement after + this.event.editor.addToArray(owner, key + 1, ifStatement); + } + } else if (isIndexedSetStatement(parent) && parent.index !== nullCoalescingExpression && !parent.additionalIndexes?.includes(nullCoalescingExpression)) { + // Create condition: obj[index] = invalid + const condition = new BinaryExpression( + new IndexedGetExpression(parent.obj, parent.index, parent.openingSquare, parent.closingSquare, undefined, parent.additionalIndexes), + createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); + + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createIndexedSetStatement({ + obj: parent.obj, + openingSquare: parent.openingSquare, + index: parent.index, + closingSquare: parent.closingSquare, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.alternate, + additionalIndexes: parent.additionalIndexes + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); + + // First, we need to create the initial indexed set statement + const initialIndexedSet = createIndexedSetStatement({ + obj: parent.obj, + openingSquare: parent.openingSquare, + index: parent.index, + closingSquare: parent.closingSquare, + equals: operator ?? parent.equals, + value: nullCoalescingExpression.consequent, + additionalIndexes: parent.additionalIndexes + }); + + // Replace the parent with a sequence: first the initial assignment, then the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + // Replace with initial assignment first + this.event.editor.setProperty(owner, key, initialIndexedSet); + // Insert the if statement after + this.event.editor.addToArray(owner, key + 1, ifStatement); + } + } + + if (ifStatement) { + //we've injected an ifStatement, so now we need to trigger a walk to handle any nested null coalescing expressions + ifStatement.walk(visitor, { walkMode: walkMode }); + } + } + /** * Given a string optionally separated by dots, find an enum related to it. * For example, all of these would return the enum: `SomeNamespace.SomeEnum.SomeMember`, SomeEnum.SomeMember, `SomeEnum` diff --git a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts index a9dfccc1a..dfab74a56 100644 --- a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts +++ b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts @@ -197,7 +197,10 @@ describe('NullCoalescingExpression', () => { end sub `, ` sub main() - a = rokucommunity_bslib_coalesce(user, false) + a = user + if a = invalid then + a = false + end if end sub `); }); @@ -209,9 +212,12 @@ describe('NullCoalescingExpression', () => { end sub `, ` sub main() - a = bslib_coalesce(user, { - "id": "default" - }) + a = user + if a = invalid then + a = { + "id": "default" + } + end if end sub `); }); @@ -374,5 +380,174 @@ describe('NullCoalescingExpression', () => { end sub `); }); + + it('transpiles null coalescing in RHS of AssignmentStatement to if statement', () => { + testTranspile(` + sub main() + a = user ?? {} + end sub + `, ` + sub main() + a = user + if a = invalid then + a = {} + end if + end sub + `); + }); + + it('transpiles null coalescing in RHS of incrementor AssignmentStatement to if statement', () => { + testTranspile(` + sub main() + a += user ?? 0 + end sub + `, ` + sub main() + a += user + if a = invalid then + a += 0 + end if + end sub + `); + }); + + it('transpiles null coalescing in RHS of DottedSetStatement to if statement', () => { + testTranspile(` + sub main() + m.a = user ?? {} + end sub + `, ` + sub main() + m.a = user + if m.a = invalid then + m.a = {} + end if + end sub + `); + }); + + it('transpiles null coalescing in RHS of incrementor DottedSetStatement to if statement', () => { + testTranspile(` + sub main() + m.a += user ?? 0 + end sub + `, ` + sub main() + m.a += user + if m.a = invalid then + m.a += 0 + end if + end sub + `); + }); + + it('transpiles null coalescing in RHS of IndexedSetStatement to if statement', () => { + testTranspile(` + sub main() + m["a"] = user ?? {} + end sub + `, ` + sub main() + m["a"] = user + if m["a"] = invalid then + m["a"] = {} + end if + end sub + `); + }); + + it('transpiles null coalescing in RHS of incrementor IndexedSetStatement to if statement', () => { + testTranspile(` + sub main() + m["a"] += user ?? 0 + end sub + `, ` + sub main() + m["a"] += user + if m["a"] = invalid then + m["a"] += 0 + end if + end sub + `); + }); + + it('supports nested null coalescing in assignment', () => { + testTranspile(` + sub main() + result = user ?? (fallback ?? {}) + end sub + `, ` + sub main() + result = user + if result = invalid then + result = fallback + if result = invalid then + result = {} + end if + end if + end sub + `); + }); + + it('supports nested null coalescing in DottedSet', () => { + testTranspile(` + sub main() + m.result = user ?? (fallback ?? {}) + end sub + `, ` + sub main() + m.result = user + if m.result = invalid then + m.result = fallback + if m.result = invalid then + m.result = {} + end if + end if + end sub + `); + }); + + it('supports nested null coalescing in IndexedSet', () => { + testTranspile(` + sub main() + m["result"] = user ?? (fallback ?? {}) + end sub + `, ` + sub main() + m["result"] = user + if m["result"] = invalid then + m["result"] = fallback + if m["result"] = invalid then + m["result"] = {} + end if + end if + end sub + `); + }); + + it('uses scope-captured functions for complex expressions', () => { + testTranspile(` + sub main() + zombie = {} + result = [ + zombie.getName() ?? "zombie" + ] + end sub + `, ` + sub main() + zombie = {} + result = [ + (function(zombie) + __bsConsequent = zombie.getName() + if __bsConsequent <> invalid then + return __bsConsequent + else + return "zombie" + end if + end function)(zombie) + ] + end sub + `); + }); }); }); From 237b9712b4497e88f6c11afdd4f3528abb32d5a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 16:43:32 +0000 Subject: [PATCH 3/5] Optimize null-coalescing operator transpilation to if/else statements Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- source/test-complex-assignment.bs | 5 ----- source/test-dotted-set.bs | 5 ----- source/test-indexed-set.bs | 5 ----- source/test-null-coalescing.bs | 5 ----- 4 files changed, 20 deletions(-) delete mode 100644 source/test-complex-assignment.bs delete mode 100644 source/test-dotted-set.bs delete mode 100644 source/test-indexed-set.bs delete mode 100644 source/test-null-coalescing.bs diff --git a/source/test-complex-assignment.bs b/source/test-complex-assignment.bs deleted file mode 100644 index 77aa1cb4f..000000000 --- a/source/test-complex-assignment.bs +++ /dev/null @@ -1,5 +0,0 @@ -sub main() - user = invalid - count += user ?? 0 - print count -end sub \ No newline at end of file diff --git a/source/test-dotted-set.bs b/source/test-dotted-set.bs deleted file mode 100644 index 1c955fc61..000000000 --- a/source/test-dotted-set.bs +++ /dev/null @@ -1,5 +0,0 @@ -sub main() - user = invalid - m.chosenUser = user ?? {} - print m.chosenUser -end sub \ No newline at end of file diff --git a/source/test-indexed-set.bs b/source/test-indexed-set.bs deleted file mode 100644 index c0ef036f0..000000000 --- a/source/test-indexed-set.bs +++ /dev/null @@ -1,5 +0,0 @@ -sub main() - user = invalid - m["chosenUser"] = user ?? {} - print m["chosenUser"] -end sub \ No newline at end of file diff --git a/source/test-null-coalescing.bs b/source/test-null-coalescing.bs deleted file mode 100644 index 640495e08..000000000 --- a/source/test-null-coalescing.bs +++ /dev/null @@ -1,5 +0,0 @@ -sub main() - user = invalid - chosenUser = user ?? {} - print chosenUser -end sub \ No newline at end of file From ad6757f7c56537e1a8a4909a3c4d44e8b0d90a76 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 19:08:17 +0000 Subject: [PATCH 4/5] Restrict null-coalescing optimization to local variable assignments only Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- .../transpile/BrsFilePreTranspileProcessor.ts | 228 +++++++----------- .../NullCoalescenceExpression.spec.ts | 89 +------ 2 files changed, 99 insertions(+), 218 deletions(-) diff --git a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts index b924b0805..c20c15f33 100644 --- a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts +++ b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts @@ -7,7 +7,7 @@ import type { Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import type { Expression, Statement } from '../../parser/AstNode'; import type { TernaryExpression, NullCoalescingExpression } from '../../parser/Expression'; -import { BinaryExpression, DottedGetExpression, IndexedGetExpression, LiteralExpression } from '../../parser/Expression'; +import { BinaryExpression, LiteralExpression } from '../../parser/Expression'; import { ParseMode } from '../../parser/Parser'; import type { IfStatement } from '../../parser/Statement'; import type { Scope } from '../../Scope'; @@ -182,17 +182,17 @@ export class BrsFilePreTranspileProcessor { } private processNullCoalescingExpression(nullCoalescingExpression: NullCoalescingExpression, visitor: ReturnType, walkMode: WalkMode) { - // Check if this null coalescing expression has mutating expressions that require scope protection + // Check if this null coalescing expression has complex expressions that require scope protection const consequentInfo = util.getExpressionInfo(nullCoalescingExpression.consequent, this.event.file); const alternateInfo = util.getExpressionInfo(nullCoalescingExpression.alternate, this.event.file); - - let hasMutatingExpression = [ + + let hasComplexExpression = [ ...consequentInfo.expressions, ...alternateInfo.expressions - ].find(e => isCallExpression(e) || isCallfuncExpression(e) || isDottedGetExpression(e)); + ].find(e => isCallExpression(e) || isCallfuncExpression(e) || isDottedGetExpression(e) || isIndexedGetExpression(e)); - // Only optimize if there are no mutating expressions - if (hasMutatingExpression) { + // Only optimize if there are no complex expressions + if (hasComplexExpression) { return; } @@ -206,18 +206,14 @@ export class BrsFilePreTranspileProcessor { } } - //if the null coalescing expression is part of a simple assignment, rewrite it as an `IfStatement` + //if the null coalescing expression is part of a simple assignment to a local variable, rewrite it as an `IfStatement` let parent = nullCoalescingExpression.findAncestor(x => !isGroupingExpression(x)); let operator: Token; //operators like `+=` will cause the RHS to be a BinaryExpression due to how the parser handles this. let's do a little magic to detect this situation if ( //parent is a binary expression isBinaryExpression(parent) && - ( - (isAssignmentStatement(parent.parent) && isVariableExpression(parent.left) && parent.left.name === parent.parent.name) || - (isDottedSetStatement(parent.parent) && isDottedGetExpression(parent.left) && parent.left.name === parent.parent.name) || - (isIndexedSetStatement(parent.parent) && isIndexedGetExpression(parent.left) && parent.left.index === parent.parent.index) - ) + isAssignmentStatement(parent.parent) && isVariableExpression(parent.left) && parent.left.name === parent.parent.name ) { //keep the correct operator (i.e. `+=`) operator = parent.operator; @@ -226,132 +222,88 @@ export class BrsFilePreTranspileProcessor { } let ifStatement: IfStatement; + // Only support AssignmentStatement to local variables if (isAssignmentStatement(parent)) { - // Create condition: variableName = invalid - const condition = new BinaryExpression( - createVariableExpression(parent.name.text), - createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), - createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) - ); - - ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), - condition: condition, - then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), - thenBranch: createBlock({ - statements: [ - createAssignmentStatement({ - name: parent.name, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.alternate - }) - ] - }), - endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) - }); - - // First, we need to create the initial assignment statement - const initialAssignment = createAssignmentStatement({ - name: parent.name, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.consequent - }); - - // Replace the parent with a sequence: first the initial assignment, then the if statement - let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; - if (owner && key !== undefined) { - // Replace with initial assignment first - this.event.editor.setProperty(owner, key, initialAssignment); - // Insert the if statement after - this.event.editor.addToArray(owner, key + 1, ifStatement); - } - } else if (isDottedSetStatement(parent)) { - // Create condition: obj.name = invalid - const condition = new BinaryExpression( - new DottedGetExpression(parent.obj, parent.name, parent.dot ?? createToken(TokenKind.Dot, '.', nullCoalescingExpression.questionQuestionToken.range)), - createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), - createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) - ); - - ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), - condition: condition, - then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), - thenBranch: createBlock({ - statements: [ - createDottedSetStatement({ - obj: parent.obj, - name: parent.name, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.alternate - }) - ] - }), - endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) - }); - - // First, we need to create the initial dotted set statement - const initialDottedSet = createDottedSetStatement({ - obj: parent.obj, - name: parent.name, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.consequent - }); - - // Replace the parent with a sequence: first the initial assignment, then the if statement - let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; - if (owner && key !== undefined) { - // Replace with initial assignment first - this.event.editor.setProperty(owner, key, initialDottedSet); - // Insert the if statement after - this.event.editor.addToArray(owner, key + 1, ifStatement); - } - } else if (isIndexedSetStatement(parent) && parent.index !== nullCoalescingExpression && !parent.additionalIndexes?.includes(nullCoalescingExpression)) { - // Create condition: obj[index] = invalid - const condition = new BinaryExpression( - new IndexedGetExpression(parent.obj, parent.index, parent.openingSquare, parent.closingSquare, undefined, parent.additionalIndexes), - createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), - createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) - ); - - ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), - condition: condition, - then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), - thenBranch: createBlock({ - statements: [ - createIndexedSetStatement({ - obj: parent.obj, - openingSquare: parent.openingSquare, - index: parent.index, - closingSquare: parent.closingSquare, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.alternate, - additionalIndexes: parent.additionalIndexes - }) - ] - }), - endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) - }); - - // First, we need to create the initial indexed set statement - const initialIndexedSet = createIndexedSetStatement({ - obj: parent.obj, - openingSquare: parent.openingSquare, - index: parent.index, - closingSquare: parent.closingSquare, - equals: operator ?? parent.equals, - value: nullCoalescingExpression.consequent, - additionalIndexes: parent.additionalIndexes - }); - - // Replace the parent with a sequence: first the initial assignment, then the if statement - let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; - if (owner && key !== undefined) { - // Replace with initial assignment first - this.event.editor.setProperty(owner, key, initialIndexedSet); - // Insert the if statement after - this.event.editor.addToArray(owner, key + 1, ifStatement); + if (operator) { + // For compound assignments like `a += user ?? 0`, we need to check the left side value first + // Create condition: user <> invalid + const condition = new BinaryExpression( + nullCoalescingExpression.consequent, + createToken(TokenKind.LessGreater, '<>', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); + + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createAssignmentStatement({ + name: parent.name, + equals: operator, + value: nullCoalescingExpression.consequent + }) + ] + }), + else: createToken(TokenKind.Else, 'else', nullCoalescingExpression.questionQuestionToken.range), + elseBranch: createBlock({ + statements: [ + createAssignmentStatement({ + name: parent.name, + equals: operator, + value: nullCoalescingExpression.alternate + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); + + // Replace the parent statement with the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + this.event.editor.setProperty(owner, key, ifStatement); + } + } else { + // For simple assignments like `a = user ?? {}`, use the original logic + // Create condition: variableName = invalid + const condition = new BinaryExpression( + createVariableExpression(parent.name.text), + createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); + + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createAssignmentStatement({ + name: parent.name, + equals: parent.equals, + value: nullCoalescingExpression.alternate + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); + + // First, we need to create the initial assignment statement + const initialAssignment = createAssignmentStatement({ + name: parent.name, + equals: parent.equals, + value: nullCoalescingExpression.consequent + }); + + // Replace the parent with a sequence: first the initial assignment, then the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + // Replace with initial assignment first + this.event.editor.setProperty(owner, key, initialAssignment); + // Insert the if statement after + this.event.editor.addToArray(owner, key + 1, ifStatement); + } } } diff --git a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts index dfab74a56..95cc4104e 100644 --- a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts +++ b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts @@ -403,74 +403,15 @@ describe('NullCoalescingExpression', () => { end sub `, ` sub main() - a += user - if a = invalid then + if user <> invalid then + a += user + else a += 0 end if end sub `); }); - it('transpiles null coalescing in RHS of DottedSetStatement to if statement', () => { - testTranspile(` - sub main() - m.a = user ?? {} - end sub - `, ` - sub main() - m.a = user - if m.a = invalid then - m.a = {} - end if - end sub - `); - }); - - it('transpiles null coalescing in RHS of incrementor DottedSetStatement to if statement', () => { - testTranspile(` - sub main() - m.a += user ?? 0 - end sub - `, ` - sub main() - m.a += user - if m.a = invalid then - m.a += 0 - end if - end sub - `); - }); - - it('transpiles null coalescing in RHS of IndexedSetStatement to if statement', () => { - testTranspile(` - sub main() - m["a"] = user ?? {} - end sub - `, ` - sub main() - m["a"] = user - if m["a"] = invalid then - m["a"] = {} - end if - end sub - `); - }); - - it('transpiles null coalescing in RHS of incrementor IndexedSetStatement to if statement', () => { - testTranspile(` - sub main() - m["a"] += user ?? 0 - end sub - `, ` - sub main() - m["a"] += user - if m["a"] = invalid then - m["a"] += 0 - end if - end sub - `); - }); - it('supports nested null coalescing in assignment', () => { testTranspile(` sub main() @@ -489,38 +430,26 @@ describe('NullCoalescingExpression', () => { `); }); - it('supports nested null coalescing in DottedSet', () => { + it('uses scope-captured functions for DottedSet expressions', () => { testTranspile(` sub main() - m.result = user ?? (fallback ?? {}) + m.a = user ?? {} end sub `, ` sub main() - m.result = user - if m.result = invalid then - m.result = fallback - if m.result = invalid then - m.result = {} - end if - end if + m.a = bslib_coalesce(user, {}) end sub `); }); - it('supports nested null coalescing in IndexedSet', () => { + it('uses scope-captured functions for IndexedSet expressions', () => { testTranspile(` sub main() - m["result"] = user ?? (fallback ?? {}) + m["a"] = user ?? {} end sub `, ` sub main() - m["result"] = user - if m["result"] = invalid then - m["result"] = fallback - if m["result"] = invalid then - m["result"] = {} - end if - end if + m["a"] = bslib_coalesce(user, {}) end sub `); }); From b3a911bfef03bff9c6648238d2f8209485356baf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 20:15:13 +0000 Subject: [PATCH 5/5] Remove compound assignment operators support from null-coalescing optimization Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- .../transpile/BrsFilePreTranspileProcessor.ts | 126 +++++------------- .../NullCoalescenceExpression.spec.ts | 8 +- 2 files changed, 38 insertions(+), 96 deletions(-) diff --git a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts index c20c15f33..f1b87a51a 100644 --- a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts +++ b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts @@ -208,102 +208,48 @@ export class BrsFilePreTranspileProcessor { //if the null coalescing expression is part of a simple assignment to a local variable, rewrite it as an `IfStatement` let parent = nullCoalescingExpression.findAncestor(x => !isGroupingExpression(x)); - let operator: Token; - //operators like `+=` will cause the RHS to be a BinaryExpression due to how the parser handles this. let's do a little magic to detect this situation - if ( - //parent is a binary expression - isBinaryExpression(parent) && - isAssignmentStatement(parent.parent) && isVariableExpression(parent.left) && parent.left.name === parent.parent.name - ) { - //keep the correct operator (i.e. `+=`) - operator = parent.operator; - //use the outer parent and skip this BinaryExpression - parent = parent.parent; - } let ifStatement: IfStatement; - // Only support AssignmentStatement to local variables + // Only support simple AssignmentStatement to local variables if (isAssignmentStatement(parent)) { - if (operator) { - // For compound assignments like `a += user ?? 0`, we need to check the left side value first - // Create condition: user <> invalid - const condition = new BinaryExpression( - nullCoalescingExpression.consequent, - createToken(TokenKind.LessGreater, '<>', nullCoalescingExpression.questionQuestionToken.range), - createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) - ); - - ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), - condition: condition, - then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), - thenBranch: createBlock({ - statements: [ - createAssignmentStatement({ - name: parent.name, - equals: operator, - value: nullCoalescingExpression.consequent - }) - ] - }), - else: createToken(TokenKind.Else, 'else', nullCoalescingExpression.questionQuestionToken.range), - elseBranch: createBlock({ - statements: [ - createAssignmentStatement({ - name: parent.name, - equals: operator, - value: nullCoalescingExpression.alternate - }) - ] - }), - endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) - }); - - // Replace the parent statement with the if statement - let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; - if (owner && key !== undefined) { - this.event.editor.setProperty(owner, key, ifStatement); - } - } else { - // For simple assignments like `a = user ?? {}`, use the original logic - // Create condition: variableName = invalid - const condition = new BinaryExpression( - createVariableExpression(parent.name.text), - createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), - createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) - ); + // For simple assignments like `a = user ?? {}`, use the original logic + // Create condition: variableName = invalid + const condition = new BinaryExpression( + createVariableExpression(parent.name.text), + createToken(TokenKind.Equal, '=', nullCoalescingExpression.questionQuestionToken.range), + createInvalidLiteral('invalid', nullCoalescingExpression.questionQuestionToken.range) + ); - ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), - condition: condition, - then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), - thenBranch: createBlock({ - statements: [ - createAssignmentStatement({ - name: parent.name, - equals: parent.equals, - value: nullCoalescingExpression.alternate - }) - ] - }), - endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) - }); + ifStatement = createIfStatement({ + if: createToken(TokenKind.If, 'if', nullCoalescingExpression.questionQuestionToken.range), + condition: condition, + then: createToken(TokenKind.Then, 'then', nullCoalescingExpression.questionQuestionToken.range), + thenBranch: createBlock({ + statements: [ + createAssignmentStatement({ + name: parent.name, + equals: parent.equals, + value: nullCoalescingExpression.alternate + }) + ] + }), + endIf: createToken(TokenKind.EndIf, 'end if', nullCoalescingExpression.questionQuestionToken.range) + }); - // First, we need to create the initial assignment statement - const initialAssignment = createAssignmentStatement({ - name: parent.name, - equals: parent.equals, - value: nullCoalescingExpression.consequent - }); + // First, we need to create the initial assignment statement + const initialAssignment = createAssignmentStatement({ + name: parent.name, + equals: parent.equals, + value: nullCoalescingExpression.consequent + }); - // Replace the parent with a sequence: first the initial assignment, then the if statement - let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; - if (owner && key !== undefined) { - // Replace with initial assignment first - this.event.editor.setProperty(owner, key, initialAssignment); - // Insert the if statement after - this.event.editor.addToArray(owner, key + 1, ifStatement); - } + // Replace the parent with a sequence: first the initial assignment, then the if statement + let { owner, key } = getOwnerAndKey(parent as Statement) ?? {}; + if (owner && key !== undefined) { + // Replace with initial assignment first + this.event.editor.setProperty(owner, key, initialAssignment); + // Insert the if statement after + this.event.editor.addToArray(owner, key + 1, ifStatement); } } diff --git a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts index 95cc4104e..c245e4801 100644 --- a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts +++ b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts @@ -396,18 +396,14 @@ describe('NullCoalescingExpression', () => { `); }); - it('transpiles null coalescing in RHS of incrementor AssignmentStatement to if statement', () => { + it('uses bslib_coalesce for compound assignment operators', () => { testTranspile(` sub main() a += user ?? 0 end sub `, ` sub main() - if user <> invalid then - a += user - else - a += 0 - end if + a += bslib_coalesce(user, 0) end sub `); });