From f8f74c6166584c23985a04dbc3a179e489c7f019 Mon Sep 17 00:00:00 2001 From: oguimbal Date: Fri, 30 Oct 2020 12:20:48 +0100 Subject: [PATCH 1/2] add 'ignoreGuardStatements' option --- .../src/instrumenter.js | 2 + .../istanbul-lib-instrument/src/visitor.js | 63 ++++- .../test/already-instrumented.test.js | 3 +- .../test/specs/ignored-guards.yaml | 229 ++++++++++++++++++ 4 files changed, 291 insertions(+), 6 deletions(-) create mode 100644 packages/istanbul-lib-instrument/test/specs/ignored-guards.yaml diff --git a/packages/istanbul-lib-instrument/src/instrumenter.js b/packages/istanbul-lib-instrument/src/instrumenter.js index 679097ac..f4355173 100644 --- a/packages/istanbul-lib-instrument/src/instrumenter.js +++ b/packages/istanbul-lib-instrument/src/instrumenter.js @@ -20,6 +20,7 @@ import readInitialCoverage from './read-coverage'; * @param {boolean} [opts.autoWrap=false] set to true to allow `return` statements outside of functions. * @param {boolean} [opts.produceSourceMap=false] set to true to produce a source map for the instrumented code. * @param {Array} [opts.ignoreClassMethods=[]] set to array of class method names to ignore for coverage. + * @param {Array} [opts.ignoreGuardStatements=[]] ignore all guard statements. Can contain any of 'returns', 'literalReturns', 'identifierReturns', 'voidReturns', 'throws', 'continues', 'breaks' * @param {Function} [opts.sourceMapUrlCallback=null] a callback function that is called when a source map URL * is found in the original code. This function is called with the source file name and the source map URL. * @param {boolean} [opts.debug=false] - turn debugging on @@ -77,6 +78,7 @@ class Instrumenter { coverageGlobalScopeFunc: opts.coverageGlobalScopeFunc, ignoreClassMethods: opts.ignoreClassMethods, + ignoreGuardStatements: opts.ignoreGuardStatements, inputSourceMap }); diff --git a/packages/istanbul-lib-instrument/src/visitor.js b/packages/istanbul-lib-instrument/src/visitor.js index ad526e82..61be40f7 100644 --- a/packages/istanbul-lib-instrument/src/visitor.js +++ b/packages/istanbul-lib-instrument/src/visitor.js @@ -25,7 +25,8 @@ class VisitState { types, sourceFilePath, inputSourceMap, - ignoreClassMethods = [] + ignoreClassMethods = [], + ignoreGuardStatements = [] ) { this.varName = genVar(sourceFilePath); this.attrs = {}; @@ -36,6 +37,7 @@ class VisitState { this.cov.inputSourceMap(inputSourceMap); } this.ignoreClassMethods = ignoreClassMethods; + this.ignoreGuardStatements = ignoreGuardStatements; this.types = types; this.sourceMappingURL = null; } @@ -409,11 +411,62 @@ function convertArrowExpression(path) { } } +function isIgnoredGuard(n) { + if (!this.ignoreGuardStatements + || n.type !== 'BlockStatement' + || !Array.isArray(this.ignoreGuardStatements) + || !this.ignoreGuardStatements.length) { + return false; + } + if (n.body.length !== 1) { + // only ignore simple bodies (signle statement) + return false; + } + const stm = n.body[0]; + if (stm.type === 'ThrowStatement') { + // ignore throws + return this.ignoreGuardStatements.includes('throws'); + } + if (stm.type === 'ContinueStatement') { + // ignore continues + return this.ignoreGuardStatements.includes('continues'); + } + if (stm.type === 'BreakStatement') { + // ignore continues + return this.ignoreGuardStatements.includes('breaks'); + } + if (stm.type === 'ReturnStatement') { + if (!this.ignoreGuardStatements) { + return false; + } + if (this.ignoreGuardStatements.includes('returns')) { + // ignore all returns + return true; + } + if (!stm.argument) { + return this.ignoreGuardStatements.includes('literalReturns') + || this.ignoreGuardStatements.includes('voidReturns'); + } + switch (stm.argument.type) { + case 'NumericLiteral': + case 'BooleanLiteral': + case 'StringLiteral': + case 'NullLiteral': + return this.ignoreGuardStatements.includes('literalReturns'); + case 'Identifier': + return stm.argument.name === 'undefined' && this.ignoreGuardStatements.includes('literalReturns') + || this.ignoreGuardStatements.includes('identifierReturns'); + + } + } + return false; +} + function coverIfBranches(path) { const n = path.node; const hint = this.hintFor(n); - const ignoreIf = hint === 'if'; - const ignoreElse = hint === 'else'; + const ignoreIf = hint === 'if' || isIgnoredGuard.call(this, n.consequent); + const ignoreElse = hint === 'else' || isIgnoredGuard.call(this, n.alternate); const branch = this.cov.newBranch('if', n.loc); if (ignoreIf) { @@ -595,6 +648,7 @@ function shouldIgnoreFile(programNode) { * @param {string} [opts.coverageGlobalScope=this] the global coverage variable scope. * @param {boolean} [opts.coverageGlobalScopeFunc=true] use an evaluated function to find coverageGlobalScope. * @param {Array} [opts.ignoreClassMethods=[]] names of methods to ignore by default on classes. + * @param {Array} [opts.ignoreGuardStatements=[]] ignore all guard statements. Can contain any of 'returns', 'literalReturns', 'identifierReturns', 'voidReturns', 'throws', 'continues', 'breaks' * @param {object} [opts.inputSourceMap=undefined] the input source map, that maps the uninstrumented code back to the * original code. */ @@ -608,7 +662,8 @@ function programVisitor(types, sourceFilePath = 'unknown.js', opts = {}) { types, sourceFilePath, opts.inputSourceMap, - opts.ignoreClassMethods + opts.ignoreClassMethods, + opts.ignoreGuardStatements ); return { enter(path) { diff --git a/packages/istanbul-lib-instrument/test/already-instrumented.test.js b/packages/istanbul-lib-instrument/test/already-instrumented.test.js index eef1928d..5a9973d3 100644 --- a/packages/istanbul-lib-instrument/test/already-instrumented.test.js +++ b/packages/istanbul-lib-instrument/test/already-instrumented.test.js @@ -17,9 +17,8 @@ function instrument(code, inputSourceMap) { }; } -const instrumented = instrument(`console.log('basic test');`); - it('should not alter already instrumented code', () => { + const instrumented = instrument(`console.log('basic test');`); const result = instrument(instrumented.code, instrumented.sourceMap); [instrumented, result].forEach(({ sourceMap }) => { // XXX Ignore source-map difference caused by: diff --git a/packages/istanbul-lib-instrument/test/specs/ignored-guards.yaml b/packages/istanbul-lib-instrument/test/specs/ignored-guards.yaml new file mode 100644 index 00000000..7af3524c --- /dev/null +++ b/packages/istanbul-lib-instrument/test/specs/ignored-guards.yaml @@ -0,0 +1,229 @@ +--- +name: ignores throw +code: | + output = -1; + if (args[0] > args [1]) throw new Error('Error !') +instrumentOpts: + ignoreGuardStatements: ["throws"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: does not ignores throw +code: | + output = -1; + if (args[0] > args [1]) throw new Error('Error !') +instrumentOpts: + ignoreGuardStatements: ["returns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [0, 1] } + statements: { "0": 1, "1": 1, "2": 0 } + +--- +name: ignores void return +code: | + output = -1; + if (args[0] > args [1]) return +instrumentOpts: + ignoreGuardStatements: ["voidReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores continue +code: | + output = 3; + while (--output>0) { + if (args[0] > args [1]) continue + } +instrumentOpts: + ignoreGuardStatements: ["continues"] +tests: + - name: default test + args: [10, 20] + out: 0 + lines: { "1": 1, "2": 1, "3": 2 } + branches: { "0": [2] } + statements: { "0": 1, "1": 1, "2": 2 } + +--- +name: ignores break +code: | + output = 3; + while (--output>0) { + if (args[0] > args [1]) break + } +instrumentOpts: + ignoreGuardStatements: ["breaks"] +tests: + - name: default test + args: [10, 20] + out: 0 + lines: { "1": 1, "2": 1, "3": 2 } + branches: { "0": [2] } + statements: { "0": 1, "1": 1, "2": 2 } + + +--- +name: ignores numeric literal returns +code: | + output = -1; + if (args[0] > args [1]) return 0 +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: does not ignore numeric literal returns +code: | + output = -1; + if (args[0] > args [1]) return 0 +instrumentOpts: + ignoreGuardStatements: ["voidReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [0, 1] } + statements: { "0": 1, "1": 1, "2": 0 } + +--- +name: ignores bool literal returns +code: | + output = -1; + if (args[0] > args [1]) return false +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores string literal returns +code: | + output = -1; + if (args[0] > args [1]) return 'nope' +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores null literal returns +code: | + output = -1; + if (args[0] > args [1]) return null +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores undefined literal returns +code: | + output = -1; + if (args[0] > args [1]) return undefined +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores identifier literal returns +code: | + output = -1; + if (args[0] > args [1]) return output +instrumentOpts: + ignoreGuardStatements: ["identifierReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: ignores complex return +code: | + output = -1; + if (args[0] > args [1]) return args[0] +instrumentOpts: + ignoreGuardStatements: ["returns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [1] } + statements: { "0": 1, "1": 1 } + +--- +name: does not ignore complex return +code: | + output = -1; + if (args[0] > args [1]) return args[0] +instrumentOpts: + ignoreGuardStatements: ["literalReturns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [0, 1] } + statements: { "0": 1, "1": 1, "2": 0 } + +--- +name: simple if single line does not ignore statement +code: | + output = -1; + if (args[0] > args [1]) output = args[0]; +instrumentOpts: + ignoreGuardStatements: ["returns"] +tests: + - name: default test + args: [10, 20] + out: -1 + lines: { "1": 1, "2": 1 } + branches: { "0": [0, 1] } + statements: { "0": 1, "1": 1, "2": 0 } From 01acda1cd293534bf4f49f32de8facf5c8961ee3 Mon Sep 17 00:00:00 2001 From: oguimbal Date: Fri, 30 Oct 2020 13:44:14 +0100 Subject: [PATCH 2/2] linting --- packages/istanbul-lib-instrument/api.md | 2 + .../istanbul-lib-instrument/src/visitor.js | 42 ++++++++++++------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/istanbul-lib-instrument/api.md b/packages/istanbul-lib-instrument/api.md index 9c046856..51a5b1a5 100644 --- a/packages/istanbul-lib-instrument/api.md +++ b/packages/istanbul-lib-instrument/api.md @@ -42,6 +42,7 @@ instead. - `opts.autoWrap` **[boolean][15]** set to true to allow `return` statements outside of functions. (optional, default `false`) - `opts.produceSourceMap` **[boolean][15]** set to true to produce a source map for the instrumented code. (optional, default `false`) - `opts.ignoreClassMethods` **[Array][16]** set to array of class method names to ignore for coverage. (optional, default `[]`) + - `opts.ignoreGuardStatements` **[Array][16]** ignore all guard statements. Can contain any of 'returns', 'literalReturns', 'identifierReturns', 'voidReturns', 'throws', 'continues', 'breaks' (optional, default `[]`) - `opts.sourceMapUrlCallback` **[Function][17]** a callback function that is called when a source map URL is found in the original code. This function is called with the source file name and the source map URL. (optional, default `null`) - `opts.debug` **[boolean][15]** turn debugging on (optional, default `false`) @@ -114,6 +115,7 @@ The exit function returns an object that currently has the following keys: - `opts.coverageGlobalScope` **[string][14]** the global coverage variable scope. (optional, default `this`) - `opts.coverageGlobalScopeFunc` **[boolean][15]** use an evaluated function to find coverageGlobalScope. (optional, default `true`) - `opts.ignoreClassMethods` **[Array][16]** names of methods to ignore by default on classes. (optional, default `[]`) + - `opts.ignoreGuardStatements` **[Array][16]** ignore all guard statements. Can contain any of 'returns', 'literalReturns', 'identifierReturns', 'voidReturns', 'throws', 'continues', 'breaks' (optional, default `[]`) - `opts.inputSourceMap` **[object][13]** the input source map, that maps the uninstrumented code back to the original code. (optional, default `undefined`) diff --git a/packages/istanbul-lib-instrument/src/visitor.js b/packages/istanbul-lib-instrument/src/visitor.js index 61be40f7..cdf60a3d 100644 --- a/packages/istanbul-lib-instrument/src/visitor.js +++ b/packages/istanbul-lib-instrument/src/visitor.js @@ -411,13 +411,17 @@ function convertArrowExpression(path) { } } -function isIgnoredGuard(n) { - if (!this.ignoreGuardStatements - || n.type !== 'BlockStatement' - || !Array.isArray(this.ignoreGuardStatements) - || !this.ignoreGuardStatements.length) { +function ignoreGuard(n) { + if (!this.ignoreGuardStatements || n.type !== 'BlockStatement') { return false; } + if (!Array.isArray(this.ignoreGuardStatements)) { + return false; + } + if (!this.ignoreGuardStatements.length) { + return false; + } + if (n.body.length !== 1) { // only ignore simple bodies (signle statement) return false; @@ -432,31 +436,37 @@ function isIgnoredGuard(n) { return this.ignoreGuardStatements.includes('continues'); } if (stm.type === 'BreakStatement') { - // ignore continues + // ignore breaks return this.ignoreGuardStatements.includes('breaks'); } if (stm.type === 'ReturnStatement') { - if (!this.ignoreGuardStatements) { - return false; - } if (this.ignoreGuardStatements.includes('returns')) { // ignore all returns return true; } if (!stm.argument) { - return this.ignoreGuardStatements.includes('literalReturns') - || this.ignoreGuardStatements.includes('voidReturns'); + // ignore void returns + return ( + this.ignoreGuardStatements.includes('literalReturns') || + this.ignoreGuardStatements.includes('voidReturns') + ); } switch (stm.argument.type) { case 'NumericLiteral': case 'BooleanLiteral': case 'StringLiteral': case 'NullLiteral': + // ignore constant literal returns return this.ignoreGuardStatements.includes('literalReturns'); case 'Identifier': - return stm.argument.name === 'undefined' && this.ignoreGuardStatements.includes('literalReturns') - || this.ignoreGuardStatements.includes('identifierReturns'); - + if (this.ignoreGuardStatements.includes('identifierReturns')) { + return true; + } + // ignore 'return undefined' + return ( + stm.argument.name === 'undefined' && + this.ignoreGuardStatements.includes('literalReturns') + ); } } return false; @@ -465,8 +475,8 @@ function isIgnoredGuard(n) { function coverIfBranches(path) { const n = path.node; const hint = this.hintFor(n); - const ignoreIf = hint === 'if' || isIgnoredGuard.call(this, n.consequent); - const ignoreElse = hint === 'else' || isIgnoredGuard.call(this, n.alternate); + const ignoreIf = hint === 'if' || ignoreGuard.call(this, n.consequent); + const ignoreElse = hint === 'else' || ignoreGuard.call(this, n.alternate); const branch = this.cov.newBranch('if', n.loc); if (ignoreIf) {