From e8ea91d174ba0e568af8036458615cd34e71d938 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Thu, 8 Apr 2021 16:32:40 +0300 Subject: [PATCH 01/14] fix issue #638, allow "node" node type of "roSGNode" --- src/brsTypes/components/ComponentFactory.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/brsTypes/components/ComponentFactory.ts b/src/brsTypes/components/ComponentFactory.ts index 29445a34f..2b0d246ee 100644 --- a/src/brsTypes/components/ComponentFactory.ts +++ b/src/brsTypes/components/ComponentFactory.ts @@ -18,6 +18,7 @@ import { export enum BrsComponentName { Node = "Node", + Node_lowerCase = "node", Group = "Group", LayoutGroup = "LayoutGroup", Rectangle = "Rectangle", @@ -68,6 +69,8 @@ export class ComponentFactory { return new LayoutGroup([], name); case BrsComponentName.Node: return new RoSGNode([], name); + case BrsComponentName.Node_lowerCase: + return new RoSGNode([], name); case BrsComponentName.Rectangle: return new Rectangle([], name); case BrsComponentName.Label: From e1486281a811354b78b0caa557aa473f6e4bf15f Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Thu, 8 Apr 2021 16:42:51 +0300 Subject: [PATCH 02/14] add lookupCI for AA and Nodes --- src/brsTypes/components/RoAssociativeArray.ts | 19 +++++++++++++++++-- src/brsTypes/components/RoSGNode.ts | 3 +++ .../components/RoAssociativeArray.test.js | 14 ++++++++++++++ test/e2e/BrsComponents.test.js | 2 ++ test/e2e/RoSGNode.test.js | 2 ++ .../components/roAssociativeArray.brs | 1 + .../components/roSGNode/roSGNode.brs | 1 + 7 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index c91fd8d05..a0ad24ec3 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -36,6 +36,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte this.keys, this.items, this.lookup, + this.lookupCI, this.setmodecasesensitive, ], ifEnum: [this.isEmpty], @@ -77,7 +78,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte .map((value: BrsType) => value); } - get(index: BrsType) { + get(index: BrsType, forceCaseInsensitive = false) { if (index.kind !== ValueKind.String) { throw new Error("Associative array indexes must be strings"); } @@ -93,7 +94,10 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte // method with the desired name separately? That last bit would work but it's pretty gross. // That'd allow roArrays to have methods with the methods not accessible via `arr["count"]`. // Same with RoAssociativeArrays I guess. - let indexValue = this.modeCaseSensitive ? index.value : index.value.toLowerCase(); + let indexValue = + this.modeCaseSensitive && !forceCaseInsensitive + ? index.value + : index.value.toLowerCase(); return this.elements.get(indexValue) || this.getMethod(index.value) || BrsInvalid.Instance; } @@ -235,6 +239,17 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, }); + private lookupCI = new Callable("lookupCI", { + signature: { + args: [new StdlibArgument("key", ValueKind.String)], + returns: ValueKind.Dynamic, + }, + impl: (interpreter: Interpreter, key: BrsString) => { + let lKey = key.value; + return this.get(new BrsString(lKey), true); + }, + }); + /** Changes the sensitive case method for lookups */ private setmodecasesensitive = new Callable("setModeCaseSensitive", { signature: { diff --git a/src/brsTypes/components/RoSGNode.ts b/src/brsTypes/components/RoSGNode.ts index 4f9340514..2dad5b30a 100644 --- a/src/brsTypes/components/RoSGNode.ts +++ b/src/brsTypes/components/RoSGNode.ts @@ -327,6 +327,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { this.keys, this.items, this.lookup, + this.lookupCI, ], ifSGNodeField: [ this.addfield, @@ -801,6 +802,8 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { }, }); + private lookupCI = new Callable("lookupCI", this.lookup.signatures[0]); + /** Adds a new field to the node, if the field already exists it doesn't change the current value. */ private addfield = new Callable("addfield", { signature: { diff --git a/test/brsTypes/components/RoAssociativeArray.test.js b/test/brsTypes/components/RoAssociativeArray.test.js index c601a4421..043b7101c 100644 --- a/test/brsTypes/components/RoAssociativeArray.test.js +++ b/test/brsTypes/components/RoAssociativeArray.test.js @@ -372,6 +372,20 @@ describe("RoAssociativeArray", () => { result2 = aa.get(new BrsString("KeY2")); expect(result2).toBe(v2); + + // check lookupCI method + let lookupCI = aa.getMethod("lookupCI"); + expect(lookupCI).toBeTruthy(); + + addreplace.call(interpreter, new BrsString("key2"), v1); + let result = lookup.call(interpreter, new BrsString("key2")); + expect(result).toBe(v1); + + result = lookupCI.call(interpreter, new BrsString("KEY2")); + expect(result).toBe(v1); + + result = lookupCI.call(interpreter, new BrsString("Key2")); + expect(result).toBe(v1); }); }); }); diff --git a/test/e2e/BrsComponents.test.js b/test/e2e/BrsComponents.test.js index 35d5557ae..4dcf108a1 100644 --- a/test/e2e/BrsComponents.test.js +++ b/test/e2e/BrsComponents.test.js @@ -60,6 +60,8 @@ describe("end to end brightscript functions", () => { "true", "can look up elements (brackets): ", "true", + "can case insensitive look up elements: ", + "true", "can check for existence: ", "true", "items() example key: ", diff --git a/test/e2e/RoSGNode.test.js b/test/e2e/RoSGNode.test.js index 6eb0882f4..0b50ba83e 100644 --- a/test/e2e/RoSGNode.test.js +++ b/test/e2e/RoSGNode.test.js @@ -103,6 +103,8 @@ describe("components/roSGNode", () => { "true", "can look up elements (brackets): ", "true", + "can case insensitive look up elements: ", + "true", "can check for existence: ", "true", "can empty itself: ", diff --git a/test/e2e/resources/components/roAssociativeArray.brs b/test/e2e/resources/components/roAssociativeArray.brs index 86cb7985d..6bc8d46c3 100644 --- a/test/e2e/resources/components/roAssociativeArray.brs +++ b/test/e2e/resources/components/roAssociativeArray.brs @@ -11,6 +11,7 @@ sub main() print "can delete elements: " aa.delete("baz") ' => true print "can look up elements: " aa.lookup("foo") = "foo" ' => true print "can look up elements (brackets): " aa["foo"] = "foo" ' => true + print "can case insensitive look up elements: " aa.lookupCI("foO") = "foo" ' => true print "can check for existence: " aa.doesExist("bar") ' => true print "items() example key: " aa.items()[0].key ' => bar print "items() example value: " aa.items()[0].value ' => 5 diff --git a/test/e2e/resources/components/roSGNode/roSGNode.brs b/test/e2e/resources/components/roSGNode/roSGNode.brs index 7dd22c2fe..20b3c420a 100644 --- a/test/e2e/resources/components/roSGNode/roSGNode.brs +++ b/test/e2e/resources/components/roSGNode/roSGNode.brs @@ -12,6 +12,7 @@ sub init() print "can delete elements: " node1.delete("baz") ' => true print "can look up elements: " node1.lookup("foo") = "foo" ' => true print "can look up elements (brackets): " node1["foo"] = "foo" ' => true + print "can case insensitive look up elements: " node1.lookupCI("foO") = "foo" ' => true print "can check for existence: " node1.doesExist("bar") ' => true node1.clear() From 44c6f5339498e65a30cf48e4ad760b214ef83d23 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Thu, 8 Apr 2021 17:39:51 +0300 Subject: [PATCH 03/14] Revert "fix issue #638, allow "node" node type of "roSGNode"" This reverts commit e8ea91d174ba0e568af8036458615cd34e71d938. --- src/brsTypes/components/ComponentFactory.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/brsTypes/components/ComponentFactory.ts b/src/brsTypes/components/ComponentFactory.ts index 2b0d246ee..29445a34f 100644 --- a/src/brsTypes/components/ComponentFactory.ts +++ b/src/brsTypes/components/ComponentFactory.ts @@ -18,7 +18,6 @@ import { export enum BrsComponentName { Node = "Node", - Node_lowerCase = "node", Group = "Group", LayoutGroup = "LayoutGroup", Rectangle = "Rectangle", @@ -69,8 +68,6 @@ export class ComponentFactory { return new LayoutGroup([], name); case BrsComponentName.Node: return new RoSGNode([], name); - case BrsComponentName.Node_lowerCase: - return new RoSGNode([], name); case BrsComponentName.Rectangle: return new Rectangle([], name); case BrsComponentName.Label: From 29809efe401e43f386e393530afcdd151e8f7c07 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Fri, 9 Apr 2021 21:23:58 +0300 Subject: [PATCH 04/14] added comments, moved `lookupCI` test in separate suit. Fixed bug with creating fields on Node by []. Fixed logic of using/saving keys in AA --- src/brsTypes/components/BrsComponent.ts | 5 +- src/brsTypes/components/RoAssociativeArray.ts | 71 ++++++++++++++----- src/brsTypes/components/RoSGNode.ts | 42 ++++++++--- src/interpreter/index.ts | 4 +- .../components/RoAssociativeArray.test.js | 29 ++++++-- test/e2e/BrsComponents.test.js | 8 +++ .../components/roAssociativeArray.brs | 9 +++ 7 files changed, 132 insertions(+), 36 deletions(-) diff --git a/src/brsTypes/components/BrsComponent.ts b/src/brsTypes/components/BrsComponent.ts index a5417dc65..2e3197c5c 100644 --- a/src/brsTypes/components/BrsComponent.ts +++ b/src/brsTypes/components/BrsComponent.ts @@ -58,9 +58,10 @@ export interface BrsIterable { /** * Retrieves an element from this component at the provided `index`. * @param index the index in this component from which to retrieve the desired element. + * @param isCaseSensitiveGet determinate whether operation of getting should be case sensitive or not. * @returns the element at `index` if one exists, otherwise throws an Error. */ - get(index: BrsType): BrsType; + get(index: BrsType, isCaseSensitiveGet?: boolean): BrsType; - set(index: BrsType, value: BrsType): BrsInvalid; + set(index: BrsType, value: BrsType, isCaseSensitiveSet?: boolean): BrsInvalid; } diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index a0ad24ec3..a5efa0c63 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -21,9 +21,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte constructor(elements: AAMember[]) { super("roAssociativeArray"); - elements.forEach((member) => - this.elements.set(member.name.value.toLowerCase(), member.value) - ); + elements.forEach((member) => this.set(member.name, member.value, true)); this.registerMethods({ ifAssociativeArray: [ @@ -78,7 +76,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte .map((value: BrsType) => value); } - get(index: BrsType, forceCaseInsensitive = false) { + get(index: BrsType, isCaseSensitiveGet = false) { if (index.kind !== ValueKind.String) { throw new Error("Associative array indexes must be strings"); } @@ -94,21 +92,49 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte // method with the desired name separately? That last bit would work but it's pretty gross. // That'd allow roArrays to have methods with the methods not accessible via `arr["count"]`. // Same with RoAssociativeArrays I guess. - let indexValue = - this.modeCaseSensitive && !forceCaseInsensitive - ? index.value - : index.value.toLowerCase(); - return this.elements.get(indexValue) || this.getMethod(index.value) || BrsInvalid.Instance; + return ( + this.findElement(index.value, isCaseSensitiveGet) || + this.getMethod(index.value) || + BrsInvalid.Instance + ); } - set(index: BrsType, value: BrsType) { + set(index: BrsType, value: BrsType, isCaseSensitiveSet = false) { if (index.kind !== ValueKind.String) { throw new Error("Associative array indexes must be strings"); } - let indexValue = this.modeCaseSensitive ? index.value : index.value.toLowerCase(); + // override old key with new one + let oldKey = this.findElementKey(index.value); + if (!this.modeCaseSensitive && oldKey) { + this.elements.delete(oldKey); + } + + let indexValue = isCaseSensitiveSet ? index.value : index.value.toLowerCase(); this.elements.set(indexValue, value); return BrsInvalid.Instance; } + /** if AA is in insensitive mode, it means that we should do insensitive search of real key */ + private findElementKey(elementKey: string, isCaseSensitiveFind = false) { + let useCaseSensitiveSearch = this.modeCaseSensitive && isCaseSensitiveFind; + elementKey = useCaseSensitiveSearch ? elementKey : elementKey.toLowerCase(); + if (this.elements.has(elementKey)) { + return elementKey; + } + + let realElementKey = undefined; + for (let key of this.elements.keys()) { + if ((useCaseSensitiveSearch ? key : key.toLowerCase()) === elementKey) { + realElementKey = key; + break; + } + } + return realElementKey; + } + + private findElement(elementKey: string, isCaseSensitiveFind = false) { + let realKey = this.findElementKey(elementKey, isCaseSensitiveFind); + return realKey !== undefined ? this.elements.get(realKey) : undefined; + } /** Removes all elements from the associative array */ private clear = new Callable("clear", { @@ -129,7 +155,8 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Boolean, }, impl: (interpreter: Interpreter, str: BrsString) => { - let deleted = this.elements.delete(str.value.toLowerCase()); + let key = this.findElementKey(str.value, this.modeCaseSensitive); + let deleted = key ? this.elements.delete(key) : false; return BrsBoolean.from(deleted); }, }); @@ -146,7 +173,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Void, }, impl: (interpreter: Interpreter, key: BrsString, value: BrsType) => { - this.set(key, value); + this.set(key, value, true); return BrsInvalid.Instance; }, }); @@ -170,7 +197,8 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, impl: (interpreter: Interpreter, str: BrsString) => { let strValue = this.modeCaseSensitive ? str.value : str.value.toLowerCase(); - return this.elements.has(strValue) ? BrsBoolean.True : BrsBoolean.False; + let key = this.findElementKey(str.value, this.modeCaseSensitive); + return key && this.elements.has(key) ? BrsBoolean.True : BrsBoolean.False; }, }); @@ -186,7 +214,9 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte return BrsInvalid.Instance; } - this.elements = new Map([...this.elements, ...obj.elements]); + obj.elements.forEach((value, key) => { + this.set(new BrsString(key), value, true); + }); return BrsInvalid.Instance; }, @@ -227,7 +257,9 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, }); - /** Given a key, returns the value associated with that key. This method is case insensitive. */ + /** Given a key, returns the value associated with that key. + * This method is case insensitive either-or case sensitive, depends on whether `setModeCasesensitive` was called or not. + * */ private lookup = new Callable("lookup", { signature: { args: [new StdlibArgument("key", ValueKind.String)], @@ -235,10 +267,11 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, impl: (interpreter: Interpreter, key: BrsString) => { let lKey = key.value; - return this.get(new BrsString(lKey)); + return this.get(new BrsString(lKey), true); }, }); + /** Given a key, returns the value associated with that key. This method always is case insensitive. */ private lookupCI = new Callable("lookupCI", { signature: { args: [new StdlibArgument("key", ValueKind.String)], @@ -246,7 +279,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, impl: (interpreter: Interpreter, key: BrsString) => { let lKey = key.value; - return this.get(new BrsString(lKey), true); + return this.get(new BrsString(lKey)); }, }); @@ -257,7 +290,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Void, }, impl: (interpreter: Interpreter) => { - this.modeCaseSensitive = !this.modeCaseSensitive; + this.modeCaseSensitive = true; return BrsInvalid.Instance; }, }); diff --git a/src/brsTypes/components/RoSGNode.ts b/src/brsTypes/components/RoSGNode.ts index 2dad5b30a..2ca35e46c 100644 --- a/src/brsTypes/components/RoSGNode.ts +++ b/src/brsTypes/components/RoSGNode.ts @@ -436,30 +436,53 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { return this.getMethod(index.value) || BrsInvalid.Instance; } - set(index: BrsType, value: BrsType, alwaysNotify: boolean = false, kind?: FieldKind) { + set(index: BrsType, value: BrsType) { if (index.kind !== ValueKind.String) { throw new Error("RoSGNode indexes must be strings"); } let mapKey = index.value.toLowerCase(); - let fieldType = kind || FieldKind.fromBrsType(value); let field = this.fields.get(mapKey); if (!field) { - // RBI does not create a new field if the value isn't valid. - if (fieldType) { - field = new Field(value, fieldType, alwaysNotify); - this.fields.set(mapKey, field); - } + // TODO: change to using interpreter.stderr + console.warn( + `=================================================================\n` + + `Warning occurred while setting a field of an RoSGNode\n` + + `-- Tried to set nonexistent field "${mapKey}" of a "${this.nodeSubtype}" node\n` + + `=================================================================\n` + ); } else if (field.canAcceptValue(value)) { // Fields are not overwritten if they haven't the same type. field.setValue(value); this.fields.set(mapKey, field); + } else { + console.warn(`BRIGHTSCRIPT: ERROR: roSGNode.AddReplace: "${field}": Type mismatch`); } return BrsInvalid.Instance; } + private _addField( + index: BrsType, + value: BrsType, + alwaysNotify: boolean = false, + kind?: FieldKind + ) { + if (index.kind !== ValueKind.String) { + throw new Error("RoSGNode indexes must be strings"); + } + + let mapKey = index.value.toLowerCase(); + let fieldType = kind || FieldKind.fromBrsType(value); + let field = this.fields.get(mapKey); + + if (!field && fieldType) { + field = new Field(value, fieldType, alwaysNotify); + this.fields.set(mapKey, field); + } + } + getParent() { return this.parent; } @@ -802,6 +825,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { }, }); + /** Given a key, returns the value associated with that key. This method is case insensitive. */ private lookupCI = new Callable("lookupCI", this.lookup.signatures[0]); /** Adds a new field to the node, if the field already exists it doesn't change the current value. */ @@ -824,7 +848,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { let fieldKind = FieldKind.fromString(type.value); if (defaultValue !== Uninitialized.Instance && !this.fields.has(fieldname.value)) { - this.set(fieldname, defaultValue, alwaysnotify.toBoolean(), fieldKind); + this._addField(fieldname, defaultValue, alwaysnotify.toBoolean(), fieldKind); } return BrsBoolean.True; @@ -845,7 +869,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { fields.getValue().forEach((value, key) => { let fieldName = new BrsString(key); if (!this.fields.has(key)) { - this.set(fieldName, value); + this._addField(fieldName, value); } }); diff --git a/src/interpreter/index.ts b/src/interpreter/index.ts index 39fc17184..3baed8b38 100644 --- a/src/interpreter/index.ts +++ b/src/interpreter/index.ts @@ -1278,7 +1278,7 @@ export class Interpreter implements Expr.Visitor, Stmt.Visitor } try { - return source.get(index); + return source.get(index, true); } catch (err) { return this.addError(new BrsError(err.message, expression.closingSquare.location)); } @@ -1537,7 +1537,7 @@ export class Interpreter implements Expr.Visitor, Stmt.Visitor let value = this.evaluate(statement.value); try { - source.set(index, value); + source.set(index, value, true); } catch (err) { return this.addError(new BrsError(err.message, statement.closingSquare.location)); } diff --git a/test/brsTypes/components/RoAssociativeArray.test.js b/test/brsTypes/components/RoAssociativeArray.test.js index 043b7101c..b812ea540 100644 --- a/test/brsTypes/components/RoAssociativeArray.test.js +++ b/test/brsTypes/components/RoAssociativeArray.test.js @@ -372,19 +372,40 @@ describe("RoAssociativeArray", () => { result2 = aa.get(new BrsString("KeY2")); expect(result2).toBe(v2); + }); + }); + describe("lookupCI", () => { + it("changes lookups to case sensitive mode", () => { + let v1 = new BrsString("value1"); + let v2 = new BrsString("value2"); + + let aa = new RoAssociativeArray([{ name: new BrsString("key1"), value: v1 }]); + + let setModeCaseSensitive = aa.getMethod("setmodecasesensitive"); + expect(setModeCaseSensitive).toBeTruthy(); + setModeCaseSensitive.call(interpreter); + + let addreplace = aa.getMethod("addreplace"); + expect(addreplace).toBeTruthy(); + addreplace.call(interpreter, new BrsString("KEY1"), v2); + + let lookup = aa.getMethod("lookup"); + expect(lookup).toBeTruthy(); // check lookupCI method let lookupCI = aa.getMethod("lookupCI"); expect(lookupCI).toBeTruthy(); - addreplace.call(interpreter, new BrsString("key2"), v1); - let result = lookup.call(interpreter, new BrsString("key2")); + let result = lookup.call(interpreter, new BrsString("key1")); expect(result).toBe(v1); - result = lookupCI.call(interpreter, new BrsString("KEY2")); + result = lookup.call(interpreter, new BrsString("KEY1")); + expect(result).toBe(v2); + + result = lookupCI.call(interpreter, new BrsString("kEy1")); expect(result).toBe(v1); - result = lookupCI.call(interpreter, new BrsString("Key2")); + result = lookupCI.call(interpreter, new BrsString("KeY1")); expect(result).toBe(v1); }); }); diff --git a/test/e2e/BrsComponents.test.js b/test/e2e/BrsComponents.test.js index 4dcf108a1..a9eb22bff 100644 --- a/test/e2e/BrsComponents.test.js +++ b/test/e2e/BrsComponents.test.js @@ -76,6 +76,14 @@ describe("end to end brightscript functions", () => { "value1", "can empty itself: ", "true", + "saved key: ", + "DD", + "saved key after accessing by dot: ", + "dd", + "saved key after accessing by index: ", + "Dd", + "AA keys size: ", + "1", ]); }); diff --git a/test/e2e/resources/components/roAssociativeArray.brs b/test/e2e/resources/components/roAssociativeArray.brs index 6bc8d46c3..0ab9bebe8 100644 --- a/test/e2e/resources/components/roAssociativeArray.brs +++ b/test/e2e/resources/components/roAssociativeArray.brs @@ -25,4 +25,13 @@ sub main() aa.clear() print "can empty itself: " aa.isEmpty() ' => true + + ' check mimic of RBI AA keys saving logic + aa = {"dd": 0, "DD":1} + print "saved key: "aa.keys()[0] ' => DD + aa.dd = 2 + print "saved key after accessing by dot: "aa.keys()[0] ' => dd + aa["Dd"] = 3 + print "saved key after accessing by index: "aa.keys()[0] ' => Dd + print "AA keys size: " aa.keys().count() ' => 1 end sub From 393c24e965364f8bdb9004e8397ce7f8fff3be11 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Fri, 9 Apr 2021 21:27:23 +0300 Subject: [PATCH 05/14] fix lint --- src/brsTypes/components/RoAssociativeArray.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index a5efa0c63..77174d1e9 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -259,7 +259,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte /** Given a key, returns the value associated with that key. * This method is case insensitive either-or case sensitive, depends on whether `setModeCasesensitive` was called or not. - * */ + */ private lookup = new Callable("lookup", { signature: { args: [new StdlibArgument("key", ValueKind.String)], From f10202c4025854b0cad9b09cd545075897734b30 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Fri, 9 Apr 2021 22:15:36 +0300 Subject: [PATCH 06/14] revert fix for node and creating fild by [] --- src/brsTypes/components/RoSGNode.ts | 41 +++++++---------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/src/brsTypes/components/RoSGNode.ts b/src/brsTypes/components/RoSGNode.ts index 2ca35e46c..b3935bff5 100644 --- a/src/brsTypes/components/RoSGNode.ts +++ b/src/brsTypes/components/RoSGNode.ts @@ -436,53 +436,30 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { return this.getMethod(index.value) || BrsInvalid.Instance; } - set(index: BrsType, value: BrsType) { + set(index: BrsType, value: BrsType, alwaysNotify: boolean = false, kind?: FieldKind) { if (index.kind !== ValueKind.String) { throw new Error("RoSGNode indexes must be strings"); } let mapKey = index.value.toLowerCase(); + let fieldType = kind || FieldKind.fromBrsType(value); let field = this.fields.get(mapKey); if (!field) { - // TODO: change to using interpreter.stderr - console.warn( - `=================================================================\n` + - `Warning occurred while setting a field of an RoSGNode\n` + - `-- Tried to set nonexistent field "${mapKey}" of a "${this.nodeSubtype}" node\n` + - `=================================================================\n` - ); + // RBI does not create a new field if the value isn't valid. + if (fieldType) { + field = new Field(value, fieldType, alwaysNotify); + this.fields.set(mapKey, field); + } } else if (field.canAcceptValue(value)) { // Fields are not overwritten if they haven't the same type. field.setValue(value); this.fields.set(mapKey, field); - } else { - console.warn(`BRIGHTSCRIPT: ERROR: roSGNode.AddReplace: "${field}": Type mismatch`); } return BrsInvalid.Instance; } - private _addField( - index: BrsType, - value: BrsType, - alwaysNotify: boolean = false, - kind?: FieldKind - ) { - if (index.kind !== ValueKind.String) { - throw new Error("RoSGNode indexes must be strings"); - } - - let mapKey = index.value.toLowerCase(); - let fieldType = kind || FieldKind.fromBrsType(value); - let field = this.fields.get(mapKey); - - if (!field && fieldType) { - field = new Field(value, fieldType, alwaysNotify); - this.fields.set(mapKey, field); - } - } - getParent() { return this.parent; } @@ -848,7 +825,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { let fieldKind = FieldKind.fromString(type.value); if (defaultValue !== Uninitialized.Instance && !this.fields.has(fieldname.value)) { - this._addField(fieldname, defaultValue, alwaysnotify.toBoolean(), fieldKind); + this.set(fieldname, defaultValue, alwaysnotify.toBoolean(), fieldKind); } return BrsBoolean.True; @@ -869,7 +846,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { fields.getValue().forEach((value, key) => { let fieldName = new BrsString(key); if (!this.fields.has(key)) { - this._addField(fieldName, value); + this.set(fieldName, value); } }); From d8f2e581a1bfd85e646774d5fd985ad559709a2c Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Fri, 9 Apr 2021 22:32:47 +0300 Subject: [PATCH 07/14] fix ut fails --- src/brsTypes/components/RoSGNode.ts | 2 +- test/brsTypes/components/RoSGNode.test.js | 2 +- test/e2e/Iterables.test.js | 2 +- test/e2e/Syntax.test.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/brsTypes/components/RoSGNode.ts b/src/brsTypes/components/RoSGNode.ts index b3935bff5..6ee7e3d4a 100644 --- a/src/brsTypes/components/RoSGNode.ts +++ b/src/brsTypes/components/RoSGNode.ts @@ -301,7 +301,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable { readonly defaultFields: FieldModel[] = [ { name: "change", type: "roAssociativeArray" }, { name: "focusable", type: "boolean" }, - { name: "focusedChild", type: "node", alwaysNotify: true }, + { name: "focusedchild", type: "node", alwaysNotify: true }, { name: "id", type: "string" }, ]; m: RoAssociativeArray = new RoAssociativeArray([]); diff --git a/test/brsTypes/components/RoSGNode.test.js b/test/brsTypes/components/RoSGNode.test.js index 6192ac007..274a1c76d 100644 --- a/test/brsTypes/components/RoSGNode.test.js +++ b/test/brsTypes/components/RoSGNode.test.js @@ -528,7 +528,7 @@ describe("RoSGNode", () => { let expected = new RoAssociativeArray([ { name: new BrsString("change"), value: new RoAssociativeArray([]) }, { name: new BrsString("focusable"), value: BrsBoolean.False }, - { name: new BrsString("focusedChild"), value: BrsInvalid.Instance }, + { name: new BrsString("focusedchild"), value: BrsInvalid.Instance }, { name: new BrsString("id"), value: new BrsString("") }, ]); result.elements.forEach((value, name) => { diff --git a/test/e2e/Iterables.test.js b/test/e2e/Iterables.test.js index cbadea109..cd0a8cd7e 100644 --- a/test/e2e/Iterables.test.js +++ b/test/e2e/Iterables.test.js @@ -37,7 +37,7 @@ describe("end to end iterables", () => { // iterate through keys "has-second-layer", "level", - "secondlayer", + "secondLayer", // twoDimensional.secondLayer.level "2", diff --git a/test/e2e/Syntax.test.js b/test/e2e/Syntax.test.js index 77c84fab2..29140829e 100644 --- a/test/e2e/Syntax.test.js +++ b/test/e2e/Syntax.test.js @@ -195,7 +195,7 @@ describe("end to end syntax", () => { expect(allArgs(outputStreams.stdout.write).filter((arg) => arg !== "\n")).toEqual([ // note: associative array keys are sorted before iteration - "createobject", + "createObject", "in", "run", "stop", From f353bdfbc0493374a188182b251431aaccc1e610 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Wed, 14 Apr 2021 23:41:25 +0300 Subject: [PATCH 08/14] resolve nits --- src/brsTypes/components/BrsComponent.ts | 6 +++--- src/brsTypes/components/RoAssociativeArray.ts | 9 +++++---- test/brsTypes/components/RoAssociativeArray.test.js | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/brsTypes/components/BrsComponent.ts b/src/brsTypes/components/BrsComponent.ts index 2e3197c5c..709202a1b 100644 --- a/src/brsTypes/components/BrsComponent.ts +++ b/src/brsTypes/components/BrsComponent.ts @@ -58,10 +58,10 @@ export interface BrsIterable { /** * Retrieves an element from this component at the provided `index`. * @param index the index in this component from which to retrieve the desired element. - * @param isCaseSensitiveGet determinate whether operation of getting should be case sensitive or not. + * @param isCaseSensitive determinate whether operation of getting should be case sensitive or not. * @returns the element at `index` if one exists, otherwise throws an Error. */ - get(index: BrsType, isCaseSensitiveGet?: boolean): BrsType; + get(index: BrsType, isCaseSensitive?: boolean): BrsType; - set(index: BrsType, value: BrsType, isCaseSensitiveSet?: boolean): BrsInvalid; + set(index: BrsType, value: BrsType, isCaseSensitive?: boolean): BrsInvalid; } diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index 77174d1e9..ce3a29785 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -76,7 +76,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte .map((value: BrsType) => value); } - get(index: BrsType, isCaseSensitiveGet = false) { + get(index: BrsType, isCaseSensitive = false) { if (index.kind !== ValueKind.String) { throw new Error("Associative array indexes must be strings"); } @@ -93,13 +93,13 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte // That'd allow roArrays to have methods with the methods not accessible via `arr["count"]`. // Same with RoAssociativeArrays I guess. return ( - this.findElement(index.value, isCaseSensitiveGet) || + this.findElement(index.value, isCaseSensitive) || this.getMethod(index.value) || BrsInvalid.Instance ); } - set(index: BrsType, value: BrsType, isCaseSensitiveSet = false) { + set(index: BrsType, value: BrsType, isCaseSensitive = false) { if (index.kind !== ValueKind.String) { throw new Error("Associative array indexes must be strings"); } @@ -109,10 +109,11 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte this.elements.delete(oldKey); } - let indexValue = isCaseSensitiveSet ? index.value : index.value.toLowerCase(); + let indexValue = isCaseSensitive ? index.value : index.value.toLowerCase(); this.elements.set(indexValue, value); return BrsInvalid.Instance; } + /** if AA is in insensitive mode, it means that we should do insensitive search of real key */ private findElementKey(elementKey: string, isCaseSensitiveFind = false) { let useCaseSensitiveSearch = this.modeCaseSensitive && isCaseSensitiveFind; diff --git a/test/brsTypes/components/RoAssociativeArray.test.js b/test/brsTypes/components/RoAssociativeArray.test.js index b812ea540..9f69dfc1b 100644 --- a/test/brsTypes/components/RoAssociativeArray.test.js +++ b/test/brsTypes/components/RoAssociativeArray.test.js @@ -374,6 +374,7 @@ describe("RoAssociativeArray", () => { expect(result2).toBe(v2); }); }); + describe("lookupCI", () => { it("changes lookups to case sensitive mode", () => { let v1 = new BrsString("value1"); From 29ea65a96828bd1a3b5702357b8ef1bca45b5d9a Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Sat, 17 Apr 2021 01:25:01 +0300 Subject: [PATCH 09/14] fix O(N) computation coplex for AA data accessing. --- src/brsTypes/components/RoAssociativeArray.ts | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index ce3a29785..ab7b3f335 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -17,6 +17,10 @@ export interface AAMember { export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIterable { readonly kind = ValueKind.Object; elements = new Map(); + /** keyMap - maps lowercased key to original one which used in this.elements to store value + * Main benefit of it is fast case insensitive access + */ + keyMap = new Map(); private modeCaseSensitive: boolean = false; constructor(elements: AAMember[]) { @@ -111,25 +115,18 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte let indexValue = isCaseSensitive ? index.value : index.value.toLowerCase(); this.elements.set(indexValue, value); + this.keyMap.set(index.value.toLowerCase(), indexValue); + return BrsInvalid.Instance; } /** if AA is in insensitive mode, it means that we should do insensitive search of real key */ private findElementKey(elementKey: string, isCaseSensitiveFind = false) { - let useCaseSensitiveSearch = this.modeCaseSensitive && isCaseSensitiveFind; - elementKey = useCaseSensitiveSearch ? elementKey : elementKey.toLowerCase(); - if (this.elements.has(elementKey)) { + if (this.modeCaseSensitive && isCaseSensitiveFind) { return elementKey; + } else { + return this.keyMap.get(elementKey.toLowerCase()); } - - let realElementKey = undefined; - for (let key of this.elements.keys()) { - if ((useCaseSensitiveSearch ? key : key.toLowerCase()) === elementKey) { - realElementKey = key; - break; - } - } - return realElementKey; } private findElement(elementKey: string, isCaseSensitiveFind = false) { @@ -145,6 +142,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte }, impl: (interpreter: Interpreter) => { this.elements.clear(); + this.keyMap.clear(); return BrsInvalid.Instance; }, }); @@ -158,6 +156,20 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte impl: (interpreter: Interpreter, str: BrsString) => { let key = this.findElementKey(str.value, this.modeCaseSensitive); let deleted = key ? this.elements.delete(key) : false; + this.keyMap.delete(str.value.toLowerCase()); + + // if we have {"DD": 0, "dD": 0}, keyMap["dd"] is pointed to "DD", + // and we delets it, then we should find another value for case insensitive accessing ("dD") + if (this.modeCaseSensitive) { + let lKey = str.value.toLowerCase(); + for (let key of this.elements.keys()) { + if (key.toLowerCase() === lKey) { + this.keyMap.set(lKey, key); + break; + } + } + } + return BrsBoolean.from(deleted); }, }); @@ -267,8 +279,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Dynamic, }, impl: (interpreter: Interpreter, key: BrsString) => { - let lKey = key.value; - return this.get(new BrsString(lKey), true); + return this.get(key, true); }, }); @@ -279,8 +290,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Dynamic, }, impl: (interpreter: Interpreter, key: BrsString) => { - let lKey = key.value; - return this.get(new BrsString(lKey)); + return this.get(key); }, }); From f7d7bbae9468c66229fec8587084e9b3494b1673 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Sat, 17 Apr 2021 01:34:53 +0300 Subject: [PATCH 10/14] refactoring for AA's `delete` method --- src/brsTypes/components/RoAssociativeArray.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index ab7b3f335..f99143385 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -156,20 +156,21 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte impl: (interpreter: Interpreter, str: BrsString) => { let key = this.findElementKey(str.value, this.modeCaseSensitive); let deleted = key ? this.elements.delete(key) : false; - this.keyMap.delete(str.value.toLowerCase()); - - // if we have {"DD": 0, "dD": 0}, keyMap["dd"] is pointed to "DD", - // and we delets it, then we should find another value for case insensitive accessing ("dD") - if (this.modeCaseSensitive) { - let lKey = str.value.toLowerCase(); - for (let key of this.elements.keys()) { - if (key.toLowerCase() === lKey) { - this.keyMap.set(lKey, key); - break; + + let lKey = str.value.toLowerCase(); + if (this.keyMap.get(lKey) === key) { + this.keyMap.delete(lKey); + // if we have {"DD": 0, "dD": 0}, keyMap["dd"] is pointed to "DD", + // and we delets it, then we should find another value for case insensitive accessing ("dD") + if (this.modeCaseSensitive) { + for (let key of this.elements.keys()) { + if (key.toLowerCase() === lKey) { + this.keyMap.set(lKey, key); + break; + } } } } - return BrsBoolean.from(deleted); }, }); From d8aade1f9d57238d510383111f2accad91dc35a2 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Tue, 20 Apr 2021 01:35:58 +0300 Subject: [PATCH 11/14] resolve PR comments --- src/brsTypes/components/RoAssociativeArray.ts | 8 ++++---- test/brsTypes/components/RoAssociativeArray.test.js | 7 +++++-- test/e2e/BrsComponents.test.js | 2 ++ test/e2e/resources/components/roAssociativeArray.brs | 1 + 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index f99143385..5a6f8829b 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -17,8 +17,8 @@ export interface AAMember { export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIterable { readonly kind = ValueKind.Object; elements = new Map(); - /** keyMap - maps lowercased key to original one which used in this.elements to store value - * Main benefit of it is fast case insensitive access + /** Maps lowercased element name to original name used in this.elements. + * Main benefit of it is fast, case-insensitive access. */ keyMap = new Map(); private modeCaseSensitive: boolean = false; @@ -161,7 +161,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte if (this.keyMap.get(lKey) === key) { this.keyMap.delete(lKey); // if we have {"DD": 0, "dD": 0}, keyMap["dd"] is pointed to "DD", - // and we delets it, then we should find another value for case insensitive accessing ("dD") + // and we delete it, then we should find another value for case insensitive accessing ("dD") if (this.modeCaseSensitive) { for (let key of this.elements.keys()) { if (key.toLowerCase() === lKey) { @@ -187,7 +187,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte returns: ValueKind.Void, }, impl: (interpreter: Interpreter, key: BrsString, value: BrsType) => { - this.set(key, value, true); + this.set(key, value, /* isCaseSensitive */ true); return BrsInvalid.Instance; }, }); diff --git a/test/brsTypes/components/RoAssociativeArray.test.js b/test/brsTypes/components/RoAssociativeArray.test.js index 9f69dfc1b..ce7ed104b 100644 --- a/test/brsTypes/components/RoAssociativeArray.test.js +++ b/test/brsTypes/components/RoAssociativeArray.test.js @@ -379,6 +379,7 @@ describe("RoAssociativeArray", () => { it("changes lookups to case sensitive mode", () => { let v1 = new BrsString("value1"); let v2 = new BrsString("value2"); + let v3 = new BrsString("value3"); let aa = new RoAssociativeArray([{ name: new BrsString("key1"), value: v1 }]); @@ -395,6 +396,8 @@ describe("RoAssociativeArray", () => { // check lookupCI method let lookupCI = aa.getMethod("lookupCI"); + // RBI uses last value for key when uses lookupCI, if do not set new value it will use old v2 value + addreplace.call(interpreter, new BrsString("Key1"), v3); expect(lookupCI).toBeTruthy(); let result = lookup.call(interpreter, new BrsString("key1")); @@ -404,10 +407,10 @@ describe("RoAssociativeArray", () => { expect(result).toBe(v2); result = lookupCI.call(interpreter, new BrsString("kEy1")); - expect(result).toBe(v1); + expect(result).toBe(v3); result = lookupCI.call(interpreter, new BrsString("KeY1")); - expect(result).toBe(v1); + expect(result).toBe(v3); }); }); }); diff --git a/test/e2e/BrsComponents.test.js b/test/e2e/BrsComponents.test.js index a9eb22bff..c1afa0e70 100644 --- a/test/e2e/BrsComponents.test.js +++ b/test/e2e/BrsComponents.test.js @@ -74,6 +74,8 @@ describe("end to end brightscript functions", () => { "value1", "lookup uses mode case too", "value1", + "lookupCI ignore mode case", + "value1", "can empty itself: ", "true", "saved key: ", diff --git a/test/e2e/resources/components/roAssociativeArray.brs b/test/e2e/resources/components/roAssociativeArray.brs index 0ab9bebe8..4c6299cd5 100644 --- a/test/e2e/resources/components/roAssociativeArray.brs +++ b/test/e2e/resources/components/roAssociativeArray.brs @@ -22,6 +22,7 @@ sub main() print "key is not found if sensitive mode is enabled" aa.doesExist("key1") ' => false print "key exits with correct casing" aa["KeY1"] ' => value1 print "lookup uses mode case too" aa.lookup("KeY1") ' => value1 + print "lookupCI ignore mode case " aa.lookupCI("kEy1") ' => value1 aa.clear() print "can empty itself: " aa.isEmpty() ' => true From 53bed838bd89e7b7e00f4c354db0861aaa390c22 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Tue, 20 Apr 2021 01:40:07 +0300 Subject: [PATCH 12/14] fix UT --- test/e2e/resources/components/roAssociativeArray.brs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/resources/components/roAssociativeArray.brs b/test/e2e/resources/components/roAssociativeArray.brs index 4c6299cd5..7f716f7b6 100644 --- a/test/e2e/resources/components/roAssociativeArray.brs +++ b/test/e2e/resources/components/roAssociativeArray.brs @@ -22,7 +22,7 @@ sub main() print "key is not found if sensitive mode is enabled" aa.doesExist("key1") ' => false print "key exits with correct casing" aa["KeY1"] ' => value1 print "lookup uses mode case too" aa.lookup("KeY1") ' => value1 - print "lookupCI ignore mode case " aa.lookupCI("kEy1") ' => value1 + print "lookupCI ignore mode case" aa.lookupCI("kEy1") ' => value1 aa.clear() print "can empty itself: " aa.isEmpty() ' => true From f0a56a747d8e978f4bbd65a5e6e73d8a6f68161d Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Wed, 21 Apr 2021 14:33:01 +0300 Subject: [PATCH 13/14] refactoring keyMap to store set of keys --- src/brsTypes/components/RoAssociativeArray.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/brsTypes/components/RoAssociativeArray.ts b/src/brsTypes/components/RoAssociativeArray.ts index 5a6f8829b..c95584ea7 100644 --- a/src/brsTypes/components/RoAssociativeArray.ts +++ b/src/brsTypes/components/RoAssociativeArray.ts @@ -20,7 +20,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte /** Maps lowercased element name to original name used in this.elements. * Main benefit of it is fast, case-insensitive access. */ - keyMap = new Map(); + keyMap = new Map>(); private modeCaseSensitive: boolean = false; constructor(elements: AAMember[]) { @@ -111,11 +111,17 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte let oldKey = this.findElementKey(index.value); if (!this.modeCaseSensitive && oldKey) { this.elements.delete(oldKey); + this.keyMap.set(oldKey.toLowerCase(), new Set()); // clear key set cuz in insensitive mode we should have 1 key in set } let indexValue = isCaseSensitive ? index.value : index.value.toLowerCase(); this.elements.set(indexValue, value); - this.keyMap.set(index.value.toLowerCase(), indexValue); + + let lkey = index.value.toLowerCase(); + if (!this.keyMap.has(lkey)) { + this.keyMap.set(lkey, new Set()); + } + this.keyMap.get(lkey)?.add(indexValue); return BrsInvalid.Instance; } @@ -125,7 +131,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte if (this.modeCaseSensitive && isCaseSensitiveFind) { return elementKey; } else { - return this.keyMap.get(elementKey.toLowerCase()); + return this.keyMap.get(elementKey.toLowerCase())?.values().next().value; } } @@ -158,18 +164,14 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte let deleted = key ? this.elements.delete(key) : false; let lKey = str.value.toLowerCase(); - if (this.keyMap.get(lKey) === key) { - this.keyMap.delete(lKey); - // if we have {"DD": 0, "dD": 0}, keyMap["dd"] is pointed to "DD", - // and we delete it, then we should find another value for case insensitive accessing ("dD") - if (this.modeCaseSensitive) { - for (let key of this.elements.keys()) { - if (key.toLowerCase() === lKey) { - this.keyMap.set(lKey, key); - break; - } - } + if (this.modeCaseSensitive) { + let keySet = this.keyMap.get(lKey); + keySet?.delete(key); + if (keySet?.size === 0) { + this.keyMap.delete(lKey); } + } else { + this.keyMap.delete(lKey); } return BrsBoolean.from(deleted); }, From 020bc4a9ed79c0690a71e5aaf142ae2a1da3f930 Mon Sep 17 00:00:00 2001 From: Vasyl Martynych Date: Wed, 21 Apr 2021 14:39:24 +0300 Subject: [PATCH 14/14] fix AA UT --- test/brsTypes/components/RoAssociativeArray.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/brsTypes/components/RoAssociativeArray.test.js b/test/brsTypes/components/RoAssociativeArray.test.js index ce7ed104b..5f79118d1 100644 --- a/test/brsTypes/components/RoAssociativeArray.test.js +++ b/test/brsTypes/components/RoAssociativeArray.test.js @@ -396,7 +396,7 @@ describe("RoAssociativeArray", () => { // check lookupCI method let lookupCI = aa.getMethod("lookupCI"); - // RBI uses last value for key when uses lookupCI, if do not set new value it will use old v2 value + addreplace.call(interpreter, new BrsString("Key1"), v3); expect(lookupCI).toBeTruthy(); @@ -407,10 +407,10 @@ describe("RoAssociativeArray", () => { expect(result).toBe(v2); result = lookupCI.call(interpreter, new BrsString("kEy1")); - expect(result).toBe(v3); + expect(result).toBe(v1); result = lookupCI.call(interpreter, new BrsString("KeY1")); - expect(result).toBe(v3); + expect(result).toBe(v1); }); }); });