Repository: rokucommunity/brighterscript (ts)

Issue:
Certain enums aren't being transpiled
Some code patterns seem to prevent enums from transpiling. 

```
serviceField = m.entitlementService = someNamespace.authorization.EntitlementService.someEnumValue ? "enabled" : "disabled"
result = Direction.up = "up" or Direction.down = "down" and Direction.up = Direction.down
```

These issues can be mitigated by wrapping the enum expression in parens, but that's gross so we should fix this.

Pull request description:
Fix bug that wasn't transpiling enums in binary expressions
Certain enums weren't being transpiled property. This was because expressions inside of `BinaryExpressions` were totally missing from the `references.expressions` collection. This fixes that issue.
fixes #604 
Fixes some enum highlighting bugs

Patch:
diff --git a/src/LanguageServer.spec.ts b/src/LanguageServer.spec.ts
index a6d9a1e3..25ed2486 100644
--- a/src/LanguageServer.spec.ts
+++ b/src/LanguageServer.spec.ts
@@ -418,7 +418,7 @@ describe('LanguageServer', () => {
             await server['syncProjects']();
 
             expect(
-                server.projects.map(x => x.projectPath)
+                server.projects.map(x => x.projectPath).sort()
             ).to.eql([
                 s`${tempDir}/root`,
                 s`${tempDir}/root/subdir`
diff --git a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts
index df97941e..5eca7386 100644
--- a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts
+++ b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts
@@ -22,15 +22,17 @@ describe('BrsFileSemanticTokensProcessor', () => {
     function expectSemanticTokens(file: BscFile, tokens: SemanticToken[]) {
         program.validate();
         expectZeroDiagnostics(program);
+        const result = util.sortByRange(
+            program.getSemanticTokens(file.srcPath)
+        );
         expect(
-            util.sortByRange(
-                program.getSemanticTokens(file.srcPath)
-            )
+            result
         ).to.eql(
             util.sortByRange(
                 tokens
             )
         );
+        return result;
     }
 
     it('matches each namespace section for class', () => {
diff --git a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts
index 2d21de55..0215ecf8 100644
--- a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts
+++ b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts
@@ -1,10 +1,9 @@
 import type { Range } from 'vscode-languageserver-protocol';
 import { SemanticTokenTypes } from 'vscode-languageserver-protocol';
-import { isBinaryExpression, isCallExpression, isCustomType, isNewExpression } from '../../astUtils/reflection';
+import { isCallExpression, isCustomType, isNewExpression } from '../../astUtils/reflection';
 import type { BrsFile } from '../../files/BrsFile';
 import type { OnGetSemanticTokensEvent } from '../../interfaces';
 import type { Locatable } from '../../lexer/Token';
-import type { Expression } from '../../parser/Expression';
 import { ParseMode } from '../../parser/Parser';
 import util from '../../util';
 
@@ -77,39 +76,29 @@ export class BrsFileSemanticTokensProcessor {
     private iterateExpressions() {
         const scope = this.event.scopes[0];
 
-        for (const referenceExpression of this.event.file.parser.references.expressions) {
-            const actualExpressions: Expression[] = [];
-            //binary expressions actually have two expressions (left and right), so handle them independently
-            if (isBinaryExpression(referenceExpression)) {
-                actualExpressions.push(referenceExpression.left, referenceExpression.right);
-            } else {
-                //assume all other expressions are a single chain
-                actualExpressions.push(referenceExpression);
+        for (let expression of this.event.file.parser.references.expressions) {
+            //lift the callee from call expressions to handle namespaced function calls
+            if (isCallExpression(expression)) {
+                expression = expression.callee;
+            } else if (isNewExpression(expression)) {
+                expression = expression.call.callee;
             }
-            for (let expression of actualExpressions) {
-                //lift the callee from call expressions to handle namespaced function calls
-                if (isCallExpression(expression)) {
-                    expression = expression.callee;
-                } else if (isNewExpression(expression)) {
-                    expression = expression.call.callee;
-                }
-                const tokens = util.getAllDottedGetParts(expression);
-                const processedNames: string[] = [];
-                for (const token of tokens ?? []) {
-                    processedNames.push(token.text?.toLowerCase());
-                    const entityName = processedNames.join('.');
+            const tokens = util.getAllDottedGetParts(expression);
+            const processedNames: string[] = [];
+            for (const token of tokens ?? []) {
+                processedNames.push(token.text?.toLowerCase());
+                const entityName = processedNames.join('.');
 
-                    if (scope.getEnumMemberMap().has(entityName)) {
-                        this.addToken(token, SemanticTokenTypes.enumMember);
-                    } else if (scope.getEnumMap().has(entityName)) {
-                        this.addToken(token, SemanticTokenTypes.enum);
-                    } else if (scope.getClassMap().has(entityName)) {
-                        this.addToken(token, SemanticTokenTypes.class);
-                    } else if (scope.getCallableByName(entityName)) {
-                        this.addToken(token, SemanticTokenTypes.function);
-                    } else if (scope.namespaceLookup.has(entityName)) {
-                        this.addToken(token, SemanticTokenTypes.namespace);
-                    }
+                if (scope.getEnumMemberMap().has(entityName)) {
+                    this.addToken(token, SemanticTokenTypes.enumMember);
+                } else if (scope.getEnumMap().has(entityName)) {
+                    this.addToken(token, SemanticTokenTypes.enum);
+                } else if (scope.getClassMap().has(entityName)) {
+                    this.addToken(token, SemanticTokenTypes.class);
+                } else if (scope.getCallableByName(entityName)) {
+                    this.addToken(token, SemanticTokenTypes.function);
+                } else if (scope.namespaceLookup.has(entityName)) {
+                    this.addToken(token, SemanticTokenTypes.namespace);
                 }
             }
         }
diff --git a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts
index d433bf47..1a301010 100644
--- a/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts
+++ b/src/bscPlugin/transpile/BrsFilePreTranspileProcessor.ts
@@ -1,8 +1,6 @@
-import { isBinaryExpression } from '../../astUtils/reflection';
 import { Cache } from '../../Cache';
 import type { BrsFile } from '../../files/BrsFile';
 import type { BeforeFileTranspileEvent } from '../../interfaces';
-import type { Expression } from '../../parser/Expression';
 import util from '../../util';
 
 export class BrsFilePreTranspileProcessor {
@@ -23,29 +21,19 @@ export class BrsFilePreTranspileProcessor {
         if ((enumLookup?.size ?? 0) === 0) {
             return;
         }
-        for (const referenceExpression of this.event.file.parser.references.expressions) {
-            const actualExpressions: Expression[] = [];
-            //binary expressions actually have two expressions (left and right), so handle them independently
-            if (isBinaryExpression(referenceExpression)) {
-                actualExpressions.push(referenceExpression.left, referenceExpression.right);
-            } else {
-                //assume all other expressions are a single chain
-                actualExpressions.push(referenceExpression);
-            }
-            for (const expression of actualExpressions) {
-                const parts = util.getAllDottedGetParts(expression)?.map(x => x.text.toLowerCase());
-                if (parts) {
-                    //get the name of the enum member
-                    const memberName = parts.pop();
-                    //get the name of the enum (including leading namespace if applicable)
-                    const enumName = parts.join('.');
-                    const lowerEnumName = enumName.toLowerCase();
-                    const theEnum = enumLookup.get(lowerEnumName)?.item;
-                    if (theEnum) {
-                        const members = membersByEnum.getOrAdd(lowerEnumName, () => theEnum.getMemberValueMap());
-                        const value = members?.get(memberName);
-                        this.event.editor.overrideTranspileResult(expression, value);
-                    }
+        for (const expression of this.event.file.parser.references.expressions) {
+            const parts = util.getAllDottedGetParts(expression)?.map(x => x.text.toLowerCase());
+            if (parts) {
+                //get the name of the enum member
+                const memberName = parts.pop();
+                //get the name of the enum (including leading namespace if applicable)
+                const enumName = parts.join('.');
+                const lowerEnumName = enumName.toLowerCase();
+                const theEnum = enumLookup.get(lowerEnumName)?.item;
+                if (theEnum) {
+                    const members = membersByEnum.getOrAdd(lowerEnumName, () => theEnum.getMemberValueMap());
+                    const value = members?.get(memberName);
+                    this.event.editor.overrideTranspileResult(expression, value);
                 }
             }
         }
diff --git a/src/parser/Parser.spec.ts b/src/parser/Parser.spec.ts
index 161888a7..7a1989c8 100644
--- a/src/parser/Parser.spec.ts
+++ b/src/parser/Parser.spec.ts
@@ -58,8 +58,9 @@ describe('parser', () => {
         it('works for references.expressions', () => {
             const parser = Parser.parse(`
                 a += 1 + 2
-                a++
-                a--
+                b += getValue1() + getValue2()
+                increment++
+                decrement--
                 some.node@.doCallfunc()
                 bravo(3 + 4).jump(callMe())
                 obj = {
@@ -80,11 +81,20 @@ describe('parser', () => {
                 end function
             `);
             const expected = [
+                '1',
+                '2',
+                'a',
                 'a += 1 + 2',
-                'a++',
-                'a--',
+                'getValue1()',
+                'getValue2()',
+                'b',
+                'b += getValue1() + getValue2()',
+                'increment++',
+                'decrement--',
                 //currently the "toString" does a transpile, so that's why this is different.
                 'some.node.callfunc("doCallfunc", invalid)',
+                '3',
+                '4',
                 '3 + 4',
                 'callMe()',
                 'bravo(3 + 4).jump(callMe())',
@@ -99,10 +109,38 @@ describe('parser', () => {
                 'call1().a.b.call2()',
                 '"bob"',
                 'name.space.getSomething()'
-            ].sort();
+            ];
+
+            expect(
+                expressionsToStrings(parser.references.expressions)
+            ).to.eql(expected);
+
+            //tell the parser we modified the AST and need to regenerate references
+            parser.invalidateReferences();
 
             expect(
                 expressionsToStrings(parser.references.expressions).sort()
+            ).to.eql(expected.sort());
+        });
+
+        it('works for references.expressions', () => {
+            const parser = Parser.parse(`
+                value = true or type(true) = "something" or Enums.A.Value = "value" and Enum1.Value = Name.Space.Enum2.Value
+            `);
+            const expected = [
+                'true',
+                'type(true)',
+                '"something"',
+                'true',
+                'Enums.A.Value',
+                '"value"',
+                'Enum1.Value',
+                'Name.Space.Enum2.Value',
+                'true or type(true) = "something" or Enums.A.Value = "value" and Enum1.Value = Name.Space.Enum2.Value'
+            ];
+
+            expect(
+                expressionsToStrings(parser.references.expressions)
             ).to.eql(expected);
 
             //tell the parser we modified the AST and need to regenerate references
@@ -110,7 +148,29 @@ describe('parser', () => {
 
             expect(
                 expressionsToStrings(parser.references.expressions).sort()
+            ).to.eql(expected.sort());
+        });
+
+        it('works for logical expression', () => {
+            const parser = Parser.parse(`
+                value = Enums.A.Value = "value"
+            `);
+            const expected = [
+                'Enums.A.Value',
+                '"value"',
+                'Enums.A.Value = "value"'
+            ];
+
+            expect(
+                expressionsToStrings(parser.references.expressions)
             ).to.eql(expected);
+
+            //tell the parser we modified the AST and need to regenerate references
+            parser.invalidateReferences();
+
+            expect(
+                expressionsToStrings(parser.references.expressions).sort()
+            ).to.eql(expected.sort());
         });
     });
 
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index d95dd8fe..b66dcaf3 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -89,7 +89,7 @@ import {
 } from './Expression';
 import type { Diagnostic, Range } from 'vscode-languageserver';
 import { Logger } from '../Logger';
-import { isAAMemberExpression, isAnnotationExpression, isCallExpression, isCallfuncExpression, isClassMethodStatement, isCommentStatement, isDottedGetExpression, isIfStatement, isIndexedGetExpression, isVariableExpression } from '../astUtils/reflection';
+import { isAAMemberExpression, isAnnotationExpression, isBinaryExpression, isCallExpression, isCallfuncExpression, isClassMethodStatement, isCommentStatement, isDottedGetExpression, isIfStatement, isIndexedGetExpression, isVariableExpression } from '../astUtils/reflection';
 import { createVisitor, WalkMode } from '../astUtils/visitors';
 import { createStringLiteral, createToken } from '../astUtils/creators';
 import { Cache } from '../Cache';
@@ -999,12 +999,14 @@ export class Parser {
         if (operator.kind === TokenKind.Equal) {
             result = new AssignmentStatement(operator, name, value, this.currentFunctionExpression);
         } else {
+            const nameExpression = new VariableExpression(name, this.currentNamespaceName);
             result = new AssignmentStatement(
                 operator,
                 name,
-                new BinaryExpression(new VariableExpression(name, this.currentNamespaceName), operator, value),
+                new BinaryExpression(nameExpression, operator, value),
                 this.currentFunctionExpression
             );
+            this.addExpressionsToReferences(nameExpression);
             //remove the right-hand-side expression from this assignment operator, and replace with the full assignment expression
             this._references.expressions.delete(value);
             this._references.expressions.add(result);
@@ -2232,6 +2234,7 @@ export class Parser {
         while (this.matchAny(TokenKind.And, TokenKind.Or)) {
             let operator = this.previous();
             let right = this.relational();
+            this.addExpressionsToReferences(expr, right);
             expr = new BinaryExpression(expr, operator, right);
         }
 
@@ -2253,12 +2256,21 @@ export class Parser {
         ) {
             let operator = this.previous();
             let right = this.additive();
+            this.addExpressionsToReferences(expr, right);
             expr = new BinaryExpression(expr, operator, right);
         }
 
         return expr;
     }
 
+    private addExpressionsToReferences(...expressions: Expression[]) {
+        for (const expression of expressions) {
+            if (!isBinaryExpression(expression)) {
+                this.references.expressions.add(expression);
+            }
+        }
+    }
+
     // TODO: bitshift
 
     private additive(): Expression {
@@ -2267,6 +2279,7 @@ export class Parser {
         while (this.matchAny(TokenKind.Plus, TokenKind.Minus)) {
             let operator = this.previous();
             let right = this.multiplicative();
+            this.addExpressionsToReferences(expr, right);
             expr = new BinaryExpression(expr, operator, right);
         }
 
@@ -2286,6 +2299,7 @@ export class Parser {
         )) {
             let operator = this.previous();
             let right = this.exponential();
+            this.addExpressionsToReferences(expr, right);
             expr = new BinaryExpression(expr, operator, right);
         }
 
@@ -2298,6 +2312,7 @@ export class Parser {
         while (this.match(TokenKind.Caret)) {
             let operator = this.previous();
             let right = this.prefixUnary();
+            this.addExpressionsToReferences(expr, right);
             expr = new BinaryExpression(expr, operator, right);
         }
 
@@ -3016,6 +3031,18 @@ export class Parser {
                     }
                 }
             },
+            BinaryExpression: (e, parent) => {
+                //walk the chain of binary expressions and add each one to the list of expressions
+                const expressions: Expression[] = [e];
+                let expression: Expression;
+                while ((expression = expressions.pop())) {
+                    if (isBinaryExpression(expression)) {
+                        expressions.push(expression.left, expression.right);
+                    } else {
+                        this._references.expressions.add(expression);
+                    }
+                }
+            },
             ArrayLiteralExpression: e => {
                 for (const element of e.elements) {
                     //keep everything except comments
@@ -3130,6 +3157,9 @@ export class References {
      *
      * Example 2:
      * `name.space.doSomething(a.b.c)` will result in 2 entries in this list. the `CallExpression` for `doSomething`, and the `.c` DottedGetExpression.
+     *
+     * Example 3:
+     * `value = SomeEnum.value > 2 or SomeEnum.otherValue < 10` will result in 4 entries. `SomeEnum.value`, `2`, `SomeEnum.otherValue`, `10`
      */
     public expressions = new Set<Expression>();
 


Test patch:
diff --git a/src/parser/tests/statement/Enum.spec.ts b/src/parser/tests/statement/Enum.spec.ts
index 4a6d0afe..cf9b965c 100644
--- a/src/parser/tests/statement/Enum.spec.ts
+++ b/src/parser/tests/statement/Enum.spec.ts
@@ -887,6 +887,22 @@ describe('EnumStatement', () => {
             `);
         });
 
+        it('handles when found in boolean expressions', () => {
+            testTranspile(`
+                sub main()
+                    result = Direction.up = "up" or Direction.down = "down" and Direction.up = Direction.down
+                end sub
+                enum Direction
+                    up = "up"
+                    down = "down"
+                end enum
+            `, `
+                sub main()
+                    result = "up" = "up" or "down" = "down" and "up" = "down"
+                end sub
+            `);
+        });
+
         it('replaces enum values in if statements', () => {
             testTranspile(`
                 sub main()