From 199ec9f2c28bf14e7da62d131ced80f1255d5f79 Mon Sep 17 00:00:00 2001
From: Mani Maghsoudlou <manidlou@gmail.com>
Date: Thu, 9 Nov 2017 13:22:59 -0800
Subject: [PATCH] Apply filter before creating parent dir in copy

---
 .../copy-prevent-copying-identical.test.js    |  8 +--
 .../copy-prevent-copying-into-itself.test.js  | 66 +++++++++----------
 lib/copy/__tests__/copy.test.js               | 19 +++++-
 lib/copy/copy.js                              |  2 +
 4 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/lib/copy/__tests__/copy-prevent-copying-identical.test.js b/lib/copy/__tests__/copy-prevent-copying-identical.test.js
index 64357105..615645b5 100644
--- a/lib/copy/__tests__/copy-prevent-copying-identical.test.js
+++ b/lib/copy/__tests__/copy-prevent-copying-identical.test.js
@@ -8,21 +8,21 @@ const klawSync = require('klaw-sync')
 
 /* global beforeEach, afterEach, describe, it */
 
-describe('+ copySync() - prevent copying identical files and dirs', () => {
+describe('+ copy() - prevent copying identical files and dirs', () => {
   let TEST_DIR = ''
   let src = ''
   let dest = ''
 
   beforeEach(done => {
-    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical')
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-identical')
     fs.emptyDir(TEST_DIR, done)
   })
 
   afterEach(done => fs.remove(TEST_DIR, done))
 
   it('should return an error if src and dest are the same', done => {
-    const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
-    const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
+    const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy')
+    const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
 
     fs.copy(fileSrc, fileDest, err => {
       assert.equal(err.message, 'Source and destination must not be the same.')
diff --git a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
index d759a1a5..9fe0ef0e 100644
--- a/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
+++ b/lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
@@ -21,43 +21,11 @@ const dat1 = 'file1'
 const dat2 = 'file2'
 const dat3 = 'file3'
 
-function testSuccess (src, dest, done) {
-  const srclen = klawSync(src).length
-  assert(srclen > 2)
-  fs.copy(src, dest, err => {
-    assert.ifError(err)
-
-    const destlen = klawSync(dest).length
-
-    assert.strictEqual(destlen, srclen)
-
-    FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied'))
-
-    const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8')
-    const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8')
-    const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8')
-    const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8')
-
-    assert.strictEqual(o0, dat0, 'file contents matched')
-    assert.strictEqual(o1, dat1, 'file contents matched')
-    assert.strictEqual(o2, dat2, 'file contents matched')
-    assert.strictEqual(o3, dat3, 'file contents matched')
-    done()
-  })
-}
-
-function testError (src, dest, done) {
-  fs.copy(src, dest, err => {
-    assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)
-    done()
-  })
-}
-
 describe('+ copy() - prevent copying into itself', () => {
   let TEST_DIR, src
 
   beforeEach(done => {
-    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself-4')
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-prevent-copying-into-itself')
     src = path.join(TEST_DIR, 'src')
     fs.mkdirpSync(src)
 
@@ -370,3 +338,35 @@ describe('+ copy() - prevent copying into itself', () => {
     })
   })
 })
+
+function testSuccess (src, dest, done) {
+  const srclen = klawSync(src).length
+  assert(srclen > 2)
+  fs.copy(src, dest, err => {
+    assert.ifError(err)
+
+    const destlen = klawSync(dest).length
+
+    assert.strictEqual(destlen, srclen)
+
+    FILES.forEach(f => assert(fs.existsSync(path.join(dest, f)), 'file copied'))
+
+    const o0 = fs.readFileSync(path.join(dest, FILES[0]), 'utf8')
+    const o1 = fs.readFileSync(path.join(dest, FILES[1]), 'utf8')
+    const o2 = fs.readFileSync(path.join(dest, FILES[2]), 'utf8')
+    const o3 = fs.readFileSync(path.join(dest, FILES[3]), 'utf8')
+
+    assert.strictEqual(o0, dat0, 'file contents matched')
+    assert.strictEqual(o1, dat1, 'file contents matched')
+    assert.strictEqual(o2, dat2, 'file contents matched')
+    assert.strictEqual(o3, dat3, 'file contents matched')
+    done()
+  })
+}
+
+function testError (src, dest, done) {
+  fs.copy(src, dest, err => {
+    assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${dest}'.`)
+    done()
+  })
+}
diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js
index 4787bfd8..d8e38f63 100644
--- a/lib/copy/__tests__/copy.test.js
+++ b/lib/copy/__tests__/copy.test.js
@@ -208,6 +208,21 @@ describe('fs-extra', () => {
     })
 
     describe('> when filter is used', () => {
+      it('should do nothing if filter fails', done => {
+        const srcDir = path.join(TEST_DIR, 'src')
+        const srcFile = path.join(srcDir, 'srcfile.css')
+        fse.outputFileSync(srcFile, 'src contents')
+        const destDir = path.join(TEST_DIR, 'dest')
+        const destFile = path.join(destDir, 'destfile.css')
+        const filter = s => path.extname(s) !== '.css' && !fs.statSync(s).isDirectory()
+
+        fse.copy(srcFile, destFile, filter, err => {
+          assert.ifError(err)
+          assert(!fs.existsSync(destDir))
+          done()
+        })
+      })
+
       it('should only copy files allowed by filter fn', done => {
         const srcFile1 = path.join(TEST_DIR, '1.css')
         fs.writeFileSync(srcFile1, '')
@@ -234,7 +249,7 @@ describe('fs-extra', () => {
         })
       })
 
-      it('should should apply filter recursively', done => {
+      it('should apply filter recursively', done => {
         const FILES = 2
         // Don't match anything that ends with a digit higher than 0:
         const filter = s => /(0|\D)$/i.test(s)
@@ -280,7 +295,7 @@ describe('fs-extra', () => {
         })
       })
 
-      it('should apply the filter to directory names', done => {
+      it('should apply filter to directory names', done => {
         const IGNORE = 'ignore'
         const filter = p => !~p.indexOf(IGNORE)
 
diff --git a/lib/copy/copy.js b/lib/copy/copy.js
index 76aaa23c..f270c1d7 100644
--- a/lib/copy/copy.js
+++ b/lib/copy/copy.js
@@ -35,6 +35,8 @@ function copy (src, dest, opts, cb) {
   // don't allow src and dest to be the same
   if (src === dest) return cb(new Error('Source and destination must not be the same.'))
 
+  if (opts.filter && !opts.filter(src, dest)) return cb()
+
   const destParent = path.dirname(dest)
   pathExists(destParent, (err, dirExists) => {
     if (err) return cb(err)