Merge pull request #631 from GuillaumeGomez/concat-rules

Handle precedence of expression operators depending on operators and not depending on the global kind of the expression

Author: GuillaumeGomez

goml-script.md

@@ -66,11 +66,16 @@ store-value: (effect, ":hover")
 set-text: ("element" + |variable|, "something " + 2 + " something else")
 ```
 
-Rules of concatenation are simple: if any of the element is a string, then it'll concatenate as a string. Examples:
+Operations are handled from the left to the right. Operations between parens are performed first:
 
 ```ignore
-1 + 2 + "a" // gives string "12a"
-1 + 2 // gives number 3
+1 + 2 + "a" + (4 * 3)
+// first the parens
+1 + 2 + "a" + 12
+// then from left to right
+3 + "a" + 12
+"3a" + 12
+"3a12"
 ```
 
 This is just a sub-part of expressions which allow more things.

src/ast.js

@@ -35,15 +35,14 @@ function replaceVariable(elem, variables, functionArgs, forceVariableAsString, e
     if (associatedValue instanceof Element) {
         // Nothing to be done in here.
         return associatedValue;
-    } else if (['number', 'string', 'boolean'].includes(typeof associatedValue)) {
-        if (typeof associatedValue === 'boolean') {
-            return new IdentElement(
-                associatedValue.toString(), startPos, endPos, lineNumber);
-        } else if (typeof associatedValue === 'number' ||
-            // eslint-disable-next-line no-extra-parens
-            (!forceVariableAsString && matchInteger(associatedValue) === true)) {
-            return new NumberElement(associatedValue, startPos, endPos, lineNumber);
-        }
+    } else if (typeof associatedValue === 'boolean') {
+        return new IdentElement(
+            associatedValue.toString(), startPos, endPos, lineNumber);
+    } else if (typeof associatedValue === 'number' ||
+        // eslint-disable-next-line no-extra-parens
+        (!forceVariableAsString && matchInteger(associatedValue) === true)) {
+        return new NumberElement(associatedValue, startPos, endPos, lineNumber);
+    } else if (typeof associatedValue === 'string') {
         return new StringElement(
             associatedValue,
             startPos,

src/commands/navigation.js

@@ -1,6 +1,5 @@
 // Commands changing the current location or reloading the page.
 
-const { cleanString } = require('../parser.js');
 const { hasError } = require('../utils.js');
 const { validator } = require('../validator.js');
 
@@ -22,7 +21,8 @@ function parseGoTo(parser) {
         return ret;
     }
 
-    const path = ret.value.value.trim();
+    const path = ret.value.getStringValue();
+    const code = ret.value.displayInCode();
 
     let goto_arg;
     const permissions = 'await arg.browser.overridePermissions(page.url(), arg.permissions);';
@@ -32,11 +32,11 @@ function parseGoTo(parser) {
         || path.startsWith('www.') === true
         || path.startsWith('file://') === true
     ) {
-        goto_arg = `"${cleanString(path)}"`;
+        goto_arg = code;
     } else if (path.startsWith('.')) {
-        goto_arg = `page.url().split("/").slice(0, -1).join("/") + "/${cleanString(path)}"`;
+        goto_arg = `page.url().split("/").slice(0, -1).join("/") + "/" + ${code}`;
     } else if (path.startsWith('/')) {
-        goto_arg = `page.url().split("/").slice(0, -1).join("/") + "${cleanString(path)}"`;
+        goto_arg = `page.url().split("/").slice(0, -1).join("/") + ${code}`;
     } else {
         return {'error': `a relative path or a full URL was expected, found \`${path}\``};
     }

src/parser.js

