Breaking: Normalize paths (closes #80) (#101)

Author: darsain

README.md

@@ -45,6 +45,8 @@ File.isCustomProp('path'); // false -> internal getter/setter
 Read more in [Extending Vinyl](#extending-vinyl).
 
 ### constructor(options)
+All internally managed paths (`cwd`, `base`, `path`, `[history]`) are normalized and remove a trailing separator.
+
 #### options.cwd
 Type: `String`<br><br>Default: `process.cwd()`
 
@@ -64,7 +66,7 @@ Path history. Has no effect if `options.path` is passed.
 Type: `Array`<br><br>Default: `options.path ? [options.path] : []`
 
 #### options.stat
-The result of an fs.stat call. See [fs.Stats](http://nodejs.org/api/fs.html#fs_class_fs_stats) for more information.
+The result of an fs.stat call. This is how you mark the file as a directory. See [isDirectory()](#isDirectory) and [fs.Stats](http://nodejs.org/api/fs.html#fs_class_fs_stats) for more information.
 
 Type: `fs.Stats`<br><br>Default: `null`
 
@@ -92,6 +94,15 @@ Returns true if file.contents is a Stream.
 ### isNull()
 Returns true if file.contents is null.
 
+### isDirectory()
+Returns true if file is a directory. File is considered a directory when:
+
+- `file.isNull()` is `true`
+- `file.stat` is an object
+- `file.stat.isDirectory()` returns `true`
+
+When constructing a Vinyl object, pass in a valid `fs.Stats` object via `options.stat`. Some operations in Vinyl might need to know the file is a directory from the get go. If you are mocking the `fs.Stats` object, ensure it has the `isDirectory()` method.
+
 ### clone([opt])
 Returns a new File object with all attributes cloned.<br>By default custom attributes are deep-cloned.
 
@@ -124,8 +135,14 @@ if (file.isBuffer()) {
 }
 ```
 
+### cwd
+Gets and sets current working directory. Defaults to `process.cwd`. Will always be normalized and remove a trailing separator.
+
+### base
+Gets and sets base directory. Used for relative pathing (typically where a glob starts). When `null` or `undefined`, it simply proxies the `file.cwd` property. Will always be normalized and remove a trailing separator.
+
 ### path
-Absolute pathname string or `undefined`. Setting to a different value pushes the old value to `history`.
+Absolute pathname string or `undefined`. Setting to a different value pushes the old value to `history`. All new values are normalized and remove a trailing separator.
 
 ### history
 Array of `path` values the file object has had, from `history[0]` (original) through `history[history.length - 1]` (current). `history` and its elements should normally be treated as read-only and only altered indirectly by setting `path`.
@@ -146,7 +163,7 @@ console.log(file.relative); // file.coffee
 ```
 
 ### dirname
-Gets and sets path.dirname for the file path.
+Gets and sets path.dirname for the file path. Will always be normalized and remove a trailing separator.
 
 Example:
 
@@ -225,6 +242,24 @@ console.log(file.extname); // .js
 console.log(file.path); // /test/file.js
 ```
 
+### symlink
+Path where the file points to in case it's a symbolic link. Will always be normalized and remove a trailing separator.
+
+## Normalization and concatenation
+Since all properties are normalized in their setters, you can just concatenate with `/`, and normalization takes care of it properly on all platforms.
+
+Example:
+
+```javascript
+var file = new File();
+file.path = '/' + 'test' + '/' + 'foo.bar';
+console.log(file.path);
+// posix => /test/foo.bar
+// win32 => \\test\\foo.bar
+```
+
+But never concatenate with `\`, since that is a valid filename character on posix system.
+
 ## Extending Vinyl
 When extending Vinyl into your own class with extra features, you need to think about a few things.
 

index.js

@@ -2,15 +2,18 @@ var path = require('path');
 var clone = require('clone');
 var cloneStats = require('clone-stats');
 var cloneBuffer = require('./lib/cloneBuffer');
+var stripTrailingSep = require('./lib/stripTrailingSep');
 var isBuffer = require('./lib/isBuffer');
 var isStream = require('./lib/isStream');
 var isNull = require('./lib/isNull');
 var inspectStream = require('./lib/inspectStream');
+var normalize = require('./lib/normalize');
 var Stream = require('stream');
 var replaceExt = require('replace-ext');
 
 var builtInFields = [
-  '_contents', '_symlink', 'contents', 'stat', 'history', 'path', 'base', 'cwd',
+  '_contents', '_symlink', 'contents', 'stat', 'history', 'path',
+  '_base', 'base', '_cwd', 'cwd',
 ];
 
 function File(file) {
@@ -20,19 +23,22 @@ function File(file) {
     file = {};
   }
 
-  // Record path change
-  var history = file.path ? [file.path] : file.history;
-  this.history = history || [];
-
-  this.cwd = file.cwd || process.cwd();
-  this.base = file.base || this.cwd;
-
   // Stat = files stats object
   this.stat = file.stat || null;
 
   // Contents = stream, buffer, or null if not read
   this.contents = file.contents || null;
 
+  // Replay path history to ensure proper normalization and trailing sep
+  var history = file.path ? [file.path] : file.history || [];
+  this.history = [];
+  history.forEach(function(path) {
+    self.path = path;
+  });
+
+  this.cwd = file.cwd || process.cwd();
+  this.base = file.base;
+
   this._isVinyl = true;
 
   this._symlink = null;
@@ -156,7 +162,7 @@ File.prototype.inspect = function() {
   var inspect = [];
 
   // Use relative path if possible
-  var filePath = (this.base && this.path) ? this.relative : this.path;
+  var filePath = this.path ? this.relative : null;
 
   if (filePath) {
     inspect.push('"' + filePath + '"');
@@ -195,12 +201,40 @@ Object.defineProperty(File.prototype, 'contents', {
   },
 });
 
+Object.defineProperty(File.prototype, 'cwd', {
+  get: function() {
+    return this._cwd;
+  },
+  set: function(cwd) {
+    if (!cwd || typeof cwd !== 'string') {
+      throw new Error('cwd must be a non-empty string.');
+    }
+    this._cwd = stripTrailingSep(normalize(cwd));
+  },
+});
+
+Object.defineProperty(File.prototype, 'base', {
+  get: function() {
+    return this._base || this._cwd;
+  },
+  set: function(base) {
+    if (base == null) {
+      delete this._base;
+      return;
+    }
+    if (typeof base !== 'string' || !base) {
+      throw new Error('base must be a non-empty string, or null/undefined.');
+    }
+    base = stripTrailingSep(normalize(base));
+    if (base !== this._cwd) {
+      this._base = base;
+    }
+  },
+});
+
 // TODO: Should this be moved to vinyl-fs?
 Object.defineProperty(File.prototype, 'relative', {
   get: function() {
-    if (!this.base) {
-      throw new Error('No base specified! Can not get relative.');
-    }
     if (!this.path) {
       throw new Error('No path specified! Can not get relative.');
     }
@@ -222,7 +256,7 @@ Object.defineProperty(File.prototype, 'dirname', {
     if (!this.path) {
       throw new Error('No path specified! Can not set dirname.');
     }
-    this.path = path.join(dirname, path.basename(this.path));
+    this.path = path.join(dirname, this.basename);
   },
 });
 
@@ -237,7 +271,7 @@ Object.defineProperty(File.prototype, 'basename', {
     if (!this.path) {
       throw new Error('No path specified! Can not set basename.');
     }
-    this.path = path.join(path.dirname(this.path), basename);
+    this.path = path.join(this.dirname, basename);
   },
 });
 
@@ -253,7 +287,7 @@ Object.defineProperty(File.prototype, 'stem', {
     if (!this.path) {
       throw new Error('No path specified! Can not set stem.');
     }
-    this.path = path.join(path.dirname(this.path), stem + this.extname);
+    this.path = path.join(this.dirname, stem + this.extname);
   },
 });
 
@@ -278,8 +312,9 @@ Object.defineProperty(File.prototype, 'path', {
   },
   set: function(path) {
     if (typeof path !== 'string') {
-      throw new Error('path should be string');
+      throw new Error('path should be a string.');
     }
+    path = stripTrailingSep(normalize(path));
 
     // Record history only when path changed
     if (path && path !== this.path) {
@@ -298,7 +333,7 @@ Object.defineProperty(File.prototype, 'symlink', {
       throw new Error('symlink should be a string');
     }
 
-    this._symlink = symlink;
+    this._symlink = stripTrailingSep(normalize(symlink));
   },
 });
 

lib/normalize.js

@@ -0,0 +1,5 @@
+var normalize = require('path').normalize;
+
+module.exports = function(str) {
+  return str === '' ? str : normalize(str);
+};

lib/stripTrailingSep.js

@@ -0,0 +1,11 @@
+module.exports = function(str) {
+  while (endsInSeparator(str)) {
+    str = str.slice(0, -1);
+  }
+  return str;
+};
+
+function endsInSeparator(str) {
+  var last = str[str.length - 1];
+  return str.length > 1 && (last === '/' || last === '\\');
+}