Migrate from detect-conflict to diff (JsDiff) (#1132)

* Change bail test to a constructor

* Reimplement detect-conflict with diff

* Add skipRegenerate option to conflicter.

This will skip a rewrite on identical files.

* Bail on create new file too.

* Update conflicter.js

* Update index.js

lib/index.js

@@ -116,11 +116,12 @@ class Generator extends EventEmitter {
     this.async = () => () => {};
 
     this.fs = FileEditor.create(this.env.sharedFs);
-    this.conflicter = new Conflicter(
-      this.env.adapter,
-      this.options.force,
-      this.options.bail
-    );
+
+    this.conflicter = new Conflicter(this.env.adapter, this.options.force, {
+      bail: this.options.bail,
+      ignoreWhitespace: this.options.whitespace,
+      skipRegenerate: this.options.skipRegenerate
+    });
 
     // Mirror the adapter log method on the generator.
     //

lib/util/conflicter.js

@@ -2,7 +2,7 @@
 const fs = require('fs');
 const path = require('path');
 const async = require('async');
-const detectConflict = require('detect-conflict');
+const jsdiff = require('diff');
 const _ = require('lodash');
 const typedError = require('error/typed');
 const binaryDiff = require('./binary-diff');
@@ -35,11 +35,27 @@ const ConflictError = typedError({
  *                           testing reproducibility)
  */
 class Conflicter {
-  constructor(adapter, force, bail = false) {
+  constructor(adapter, force, options = {}) {
+    if (typeof options === 'boolean') {
+      this.bail = options;
+    } else {
+      this.bail = options.bail;
+      this.ignoreWhitespace = options.ignoreWhitespace;
+      this.skipRegenerate = options.skipRegenerate;
+    }
+
     this.force = force === true;
-    this.bail = bail === true;
     this.adapter = adapter;
     this.conflicts = [];
+
+    this.diffOptions = options.diffOptions;
+
+    if (this.bail) {
+      // Set ignoreWhitespace as true by default for bail.
+      // Probably just testing, so don't override.
+      this.ignoreWhitespace = true;
+      this.skipRegenerate = true;
+    }
   }
 
   /**
@@ -104,12 +120,70 @@ class Conflicter {
    * @param  {Object}   file File object respecting this interface: { path, contents }
    */
   _printDiff(file) {
-    if (binaryDiff.isBinary(file.path, file.contents)) {
+    if (file.binary === undefined) {
+      file.binary = binaryDiff.isBinary(file.path, file.contents);
+    }
+
+    if (file.binary) {
       this.adapter.log.writeln(binaryDiff.diff(file.path, file.contents));
     } else {
       const existing = fs.readFileSync(file.path);
-      this.adapter.diff(existing.toString(), (file.contents || '').toString());
+      this.adapter.diff(
+        existing.toString(),
+        (file.contents || '').toString(),
+        file.changes
+      );
+    }
+  }
+
+  /**
+   * Detect conflicts between file contents at `filepath` with the `contents` passed to the
+   * function
+   *
+   * If `filepath` points to a folder, we'll always return true.
+   *
+   * Based on detect-conflict module
+   *
+   * @param  {Object}   file File object respecting this interface: { path, contents }
+   * @return {Boolean} `true` if there's a conflict, `false` otherwise.
+   */
+  _detectConflict(file) {
+    let contents = file.contents;
+    const filepath = path.resolve(file.path);
+
+    // If file path point to a directory, then it's not safe to write
+    if (fs.statSync(filepath).isDirectory()) return true;
+
+    if (file.binary === undefined) {
+      file.binary = binaryDiff.isBinary(file.path, file.contents);
+    }
+
+    const actual = fs.readFileSync(path.resolve(filepath));
+
+    if (!(contents instanceof Buffer)) {
+      contents = Buffer.from(contents || '', 'utf8');
     }
+
+    if (file.binary) {
+      return actual.toString('hex') !== contents.toString('hex');
+    }
+
+    if (this.ignoreWhitespace) {
+      file.changes = jsdiff.diffWords(
+        actual.toString(),
+        contents.toString(),
+        this.diffOptions
+      );
+    } else {
+      file.changes = jsdiff.diffLines(
+        actual.toString(),
+        contents.toString(),
+        this.diffOptions
+      );
+    }
+
+    const changes = file.changes;
+    return changes.length > 1 || changes[0].added || changes[0].removed;
   }
 
   /**
@@ -131,6 +205,11 @@ class Conflicter {
 
     if (!fs.existsSync(file.path)) {
       this.adapter.log.create(rfilepath);
+      if (this.bail) {
+        this.adapter.log.writeln('Aborting ...');
+        throw new ConflictError();
+      }
+
       cb('create');
       return;
     }
@@ -141,7 +220,7 @@ class Conflicter {
       return;
     }
 
-    if (detectConflict(file.path, file.contents)) {
+    if (this._detectConflict(file)) {
       this.adapter.log.conflict(rfilepath);
       if (this.bail) {
         this._printDiff(file);
@@ -152,7 +231,11 @@ class Conflicter {
       this._ask(file, cb);
     } else {
       this.adapter.log.identical(rfilepath);
-      cb('identical');
+      if (this.skipRegenerate) {
+        cb('skip');
+      } else {
+        cb('identical');
+      }
     }
   }
 

package.json

@@ -61,7 +61,7 @@
     "dargs": "^6.1.0",
     "dateformat": "^3.0.3",
     "debug": "^4.1.1",
-    "detect-conflict": "^1.0.0",
+    "diff": "^4.0.1",
     "error": "^7.0.2",
     "find-up": "^3.0.0",
     "github-username": "^3.0.0",

test/conflicter.js

@@ -115,14 +115,102 @@ describe('Conflicter', () => {
     });
 
     it('abort on first conflict', function(done) {
-      this.conflicter.bail = true;
+      const conflicter = new Conflicter(new TestAdapter(), false, true);
       assert.throws(
-        this.conflicter.collision.bind(this.conflicter, this.conflictingFile),
+        conflicter.collision.bind(conflicter, this.conflictingFile),
         /^ConflicterConflictError: Process aborted by conflict$/
       );
       done();
     });
 
+    it('abort on first conflict with whitespace changes', function(done) {
+      const conflicter = new Conflicter(new TestAdapter(), false, {
+        bail: true
+      });
+      conflicter.collision(
+        {
+          path: path.join(__dirname, 'fixtures/file-conflict.txt'),
+          contents: `initial
+                 content
+      `
+        },
+        status => {
+          assert.equal(status, 'skip');
+          done();
+        }
+      );
+    });
+
+    it('abort on create new file', function(done) {
+      const conflicter = new Conflicter(new TestAdapter(), false, {
+        bail: true
+      });
+      assert.throws(
+        conflicter.collision.bind(conflicter, {
+          path: 'file-who-does-not-exist2.js',
+          contents: ''
+        }),
+        /^ConflicterConflictError: Process aborted by conflict$/
+      );
+      done();
+    });
+
+    it('does not give a conflict with ignoreWhitespace', function(done) {
+      const conflicter = new Conflicter(new TestAdapter(), false, {
+        ignoreWhitespace: true
+      });
+
+      conflicter.collision(
+        {
+          path: path.join(__dirname, 'fixtures/file-conflict.txt'),
+          contents: `initial
+           content
+`
+        },
+        status => {
+          assert.equal(status, 'identical');
+          done();
+        }
+      );
+    });
+
+    it('skip rewrite with ignoreWhitespace and skipRegenerate', function(done) {
+      const conflicter = new Conflicter(new TestAdapter(), false, {
+        ignoreWhitespace: true,
+        skipRegenerate: true
+      });
+
+      conflicter.collision(
+        {
+          path: path.join(__dirname, 'fixtures/file-conflict.txt'),
+          contents: `initial
+           content
+`
+        },
+        status => {
+          assert.equal(status, 'skip');
+          done();
+        }
+      );
+    });
+
+    it('does give a conflict without ignoreWhitespace', function(done) {
+      const conflicter = new Conflicter(new TestAdapter({ action: 'skip' }));
+
+      conflicter.collision(
+        {
+          path: path.join(__dirname, 'fixtures/file-conflict.txt'),
+          contents: `initial
+           content
+`
+        },
+        status => {
+          assert.equal(status, 'skip');
+          done();
+        }
+      );
+    });
+
     it('does not give a conflict on same binary files', function(done) {
       this.conflicter.collision(
         {