From 2a089f82cc4c0fdc9a1fb64970b46ced6e9d856b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Tue, 15 Dec 2015 12:33:53 +0100 Subject: [PATCH 1/4] Handle ExportNamedDeclarations --- lib/instrumenter.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/instrumenter.js b/lib/instrumenter.js index d2995f85..70a08120 100644 --- a/lib/instrumenter.js +++ b/lib/instrumenter.js @@ -790,6 +790,8 @@ coverStatement: function (node, walker) { var sName, incrStatementCount, + parent, + insertionTarget, grandParent; this.maybeSkipNode(node, 'next'); @@ -817,7 +819,19 @@ ) ) ); - this.splice(incrStatementCount, node, walker); + + // The insertion target for `incrStatementCount` is usually the current node. + insertionTarget = node; + + // However, to handle ExportNamedDeclarations, like `export var ...` + // We must insert the statement before our parent node, + // if it is such an export statement. + parent = walker.parent(); + if (parent && parent.node.type === SYNTAX.ExportNamedDeclaration.name) { + insertionTarget = parent; + } + + this.splice(incrStatementCount, insertionTarget, walker); } }, @@ -1048,4 +1062,3 @@ } }(typeof module !== 'undefined' && typeof module.exports !== 'undefined' && typeof exports !== 'undefined')); - From 1d49eb954dfbbd6e42ad9ef11f9aac05baf73e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Thu, 17 Dec 2015 17:03:59 +0100 Subject: [PATCH 2/4] Defer ExportNamedDeclarations to coverExport --- lib/instrumenter.js | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/instrumenter.js b/lib/instrumenter.js index 70a08120..bb7154f6 100644 --- a/lib/instrumenter.js +++ b/lib/instrumenter.js @@ -397,6 +397,7 @@ this.walker = new Walker({ ArrowFunctionExpression: [ this.arrowBlockConverter ], ExpressionStatement: this.coverStatement, + ExportNamedDeclaration: this.coverExport, BreakStatement: this.coverStatement, ContinueStatement: this.coverStatement, DebuggerStatement: this.coverStatement, @@ -791,7 +792,6 @@ var sName, incrStatementCount, parent, - insertionTarget, grandParent; this.maybeSkipNode(node, 'next'); @@ -820,21 +820,36 @@ ) ); - // The insertion target for `incrStatementCount` is usually the current node. - insertionTarget = node; - - // However, to handle ExportNamedDeclarations, like `export var ...` - // We must insert the statement before our parent node, - // if it is such an export statement. + // We let `coverExport` handle ExportNamedDeclarations. parent = walker.parent(); if (parent && parent.node.type === SYNTAX.ExportNamedDeclaration.name) { - insertionTarget = parent; + return; } - this.splice(incrStatementCount, insertionTarget, walker); + this.splice(incrStatementCount, node, walker); } }, + coverExport: function (node, walker) { + var sName, incrStatementCount; + + if ( !node.declaration ) return; + + this.maybeSkipNode(node, 'next'); + + sName = this.statementName(node.loc); + incrStatementCount = astgen.statement( + astgen.postIncrement( + astgen.subscript( + astgen.dot(astgen.variable(this.currentState.trackerVar), astgen.variable('s')), + astgen.stringLiteral(sName) + ) + ) + ); + + this.splice(incrStatementCount, node, walker); + }, + splice: function (statements, node, walker) { var targetNode = walker.isLabeled() ? walker.parent().node : node; targetNode.prepend = targetNode.prepend || []; From 8c4343fbb2dbb31e989ac0f369883deb2f75d89c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Mon, 4 Jan 2016 09:44:57 +0100 Subject: [PATCH 3/4] Fix lint warnings and tests for exports. Tweaked helper. --- lib/instrumenter.js | 43 +++++++++++++------------ test/es6.js | 23 +++++++++---- test/helper.js | 5 +++ test/instrumentation/test-es6-export.js | 18 +++++++++++ 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/lib/instrumenter.js b/lib/instrumenter.js index bb7154f6..42a547c7 100644 --- a/lib/instrumenter.js +++ b/lib/instrumenter.js @@ -807,10 +807,19 @@ } } } + if (node.type === SYNTAX.FunctionDeclaration.name) { - sName = this.statementName(node.loc, 1); + // Called for the side-effect of setting the function's statement count to 1. + this.statementName(node.loc, 1); } else { + // We let `coverExport` handle ExportNamedDeclarations. + parent = walker.parent(); + if (parent && parent.node.type === SYNTAX.ExportNamedDeclaration.name) { + return; + } + sName = this.statementName(node.loc); + incrStatementCount = astgen.statement( astgen.postIncrement( astgen.subscript( @@ -820,34 +829,28 @@ ) ); - // We let `coverExport` handle ExportNamedDeclarations. - parent = walker.parent(); - if (parent && parent.node.type === SYNTAX.ExportNamedDeclaration.name) { - return; - } - this.splice(incrStatementCount, node, walker); } }, coverExport: function (node, walker) { - var sName, incrStatementCount; + var sName, incrStatementCount; - if ( !node.declaration ) return; + if ( !node.declaration || !node.declaration.declarations ) { return; } - this.maybeSkipNode(node, 'next'); + this.maybeSkipNode(node, 'next'); - sName = this.statementName(node.loc); - incrStatementCount = astgen.statement( - astgen.postIncrement( - astgen.subscript( - astgen.dot(astgen.variable(this.currentState.trackerVar), astgen.variable('s')), - astgen.stringLiteral(sName) - ) - ) - ); + sName = this.statementName(node.declaration.loc); + incrStatementCount = astgen.statement( + astgen.postIncrement( + astgen.subscript( + astgen.dot(astgen.variable(this.currentState.trackerVar), astgen.variable('s')), + astgen.stringLiteral(sName) + ) + ) + ); - this.splice(incrStatementCount, node, walker); + this.splice(incrStatementCount, node, walker); }, splice: function (statements, node, walker) { diff --git a/test/es6.js b/test/es6.js index ec7a63fb..b4e96cc3 100644 --- a/test/es6.js +++ b/test/es6.js @@ -1,16 +1,25 @@ var esprima = require('esprima'); function tryThis(str, feature) { - try { - /*jshint evil: true */ - eval(str); - } catch (ex) { - console.error('ES6 feature [' + feature + '] is not available in this environment'); - return false; + // We can test instrumentation of exports even if the environment doesn't support them. + if (feature !== 'export') { + try { + /*jshint evil: true */ + eval(str); + } catch (ex) { + console.error('ES6 feature [' + feature + '] is not available in this environment'); + return false; + } } + // esprima parses sources with sourceType 'script' per default. + // The only way to enable `import`/`export` is to parse as sourceType 'module'. try { - esprima.parse(str); + try { + esprima.parse(str); + } catch (ex) { + esprima.parse(str, { sourceType: 'module' }); + } } catch (ex) { console.error('ES6 feature [' + feature + '] is not yet supported by esprima mainline'); return false; diff --git a/test/helper.js b/test/helper.js index b6ee52ec..3f3d8420 100644 --- a/test/helper.js +++ b/test/helper.js @@ -104,6 +104,11 @@ function setup(file, codeArray, opts) { } return; } + + // `export`/`import` cannot be wrapped inside a function. + // For our purposes, simply remove the `export` from export declarations. + generated = generated.replace(/export (var|function|let|const)/g, '$1'); + var wrappedCode = '(function (args) { var output;\n' + generated + '\nreturn output;\n})', fn; global[coverageVariable] = undefined; diff --git a/test/instrumentation/test-es6-export.js b/test/instrumentation/test-es6-export.js index 4ecefd9d..e1578647 100644 --- a/test/instrumentation/test-es6-export.js +++ b/test/instrumentation/test-es6-export.js @@ -21,6 +21,24 @@ if (require('../es6').isExportAvailable()) { statements: {'1': 1, '2': 1, '3': 1} }); test.done(); + }, + + 'should cover export declarations': function (test) { + code = [ + 'export var a = 2, b = 3;', + 'output = a + b' + ]; + verifier = helper.verifier(__filename, code, { + esModules: true, + noAutoWrap: true + }); + verifier.verify(test, [], 5, { + lines: {'1':1, '2': 1}, + branches: {}, + functions: {}, + statements: {'1': 1, '2': 1} + }); + test.done(); } }; } From d8282557b3caa5f3d28b2fadb8b223474c67475b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersva=CC=88rd?= Date: Thu, 24 Mar 2016 10:00:45 +0100 Subject: [PATCH 4/4] Update according to @guybedford's comments --- lib/instrumenter.js | 2 +- test/es6.js | 18 +++++++-------- test/helper.js | 4 +++- test/instrumentation/test-es6-export.js | 30 ++++++++++++------------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/instrumenter.js b/lib/instrumenter.js index 42a547c7..6273a5eb 100644 --- a/lib/instrumenter.js +++ b/lib/instrumenter.js @@ -815,7 +815,7 @@ // We let `coverExport` handle ExportNamedDeclarations. parent = walker.parent(); if (parent && parent.node.type === SYNTAX.ExportNamedDeclaration.name) { - return; + return; } sName = this.statementName(node.loc); diff --git a/test/es6.js b/test/es6.js index b4e96cc3..e47c7fec 100644 --- a/test/es6.js +++ b/test/es6.js @@ -1,15 +1,12 @@ var esprima = require('esprima'); function tryThis(str, feature) { - // We can test instrumentation of exports even if the environment doesn't support them. - if (feature !== 'export') { - try { - /*jshint evil: true */ - eval(str); - } catch (ex) { - console.error('ES6 feature [' + feature + '] is not available in this environment'); - return false; - } + try { + /*jshint evil: true */ + eval(str); + } catch (ex) { + console.error('ES6 feature [' + feature + '] is not available in this environment'); + return false; } // esprima parses sources with sourceType 'script' per default. @@ -51,6 +48,7 @@ module.exports = { }, isExportAvailable: function () { - return tryThis('export default function foo() {}', 'export'); + // We can test instrumentation of exports even if the environment doesn't support them. + return true; } }; diff --git a/test/helper.js b/test/helper.js index 3f3d8420..59678417 100644 --- a/test/helper.js +++ b/test/helper.js @@ -107,7 +107,9 @@ function setup(file, codeArray, opts) { // `export`/`import` cannot be wrapped inside a function. // For our purposes, simply remove the `export` from export declarations. - generated = generated.replace(/export (var|function|let|const)/g, '$1'); + if ( opts.esModules ) { + generated = generated.replace(/export (var|function|let|const)/g, '$1'); + } var wrappedCode = '(function (args) { var output;\n' + generated + '\nreturn output;\n})', fn; diff --git a/test/instrumentation/test-es6-export.js b/test/instrumentation/test-es6-export.js index e1578647..0efebbec 100644 --- a/test/instrumentation/test-es6-export.js +++ b/test/instrumentation/test-es6-export.js @@ -24,21 +24,21 @@ if (require('../es6').isExportAvailable()) { }, 'should cover export declarations': function (test) { - code = [ - 'export var a = 2, b = 3;', - 'output = a + b' - ]; - verifier = helper.verifier(__filename, code, { - esModules: true, - noAutoWrap: true - }); - verifier.verify(test, [], 5, { - lines: {'1':1, '2': 1}, - branches: {}, - functions: {}, - statements: {'1': 1, '2': 1} - }); - test.done(); + code = [ + 'export var a = 2, b = 3;', + 'output = a + b' + ]; + verifier = helper.verifier(__filename, code, { + esModules: true, + noAutoWrap: true + }); + verifier.verify(test, [], 5, { + lines: {'1':1, '2': 1}, + branches: {}, + functions: {}, + statements: {'1': 1, '2': 1} + }); + test.done(); } }; }