Merge pull request #571 from GuillaumeGomez/improve-generated-code

Improve generated code

src/commands/compare.js

@@ -69,7 +69,6 @@ function parseCompareElementsTextFalse(parser) {
     return parseCompareElementsTextInner(parser, true);
 }
 
-// * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2, ["attr"])
 function parseCompareElementsAttributeInner(parser, assertFalse) {
     const operators = ['<', '<=', '>', '>=', '='];
     const ret = validator(parser,
@@ -160,119 +159,89 @@ ${indentString(comparison, 1)}
 // Possible inputs:
 //
 // * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2, ["attr"])
+// * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2, ["attr"], "operator")
 function parseCompareElementsAttribute(parser) {
     return parseCompareElementsAttributeInner(parser, false);
 }
 
 // Possible inputs:
 //
 // * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2", ["attr"])
+// * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2, ["attr"], "operator")
 function parseCompareElementsAttributeFalse(parser) {
     return parseCompareElementsAttributeInner(parser, true);
 }
 
+// * ("CSS selector 1" | "XPath 1", "CSS selector 2" | "XPath 2, ["CSS properties"])
 function parseCompareElementsCssInner(parser, assertFalse) {
-    const elems = parser.elems;
-
-    if (elems.length === 0) {
-        return {'error': 'expected a tuple, found nothing'};
-    } else if (elems.length !== 1 || elems[0].kind !== 'tuple') {
-        return {
-            'error': `expected a tuple, found \`${parser.getRawArgs()}\``,
-        };
-    }
-    const tuple = elems[0].getRaw();
-    if (tuple.length !== 3) {
-        let err = `expected 3 elements in the tuple, found ${tuple.length} element`;
-        if (tuple.length > 1) {
-            err += 's';
-        }
-        return {'error': err};
-    } else if (tuple[0].kind !== 'string') {
-        return {
-            'error': 'expected first argument to be a CSS selector or an XPath, ' +
-                `found ${tuple[0].getArticleKind()}`,
-        };
-    } else if (tuple[1].kind !== 'string') {
-        return {
-            'error': 'expected second argument to be a CSS selector or an XPath, ' +
-                `found ${tuple[1].getArticleKind()}`,
-        };
-    } else if (tuple[2].kind !== 'array') {
-        return {
-            'error': 'expected third argument to be an array of string, ' +
-                `found ${tuple[2].getArticleKind()}`,
-        };
-    }
-    const array = tuple[2].getRaw();
-    if (array.length > 0 && array[0].kind !== 'string') {
-        return {'error': `expected an array of strings, found \`${tuple[2].getErrorText()}\``};
+    const ret = validator(parser,
+        {
+            kind: 'tuple',
+            elements: [
+                { kind: 'selector' },
+                { kind: 'selector' },
+                {
+                    kind: 'array',
+                    allowEmpty: false,
+                    valueTypes: {
+                        'string': [],
+                    },
+                },
+            ],
+        },
+    );
+    if (hasError(ret)) {
+        return ret;
     }
 
-    const [insertBefore, insertAfter] = getInsertStrings(assertFalse, true);
+    const tuple = ret.value.entries;
+    const selector1 = tuple[0].value;
+    const selector2 = tuple[1].value;
+    const properties = tuple[2].value.entries.map(e => `"${e.getStringValue()}"`).join(',');
+    const needColorCheck = tuple[2].value.entries.some(e => e.getStringValue() === 'color');
 
-    const selector1 = tuple[0].getSelector();
-    if (selector1.error !== undefined) {
-        return selector1;
-    }
+    const [insertBefore, insertAfter] = getInsertStrings(assertFalse, true);
     const pseudo1 = !selector1.isXPath && selector1.pseudo !== null ?
         `, "${selector1.pseudo}"` : '';
-
-    const selector2 = tuple[1].getSelector();
-    if (selector2.error !== undefined) {
-        return selector2;
-    }
     const pseudo2 = !selector2.isXPath && selector2.pseudo !== null ?
         `, "${selector2.pseudo}"` : '';
 
     const varName = 'parseCompareElementsCss';
     const selectors = getAndSetElements(selector1, varName + '1', false) + '\n' +
         getAndSetElements(selector2, varName + '2', false) + '\n';
 
-    let arr = '';
-    let needColorCheck = false;
-    for (const entry of array) {
-        const key = entry.getStringValue();
-        if (key.length === 0) {
-            return {
-                'error': 'Empty CSS property keys ("" or \'\') are not allowed',
-            };
-        }
-        if (arr.length > 0) {
-            arr += ',';
-        }
-        if (key === 'color') {
-            needColorCheck = true;
-        }
-        arr += `"${key}"`;
-    }
-
-    const code = `const properties = [${arr}];\n` +
-    'for (const css_property of properties) {\n' +
-        `${insertBefore}let style1_1 = e1.style[css_property];\n` +
-            'let style1_2 = computed_style1[css_property];\n' +
-            'let style2_1 = e2.style[css_property];\n' +
-            'let style2_2 = computed_style2[css_property];\n' +
-            'if (style1_1 != style2_1 && style1_1 != style2_2 && ' +
-            'style1_2 != style2_1 && style1_2 != style2_2) {\n' +
-            'throw \'CSS property `\' + css_property + \'` did not match: \' + ' +
-            `style1_2 + ' != ' + style2_2; }${insertAfter}\n` +
-    '}\n';
+    const code = `\
+const properties = [${properties}];
+for (const css_prop of properties) {
+    ${insertBefore}let style1_1 = e1.style[css_prop];
+    let style1_2 = computed_style1[css_prop];
+    let style2_1 = e2.style[css_prop];
+    let style2_2 = computed_style2[css_prop];
+    if (style1_1 != style2_1 && style1_1 != style2_2 &&
+        style1_2 != style2_1 && style1_2 != style2_2)
+    {
+        throw 'CSS property \`' + css_prop + '\` did not match: ' + style1_2 + ' != ' + style2_2;
+    }${insertAfter}
+}
+`;
 
     const instructions = [];
     if (needColorCheck) {
-        instructions.push('if (!arg.showText) {\n' +
-            `throw "${COLOR_CHECK_ERROR}";\n` +
-            '}',
+        instructions.push(
+            `\
+if (!arg.showText) {
+    throw "${COLOR_CHECK_ERROR}";
+}`,
         );
     }
     instructions.push(
-        selectors +
-        'await page.evaluate((e1, e2) => {' +
-        `let computed_style1 = getComputedStyle(e1${pseudo1});\n` +
-        `let computed_style2 = getComputedStyle(e2${pseudo2});\n` +
-        code +
-        `}, ${varName}1, ${varName}2);`,
+        `\
+${selectors}
+await page.evaluate((e1, e2) => {
+    let computed_style1 = getComputedStyle(e1${pseudo1});
+    let computed_style2 = getComputedStyle(e2${pseudo2});
+${indentString(code, 1)}
+}, ${varName}1, ${varName}2);`,
     );
     return {
         'instructions': instructions,

src/validator.js

@@ -272,6 +272,9 @@ function validateArray(parser, allowedSyntax, validator) {
     if (arrayElems.length === 0) {
         return parser;
     }
+    // Allowed by default and optional to specify.
+    const allowEmpty = allowedSyntax.allowEmpty !== false;
+
     const allowedForType = getObjectValue(allowedSyntax.valueTypes, arrayElems[0].kind);
     if (allowedForType === undefined) {
         const exp = Object.keys(allowedSyntax.valueTypes).join(' or ');
@@ -293,6 +296,15 @@ ${listValues(allowedForType)}`,
             }
         }
     }
+    if (!allowEmpty) {
+        for (const value of arrayElems) {
+            if (value.getStringValue().length === 0) {
+                return validator.makeError(
+                    `empty values (\`${value.getErrorText()}\`) are not allowed`,
+                );
+            }
+        }
+    }
     return parser;
 }
 

tests/test-js/api-output/parseCompareElementsCss/basic-1.toml

@@ -3,17 +3,23 @@ instructions = [
 if (parseCompareElementsCss1 === null) { throw '\"a\" not found'; }
 let parseCompareElementsCss2 = await page.$(\"b\");
 if (parseCompareElementsCss2 === null) { throw '\"b\" not found'; }
-await page.evaluate((e1, e2) => {let computed_style1 = getComputedStyle(e1);
-let computed_style2 = getComputedStyle(e2);
-const properties = [\"margin\"];
-for (const css_property of properties) {
-let style1_1 = e1.style[css_property];
-let style1_2 = computed_style1[css_property];
-let style2_1 = e2.style[css_property];
-let style2_2 = computed_style2[css_property];
-if (style1_1 != style2_1 && style1_1 != style2_2 && style1_2 != style2_1 && style1_2 != style2_2) {
-throw 'CSS property `' + css_property + '` did not match: ' + style1_2 + ' != ' + style2_2; }
-}
+
+await page.evaluate((e1, e2) => {
+    let computed_style1 = getComputedStyle(e1);
+    let computed_style2 = getComputedStyle(e2);
+    const properties = [\"margin\"];
+    for (const css_prop of properties) {
+        let style1_1 = e1.style[css_prop];
+        let style1_2 = computed_style1[css_prop];
+        let style2_1 = e2.style[css_prop];
+        let style2_2 = computed_style2[css_prop];
+        if (style1_1 != style2_1 && style1_1 != style2_2 &&
+            style1_2 != style2_1 && style1_2 != style2_2)
+        {
+            throw 'CSS property `' + css_prop + '` did not match: ' + style1_2 + ' != ' + style2_2;
+        }
+    }
+
 }, parseCompareElementsCss1, parseCompareElementsCss2);""",
 ]
 wait = false

tests/test-js/api-output/parseCompareElementsCss/color-1.toml

@@ -1,22 +1,28 @@
 instructions = [
   """if (!arg.showText) {
-throw \"`show-text: true` needs to be used before checking for `color` (otherwise the browser doesn't compute it)\";
+    throw \"`show-text: true` needs to be used before checking for `color` (otherwise the browser doesn't compute it)\";
 }""",
   """let parseCompareElementsCss1 = await page.$(\"a\");
 if (parseCompareElementsCss1 === null) { throw '\"a\" not found'; }
 let parseCompareElementsCss2 = await page.$(\"b\");
 if (parseCompareElementsCss2 === null) { throw '\"b\" not found'; }
-await page.evaluate((e1, e2) => {let computed_style1 = getComputedStyle(e1);
-let computed_style2 = getComputedStyle(e2);
-const properties = [\"color\"];
-for (const css_property of properties) {
-let style1_1 = e1.style[css_property];
-let style1_2 = computed_style1[css_property];
-let style2_1 = e2.style[css_property];
-let style2_2 = computed_style2[css_property];
-if (style1_1 != style2_1 && style1_1 != style2_2 && style1_2 != style2_1 && style1_2 != style2_2) {
-throw 'CSS property `' + css_property + '` did not match: ' + style1_2 + ' != ' + style2_2; }
-}
+
+await page.evaluate((e1, e2) => {
+    let computed_style1 = getComputedStyle(e1);
+    let computed_style2 = getComputedStyle(e2);
+    const properties = [\"color\"];
+    for (const css_prop of properties) {
+        let style1_1 = e1.style[css_prop];
+        let style1_2 = computed_style1[css_prop];
+        let style2_1 = e2.style[css_prop];
+        let style2_2 = computed_style2[css_prop];
+        if (style1_1 != style2_1 && style1_1 != style2_2 &&
+            style1_2 != style2_1 && style1_2 != style2_2)
+        {
+            throw 'CSS property `' + css_prop + '` did not match: ' + style1_2 + ' != ' + style2_2;
+        }
+    }
+
 }, parseCompareElementsCss1, parseCompareElementsCss2);""",
 ]
 wait = false

tests/test-js/api-output/parseCompareElementsCss/err-1.toml

@@ -1 +1 @@
-error = """expected third argument to be an array of string, found a tuple"""
+error = """expected third element of the tuple to be an array, found `()` (a tuple)"""

tests/test-js/api-output/parseCompareElementsCss/err-10.toml

@@ -1 +1 @@
-error = """expected first argument to be a CSS selector or an XPath, found a tuple"""
+error = """expected first element of the tuple to be a selector, found `()` (a tuple)"""

tests/test-js/api-output/parseCompareElementsCss/err-11.toml

@@ -1 +1 @@
-error = """expected 3 elements in the tuple, found 4 elements"""
+error = """expected a tuple of 3 elements, found 4"""

tests/test-js/api-output/parseCompareElementsCss/err-12.toml

@@ -1 +1 @@
-error = """expected third argument to be an array of string, found a number"""
+error = """expected third element of the tuple to be an array, found `1` (a number)"""

tests/test-js/api-output/parseCompareElementsCss/err-13.toml

@@ -1 +1 @@
-error = """Empty CSS property keys (\"\" or '') are not allowed"""
+error = """empty values (`\"\"`) are not allowed (third element of the tuple)"""

tests/test-js/api-output/parseCompareElementsCss/err-2.toml

@@ -1 +1 @@
-error = """expected third argument to be an array of string, found a tuple"""
+error = """expected third element of the tuple to be an array, found `(1)` (a tuple)"""

tests/test-js/api-output/parseCompareElementsCss/err-3.toml

@@ -1 +1 @@
-error = """expected a tuple, found `\"a\"`"""
+error = """expected a tuple, found `\"a\"` (a string)"""

tests/test-js/api-output/parseCompareElementsCss/err-4.toml

@@ -1 +1 @@
-error = """expected a tuple, found `1`"""
+error = """expected a tuple, found `1` (a number)"""

tests/test-js/api-output/parseCompareElementsCss/err-5.toml

@@ -1 +1 @@
-error = """expected 3 elements in the tuple, found 0 element"""
+error = """expected a tuple of 3 elements, found 0"""

tests/test-js/api-output/parseCompareElementsCss/err-6.toml

@@ -1 +1 @@
-error = """expected a tuple, found `[]`"""
+error = """expected a tuple, found `[]` (an array)"""

tests/test-js/api-output/parseCompareElementsCss/err-7.toml

@@ -1 +1 @@
-error = """expected 3 elements in the tuple, found 1 element"""
+error = """expected a tuple of 3 elements, found 1"""

tests/test-js/api-output/parseCompareElementsCss/err-8.toml

@@ -1 +1 @@
-error = """expected second argument to be a CSS selector or an XPath, found a number"""
+error = """expected second element of the tuple to be a selector, found `1` (a number)"""

tests/test-js/api-output/parseCompareElementsCss/err-9.toml

@@ -1 +1 @@
-error = """expected first argument to be a CSS selector or an XPath, found a number"""
+error = """expected first element of the tuple to be a selector, found `1` (a number)"""

tests/test-js/api-output/parseCompareElementsCss/multiline-1.toml

@@ -1 +1 @@
-error = """expected third argument to be an array of string, found a number"""
+error = """expected third element of the tuple to be an array, found `1` (a number)"""

tests/test-js/api-output/parseCompareElementsCss/multiline-2.toml

@@ -5,17 +5,23 @@ parseCompareElementsCss1 = parseCompareElementsCss1[0];
 let parseCompareElementsCss2 = await page.$x(\"//b\");
 if (parseCompareElementsCss2.length === 0) { throw 'XPath \"//b\" not found'; }
 parseCompareElementsCss2 = parseCompareElementsCss2[0];
-await page.evaluate((e1, e2) => {let computed_style1 = getComputedStyle(e1);
-let computed_style2 = getComputedStyle(e2);
-const properties = [\"margin\"];
-for (const css_property of properties) {
-let style1_1 = e1.style[css_property];
-let style1_2 = computed_style1[css_property];
-let style2_1 = e2.style[css_property];
-let style2_2 = computed_style2[css_property];
-if (style1_1 != style2_1 && style1_1 != style2_2 && style1_2 != style2_1 && style1_2 != style2_2) {
-throw 'CSS property `' + css_property + '` did not match: ' + style1_2 + ' != ' + style2_2; }
-}
+
+await page.evaluate((e1, e2) => {
+    let computed_style1 = getComputedStyle(e1);
+    let computed_style2 = getComputedStyle(e2);
+    const properties = [\"margin\"];
+    for (const css_prop of properties) {
+        let style1_1 = e1.style[css_prop];
+        let style1_2 = computed_style1[css_prop];
+        let style2_1 = e2.style[css_prop];
+        let style2_2 = computed_style2[css_prop];
+        if (style1_1 != style2_1 && style1_1 != style2_2 &&
+            style1_2 != style2_1 && style1_2 != style2_2)
+        {
+            throw 'CSS property `' + css_prop + '` did not match: ' + style1_2 + ' != ' + style2_2;
+        }
+    }
+
 }, parseCompareElementsCss1, parseCompareElementsCss2);""",
 ]
 wait = false

tests/test-js/api-output/parseCompareElementsCss/xpath-1.toml

@@ -4,17 +4,23 @@ if (parseCompareElementsCss1.length === 0) { throw 'XPath \"//a\" not found'; }
 parseCompareElementsCss1 = parseCompareElementsCss1[0];
 let parseCompareElementsCss2 = await page.$(\"b\");
 if (parseCompareElementsCss2 === null) { throw '\"b\" not found'; }
-await page.evaluate((e1, e2) => {let computed_style1 = getComputedStyle(e1);
-let computed_style2 = getComputedStyle(e2);
-const properties = [\"margin\"];
-for (const css_property of properties) {
-let style1_1 = e1.style[css_property];
-let style1_2 = computed_style1[css_property];
-let style2_1 = e2.style[css_property];
-let style2_2 = computed_style2[css_property];
-if (style1_1 != style2_1 && style1_1 != style2_2 && style1_2 != style2_1 && style1_2 != style2_2) {
-throw 'CSS property `' + css_property + '` did not match: ' + style1_2 + ' != ' + style2_2; }
-}
+
+await page.evaluate((e1, e2) => {
+    let computed_style1 = getComputedStyle(e1);
+    let computed_style2 = getComputedStyle(e2);
+    const properties = [\"margin\"];
+    for (const css_prop of properties) {
+        let style1_1 = e1.style[css_prop];
+        let style1_2 = computed_style1[css_prop];
+        let style2_1 = e2.style[css_prop];
+        let style2_2 = computed_style2[css_prop];
+        if (style1_1 != style2_1 && style1_1 != style2_2 &&
+            style1_2 != style2_1 && style1_2 != style2_2)
+        {
+            throw 'CSS property `' + css_prop + '` did not match: ' + style1_2 + ' != ' + style2_2;
+        }
+    }
+
 }, parseCompareElementsCss1, parseCompareElementsCss2);""",
 ]
 wait = false