WIP: Fix option namespace pollution

index.js

@@ -103,6 +103,8 @@ function Command(name) {
   this._allowUnknownOption = false;
   this._args = [];
   this._name = name || '';
+  this.optMap = new Map();
+  this._storeOptionsAsProperties = true; // legacy behaviour by default
 }
 
 /**
@@ -376,6 +378,11 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {
     oname = option.name(),
     name = option.attributeName();
 
+  if (self._storeOptionsAsProperties && self[name] !== undefined) {
+    console.log("Setup warning: option name '%s' conflicts with internal property", name);
+    console.log('   Either change option name, or setFeatureFlags with storeOptionsAsProperties:false and update code');
+  }
+
   // default as 3rd arg
   if (typeof fn !== 'function') {
     if (fn instanceof RegExp) {
@@ -396,7 +403,10 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {
     if (!option.bool) defaultValue = true;
     // preassign only if we have a default
     if (defaultValue !== undefined) {
-      self[name] = defaultValue;
+      self.optMap.set(name, defaultValue);
+      if (self._storeOptionsAsProperties) {
+        self[name] = defaultValue;
+      }
       option.defaultValue = defaultValue;
     }
   }
@@ -409,22 +419,28 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {
   this.on('option:' + oname, function(val) {
     // coercion
     if (val !== null && fn) {
-      val = fn(val, self[name] === undefined ? defaultValue : self[name]);
+      val = fn(val, self.optMap.has(name) ? self.optMap.get(name) : defaultValue);
     }
 
     // unassigned or bool
-    if (typeof self[name] === 'boolean' || typeof self[name] === 'undefined') {
+    if (typeof self.optMap.get(name) === 'boolean' || !self.optMap.has(name)) {
       // if no value, bool true, and we have a default, then use it!
       if (val == null) {
-        self[name] = option.bool
+        self.optMap.set(name, option.bool
           ? defaultValue || true
-          : false;
+          : false);
       } else {
-        self[name] = val;
+        self.optMap.set(name, val);
+      }
+      if (self._storeOptionsAsProperties) {
+        self[name] = self.optMap.get(name);
       }
     } else if (val !== null) {
       // reassign
-      self[name] = val;
+      self.optMap.set(name, val);
+      if (self._storeOptionsAsProperties) {
+        self[name] = val;
+      }
     }
   });
 
@@ -443,6 +459,23 @@ Command.prototype.allowUnknownOption = function(arg) {
   return this;
 };
 
+/**
+ * Set featureFlags.
+ *
+ * @param {Object} boolean values for feature flags
+ * @return {Command} for chaining
+ * @api public
+ */
+Command.prototype.setFeatureFlags = function(flags) {
+  const storeOptionsAsPropertiesStr = 'storeOptionsAsProperties';
+  if (typeof flags === 'object') {
+    if (Object.prototype.hasOwnProperty.call(flags, storeOptionsAsPropertiesStr)) {
+      this._storeOptionsAsProperties = flags[storeOptionsAsPropertiesStr];
+    }
+  }
+  return this;
+};
+
 /**
  * Parse `argv`, settings options and invoking commands when defined.
  *
@@ -771,13 +804,17 @@ Command.prototype.parseOptions = function(argv) {
  * @api public
  */
 Command.prototype.opts = function() {
-  var result = {},
-    len = this.options.length;
+  var result = {};
 
-  for (var i = 0; i < len; i++) {
-    var key = this.options[i].attributeName();
-    result[key] = key === this._versionOptionName ? this._version : this[key];
+  for (const [key, val] of this.optMap.entries()) {
+    result[key] = val;
   }
+
+  // Preserve legacy behaviour that version added into result.
+  if (this._versionOptionName) {
+    result[this._versionOptionName] = this._version;
+  }
+
   return result;
 };