From 70551febc8eea1db62ad87aae7bd727ea4a718b6 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Sun, 9 Feb 2020 08:07:55 +0000 Subject: [PATCH] improve `test/reduce` (#3710) - suppress several instances of malformed AST generation - improve resilience & reporting against malformed ASTs --- test/mocha/reduce.js | 6 ++ test/reduce.js | 144 ++++++++++++++++++++++++++++++------------- 2 files changed, 108 insertions(+), 42 deletions(-) diff --git a/test/mocha/reduce.js b/test/mocha/reduce.js index 99fc7947..5ab4e5b7 100644 --- a/test/mocha/reduce.js +++ b/test/mocha/reduce.js @@ -57,4 +57,10 @@ describe("test/reduce.js", function() { assert.ok(err instanceof Error); assert.strictEqual(err.stack.split(/\n/)[0], "DefaultsError: `unsafe_regex` is not a supported option"); }); + it("Should report on test case with invalid syntax", function() { + var result = reduce_test("var 0 = 1;"); + var err = result.error; + assert.ok(err instanceof Error); + assert.strictEqual(err.stack.split(/\n/)[0], "SyntaxError: Name expected"); + }); }); diff --git a/test/reduce.js b/test/reduce.js index a54bd1a7..2a4dad0b 100644 --- a/test/reduce.js +++ b/test/reduce.js @@ -52,11 +52,12 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) if (CHANGED) return; // quick ignores - if (node instanceof U.AST_Toplevel) return; if (node instanceof U.AST_Accessor) return; if (node instanceof U.AST_Directive) return; - // if (node instanceof U.AST_Var) return; - // if (node instanceof U.AST_VarDef) return; + if (node instanceof U.AST_Label) return; + if (node instanceof U.AST_LabelRef) return; + if (node instanceof U.AST_SymbolDeclaration) return; + if (node instanceof U.AST_Toplevel) return; var parent = tt.parent(); @@ -85,12 +86,17 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) CHANGED = true; return null; } + if ((parent instanceof U.AST_For || parent instanceof U.AST_ForIn) + && parent.init === node && node instanceof U.AST_Var) { + // preserve for (var ...) + return node; + } // node specific permutations with no parent logic if (node instanceof U.AST_Array) { var expr = node.elements[0]; - if (expr) { + if (expr && !(expr instanceof U.AST_Hole)) { node.start._permute++; CHANGED = true; return expr; @@ -151,12 +157,9 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) expr = node.condition; // AST_While - fall through } } - if (expr) { + if (expr && (expr !== node.body || !has_loopcontrol(expr, node, parent))) { CHANGED = true; - return expr instanceof U.AST_Statement ? expr : new U.AST_SimpleStatement({ - body: expr, - start: {}, - }); + return to_statement(expr); } } else if (node instanceof U.AST_PropAccess) { @@ -177,12 +180,9 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) node.body, ][ (node.start._permute * steps | 0) % 4 ]; node.start._permute += step; - if (expr) { + if (expr && (expr !== node.body || !has_loopcontrol(expr, node, parent))) { CHANGED = true; - return expr instanceof U.AST_Statement ? expr : new U.AST_SimpleStatement({ - body: expr, - start: {}, - }); + return to_statement(expr); } } else if (node instanceof U.AST_ForIn) { @@ -192,12 +192,9 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) node.body, ][ (node.start._permute * steps | 0) % 3 ]; node.start._permute += step; - if (expr) { + if (expr && (expr !== node.body || !has_loopcontrol(expr, node, parent))) { CHANGED = true; - return expr instanceof U.AST_Statement ? expr : new U.AST_SimpleStatement({ - body: expr, - start: {}, - }); + return to_statement(expr); } } else if (node instanceof U.AST_If) { @@ -224,15 +221,15 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) var expr = [ node.expression, // switch expression node.body[0] && node.body[0].expression, // first case expression or undefined - node.body[0] && node.body[0].body[0], // first case body statement or undefined - ][ (node.start._permute * steps | 0) % 3 ]; + node.body[0] && node.body[0], // first case body or undefined + ][ (node.start._permute * steps | 0) % 4 ]; node.start._permute += step; - if (expr) { + if (expr && (!(expr instanceof U.AST_Statement) || !has_loopcontrol(expr, node, parent))) { CHANGED = true; - return expr instanceof U.AST_Statement ? expr : new U.AST_SimpleStatement({ - body: expr, + return expr instanceof U.AST_SwitchBranch ? new U.AST_BlockStatement({ + body: expr.body.slice(), start: {}, - }); + }) : to_statement(expr); } } else if (node instanceof U.AST_Try) { @@ -277,7 +274,8 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) // replace or skip statement if (node instanceof U.AST_Statement) { if (node instanceof U.AST_LabeledStatement - && node.body instanceof U.AST_Statement) { + && node.body instanceof U.AST_Statement + && !has_loopcontrol(node.body, node.body, node)) { // replace labelled statement with its non-labelled body node.start._permute = REPLACEMENTS.length; CHANGED = true; @@ -287,15 +285,46 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) CHANGED = true; return List.skip; } + + // remove this node unless its the sole element of a (transient) sequence + if (!(parent instanceof U.AST_Sequence) || parent.expressions.length > 1) { + node.start._permute++; + CHANGED = true; + return List.skip; + } } - // replace or remove this node depending on whether it's in a list + // replace this node var newNode = new U.parse(REPLACEMENTS[node.start._permute % REPLACEMENTS.length | 0], { expression: true, }); + if (is_statement(node)) { + newNode = new U.AST_SimpleStatement({ + body: newNode, + start: {}, + }); + } newNode.start._permute = ++node.start._permute; CHANGED = true; - return in_list ? List.skip : newNode; + return newNode; + }, function(node, in_list) { + if (node instanceof U.AST_Sequence) { + // expand single-element sequence + if (node.expressions.length == 1) return node.expressions[0]; + } + else if (node instanceof U.AST_Try) { + // expand orphaned try block + if (!node.bcatch && !node.bfinally) return new U.AST_BlockStatement({ + body: node.body, + start: {} + }); + } + else if (node instanceof U.AST_Var) { + // remove empty var statement + if (node.definitions.length == 0) return in_list ? List.skip : new U.AST_EmptyStatement({ + start: {} + }); + } }); for (var pass = 1; pass <= 3; ++pass) { @@ -321,22 +350,28 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) // AST is not well formed. // no harm done - just log the error, ignore latest change and continue iterating. console.error("*** Error generating code from AST."); - console.error(ex.stack); + console.error(ex); console.error("*** Discarding permutation and continuing."); + continue; } - if (code) { - var diff = producesDifferentResultWhenMinified(code, minify_options, timeout); - if (diff) { - if (diff.timed_out) { - // can't trust the validity of `code_ast` and `code` when timed out. - // no harm done - just ignore latest change and continue iterating. - if (timeout < max_timeout) timeout += 250; - } else { - // latest permutation is valid, so use it as the basis of new changes - testcase_ast = code_ast; - testcase = code; - differs = diff; - } + var diff = producesDifferentResultWhenMinified(code, minify_options, timeout); + if (diff) { + if (diff.timed_out) { + // can't trust the validity of `code_ast` and `code` when timed out. + // no harm done - just ignore latest change and continue iterating. + if (timeout < max_timeout) timeout += 250; + } else if (diff.error) { + // something went wrong during minify() - could be malformed AST or genuine bug. + // no harm done - just log code & error, ignore latest change and continue iterating. + console.error("*** Error during minification."); + console.error(code); + console.error(diff.error); + console.error("*** Discarding permutation and continuing."); + } else { + // latest permutation is valid, so use it as the basis of new changes + testcase_ast = code_ast; + testcase = code; + differs = diff; } } } @@ -365,6 +400,31 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options) return result; }; +function has_loopcontrol(body, loop, label) { + var found = false; + var tw = new U.TreeWalker(function(node) { + if (found) return true; + if (node instanceof U.AST_LoopControl && this.loopcontrol_target(node) === loop) { + return found = true; + } + }); + if (label instanceof U.AST_LabeledStatement) tw.push(label); + tw.push(loop); + body.walk(tw); + return found; +} + +function is_statement(node) { + return node instanceof U.AST_Statement && !(node instanceof U.AST_Function); +} + +function to_statement(node) { + return is_statement(node) ? node : new U.AST_SimpleStatement({ + body: node, + start: {}, + }); +} + function producesDifferentResultWhenMinified(code, minify_options, timeout) { var minified = U.minify(code, minify_options); if (minified.error) return minified; -- 2.34.1