@@ -21,7 +21,7 @@ function isLetter(c) {
 function matchInteger(s) {
     if (typeof s === 'number' || s instanceof Number) {
         return true;
-    } else if (typeof s === 'string' || s instanceof String) {
+    } else if ((typeof s === 'string' || s instanceof String) && s.length !== 0) {
         for (let i = s.length - 1; i >= 0; --i) {
             if (!isNumber(s.charAt(i))) {
                 return false;
@@ -158,39 +158,6 @@ function isArrayElementCompatible(expected, elem) {
     return expected.kind === elem.kind;
 }
 
-// Used to concatenate all elements into a string, like `1 + "a" + 12` -> "1a12".
-function concatExprAsString(elems) {
-    let out = '';
-    for (const elem of elems) {
-        if (elem.kind === 'operator') {
-            continue;
-        } else if (['number', 'string', 'boolean'].includes(elem.kind)) {
-            out += elem.value;
-        } else {
-            out += concatExprAsString(elem.value);
-        }
-    }
-    return out;
-}
-
-function concatExprAsObjectPath(elems) {
-    let s = '';
-    const parts = [];
-
-    for (const elem of elems) {
-        if (elem.kind === 'operator') {
-            continue;
-        } else if (elem.kind !== 'object-path') {
-            s += concatExprAsString([elem]);
-        } else {
-            elem.value[0].value = s + elem.value[0].value;
-            parts.push(...elem.value);
-            s = '';
-        }
-    }
-    return parts;
-}
-
 // This function is used when generating an expression interpreted as a boolean.
 function concatExprAsExpr(elems) {
     const out = [];
@@ -261,24 +228,19 @@ function canBeCompared(kind1, kind2) {
 }
 
 function convertAsString(elem) {
-    if (elem.value.some(v => v.kind === 'object-path')) {
-        return new ObjectPathElement(
-            concatExprAsObjectPath(elem.value),
-            elem.startPos,
-            elem.endPos,
-            elem.fullText,
-            elem.line,
-            elem.error,
-        );
-    }
-    return new StringElement(
-        concatExprAsString(elem.value),
-        elem.startPos,
-        elem.endPos,
-        elem.fullText,
-        elem.line,
-        elem.error,
-    );
+    const pos = elem.value.findIndex(v => v.kind === 'object-path');
+    elem.kind = 'string';
+    if (pos !== -1) {
+        // We remove the object-path from the expression.
+        const objPath = elem.value.pop();
+        // We remove the first item of the object path and push it at the end of the expression.
+        elem.value.push(...objPath.value.splice(0, 1));
+        // We put the expression as the first element of the object path.
+        objPath.value.splice(0, 0, elem);
+        // All done!
+        return objPath;
+    }
+    return elem;
 }
 
 function convertExprAs(elem, convertAs) {
@@ -363,7 +325,8 @@ class Element {
     }
 
     isRecursive() {
-        // Only Tuple and JSON elements are "recursive" (meaning they can contain sub-levels).
+        // Expression, Array, Tuple and JSON elements are "recursive" (meaning they can contain
+        // sub-levels).
         return false;
     }
 
@@ -433,6 +396,18 @@ class ExpressionElement extends Element {
         super('expression', value, startPos, endPos, fullText, line, error);
     }
 
+    isRecursive() {
+        return true;
+    }
+
+    displayInCode() {
+        return this.value.map(v => v.displayInCode()).join('');
+    }
+
+    getStringValue() {
+        return this.value.map(v => v.getStringValue()).join('');
+    }
+
     clone() {
         const elems = this.value.map(elem => elem.clone());
         return new this.constructor(
@@ -537,8 +512,8 @@ class ObjectPathElement extends Element {
         super('object-path', value, startPos, endPos, fullText, line, error);
     }
 
-    getStringValue(trim, clean = true) {
-        const content = this.value.map(v => `"${v.getStringValue(clean)}"`).join(',');
+    getStringValue() {
+        const content = this.value.map(v => `${v.displayInCode()}`).join(',');
         return `[${content}]`;
     }
 

tests/api-output/parseGoTo/basic-4.toml

@@ -1,5 +1,5 @@
 instructions = [
-  """const url = page.url().split(\"/\").slice(0, -1).join(\"/\") + \"/./a\";
+  """const url = page.url().split(\"/\").slice(0, -1).join(\"/\") + \"/\" + \"./a\";
 try {
     await page.goto(url);
 } catch(exc) {

tests/api-output/parseGoTo/var-1.toml

@@ -1,5 +1,5 @@
 instructions = [
-  """const url = \"file://foo/a\";
+  """const url = \"file://\"+\"foo\"+\"/a\";
 try {
     await page.goto(url);
 } catch(exc) {

tests/api-output/parseGoTo/var-3.toml

@@ -1,5 +1,5 @@
 instructions = [
-  """const url = \"http://foo/tadam/fa\";
+  """const url = \"http://foo/\"+\"tadam/\"+\"fa\";
 try {
     await page.goto(url);
 } catch(exc) {

tests/api-output/parseGoTo/var-4.toml

@@ -1,5 +1,5 @@
 instructions = [
-  """const url = \"http://foo/tadam/fa\";
+  """const url = \"http://foo/\"+\"tadam\"+\"/fa\";
 try {
     await page.goto(url);
 } catch(exc) {

tests/ui/assert-expr.goml

@@ -18,6 +18,8 @@ assert: ["1", "2"] != ["1", "2"]
 store-value: (var, (1 + 2) * 4 + 1)
 assert: |var| == 13
 assert: |var| != 12 && 1 < 2
+assert: 1 + 2 + "a" == "3a"
+assert: 1 + 2 + "a" + (4 * 3) == "3a12"
 
 // Should fail.
 assert: |var| != 13

tests/ui/assert-expr.output

@@ -2,12 +2,12 @@
 
 assert-expr... FAILED
 [ERROR] `tests/ui/assert-expr.goml` line 15: Condition `!compareArrayLike(["1", "2"], ["1", "2"])` was evaluated as false: for command `assert: ["1", "2"] != ["1", "2"]`
-[ERROR] `tests/ui/assert-expr.goml` line 23: Condition `13 != 13` was evaluated as false: for command `assert: |var| != 13`
-[ERROR] `tests/ui/assert-expr.goml` line 24: assert didn't fail: for command `assert-false: |var| == 13`
-[ERROR] `tests/ui/assert-expr.goml` line 25: Condition `"a" == 13` was evaluated as false: for command `assert: |var2| == |var|`
-[ERROR] `tests/ui/assert-expr.goml` line 26: Condition `compareJson({"b": 3}, {"a": 2})` was evaluated as false: for command `assert: {"b": 3} == |var3|`
-[ERROR] `tests/ui/assert-expr.goml` line 27: Condition `compareArrayLike([1, "a"], [2, 3])` was evaluated as false: for command `assert: (1, "a") == (2, 3)`
-[ERROR] `tests/ui/assert-expr.goml` line 28: Condition `compareArrayLike([1, 2], ["a", "b"])` was evaluated as false: for command `assert: [1, 2] == ["a", "b"]`
+[ERROR] `tests/ui/assert-expr.goml` line 25: Condition `13 != 13` was evaluated as false: for command `assert: |var| != 13`
+[ERROR] `tests/ui/assert-expr.goml` line 26: assert didn't fail: for command `assert-false: |var| == 13`
+[ERROR] `tests/ui/assert-expr.goml` line 27: Condition `"a" == 13` was evaluated as false: for command `assert: |var2| == |var|`
+[ERROR] `tests/ui/assert-expr.goml` line 28: Condition `compareJson({"b": 3}, {"a": 2})` was evaluated as false: for command `assert: {"b": 3} == |var3|`
+[ERROR] `tests/ui/assert-expr.goml` line 29: Condition `compareArrayLike([1, "a"], [2, 3])` was evaluated as false: for command `assert: (1, "a") == (2, 3)`
+[ERROR] `tests/ui/assert-expr.goml` line 30: Condition `compareArrayLike([1, 2], ["a", "b"])` was evaluated as false: for command `assert: [1, 2] == ["a", "b"]`
 
 
 <= doc-ui tests done: 0 succeeded, 1 failed