From ddabf8ab9803b667330a0f682e28786af773df1b Mon Sep 17 00:00:00 2001 From: Marcelo Lv Cabral Date: Fri, 3 Jan 2025 08:43:26 -0700 Subject: [PATCH 1/2] Fixed dot chaining error scenarios (#83) * Fixed dot chaining operator error scenarios * Fixed Syntax e2e test case * Fixed test case * Fixed prettier --- src/interpreter/index.ts | 22 +++++++-------- test/e2e/Syntax.test.js | 4 ++- .../resources/optional-chaining-operators.brs | 28 +++++++++++++------ 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/interpreter/index.ts b/src/interpreter/index.ts index 45ef9b1a..f30ee7e2 100644 --- a/src/interpreter/index.ts +++ b/src/interpreter/index.ts @@ -1297,24 +1297,22 @@ export class Interpreter implements Expr.Visitor, Stmt.Visitor } let boxedSource = isBoxable(source) ? source.box() : source; + let errorDetail = RuntimeErrorDetail.DotOnNonObject; if (boxedSource instanceof BrsComponent) { - // This check is supposed to be placed below the try/catch block, + const invalidSource = BrsInvalid.Instance.equalTo(source).toBoolean(); + // This check is supposed to be placed after method check, // but it's here to mimic the behavior of Roku, if they fix, we move it. - if (source instanceof BrsInvalid && expression.optional) { + if (invalidSource && expression.optional) { return source; } - try { - const method = boxedSource.getMethod(expression.name.text); - if (method) { - return method; - } - } catch (err: any) { - this.addError(new BrsError(err.message, expression.name.location)); + const method = boxedSource.getMethod(expression.name.text); + if (method) { + return method; + } else if (!invalidSource) { + errorDetail = RuntimeErrorDetail.MemberFunctionNotFound; } } - this.addError( - new RuntimeError(RuntimeErrorDetail.DotOnNonObject, expression.name.location) - ); + this.addError(new RuntimeError(errorDetail, expression.name.location)); } visitIndexedGet(expression: Expr.IndexedGet): BrsType { diff --git a/test/e2e/Syntax.test.js b/test/e2e/Syntax.test.js index b3c8893a..c7283963 100644 --- a/test/e2e/Syntax.test.js +++ b/test/e2e/Syntax.test.js @@ -135,13 +135,15 @@ describe("end to end syntax", () => { await execute([resourceFile("optional-chaining-operators.brs")], outputStreams); expect(allArgs(outputStreams.stdout.write).filter((arg) => arg !== "\n")).toEqual([ + " 236", "invalid", "invalid", "invalid", "invalid", "invalid", "invalid", - "invalid", + "invalid as string", + " 244", ]); }); diff --git a/test/e2e/resources/optional-chaining-operators.brs b/test/e2e/resources/optional-chaining-operators.brs index 4678bfd0..ec682c75 100644 --- a/test/e2e/resources/optional-chaining-operators.brs +++ b/test/e2e/resources/optional-chaining-operators.brs @@ -1,11 +1,21 @@ sub Main() - thing = invalid - print thing?.property - print thing?.functionCall2?() - print thing?[0] - print thing?[0]?.property - print thing?[0]?.functionCall2?() - print thing?.functionCall?(thing?[0]?.property, thing?[0]?.functionCall2?()) - node = {} - print node?.focusedChild?.id.toStr() + thing = invalid + try + print thing.property + catch e + print e.number + end try + print thing?.property + print thing?.functionCall2?() + print thing?[0] + print thing?[0]?.property + print thing?[0]?.functionCall2?() + print thing?.functionCall?(thing?[0]?.property, thing?[0]?.functionCall2?()) + print thing.toStr() + " as string" + di = CreateObject("roDeviceInfo") + try + print di.action() + catch e + print e.number + end try end sub \ No newline at end of file From 0afd6b66cd18679694be9008eb136a04060b50eb Mon Sep 17 00:00:00 2001 From: Marcelo Lv Cabral Date: Fri, 3 Jan 2025 09:12:33 -0700 Subject: [PATCH 2/2] Implemented `ObjFun()` and aligned behavior of `CreateObject()` with Roku (#84) * Implemented `ObjFun()` global function and support for `variadic` arguments on `Callable` * Aligned the behavior of CreateObject with Roku. Fixes #80 * Fixed prettier issue --------- Co-authored-by: Bronley Plumb --- src/brsTypes/Callable.ts | 6 +- src/brsTypes/components/BrsObjects.ts | 97 +++++++++++++++---- src/brsTypes/components/RoBoolean.ts | 6 +- src/brsTypes/index.ts | 8 +- src/interpreter/index.ts | 2 +- src/stdlib/CreateObject.ts | 34 +++++-- src/stdlib/GlobalUtilities.ts | 33 ++++++- test/e2e/StdLib.test.js | 29 ++++++ .../e2e/resources/components/roIntrinsics.brs | 18 ++-- test/e2e/resources/stdlib/create-object.brs | 30 ++++++ .../e2e/resources/stdlib/global-utilities.brs | 4 + test/stdlib/GlobalUtilities.test.js | 43 +++++++- 12 files changed, 260 insertions(+), 50 deletions(-) create mode 100644 test/e2e/resources/stdlib/create-object.brs diff --git a/src/brsTypes/Callable.ts b/src/brsTypes/Callable.ts index c0b058f0..0d235dd5 100644 --- a/src/brsTypes/Callable.ts +++ b/src/brsTypes/Callable.ts @@ -64,7 +64,9 @@ export class StdlibArgument implements Argument { /** A BrightScript `function` or `sub`'s signature. */ export interface Signature { /** The set of arguments a function accepts. */ - readonly args: ReadonlyArray; + readonly args: Argument[]; + /** Whether the function accepts a variable number of arguments. */ + readonly variadic?: boolean; /** The type of BrightScript value the function will return. `sub`s must use `ValueKind.Void`. */ readonly returns: Brs.ValueKind; } @@ -280,7 +282,7 @@ export class Callable implements Brs.BrsValue { expected: signature.args.length.toString(), received: args.length.toString(), }); - } else if (args.length > signature.args.length) { + } else if (!signature.variadic && args.length > signature.args.length) { reasons.push({ reason: MismatchReason.TooManyArguments, expected: signature.args.length.toString(), diff --git a/src/brsTypes/components/BrsObjects.ts b/src/brsTypes/components/BrsObjects.ts index 95269c95..e17aad40 100644 --- a/src/brsTypes/components/BrsObjects.ts +++ b/src/brsTypes/components/BrsObjects.ts @@ -25,37 +25,94 @@ import { BrsComponent } from "./BrsComponent"; import { RoAppInfo } from "./RoAppInfo"; import { RoPath } from "./RoPath"; +// Class to define a case-insensitive map of BrightScript objects. +class BrsObjectsMap { + private readonly map = new Map< + string, + { originalKey: string; value: Function; params: number } + >(); + + constructor(entries: [string, Function, number?][]) { + entries.forEach(([key, value, params]) => this.set(key, value, params)); + } + + get(key: string) { + const entry = this.map.get(key.toLowerCase()); + return entry ? entry.value : undefined; + } + + set(key: string, value: Function, params?: number) { + return this.map.set(key.toLowerCase(), { + originalKey: key, + value: value, + params: params ?? 0, + }); + } + + has(key: string) { + return this.map.has(key.toLowerCase()); + } + + delete(key: string) { + return this.map.delete(key.toLowerCase()); + } + + clear() { + return this.map.clear(); + } + + values() { + return Array.from(this.map.values()).map((entry) => entry.value); + } + + keys() { + return Array.from(this.map.values()).map((entry) => entry.originalKey); + } + + // Returns the number of parameters required by the object constructor. + // >=0 = exact number of parameters required + // -1 = ignore parameters, create object with no parameters + // -2 = do not check for minimum number of parameters + params(key: string) { + const entry = this.map.get(key.toLowerCase()); + return entry ? entry.params : -1; + } +} + /** Map containing a list of BrightScript components that can be created. */ -export const BrsObjects = new Map([ - ["roassociativearray", (_: Interpreter) => new RoAssociativeArray([])], +export const BrsObjects = new BrsObjectsMap([ + ["roAssociativeArray", (_: Interpreter) => new RoAssociativeArray([])], [ - "roarray", + "roArray", (interpreter: Interpreter, capacity: Int32 | Float, resizable: BrsBoolean) => new RoArray(capacity, resizable), + 2, ], - ["rolist", (_: Interpreter) => new RoList([])], - ["robytearray", (_: Interpreter) => new RoByteArray()], - ["rodatetime", (_: Interpreter) => new RoDateTime()], - ["rotimespan", (_: Interpreter) => new Timespan()], - ["rodeviceinfo", (_: Interpreter) => new RoDeviceInfo()], + ["roList", (_: Interpreter) => new RoList([])], + ["roByteArray", (_: Interpreter) => new RoByteArray()], + ["roDateTime", (_: Interpreter) => new RoDateTime()], + ["roTimespan", (_: Interpreter) => new Timespan()], + ["roDeviceInfo", (_: Interpreter) => new RoDeviceInfo()], [ - "rosgnode", + "roSGNode", (interpreter: Interpreter, nodeType: BrsString) => createNodeByType(interpreter, nodeType), + 1, ], [ - "roregex", + "roRegex", (_: Interpreter, expression: BrsString, flags: BrsString) => new RoRegex(expression, flags), + 2, ], - ["roxmlelement", (_: Interpreter) => new RoXMLElement()], - ["rostring", (_: Interpreter) => new RoString()], - ["roboolean", (_: Interpreter, literal: BrsBoolean) => new roBoolean(literal)], - ["rodouble", (_: Interpreter, literal: Double) => new roDouble(literal)], - ["rofloat", (_: Interpreter, literal: Float) => new roFloat(literal)], - ["roint", (_: Interpreter, literal: Int32) => new roInt(literal)], - ["rolonginteger", (_: Interpreter, literal: Int64) => new roLongInteger(literal)], - ["roappinfo", (_: Interpreter) => new RoAppInfo()], - ["ropath", (interpreter: Interpreter, path: BrsString) => new RoPath(path)], - ["roinvalid", (_: Interpreter) => new roInvalid()], + ["roXMLElement", (_: Interpreter) => new RoXMLElement()], + ["roString", (_: Interpreter) => new RoString(), -1], + ["roBoolean", (_: Interpreter, literal: BrsBoolean) => new roBoolean(literal), -1], + ["roDouble", (_: Interpreter, literal: Double) => new roDouble(literal), -1], + ["roFloat", (_: Interpreter, literal: Float) => new roFloat(literal), -1], + ["roInt", (_: Interpreter, literal: Int32) => new roInt(literal), -1], + ["roLongInteger", (_: Interpreter, literal: Int64) => new roLongInteger(literal), -1], + ["roAppInfo", (_: Interpreter) => new RoAppInfo()], + ["roPath", (_: Interpreter, path: BrsString) => new RoPath(path), 1], + ["roInvalid", (_: Interpreter) => new roInvalid(), -1], ]); /** diff --git a/src/brsTypes/components/RoBoolean.ts b/src/brsTypes/components/RoBoolean.ts index 3eb3274d..c248b04d 100644 --- a/src/brsTypes/components/RoBoolean.ts +++ b/src/brsTypes/components/RoBoolean.ts @@ -7,7 +7,7 @@ import { Unboxable } from "../Boxing"; export class roBoolean extends BrsComponent implements BrsValue, Unboxable { readonly kind = ValueKind.Object; - private intrinsic: BrsBoolean; + private intrinsic: BrsBoolean = BrsBoolean.False; public getValue(): boolean { return this.intrinsic.toBoolean(); @@ -16,7 +16,9 @@ export class roBoolean extends BrsComponent implements BrsValue, Unboxable { constructor(initialValue: BrsBoolean) { super("roBoolean"); - this.intrinsic = initialValue; + if (initialValue instanceof BrsBoolean) { + this.intrinsic = initialValue; + } this.registerMethods({ ifBoolean: [this.getBoolean, this.setBoolean], ifToStr: [this.toStr], diff --git a/src/brsTypes/index.ts b/src/brsTypes/index.ts index 9597b8cf..f98f0efb 100644 --- a/src/brsTypes/index.ts +++ b/src/brsTypes/index.ts @@ -77,7 +77,7 @@ export * from "./coercion"; * @returns `true` if `value` is a numeric value, otherwise `false`. */ export function isBrsNumber(value: BrsType): value is BrsNumber { - return NumberKinds.has(value.kind); + return NumberKinds.has(value?.kind); } export function isNumberKind(kind: ValueKind): boolean { @@ -105,7 +105,7 @@ export const PrimitiveKinds = new Set([ * @returns `true` if `value` is a string, otherwise `false`. */ export function isBrsString(value: BrsType): value is BrsString { - return value.kind === ValueKind.String || value instanceof RoString; + return value?.kind === ValueKind.String || value instanceof RoString; } /** @@ -114,7 +114,7 @@ export function isBrsString(value: BrsType): value is BrsString { * @returns `true` if `value` if a boolean, otherwise `false`. */ export function isBrsBoolean(value: BrsType): value is BrsBoolean { - return value.kind === ValueKind.Boolean; + return value?.kind === ValueKind.Boolean; } /** @@ -123,7 +123,7 @@ export function isBrsBoolean(value: BrsType): value is BrsBoolean { * @returns `true` if `value` is a Callable value, otherwise `false`. */ export function isBrsCallable(value: BrsType): value is Callable { - return value.kind === ValueKind.Callable; + return value?.kind === ValueKind.Callable; } /** diff --git a/src/interpreter/index.ts b/src/interpreter/index.ts index f30ee7e2..40a24b2f 100644 --- a/src/interpreter/index.ts +++ b/src/interpreter/index.ts @@ -1195,7 +1195,7 @@ export class Interpreter implements Expr.Visitor, Stmt.Visitor let signature = satisfiedSignature.signature; args = args.map((arg, index) => { // any arguments of type "object" must be automatically boxed - if (signature.args[index].type.kind === ValueKind.Object && isBoxable(arg)) { + if (signature.args[index]?.type.kind === ValueKind.Object && isBoxable(arg)) { return arg.box(); } diff --git a/src/stdlib/CreateObject.ts b/src/stdlib/CreateObject.ts index 3e2e74e1..e2d78864 100644 --- a/src/stdlib/CreateObject.ts +++ b/src/stdlib/CreateObject.ts @@ -8,20 +8,15 @@ import { RoAssociativeArray, } from "../brsTypes"; import { BrsObjects } from "../brsTypes/components/BrsObjects"; +import { RuntimeError, RuntimeErrorDetail } from "../Error"; import { Interpreter } from "../interpreter"; import { MockNode } from "../extensions/MockNode"; /** Creates a new instance of a given brightscript component (e.g. roAssociativeArray) */ export const CreateObject = new Callable("CreateObject", { signature: { - args: [ - new StdlibArgument("objName", ValueKind.String), - new StdlibArgument("arg1", ValueKind.Dynamic, BrsInvalid.Instance), - new StdlibArgument("arg2", ValueKind.Dynamic, BrsInvalid.Instance), - new StdlibArgument("arg3", ValueKind.Dynamic, BrsInvalid.Instance), - new StdlibArgument("arg4", ValueKind.Dynamic, BrsInvalid.Instance), - new StdlibArgument("arg5", ValueKind.Dynamic, BrsInvalid.Instance), - ], + args: [new StdlibArgument("objName", ValueKind.String)], + variadic: true, returns: ValueKind.Dynamic, }, impl: (interpreter: Interpreter, objName: BrsString, ...additionalArgs: BrsType[]) => { @@ -40,8 +35,27 @@ export const CreateObject = new Callable("CreateObject", { return new MockNode(possibleMock, objToMock); } let ctor = BrsObjects.get(objName.value.toLowerCase()); - - if (ctor) { + if (ctor === undefined) { + let msg = `BRIGHTSCRIPT: ERROR: Runtime: unknown classname "${ + objName.value + }": ${interpreter.formatLocation()}\n`; + interpreter.stderr.write(msg); + } else { + const minParams = BrsObjects.params(objName.value.toLowerCase()); + if (minParams === -1) { + additionalArgs = []; + } else if (minParams > 0 && additionalArgs.length === 0) { + interpreter.stderr.write( + `BRIGHTSCRIPT: ERROR: Runtime: "${ + objName.value + }": invalid number of parameters: ${interpreter.formatLocation()}\n` + ); + return BrsInvalid.Instance; + } else if (minParams >= 0 && additionalArgs.length !== minParams) { + interpreter.addError( + new RuntimeError(RuntimeErrorDetail.RoWrongNumberOfParams, interpreter.location) + ); + } try { return ctor(interpreter, ...additionalArgs); } catch (err: any) { diff --git a/src/stdlib/GlobalUtilities.ts b/src/stdlib/GlobalUtilities.ts index 6981025b..56b64d0b 100644 --- a/src/stdlib/GlobalUtilities.ts +++ b/src/stdlib/GlobalUtilities.ts @@ -5,11 +5,11 @@ import { BrsString, BrsType, StdlibArgument, - RoAssociativeArray, BrsInterface, + BrsComponent, } from "../brsTypes"; import { isBoxable } from "../brsTypes/Boxing"; -import { BrsComponent } from "../brsTypes/components/BrsComponent"; +import { RuntimeError, RuntimeErrorDetail } from "../Error"; import { Interpreter } from "../interpreter"; let warningShown = false; @@ -66,3 +66,32 @@ export const FindMemberFunction = new Callable("FindMemberFunction", { return BrsInvalid.Instance; }, }); + +export const ObjFun = new Callable("ObjFun", { + signature: { + args: [ + new StdlibArgument("object", ValueKind.Object), + new StdlibArgument("iface", ValueKind.Interface), + new StdlibArgument("funName", ValueKind.String), + ], + variadic: true, + returns: ValueKind.Dynamic, + }, + impl: ( + interpreter: Interpreter, + object: BrsComponent, + iface: BrsInterface, + funName: BrsString, + ...args: BrsType[] + ): BrsType => { + for (let [_, objI] of object.interfaces) { + if (iface.name === objI.name && iface.hasMethod(funName.value)) { + const func = object.getMethod(funName.value); + return func?.call(interpreter, ...args) || BrsInvalid.Instance; + } + } + interpreter.addError( + new RuntimeError(RuntimeErrorDetail.MemberFunctionNotFound, interpreter.location) + ); + }, +}); diff --git a/test/e2e/StdLib.test.js b/test/e2e/StdLib.test.js index 7b5f864b..868ddb21 100644 --- a/test/e2e/StdLib.test.js +++ b/test/e2e/StdLib.test.js @@ -159,6 +159,35 @@ describe("end to end standard libary", () => { "", "", "", + "", + "true", + ]); + }); + + test("stdlib/create-object.brs", async () => { + await execute([resourceFile("stdlib", "create-object.brs")], outputStreams); + + expect(allArgs(outputStreams.stdout.write).filter((arg) => arg !== "\n")).toEqual([ + "false", + "false", + " 0", + " 0", + " 0", + " 0", + " 0", + " 0", + " 0", + " 0", + "invalid", + "", + "", + "", + "", + "false", + " 0", + " 245", + "invalid", + " 245", ]); }); }); diff --git a/test/e2e/resources/components/roIntrinsics.brs b/test/e2e/resources/components/roIntrinsics.brs index 701ba03b..f3ae7110 100644 --- a/test/e2e/resources/components/roIntrinsics.brs +++ b/test/e2e/resources/components/roIntrinsics.brs @@ -1,10 +1,15 @@ sub main() - booleanObjectA = createObject("roBoolean", true) - booleanObjectB = createObject("roBoolean", false) - doubleObject = createObject("roDouble", 123.456) - floatObject = createObject("roFloat", 789.012) - integerObject = createObject("roInt", 23) - longIntegerObject = createObject("roLongInteger", 2000111222333) + booleanObjectA = createObject("roBoolean") + booleanObjectA.setBoolean(true) + booleanObjectB = createObject("roBoolean") + doubleObject = createObject("roDouble") + doubleObject.setDouble(123.456) + floatObject = createObject("roFloat") + floatObject.setFloat(789.012) + integerObject = createObject("roInt") + integerObject.setInt(23) + longIntegerObject = createObject("roLongInteger") + longIntegerObject.setLongInt(2000111222333) print "Boolean object A " booleanObjectA.toStr() print "Boolean object B " booleanObjectB @@ -20,5 +25,4 @@ sub main() print "Integer to string "integerObject.toStr() print "LongInteger object type"type(longIntegerObject) print "LongInteger to string "longIntegerObject.toStr() - end sub diff --git a/test/e2e/resources/stdlib/create-object.brs b/test/e2e/resources/stdlib/create-object.brs new file mode 100644 index 00000000..5d8823c8 --- /dev/null +++ b/test/e2e/resources/stdlib/create-object.brs @@ -0,0 +1,30 @@ +sub main() + print createObject("roBoolean") + print createObject("roBoolean", true) + print createObject("roDouble") + print createObject("roDouble", 1.0) + print createObject("roFloat") + print createObject("roFloat", 1.0) + print createObject("roInt") + print createObject("roInt", 1, 2, 3) + print createObject("roLongInteger") + print createObject("roLongInteger", 1) + print createObject("roScreen") + print createObject("roString") + print createObject("roString", "hello") + print createObject("roInvalid") + print createObject("roInvalid", 1) + print createObject("roSGNode", "Node").focusable + print createObject("roAssociativeArray").count() + try + print createObject("roAssociativeArray", {a: 1, b: 2}).count() + catch e + print e.number + end try + print createObject("roRegex") + try + print createObject("roRegex", 1) + catch e + print e.number + end try +end sub \ No newline at end of file diff --git a/test/e2e/resources/stdlib/global-utilities.brs b/test/e2e/resources/stdlib/global-utilities.brs index 37d8bb40..acdcce67 100644 --- a/test/e2e/resources/stdlib/global-utilities.brs +++ b/test/e2e/resources/stdlib/global-utilities.brs @@ -7,4 +7,8 @@ sub main() print GetInterface("", "ifStringOps") print FindMemberFunction(1, "tostr") print GetInterface(1, "iftostr") + di = createObject("roDeviceInfo") + iface = getInterface(di, "ifDeviceInfo") + print objFun(di, iface, "getModel") + print objFun(di, iface, "canDecodeVideo", {codec: "mpeg2"}).result end sub diff --git a/test/stdlib/GlobalUtilities.test.js b/test/stdlib/GlobalUtilities.test.js index 0cb48314..936a93a1 100644 --- a/test/stdlib/GlobalUtilities.test.js +++ b/test/stdlib/GlobalUtilities.test.js @@ -1,6 +1,6 @@ const brs = require("../../lib"); -const { RoAssociativeArray, BrsString, BrsInvalid, BrsInterface, RoSGNode } = brs.types; -const { GetInterface, FindMemberFunction } = require("../../lib/stdlib"); +const { RoAssociativeArray, BrsString, BrsInvalid, BrsInterface, RoSGNode, Int32 } = brs.types; +const { GetInterface, FindMemberFunction, ObjFun } = require("../../lib/stdlib"); const { Interpreter } = require("../../lib/interpreter"); describe("global utility functions", () => { @@ -58,4 +58,43 @@ describe("global utility functions", () => { expect(memberFunction.name).toBe("ifSGNodeDict"); }); }); + describe("ObjFun", () => { + it("successfully call a method of a function with no arguments", () => { + let assocarray = new RoAssociativeArray([ + { name: new BrsString("letter1"), value: new BrsString("a") }, + { name: new BrsString("letter2"), value: new BrsString("b") }, + ]); + let iface = GetInterface.call( + interpreter, + assocarray, + new BrsString("ifAssociativeArray") + ); + expect(iface).toBeInstanceOf(BrsInterface); + expect(iface.name).toBe("ifAssociativeArray"); + let result = ObjFun.call(interpreter, assocarray, iface, new BrsString("Count")); + expect(result).toEqual(new Int32(2)); + }); + + it("successfully call a method of a function with arguments", () => { + let assocarray = new RoAssociativeArray([ + { name: new BrsString("letter1"), value: new BrsString("a") }, + { name: new BrsString("letter2"), value: new BrsString("b") }, + ]); + let iface = GetInterface.call( + interpreter, + assocarray, + new BrsString("ifAssociativeArray") + ); + expect(iface).toBeInstanceOf(BrsInterface); + expect(iface.name).toBe("ifAssociativeArray"); + let result = ObjFun.call( + interpreter, + assocarray, + iface, + new BrsString("lookup"), + new BrsString("letter1") + ); + expect(result).toEqual(new BrsString("a")); + }); + }); });