fix move when overwtite is true and dest exists

jprichardson · Feb 6, 2018 · 5431e49 · 5431e49
1 parent 7149691
commit 5431e49
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 62 deletions.
diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js
@@ -69,32 +69,6 @@ describe('+ move()', () => {
       })
     })
 
-    it('should throw error if src is file and dest is directory', done => {
-      const src = path.join(TEST_DIR, 'a-file')
-      const dest = path.join(TEST_DIR, 'a-folder')
-
-      // verify file exists already
-      assert(fs.existsSync(dest))
-
-      fse.move(src, dest, {overwrite: true}, err => {
-        assert.strictEqual(err.message, `Cannot move file '${src}' to directory '${dest}'.`)
-        done()
-      })
-    })
-
-    it('should throw error if src is directory and dest is file', done => {
-      const src = path.join(TEST_DIR, 'a-folder')
-      const dest = path.join(TEST_DIR, 'a-file')
-
-      // verify file exists already
-      assert(fs.existsSync(dest))
-
-      fse.move(src, dest, {overwrite: true}, err => {
-        assert.strictEqual(err.message, `Cannot move directory '${src}' to file '${dest}'.`)
-        done()
-      })
-    })
-
     it('should overwrite the destination directory', done => {
       // Create src
       const src = path.join(TEST_DIR, 'src')

diff --git a/lib/move/index.js b/lib/move/index.js
@@ -6,9 +6,7 @@ const path = require('path')
 const copy = require('../copy').copy
 const remove = require('../remove').remove
 const mkdirp = require('../mkdirs').mkdirp
-
-const destIsFile = Symbol('destIsFile')
-const destIsDir = Symbol('destIsDir')
+const pathExists = require('../path-exists').pathExists
 
 function move (src, dest, opts, cb) {
   if (typeof opts === 'function') {
@@ -26,41 +24,30 @@ function move (src, dest, opts, cb) {
   fs.stat(src, (err, st) => {
     if (err) return cb(err)
 
-    const srcIsDir = st.isDirectory()
-    if (srcIsDir && isSrcSubdir(src, dest)) return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`))
-
-    checkDest(dest, (err, res) => {
+    if (st.isDirectory() && isSrcSubdir(src, dest)) {
+      return cb(new Error(`Cannot move '${src}' to a subdirectory of itself, '${dest}'.`))
+    }
+    mkdirp(path.dirname(dest), err => {
       if (err) return cb(err)
-      if (!overwrite && res) return cb(new Error('dest already exists.'))
-      if (srcIsDir && res === destIsFile) return cb(new Error(`Cannot move directory '${src}' to file '${dest}'.`))
-      if (!srcIsDir && res === destIsDir) return cb(new Error(`Cannot move file '${src}' to directory '${dest}'.`))
       return doRename(src, dest, overwrite, cb)
     })
   })
 }
 
 function doRename (src, dest, overwrite, cb) {
-  mkdirp(path.dirname(dest), err => {
+  if (overwrite) {
+    return remove(dest, err => {
+      if (err) return cb(err)
+      return rename(src, dest, overwrite, cb)
+    })
+  }
+  pathExists(dest, (err, destExists) => {
     if (err) return cb(err)
-    if (overwrite) return renameOverwrite(src, dest, overwrite, cb)
+    if (destExists) return cb(new Error('dest already exists.'))
     return rename(src, dest, overwrite, cb)
   })
 }
 
-function renameOverwrite (src, dest, overwrite, cb) {
-  fs.rename(src, dest, err => {
-    if (!err) return cb()
-    if (err.code === 'ENOTEMPTY' || err.code === 'EEXIST' || err.code === 'EPERM') {
-      return remove(dest, err => {
-        if (err) return cb(err)
-        return move(src, dest, {overwrite: overwrite}, cb)
-      })
-    }
-    if (err.code !== 'EXDEV') return cb(err)
-    return moveAcrossDevice(src, dest, overwrite, cb)
-  })
-}
-
 function rename (src, dest, overwrite, cb) {
   fs.rename(src, dest, err => {
     if (!err) return cb()
@@ -82,16 +69,6 @@ function moveAcrossDevice (src, dest, overwrite, cb) {
   })
 }
 
-function checkDest (dest, cb) {
-  fs.stat(dest, (err, st) => {
-    if (err) {
-      if (err.code === 'ENOENT') return cb()
-      return cb(err)
-    }
-    return cb(null, st.isDirectory() ? destIsDir : destIsFile)
-  })
-}
-
 function isSrcSubdir (src, dest) {
   const srcArray = src.split(path.sep)
   const destArray = dest.split(path.sep)