feat: Allow nyc instrument to instrument code in place (#1149)

This change adds the `--in-place` switch to nyc instrument
If `--in-place` is specified, the output directory will be set to equal the input directory for the instrument command.
This has the effect of replacing any file in the input directory that should be instrumented, with its instrumented counterpart.
The command will throw an error if the --delete option is specified.
The only way to instrument in place is with the --in-place switch, setting the input and output directories to be the same will not work
If `--in-place` is set the instrument command ignores any output directory specified with the command
If `--in-place` is set the instrument command will disable the `--complete-copy` switch as it is unnecessary.

Author: AndrewFinlay

docs/instrument.md

@@ -20,15 +20,17 @@ For example, `nyc instrument . ./output` will produce instrumented versions of a
 
 The `--delete` option will remove the existing output directory before instrumenting.
 
+The `--in-place` option will allow you to run the instrument command.
+
 The `--complete-copy` option will copy all remaining files from the `input` directory to the `output` directory.
 When using `--complete-copy` nyc will not copy the contents of a `.git` folder to the output directory.
 
 **Note:** `--complete-copy` will dereference any symlinks during the copy process, this may stop scripts running properly from the output directory.
 
 ## Streaming instrumentation
 
-`nyc instrument` will stream instrumented source directly to `stdout`, that can then be piped to another process.
-You can use this behaviour to create a server that can instrument files on request.
+`nyc instrument <input>` will stream instrumented source directly to `stdout` and that output can then be piped to another process.
+You can use this behaviour to create a server that dynamically instruments files on request.
 The following example shows streaming instrumentation middleware capable of instrumenting files on request.
 
 ```javascript

lib/commands/instrument.js

@@ -31,32 +31,37 @@ exports.builder = function (yargs) {
     .option('source-map', {
       default: true,
       type: 'boolean',
-      description: 'should nyc detect and handle source maps?'
+      describe: 'should nyc detect and handle source maps?'
     })
     .option('produce-source-map', {
       default: false,
       type: 'boolean',
-      description: "should nyc's instrumenter produce source maps?"
+      describe: "should nyc's instrumenter produce source maps?"
     })
     .option('compact', {
       default: true,
       type: 'boolean',
-      description: 'should the output be compacted?'
+      describe: 'should the output be compacted?'
     })
     .option('preserve-comments', {
       default: true,
       type: 'boolean',
-      description: 'should comments be preserved in the output?'
+      describe: 'should comments be preserved in the output?'
     })
     .option('instrument', {
       default: true,
       type: 'boolean',
-      description: 'should nyc handle instrumentation?'
+      describe: 'should nyc handle instrumentation?'
+    })
+    .option('in-place', {
+      default: false,
+      type: 'boolean',
+      describe: 'should nyc run the instrumentation in place?'
     })
     .option('exit-on-error', {
       default: false,
       type: 'boolean',
-      description: 'should nyc exit when an instrumentation failure occurs?'
+      describe: 'should nyc exit when an instrumentation failure occurs?'
     })
     .option('include', {
       alias: 'n',
@@ -92,26 +97,30 @@ exports.builder = function (yargs) {
     .example('$0 instrument ./lib ./output', 'instrument all .js files in ./lib with coverage and output in ./output')
 }
 
+const errorExit = msg => {
+  console.error(`nyc instrument failed:  ${msg}`)
+  process.exitCode = 1
+}
+
 exports.handler = function (argv) {
-  if (argv.output && (path.resolve(argv.cwd, argv.input) === path.resolve(argv.cwd, argv.output))) {
-    console.error(`nyc instrument failed: cannot instrument files in place, <input> must differ from <output>`)
-    process.exitCode = 1
-    return
+  if (argv.output && !argv.inPlace && (path.resolve(argv.cwd, argv.input) === path.resolve(argv.cwd, argv.output))) {
+    return errorExit(`cannot instrument files in place, <input> must differ from <output>.  Set '--in-place' to force`)
   }
 
   if (path.relative(argv.cwd, path.resolve(argv.cwd, argv.input)).startsWith('..')) {
-    console.error(`nyc instrument failed: cannot instrument files outside of project root directory`)
-    process.exitCode = 1
-    return
+    return errorExit('cannot instrument files outside project root directory')
+  }
+
+  if (argv.delete && argv.inPlace) {
+    return errorExit(`cannot use '--delete' when instrumenting files in place`)
   }
 
   if (argv.delete && argv.output && argv.output.length !== 0) {
     const relPath = path.relative(process.cwd(), path.resolve(argv.output))
     if (relPath !== '' && !relPath.startsWith('..')) {
       rimraf.sync(argv.output)
     } else {
-      console.error(`nyc instrument failed: attempt to delete '${process.cwd()}' or containing directory.`)
-      process.exit(1)
+      return errorExit(`attempt to delete '${process.cwd()}' or containing directory.`)
     }
   }
 
@@ -120,12 +129,11 @@ exports.handler = function (argv) {
     ? './lib/instrumenters/istanbul'
     : './lib/instrumenters/noop'
 
-  const nyc = new NYC(argv)
+  if (argv.inPlace) {
+    argv.output = argv.input
+    argv.completeCopy = false
+  }
 
-  nyc.instrumentAllFiles(argv.input, argv.output, err => {
-    if (err) {
-      console.error(err.message)
-      process.exitCode = 1
-    }
-  })
+  const nyc = new NYC(argv)
+  nyc.instrumentAllFiles(argv.input, argv.output, err => err ? errorExit(err.message) : null)
 }

tap-snapshots/test-nyc-integration.js-TAP.test.js

@@ -103,6 +103,9 @@ All files                        |       0 |        0 |       0 |       0 |
   test.js                        |       0 |        0 |       0 |       0 |                         
  cli/fakebin                     |       0 |      100 |     100 |       0 |                         
   npm-template.js                |       0 |      100 |     100 |       0 | 2,3,4,7,9               
+ cli/instrument-inplace          |       0 |      100 |       0 |       0 |                         
+  file1.js                       |       0 |      100 |       0 |       0 | 2,5                     
+  file2.js                       |       0 |      100 |       0 |       0 | 2,5                     
  cli/nyc-config-js               |       0 |        0 |     100 |       0 |                         
   ignore.js                      |       0 |      100 |     100 |       0 | 1                       
   index.js                       |       0 |        0 |     100 |       0 | 1,3,5,7,8,9,10          
@@ -126,7 +129,7 @@ exports[`test/nyc-integration.js TAP --ignore-class-method skips methods that ma
 ---------------------------------|---------|----------|---------|---------|-------------------------
 File                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s       
 ---------------------------------|---------|----------|---------|---------|-------------------------
-All files                        |    1.49 |        0 |    5.56 |    1.96 |                         
+All files                        |    1.45 |        0 |       5 |    1.89 |                         
  cli                             |    2.08 |        0 |    5.56 |    3.13 |                         
   args.js                        |       0 |      100 |     100 |       0 | 1                       
   by-arg2.js                     |       0 |        0 |     100 |       0 | 1,2,3,4,5,7             
@@ -144,6 +147,9 @@ All files                        |    1.49 |        0 |    5.56 |    1.96 |
   skip-full.js                   |       0 |      100 |     100 |       0 | 1,2                     
  cli/fakebin                     |       0 |      100 |     100 |       0 |                         
   npm-template.js                |       0 |      100 |     100 |       0 | 2,3,4,7,9               
+ cli/instrument-inplace          |       0 |      100 |       0 |       0 |                         
+  file1.js                       |       0 |      100 |       0 |       0 | 2,5                     
+  file2.js                       |       0 |      100 |       0 |       0 | 2,5                     
  cli/nyc-config-js               |       0 |        0 |     100 |       0 |                         
   ignore.js                      |       0 |      100 |     100 |       0 | 1                       
   index.js                       |       0 |        0 |     100 |       0 | 1,3,5,7,8,9,10          

test/fixtures/cli/instrument-inplace/file1.js

@@ -0,0 +1,5 @@
+function test() {
+  return 'file1'
+}
+
+module.exports = test

test/fixtures/cli/instrument-inplace/file2.js

@@ -0,0 +1,5 @@
+function test() {
+	return 'file2'
+}
+
+module.exports = test

test/nyc-integration-old.js

@@ -7,6 +7,7 @@ const fixturesCLI = path.resolve(__dirname, './fixtures/cli')
 const fakebin = path.resolve(fixturesCLI, 'fakebin')
 const fs = require('fs')
 const isWindows = require('is-windows')()
+const cpFile = require('cp-file')
 const rimraf = require('rimraf')
 const makeDir = require('make-dir')
 const { spawn } = require('child_process')
@@ -325,7 +326,7 @@ describe('the nyc cli', function () {
       })
 
       it('aborts if trying to write files in place', function (done) {
-        const args = [bin, 'instrument', '--delete', './', './']
+        const args = [bin, 'instrument', './', './']
 
         const proc = spawn(process.execPath, args, {
           cwd: fixturesCLI,
@@ -339,7 +340,54 @@ describe('the nyc cli', function () {
 
         proc.on('close', function (code) {
           code.should.equal(1)
-          stderr.should.include('nyc instrument failed: cannot instrument files in place')
+          stderr.should.include('cannot instrument files in place')
+          done()
+        })
+      })
+
+      it('can write files in place with --in-place switch', function (done) {
+        const args = [bin, 'instrument', '--in-place', '--include', '*/file1.js', './test-instrument-inplace']
+
+        const sourceDir = path.resolve(fixturesCLI, 'instrument-inplace')
+        const destDir = path.resolve(fixturesCLI, 'test-instrument-inplace')
+        makeDir.sync(destDir)
+        cpFile.sync(path.join(sourceDir, 'file1.js'), path.join(destDir, 'file1.js'))
+        cpFile.sync(path.join(sourceDir, 'file2.js'), path.join(destDir, 'file2.js'))
+
+        const proc = spawn(process.execPath, args, {
+          cwd: fixturesCLI,
+          env: env
+        })
+
+        proc.on('close', function (code) {
+          code.should.equal(0)
+          const file1 = path.resolve(destDir, 'file1.js')
+          fs.readFileSync(file1, 'utf8')
+            .should.match(/var cov_/)
+          const file2 = path.resolve(destDir, 'file2.js')
+          fs.readFileSync(file2, 'utf8')
+            .should.not.match(/var cov_/)
+          rimraf.sync(destDir)
+          done()
+        })
+      })
+
+      it('aborts if trying to delete while writing files in place', function (done) {
+        const args = [bin, 'instrument', '--in-place', '--delete', '--include', 'file1.js', './instrument-inplace']
+
+        const proc = spawn(process.execPath, args, {
+          cwd: fixturesCLI,
+          env: env
+        })
+
+        let stderr = ''
+        proc.stderr.on('data', function (chunk) {
+          stderr += chunk
+        })
+
+        proc.on('close', function (code) {
+          code.should.equal(1)
+          stderr.should.include(`cannot use '--delete' when instrumenting files in place`)
           done()
         })
       })
@@ -359,7 +407,7 @@ describe('the nyc cli', function () {
 
         proc.on('close', function (code) {
           code.should.equal(1)
-          stderr.should.include('nyc instrument failed: cannot instrument files outside of project root directory')
+          stderr.should.include('cannot instrument files outside project root directory')
           done()
         })
       })
@@ -421,7 +469,7 @@ describe('the nyc cli', function () {
 
           proc.on('close', function (code) {
             code.should.equal(1)
-            stderr.should.include('nyc instrument failed: attempt to delete')
+            stderr.should.include('attempt to delete')
             done()
           })
         })
@@ -441,7 +489,7 @@ describe('the nyc cli', function () {
 
           proc.on('close', function (code) {
             code.should.equal(1)
-            stderr.should.include('nyc instrument failed: attempt to delete')
+            stderr.should.include('attempt to delete')
             done()
           })
         })