Skip to content

Commit

Permalink
feat(stdlib): Add lookupCI for assocarray and Node (#639)
Browse files Browse the repository at this point in the history
resolves #629
Co-authored-by: Vasyl Martynych <[email protected]>
  • Loading branch information
Vasya-M authored Apr 30, 2021
1 parent 545df78 commit 08baf5e
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 24 deletions.
5 changes: 3 additions & 2 deletions src/brsTypes/components/BrsComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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): BrsType;
get(index: BrsType, isCaseSensitive?: boolean): BrsType;

set(index: BrsType, value: BrsType): BrsInvalid;
set(index: BrsType, value: BrsType, isCaseSensitive?: boolean): BrsInvalid;
}
94 changes: 78 additions & 16 deletions src/brsTypes/components/RoAssociativeArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ export interface AAMember {
export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIterable {
readonly kind = ValueKind.Object;
elements = new Map<string, BrsType>();
/** Maps lowercased element name to original name used in this.elements.
* Main benefit of it is fast, case-insensitive access.
*/
keyMap = new Map<string, Set<string>>();
private modeCaseSensitive: boolean = false;

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: [
Expand All @@ -36,6 +38,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte
this.keys,
this.items,
this.lookup,
this.lookupCI,
this.setmodecasesensitive,
],
ifEnum: [this.isEmpty],
Expand Down Expand Up @@ -77,7 +80,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte
.map((value: BrsType) => value);
}

get(index: BrsType) {
get(index: BrsType, isCaseSensitive = false) {
if (index.kind !== ValueKind.String) {
throw new Error("Associative array indexes must be strings");
}
Expand All @@ -93,19 +96,50 @@ 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();
return this.elements.get(indexValue) || this.getMethod(index.value) || BrsInvalid.Instance;
return (
this.findElement(index.value, isCaseSensitive) ||
this.getMethod(index.value) ||
BrsInvalid.Instance
);
}

set(index: BrsType, value: BrsType) {
set(index: BrsType, value: BrsType, isCaseSensitive = 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);
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);

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;
}

/** if AA is in insensitive mode, it means that we should do insensitive search of real key */
private findElementKey(elementKey: string, isCaseSensitiveFind = false) {
if (this.modeCaseSensitive && isCaseSensitiveFind) {
return elementKey;
} else {
return this.keyMap.get(elementKey.toLowerCase())?.values().next().value;
}
}

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", {
signature: {
Expand All @@ -114,6 +148,7 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte
},
impl: (interpreter: Interpreter) => {
this.elements.clear();
this.keyMap.clear();
return BrsInvalid.Instance;
},
});
Expand All @@ -125,7 +160,19 @@ 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;

let lKey = str.value.toLowerCase();
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);
},
});
Expand All @@ -142,7 +189,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, /* isCaseSensitive */ true);
return BrsInvalid.Instance;
},
});
Expand All @@ -166,7 +213,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;
},
});

Expand All @@ -182,7 +230,9 @@ export class RoAssociativeArray extends BrsComponent implements BrsValue, BrsIte
return BrsInvalid.Instance;
}

this.elements = new Map<string, BrsType>([...this.elements, ...obj.elements]);
obj.elements.forEach((value, key) => {
this.set(new BrsString(key), value, true);
});

return BrsInvalid.Instance;
},
Expand Down Expand Up @@ -223,15 +273,27 @@ 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)],
returns: ValueKind.Dynamic,
},
impl: (interpreter: Interpreter, key: BrsString) => {
let lKey = key.value;
return this.get(new BrsString(lKey));
return this.get(key, 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)],
returns: ValueKind.Dynamic,
},
impl: (interpreter: Interpreter, key: BrsString) => {
return this.get(key);
},
});

Expand All @@ -242,7 +304,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;
},
});
Expand Down
6 changes: 5 additions & 1 deletion src/brsTypes/components/RoSGNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
Expand All @@ -327,6 +327,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable {
this.keys,
this.items,
this.lookup,
this.lookupCI,
],
ifSGNodeField: [
this.addfield,
Expand Down Expand Up @@ -801,6 +802,9 @@ 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. */
private addfield = new Callable("addfield", {
signature: {
Expand Down
4 changes: 2 additions & 2 deletions src/interpreter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ export class Interpreter implements Expr.Visitor<BrsType>, Stmt.Visitor<BrsType>
}

try {
return source.get(index);
return source.get(index, true);
} catch (err) {
return this.addError(new BrsError(err.message, expression.closingSquare.location));
}
Expand Down Expand Up @@ -1537,7 +1537,7 @@ export class Interpreter implements Expr.Visitor<BrsType>, Stmt.Visitor<BrsType>
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));
}
Expand Down
39 changes: 39 additions & 0 deletions test/brsTypes/components/RoAssociativeArray.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,5 +374,44 @@ describe("RoAssociativeArray", () => {
expect(result2).toBe(v2);
});
});

describe("lookupCI", () => {
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 }]);

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");

addreplace.call(interpreter, new BrsString("Key1"), v3);
expect(lookupCI).toBeTruthy();

let result = lookup.call(interpreter, new BrsString("key1"));
expect(result).toBe(v1);

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("KeY1"));
expect(result).toBe(v1);
});
});
});
});
2 changes: 1 addition & 1 deletion test/brsTypes/components/RoSGNode.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/BrsComponents.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: ",
Expand All @@ -72,8 +74,18 @@ describe("end to end brightscript functions", () => {
"value1",
"lookup uses mode case too",
"value1",
"lookupCI ignore mode case",
"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",
]);
});

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/Iterables.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe("end to end iterables", () => {
// iterate through keys
"has-second-layer",
"level",
"secondlayer",
"secondLayer",

// twoDimensional.secondLayer.level
"2",
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/RoSGNode.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: ",
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/Syntax.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,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",
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/resources/components/roAssociativeArray.brs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,7 +22,17 @@ 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

' 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
1 change: 1 addition & 0 deletions test/e2e/resources/components/roSGNode/roSGNode.brs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 08baf5e

Please sign in to comment.