Fixes How BinaryOperators work on Unions (#1400)

* Binary operators on Unions work better now * Used same logic on unary operators as well * Updated test to check result type of complex binrary operation * Binary and Unary operators handle ObjectType - issue #1387
rokucommunity · Jan 14, 2025 · 7fb92ff · 7fb92ff
1 parent d3ff753
commit 7fb92ff
Show file tree

Hide file tree

Showing 12 changed files with 145 additions and 10 deletions.
diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts
@@ -829,16 +829,16 @@ describe('HoverProcessor', () => {
                     if deviceInfo.getClockFormat() = "24h" then
                         hour = stringUtil.pad(hour)
                         ' "22:01" | "01:01"
-                        return substitute("{0}:{1}", hour, minutes as string)
+                        return substitute("{0}:{1}", hour as string, minutes as string)
                     else
                         hour = hour mod 12
                         if hour = 0 then hour = 12
                         ' "10:01 AM" | "1:01 AM"
-                        return substitute("{0}:{1} {2}", hour, minutes as string, meridiem)
+                        return substitute("{0}:{1} {2}", hour.toStr(), minutes as string, meridiem)
                     end if
                 end function
             `);
-            const expectedHourHoverStr = `hour as dynamic`;
+            const expectedHourHoverStr = `hour as integer or string`;
 
             program.validate();
             expectZeroDiagnostics(program);

diff --git a/src/bscPlugin/validation/ScopeValidator.spec.ts b/src/bscPlugin/validation/ScopeValidator.spec.ts
@@ -3487,6 +3487,18 @@ describe('ScopeValidator', () => {
                 DiagnosticMessages.operatorTypeMismatch('=', 'uninitialized', 'invalid').message
             ]);
         });
+
+        it('allows string comparisons with object', () => {
+            program.setFile<BrsFile>('source/main.brs', `
+                sub test(x as object)
+                    if x <> "test"
+                        print x
+                    end if
+                end sub
+            `);
+            program.validate();
+            expectZeroDiagnostics(program);
+        });
     });
 
     describe('memberAccessibilityMismatch', () => {

diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts
@@ -24,7 +24,7 @@ import { SymbolTypeFlag } from '../SymbolTypeFlag';
 import { ClassType, EnumType, FloatType, InterfaceType } from '../types';
 import type { StandardizedFileEntry } from 'roku-deploy';
 import * as fileUrl from 'file-url';
-import { isAALiteralExpression, isBlock } from '../astUtils/reflection';
+import { isAALiteralExpression, isBlock, isFunctionExpression } from '../astUtils/reflection';
 import type { AALiteralExpression } from '../parser/Expression';
 import { CallExpression, FunctionExpression, LiteralExpression } from '../parser/Expression';
 import { Logger } from '@rokucommunity/logger';
@@ -192,7 +192,7 @@ describe('BrsFile', () => {
     it('does not crazy during validation with unique binary operator', () => {
         //monitor the logging system, if we detect an error, this test fails
         const spy = sinon.spy(Logger.prototype, 'error');
-        program.setFile('source/main.bs', `
+        const file = program.setFile<BrsFile>('source/main.bs', `
             namespace date
                 function timeElapsedInDay()
                     time = 1
@@ -215,6 +215,14 @@ describe('BrsFile', () => {
         expect(
             spy.getCalls().map(x => (x.args?.[0] as string)?.toString()).filter(x => x?.includes('Error when calling plugin'))
         ).to.eql([]);
+
+        // Check the result type too
+        const sourceScope = program.getScopeByName('source');
+        sourceScope.linkSymbolTable();
+        const timeElapsedFunc = file.ast.findChild<FunctionExpression>(isFunctionExpression);
+        const symbolTable = timeElapsedFunc.body.getSymbolTable();
+        const offsetType = symbolTable.getSymbolType('offset', { flags: SymbolTypeFlag.runtime });
+        expectTypeToBe(offsetType, IntegerType);
     });
 
     it('supports the third parameter in CreateObject', () => {

diff --git a/src/types/BscType.ts b/src/types/BscType.ts
@@ -139,4 +139,11 @@ export abstract class BscType {
         }
         this.hasAddedBuiltInInterfaces = true;
     }
+
+    /**
+     * The level of priority of this type when in a binary operation
+     * For example Float is higher priority than integer, so Float + Int => Float
+     * Lower numbers have higher priority
+     */
+    readonly binaryOpPriorityLevel: number = 0;
 }
diff --git a/src/types/DoubleType.ts b/src/types/DoubleType.ts
@@ -41,6 +41,8 @@ export class DoubleType extends BscType {
     public isEqual(targetType: BscType): boolean {
         return isDoubleType(targetType);
     }
+
+    readonly binaryOpPriorityLevel = 1;
 }
 
 BuiltInInterfaceAdder.primitiveTypeInstanceCache.set('double', DoubleType.instance);
diff --git a/src/types/DynamicType.ts b/src/types/DynamicType.ts
@@ -50,6 +50,8 @@ export class DynamicType extends BscType {
     getMemberType(memberName: string, options: GetTypeOptions) {
         return DynamicType.instance;
     }
+
+
 }
 
 BuiltInInterfaceAdder.primitiveTypeInstanceCache.set('dynamic', DynamicType.instance);
diff --git a/src/types/FloatType.ts b/src/types/FloatType.ts
@@ -42,6 +42,8 @@ export class FloatType extends BscType {
     public isEqual(targetType: BscType): boolean {
         return isFloatType(targetType);
     }
+
+    readonly binaryOpPriorityLevel = 2;
 }
 
 BuiltInInterfaceAdder.primitiveTypeInstanceCache.set('float', FloatType.instance);
diff --git a/src/types/IntegerType.ts b/src/types/IntegerType.ts
@@ -41,6 +41,8 @@ export class IntegerType extends BscType {
     isEqual(otherType: BscType) {
         return isIntegerType(otherType);
     }
+
+    readonly binaryOpPriorityLevel = 4;
 }
 
 BuiltInInterfaceAdder.primitiveTypeInstanceCache.set('integer', IntegerType.instance);
diff --git a/src/types/LongIntegerType.ts b/src/types/LongIntegerType.ts
@@ -41,6 +41,8 @@ export class LongIntegerType extends BscType {
     isEqual(targetType: BscType): boolean {
         return isLongIntegerType(targetType);
     }
+
+    readonly binaryOpPriorityLevel = 3;
 }
 
 BuiltInInterfaceAdder.primitiveTypeInstanceCache.set('longinteger', LongIntegerType.instance);
diff --git a/src/types/UnionType.ts b/src/types/UnionType.ts
@@ -64,7 +64,7 @@ export class UnionType extends BscType {
     }
 
     isTypeCompatible(targetType: BscType, data?: TypeCompatibilityData): boolean {
-        if (isDynamicType(targetType) || isObjectType(targetType)) {
+        if (isDynamicType(targetType) || isObjectType(targetType) || this === targetType) {
             return true;
         }
         if (isEnumTypeCompatible(this, targetType, data)) {
@@ -86,7 +86,6 @@ export class UnionType extends BscType {
             }
         }
 
-
         return false;
     }
     toString(): string {
@@ -106,6 +105,9 @@ export class UnionType extends BscType {
         if (!isUnionType(targetType)) {
             return false;
         }
+        if (this === targetType) {
+            return true;
+        }
         return this.isTypeCompatible(targetType) && targetType.isTypeCompatible(this);
     }
 

diff --git a/src/util.spec.ts b/src/util.spec.ts
@@ -12,7 +12,7 @@ import { NamespaceType } from './types/NamespaceType';
 import { ClassType } from './types/ClassType';
 import { ReferenceType } from './types/ReferenceType';
 import { SymbolTypeFlag } from './SymbolTypeFlag';
-import { BooleanType, DoubleType, DynamicType, FloatType, IntegerType, InvalidType, LongIntegerType, StringType, TypedFunctionType, VoidType } from './types';
+import { BooleanType, DoubleType, DynamicType, FloatType, IntegerType, InvalidType, LongIntegerType, ObjectType, StringType, TypedFunctionType, UnionType, VoidType } from './types';
 import { TokenKind } from './lexer/TokenKind';
 import { createToken } from './astUtils/creators';
 import { createDottedIdentifier, createVariableExpression } from './astUtils/creators';
@@ -1100,6 +1100,31 @@ describe('util', () => {
             // "and" / "or" are bitwise operators with number
             expectTypeToBe(util.binaryOperatorResultType(IntegerType.instance, createToken(TokenKind.Or), DynamicType.instance), IntegerType);
         });
+
+        it('handles union types ', () => {
+            const floatIntUnion = new UnionType([IntegerType.instance, FloatType.instance]);
+            expectTypeToBe(util.binaryOperatorResultType(floatIntUnion, createToken(TokenKind.Plus), DoubleType.instance), DoubleType);
+        });
+
+        it('handles 2 union types', () => {
+            const floatIntUnion = new UnionType([IntegerType.instance, FloatType.instance]);
+            expectTypeToBe(util.binaryOperatorResultType(floatIntUnion, createToken(TokenKind.Plus), floatIntUnion), FloatType);
+        });
+
+        it('handles union types with diverse member types', () => {
+            const myUnion = new UnionType([FloatType.instance, StringType.instance, BooleanType.instance]);
+            expectTypeToBe(util.binaryOperatorResultType(myUnion, createToken(TokenKind.Plus), myUnion), DynamicType);
+        });
+
+        it('handles union types with self-referencing unions', () => {
+            const myUnion = new UnionType([FloatType.instance, StringType.instance, DynamicType.instance]);
+            myUnion.addType(myUnion);
+            expectTypeToBe(util.binaryOperatorResultType(myUnion, createToken(TokenKind.Plus), myUnion), DynamicType);
+        });
+
+        it('handles object Types', () => {
+            expectTypeToBe(util.binaryOperatorResultType(new ObjectType(), createToken(TokenKind.Plus), IntegerType.instance), IntegerType);
+        });
     });
 
     describe('unaryOperatorResultType', () => {
@@ -1124,6 +1149,10 @@ describe('util', () => {
             expectTypeToBe(util.unaryOperatorResultType(notToken, LongIntegerType.instance), LongIntegerType);
             expect(util.unaryOperatorResultType(notToken, VoidType.instance)).to.be.undefined;
         });
+
+        it('handles object Types', () => {
+            expectTypeToBe(util.unaryOperatorResultType(createToken(TokenKind.Minus), new ObjectType()), ObjectType);
+        });
     });
 
     describe('getTokenDocumentation', () => {

diff --git a/src/util.ts b/src/util.ts
@@ -26,7 +26,7 @@ import type { CallExpression, CallfuncExpression, DottedGetExpression, FunctionP
 import { LogLevel, createLogger } from './logging';
 import { isToken, type Identifier, type Locatable, type Token } from './lexer/Token';
 import { TokenKind } from './lexer/TokenKind';
-import { isAnyReferenceType, isBinaryExpression, isBooleanType, isBrsFile, isCallExpression, isCallableType, isCallfuncExpression, isClassType, isDottedGetExpression, isDoubleType, isDynamicType, isEnumMemberType, isExpression, isFloatType, isIndexedGetExpression, isInvalidType, isLiteralString, isLongIntegerType, isNamespaceStatement, isNamespaceType, isNewExpression, isNumberType, isPrimitiveType, isReferenceType, isStatement, isStringType, isTypeExpression, isTypedArrayExpression, isTypedFunctionType, isUninitializedType, isUnionType, isVariableExpression, isVoidType, isXmlAttributeGetExpression, isXmlFile } from './astUtils/reflection';
+import { isAnyReferenceType, isBinaryExpression, isBooleanType, isBrsFile, isCallExpression, isCallableType, isCallfuncExpression, isClassType, isDottedGetExpression, isDoubleType, isDynamicType, isEnumMemberType, isExpression, isFloatType, isIndexedGetExpression, isInvalidType, isLiteralString, isLongIntegerType, isNamespaceStatement, isNamespaceType, isNewExpression, isNumberType, isObjectType, isPrimitiveType, isReferenceType, isStatement, isStringType, isTypeExpression, isTypedArrayExpression, isTypedFunctionType, isUninitializedType, isUnionType, isVariableExpression, isVoidType, isXmlAttributeGetExpression, isXmlFile } from './astUtils/reflection';
 import { WalkMode } from './astUtils/visitors';
 import { SourceNode } from 'source-map';
 import * as requireRelative from 'require-relative';
@@ -1551,6 +1551,17 @@ export class Util {
             });
         }
 
+        // Try to find a common value of union type
+        leftType = getUniqueType([leftType], unionTypeFactory);
+        rightType = getUniqueType([rightType], unionTypeFactory);
+
+        if (isUnionType(leftType)) {
+            leftType = this.getHighestPriorityType(leftType.types);
+        }
+        if (isUnionType(rightType)) {
+            rightType = this.getHighestPriorityType(rightType.types);
+        }
+
         if (isVoidType(leftType) || isVoidType(rightType) || isUninitializedType(leftType) || isUninitializedType(rightType)) {
             return undefined;
         }
@@ -1561,6 +1572,15 @@ export class Util {
         if (isEnumMemberType(rightType)) {
             rightType = rightType.underlyingType;
         }
+
+        // treat object type like dynamic
+        if (isObjectType(leftType)) {
+            leftType = DynamicType.instance;
+        }
+        if (isObjectType(rightType)) {
+            rightType = DynamicType.instance;
+        }
+
         let hasDouble = isDoubleType(leftType) || isDoubleType(rightType);
         let hasFloat = isFloatType(leftType) || isFloatType(rightType);
         let hasLongInteger = isLongIntegerType(leftType) || isLongIntegerType(rightType);
@@ -1723,16 +1743,63 @@ export class Util {
         return undefined;
     }
 
+    public getHighestPriorityType(types: BscType[], depth = 0): BscType {
+        let result: BscType;
+        if (depth > 4) {
+            // shortcut for very complicated types, or self-referencing union types
+            return DynamicType.instance;
+        }
+        for (let type of types) {
+            if (isUnionType(type)) {
+                type = getUniqueType([type], unionTypeFactory);
+                if (isUnionType(type)) {
+                    type = this.getHighestPriorityType(type.types, depth + 1);
+                }
+            }
+            if (!result) {
+                result = type;
+            } else {
+                if (type.binaryOpPriorityLevel < result.binaryOpPriorityLevel) {
+                    result = type;
+                } else if (type.binaryOpPriorityLevel === result.binaryOpPriorityLevel && !result.isEqual(type)) {
+                    // equal priority types, but not equal types, like Boolean and String... just be dynamic at this point
+                    result = DynamicType.instance;
+                }
+            }
+            if (isUninitializedType(type)) {
+                return type;
+            }
+            if (isVoidType(type)) {
+                return type;
+            }
+            if (isInvalidType(type)) {
+                return type;
+            }
+            if (isObjectType(type) && !isDynamicType(type)) {
+                result = type;
+            }
+            if (isDynamicType(type)) {
+                result = type;
+            }
+        }
+        return result ?? DynamicType.instance;
+    }
+
     /**
      * Return the type of the result of a binary operator
      */
     public unaryOperatorResultType(operator: Token, exprType: BscType): BscType {
 
+        if (isUnionType(exprType)) {
+            exprType = this.getHighestPriorityType(exprType.types);
+        }
+
         if (isVoidType(exprType) || isInvalidType(exprType) || isUninitializedType(exprType)) {
             return undefined;
         }
 
-        if (isDynamicType(exprType)) {
+
+        if (isDynamicType(exprType) || isObjectType(exprType)) {
             return exprType;
         }