From 72e81b89e645e47a45a82db2e3599ddcec48aacc Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 14 Jan 2025 10:41:17 -0400 Subject: [PATCH 1/4] Binary operators on Unions work better now --- src/bscPlugin/hover/HoverProcessor.spec.ts | 6 +-- src/types/BscType.ts | 7 +++ src/types/DoubleType.ts | 2 + src/types/DynamicType.ts | 2 + src/types/FloatType.ts | 2 + src/types/IntegerType.ts | 2 + src/types/LongIntegerType.ts | 2 + src/types/ReferenceType.ts | 2 +- src/types/UnionType.ts | 6 ++- src/util.spec.ts | 23 +++++++++- src/util.ts | 50 ++++++++++++++++++++++ 11 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts index b57d7f89d..835cdaa93 100644 --- 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/types/BscType.ts b/src/types/BscType.ts index af70f8871..ec14a205d 100644 --- 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 index fe406d8d3..3a7991789 100644 --- 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 index 1bc5d566f..f82c761a0 100644 --- 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 index 101f0dfa9..50d87f6a1 100644 --- 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 index 496caf934..cec7cd1da 100644 --- 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 index 5b653c1fb..49dbb09d9 100644 --- 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/ReferenceType.ts b/src/types/ReferenceType.ts index bcd7aece6..befa053be 100644 --- a/src/types/ReferenceType.ts +++ b/src/types/ReferenceType.ts @@ -426,7 +426,7 @@ export class BinaryOperatorReferenceType extends BscType { return () => undefined; } } else { - resultType = binaryOpResolver(this.leftType, this.operator, this.rightType); + resultType = binaryOpResolver(this.leftType, this.operator, this.rightType) ?? DynamicType.instance; this.cachedType = resultType; } diff --git a/src/types/UnionType.ts b/src/types/UnionType.ts index 8d5e1840d..7c3889e6f 100644 --- 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 index c6fd7a001..76574a552 100644 --- 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, 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,27 @@ 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); + }); }); describe('unaryOperatorResultType', () => { diff --git a/src/util.ts b/src/util.ts index cbb15cfd7..567df03cf 100644 --- a/src/util.ts +++ b/src/util.ts @@ -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.getHighestPriorityNumberType(leftType.types); + } + if (isUnionType(rightType)) { + rightType = this.getHighestPriorityNumberType(rightType.types); + } + if (isVoidType(leftType) || isVoidType(rightType) || isUninitializedType(leftType) || isUninitializedType(rightType)) { return undefined; } @@ -1723,6 +1734,45 @@ export class Util { return undefined; } + public getHighestPriorityNumberType(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.getHighestPriorityNumberType(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 (isDynamicType(type)) { + result = type; + } + } + return result ?? DynamicType.instance; + } + /** * Return the type of the result of a binary operator */ From 6f910998b70ef78477f0e59df40994dfa8ad76d3 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 14 Jan 2025 10:43:26 -0400 Subject: [PATCH 2/4] Used same logic on unary operators as well --- src/util.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/util.ts b/src/util.ts index 567df03cf..05be92f46 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1556,10 +1556,10 @@ export class Util { rightType = getUniqueType([rightType], unionTypeFactory); if (isUnionType(leftType)) { - leftType = this.getHighestPriorityNumberType(leftType.types); + leftType = this.getHighestPriorityType(leftType.types); } if (isUnionType(rightType)) { - rightType = this.getHighestPriorityNumberType(rightType.types); + rightType = this.getHighestPriorityType(rightType.types); } if (isVoidType(leftType) || isVoidType(rightType) || isUninitializedType(leftType) || isUninitializedType(rightType)) { @@ -1734,7 +1734,7 @@ export class Util { return undefined; } - public getHighestPriorityNumberType(types: BscType[], depth = 0): BscType { + public getHighestPriorityType(types: BscType[], depth = 0): BscType { let result: BscType; if (depth > 4) { // shortcut for very complicated types, or self-referencing union types @@ -1744,7 +1744,7 @@ export class Util { if (isUnionType(type)) { type = getUniqueType([type], unionTypeFactory); if (isUnionType(type)) { - type = this.getHighestPriorityNumberType(type.types, depth + 1); + type = this.getHighestPriorityType(type.types, depth + 1); } } if (!result) { @@ -1778,10 +1778,15 @@ export class Util { */ 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)) { return exprType; } From 3c2416f7fa111aafaddb786de7568392a8e6a82b Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 14 Jan 2025 10:50:58 -0400 Subject: [PATCH 3/4] Updated test to check result type of complex binrary operation --- src/files/BrsFile.spec.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 79a672650..19084ed88 100644 --- 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('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(isFunctionExpression); + const symbolTable = timeElapsedFunc.body.getSymbolTable(); + const offsetType = symbolTable.getSymbolType('offset', { flags: SymbolTypeFlag.runtime }); + expectTypeToBe(offsetType, IntegerType); }); it('supports the third parameter in CreateObject', () => { From 152da3b7f2e2cccb7b8a5a3bc83be0780e6002ae Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Tue, 14 Jan 2025 12:14:37 -0400 Subject: [PATCH 4/4] Binary and Unary operators handle ObjectType - issue #1387 --- src/bscPlugin/validation/ScopeValidator.spec.ts | 12 ++++++++++++ src/util.spec.ts | 10 +++++++++- src/util.ts | 16 ++++++++++++++-- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/bscPlugin/validation/ScopeValidator.spec.ts b/src/bscPlugin/validation/ScopeValidator.spec.ts index 69ad1601c..8bac8a39a 100644 --- 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('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/util.spec.ts b/src/util.spec.ts index 76574a552..51aadbb68 100644 --- 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, UnionType, 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'; @@ -1121,6 +1121,10 @@ describe('util', () => { 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', () => { @@ -1145,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 index 05be92f46..ace9c089b 100644 --- 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'; @@ -1572,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); @@ -1766,6 +1775,9 @@ export class Util { if (isInvalidType(type)) { return type; } + if (isObjectType(type) && !isDynamicType(type)) { + result = type; + } if (isDynamicType(type)) { result = type; } @@ -1787,7 +1799,7 @@ export class Util { } - if (isDynamicType(exprType)) { + if (isDynamicType(exprType) || isObjectType(exprType)) { return exprType; }