Repository: rokucommunity/brighterscript
Pull request #1101 (2024-03-11 16:36:19)

Prevent unused variable warnings on ternary and null coalescence expressions
Prevents unused variable warnings when transpiling ternary and null coalescence expressions if they include enums and constants.

Addresses: #1091 


Patch:
diff --git a/src/parser/Expression.ts b/src/parser/Expression.ts
index c43931be..6ab35a2d 100644
--- a/src/parser/Expression.ts
+++ b/src/parser/Expression.ts
@@ -1393,8 +1393,9 @@ export class TernaryExpression extends Expression {
 
     transpile(state: BrsTranspileState) {
         let result = [] as TranspileResult;
-        let consequentInfo = util.getExpressionInfo(this.consequent!);
-        let alternateInfo = util.getExpressionInfo(this.alternate!);
+        const file = state.file;
+        let consequentInfo = util.getExpressionInfo(this.consequent!, file);
+        let alternateInfo = util.getExpressionInfo(this.alternate!, file);
 
         //get all unique variable names used in the consequent and alternate, and sort them alphabetically so the output is consistent
         let allUniqueVarNames = [...new Set([...consequentInfo.uniqueVarNames, ...alternateInfo.uniqueVarNames])].sort();
@@ -1409,7 +1410,7 @@ export class TernaryExpression extends Expression {
                     this.questionMarkToken,
                     //write all the scope variables as parameters.
                     //TODO handle when there are more than 31 parameters
-                    `(function(__bsCondition, ${allUniqueVarNames.join(', ')})`
+                    `(function(${['__bsCondition', ...allUniqueVarNames].join(', ')})`
                 ),
                 state.newline,
                 //double indent so our `end function` line is still indented one at the end
@@ -1433,7 +1434,7 @@ export class TernaryExpression extends Expression {
                 state.indent(-1),
                 state.sourceNode(this.questionMarkToken, 'end function)('),
                 ...this.test.transpile(state),
-                state.sourceNode(this.questionMarkToken, `, ${allUniqueVarNames.join(', ')})`)
+                state.sourceNode(this.questionMarkToken, `${['', ...allUniqueVarNames].join(', ')})`)
             );
             state.blockDepth--;
         } else {
@@ -1476,8 +1477,8 @@ export class NullCoalescingExpression extends Expression {
 
     transpile(state: BrsTranspileState) {
         let result = [] as TranspileResult;
-        let consequentInfo = util.getExpressionInfo(this.consequent);
-        let alternateInfo = util.getExpressionInfo(this.alternate);
+        let consequentInfo = util.getExpressionInfo(this.consequent, state.file);
+        let alternateInfo = util.getExpressionInfo(this.alternate, state.file);
 
         //get all unique variable names used in the consequent and alternate, and sort them alphabetically so the output is consistent
         let allUniqueVarNames = [...new Set([...consequentInfo.uniqueVarNames, ...alternateInfo.uniqueVarNames])].sort();
diff --git a/src/util.ts b/src/util.ts
index 62ff230a..805a6887 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -1189,7 +1189,7 @@ export class Util {
      * Gathers expressions, variables, and unique names from an expression.
      * This is mostly used for the ternary expression
      */
-    public getExpressionInfo(expression: Expression): ExpressionInfo {
+    public getExpressionInfo(expression: Expression, file: BrsFile): ExpressionInfo {
         const expressions = [expression];
         const variableExpressions = [] as VariableExpression[];
         const uniqueVarNames = new Set<string>();
@@ -1213,7 +1213,15 @@ export class Util {
         //handle the expression itself (for situations when expression is a VariableExpression)
         expressionWalker(expression);
 
-        return { expressions: expressions, varExpressions: variableExpressions, uniqueVarNames: [...uniqueVarNames] };
+        const scope = file.program.getFirstScopeForFile(file);
+        const filteredVarNames = [...uniqueVarNames].filter((varName: string) => {
+            const varNameLower = varName.toLowerCase();
+            // TODO: include namespaces in this filter
+            return !scope.getEnumMap().has(varNameLower) &&
+                !scope.getConstMap().has(varNameLower);
+        });
+
+        return { expressions: expressions, varExpressions: variableExpressions, uniqueVarNames: filteredVarNames };
     }
 
 


Test patch:
diff --git a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts
index 88ea8145..e0fc7e08 100644
--- a/src/parser/tests/expression/NullCoalescenceExpression.spec.ts
+++ b/src/parser/tests/expression/NullCoalescenceExpression.spec.ts
@@ -280,5 +280,51 @@ describe('NullCoalescingExpression', () => {
                 end sub
             `);
         });
+
+        it('ignores enum variable names', () => {
+            testTranspile(`
+                enum Direction
+                    up = "up"
+                end enum
+                sub main()
+                    d = invalid
+                    a = d ?? Direction.up
+                end sub
+            `, `
+                sub main()
+                    d = invalid
+                    a = (function(d)
+                            __bsConsequent = d
+                            if __bsConsequent <> invalid then
+                                return __bsConsequent
+                            else
+                                return "up"
+                            end if
+                        end function)(d)
+                end sub
+            `);
+        });
+
+        it('ignores const variable names', () => {
+            testTranspile(`
+                const USER = "user"
+                sub main()
+                    settings = {}
+                    a = m.defaults.getAccount(settings.name) ?? USER
+                end sub
+            `, `
+                sub main()
+                    settings = {}
+                    a = (function(m, settings)
+                            __bsConsequent = m.defaults.getAccount(settings.name)
+                            if __bsConsequent <> invalid then
+                                return __bsConsequent
+                            else
+                                return "user"
+                            end if
+                        end function)(m, settings)
+                end sub
+            `);
+        });
     });
 });
diff --git a/src/parser/tests/expression/TernaryExpression.spec.ts b/src/parser/tests/expression/TernaryExpression.spec.ts
index c1426506..3768ffe8 100644
--- a/src/parser/tests/expression/TernaryExpression.spec.ts
+++ b/src/parser/tests/expression/TernaryExpression.spec.ts
@@ -502,6 +502,55 @@ describe('ternary expressions', () => {
             `);
         });
 
+        it('ignores enum variable names', () => {
+            testTranspile(`
+                enum Direction
+                    up = "up"
+                    down = "down"
+                end enum
+                sub main()
+                    d = Direction.up
+                    theDir = d = Direction.up ? Direction.up : false
+                end sub
+            `, `
+                sub main()
+                    d = "up"
+                    theDir = (function(__bsCondition)
+                            if __bsCondition then
+                                return "up"
+                            else
+                                return false
+                            end if
+                        end function)(d = "up")
+                end sub
+            `);
+        });
+
+        it('ignores const variable names', () => {
+            testTranspile(`
+                enum Direction
+                    up = "up"
+                    down = "down"
+                end enum
+                const UP = "up"
+                sub main()
+                    d = Direction.up
+                    theDir = d = Direction.up ? UP : Direction.down
+                end sub
+            `, `
+                sub main()
+                    d = "up"
+                    theDir = (function(__bsCondition)
+                            if __bsCondition then
+                                return "up"
+                            else
+                                return "down"
+                            end if
+                        end function)(d = "up")
+                end sub
+            `);
+        });
+
         it('supports scope-captured outer, and simple inner', () => {
             testTranspile(
                 `