Repository: sjbarag/brs (js)

Issue:
AppendString() not working
The following function generates a 22 digits number on Roku native brightscript:
```
function generateSessionId()
    MAX_SIZE = 22
    index = 0
    sessionId = box("")
    while index <= MAX_SIZE
        index = index + 1
        sessionId.appendString(Rnd(9).toStr(), 1)
    end while
    return sessionId
end function
```

On **brs**, it generates an empty string.

NOTE: The `box()`function is not available but I'm using a mock that just returns the same value passed as argument. 
See [Box(x as Dynamic) as Object](https://developer.roku.com/en-gb/docs/references/brightscript/language/runtime-functions.md#boxx-as-dynamic-as-object)

Pull request:
fix(rsg): Adds optional arg to RoString constructor
I found that when creating an string like this:
```
str = createObject("roString")
```
The intrinsic string doesn't have initial value because we were always expecting a second argument in the `createObject` syntax, then when we tried to use methods like `appendString` to the object they won't work because there is no initial string.
I made optional the argument in the `RoString` class constructor so it doesn't break the string boxing functionality but at the same time makes things consistent with the Roku implementation of `roString` which doesn't take a second argument.

Thanks to @adheus who find the bug

Fixes #513 

Patch:
diff --git a/src/brsTypes/components/BrsObjects.ts b/src/brsTypes/components/BrsObjects.ts
index 33001ef..495f5d2 100644
--- a/src/brsTypes/components/BrsObjects.ts
+++ b/src/brsTypes/components/BrsObjects.ts
@@ -32,7 +32,7 @@ export const BrsObjects = new Map<string, Function>([
         "roregex",
         (_: Interpreter, expression: BrsString, flags: BrsString) => new RoRegex(expression, flags),
     ],
-    ["rostring", (_: Interpreter, literal: BrsString) => new RoString(literal)],
+    ["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)],
diff --git a/src/brsTypes/components/RoString.ts b/src/brsTypes/components/RoString.ts
index 4cedcd9..e537c04 100644
--- a/src/brsTypes/components/RoString.ts
+++ b/src/brsTypes/components/RoString.ts
@@ -10,16 +10,19 @@ import { Float } from "../Float";
 
 export class RoString extends BrsComponent implements BrsValue, Comparable, Unboxable {
     readonly kind = ValueKind.Object;
-    private intrinsic: BrsString;
+    private intrinsic: BrsString = new BrsString("");
 
     public getValue(): string {
         return this.intrinsic.value;
     }
 
-    constructor(initialValue: BrsString) {
+    constructor(initialValue?: BrsString) {
         super("roString");
 
-        this.intrinsic = initialValue;
+        if (initialValue) {
+            this.intrinsic = initialValue;
+        }
+
         this.registerMethods({
             ifString: [this.setString, this.getString],
             ifStringOps: [
@@ -34,6 +37,7 @@ export class RoString extends BrsComponent implements BrsValue, Comparable, Unbo
                 this.toInt,
                 this.toFloat,
                 this.tokenize,
+                this.setString,
                 this.split,
                 this.getEntityEncode,
                 this.escape,
@@ -91,21 +95,36 @@ export class RoString extends BrsComponent implements BrsValue, Comparable, Unbo
         return this.intrinsic.toString();
     }
 
-    // ---------- ifStringOps ----------
-    /** Sets the string to the first len characters of s. */
-    private setString = new Callable("SetString", {
-        signature: {
-            args: [
-                new StdlibArgument("s", ValueKind.String),
-                new StdlibArgument("len", ValueKind.Int32),
-            ],
-            returns: ValueKind.Void,
-        },
-        impl: (_interpreter, s: BrsString, len: Int32) => {
-            this.intrinsic = new BrsString(s.value.substr(0, len.getValue()));
-            return BrsInvalid.Instance;
+    /**
+     * Sets the string to the first len characters of s.
+     * Note: this method is implemented in the ifString and ifStringOps interfaces
+     */
+    private setString = new Callable(
+        "SetString",
+        {
+            signature: {
+                args: [new StdlibArgument("s", ValueKind.String)],
+                returns: ValueKind.Void,
+            },
+            impl: (_interpreter, s: BrsString) => {
+                this.intrinsic = new BrsString(s.value);
+                return BrsInvalid.Instance;
+            },
         },
-    });
+        {
+            signature: {
+                args: [
+                    new StdlibArgument("s", ValueKind.String),
+                    new StdlibArgument("len", ValueKind.Int32),
+                ],
+                returns: ValueKind.Void,
+            },
+            impl: (_interpreter, s: BrsString, len: Int32) => {
+                this.intrinsic = new BrsString(s.value.substr(0, len.getValue()));
+                return BrsInvalid.Instance;
+            },
+        }
+    );
 
     private getString = new Callable("GetString", {
         signature: {
@@ -115,6 +134,7 @@ export class RoString extends BrsComponent implements BrsValue, Comparable, Unbo
         impl: (_interpreter) => this.intrinsic,
     });
 
+    // ---------- ifStringOps ----------
     /** Appends the first len characters of s to the end of the string. */
     private appendString = new Callable("AppendString", {
         signature: {


Test patch:
diff --git a/test/brsTypes/components/RoString.test.js b/test/brsTypes/components/RoString.test.js
index a315687..c3314db 100644
--- a/test/brsTypes/components/RoString.test.js
+++ b/test/brsTypes/components/RoString.test.js
@@ -3,6 +3,13 @@ const { Int32, Float, BrsString, RoString, RoArray, BrsBoolean, Callable } = brs
 const { Interpreter } = require("../../../lib/interpreter");
 
 describe("RoString", () => {
+    describe("constructor", () => {
+        it("starts with empty string when no arg is passed to constructor", () => {
+            let a = new RoString();
+            expect(a.equalTo(new BrsString(""))).toBe(BrsBoolean.True);
+        });
+    });
+
     describe("equality", () => {
         it("compares to intrinsic strings", () => {
             let a = new RoString(new BrsString("foo"));
@@ -43,19 +50,15 @@ describe("RoString", () => {
                 expect(setString).toBeInstanceOf(Callable);
             });
 
-            it("overwrites its stored string", () => {
-                setString.call(interpreter, new BrsString("after"), new Int32(2));
-                expect(s.intrinsic).toEqual(new BrsString("af"));
+            it("sets a string into the object", () => {
+                setString.call(interpreter, new BrsString("hello"));
+                expect(s.intrinsic).toEqual(new BrsString("hello"));
             });
 
-            it("overwrites an empty string for zero `len`", () => {
-                setString.call(interpreter, new BrsString("after"), new Int32(0));
-                expect(s.intrinsic).toEqual(new BrsString(""));
-            });
-
-            it("overwrites an empty string for negative `len`", () => {
-                setString.call(interpreter, new BrsString("after"), new Int32(-1));
-                expect(s.intrinsic).toEqual(new BrsString(""));
+            it("overwrites string value previously set", () => {
+                setString.call(interpreter, new BrsString("hello"));
+                setString.call(interpreter, new BrsString("world"));
+                expect(s.intrinsic).toEqual(new BrsString("world"));
             });
         });
 
@@ -412,6 +415,31 @@ describe("RoString", () => {
             it.todo("splits by multi-character delimiter");
         });
 
+        describe("seString", () => {
+            let s, setString;
+
+            beforeEach(() => {
+                s = new RoString(new BrsString("before"));
+                setString = s.getMethod("setString");
+                expect(setString).toBeInstanceOf(Callable);
+            });
+
+            it("overwrites its stored string", () => {
+                setString.call(interpreter, new BrsString("after"), new Int32(2));
+                expect(s.intrinsic).toEqual(new BrsString("af"));
+            });
+
+            it("overwrites an empty string for zero `len`", () => {
+                setString.call(interpreter, new BrsString("after"), new Int32(0));
+                expect(s.intrinsic).toEqual(new BrsString(""));
+            });
+
+            it("overwrites an empty string for negative `len`", () => {
+                setString.call(interpreter, new BrsString("after"), new Int32(-1));
+                expect(s.intrinsic).toEqual(new BrsString(""));
+            });
+        });
+
         describe("split", () => {
             let split;
 
diff --git a/test/e2e/BrsComponents.test.js b/test/e2e/BrsComponents.test.js
index 615d389..359eaab 100644
--- a/test/e2e/BrsComponents.test.js
+++ b/test/e2e/BrsComponents.test.js
@@ -296,8 +296,10 @@ describe("end to end brightscript functions", () => {
 
         expect(allArgs(outputStreams.stderr.write)).toEqual([]);
         expect(allArgs(outputStreams.stdout.write).filter((arg) => arg !== "\n")).toEqual([
+            "hello",
             "bar",
             "bar",
+            "foo",
             "true", // comparison
             "5", // length
             "b", // split("/")[1]
diff --git a/test/e2e/resources/components/roString.brs b/test/e2e/resources/components/roString.brs
index 2b06063..0f4c98b 100644
--- a/test/e2e/resources/components/roString.brs
+++ b/test/e2e/resources/components/roString.brs
@@ -1,12 +1,18 @@
 sub main()
     ' direct creation
-    r = createObject("RoString", "foo")
+    r = createObject("RoString")
+
+    r.appendString("hello", 5) ' appends hello to the default empty string
+    print r.getString() ' => "hello"
 
     s = "bar"
     print s.getString() ' => "bar"
     print s.toStr() ' => "bar" (again)
 
-    r.setString("boo!", 1)
+    r.setString("foo") ' setString in the ifString interface
+    print r.getString() ' => "foo"
+
+    r.setString("boo!", 1) ' setString in the ifStringOps interface, replaces old value
     r.appendString("ar", 10)
 
     ' comparisons