Allow array value for "safeContextKeyword" rule

Fixes jscs-dev#198
markelog · Feb 2, 2014 · 9e03c9f · 9e03c9f
1 parent 41dcd74
commit 9e03c9f
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 42 deletions.
diff --git a/README.md b/README.md
@@ -1608,14 +1608,14 @@ var d = new e();
 
 Option to check `var that = this` expressions
 
-Type: `String`
+Type: 'Array' or `String`
 
 Values: String value used for context local declaration
 
 #### Example
 
 ```js
-"safeContextKeyword": "that"
+"safeContextKeyword": [ "that" ]
 ```
 
 ##### Valid

diff --git a/lib/rules/safe-context-keyword.js b/lib/rules/safe-context-keyword.js
@@ -4,32 +4,34 @@ module.exports = function() {};
 
 module.exports.prototype = {
 
-    configure: function(keyword) {
-        assert(typeof keyword === 'string', 'safeContextKeyword option requires string value');
+    configure: function(keywords) {
+        assert(
+            typeof keywords === 'string' ||
+            typeof Array.isArray(keywords),
+            'safeContextKeyword option requires string or array value'
+        );
 
-        this._keyword = keyword;
+        this._keywords = keywords;
     },
 
     getOptionName: function () {
         return 'safeContextKeyword';
     },
 
     check: function(file, errors) {
-        var keyword = this._keyword;
+        var keywords = typeof this._keywords === 'string' ? [ this._keywords ] : this._keywords;
 
         // var that = this
         file.iterateNodesByType('VariableDeclaration', function (node) {
-
             for (var i = 0; i < node.declarations.length; i++) {
                 var decl = node.declarations[i];
 
-                if (
-                    // decl.init === null in case of "var foo;"
-                    decl.init &&
-                    (decl.init.type === 'ThisExpression' && decl.id.name !== keyword)
+                // decl.init === null in case of "var foo;"
+                if (decl.init &&
+                    (decl.init.type === 'ThisExpression' && checkKeywords(decl.id.name, keywords))
                 ) {
                     errors.add(
-                        'You should use "' + keyword + '" to safe "this"',
+                        'You should use "' + keywords.join('" or "') + '" to safe "this"',
                         node.loc.start
                     );
                 }
@@ -42,14 +44,29 @@ module.exports.prototype = {
             if (
                 // filter property assignments "foo.bar = this"
                 node.left.type === 'Identifier' &&
-                (node.right.type === 'ThisExpression' && node.left.name !== keyword)
+                (node.right.type === 'ThisExpression' && checkKeywords(node.left.name, keywords))
             ) {
                 errors.add(
-                    'You should use "' + keyword + '" to safe "this"',
+                    'You should use "' + keywords.join('" or "') + '" to safe "this"',
                     node.loc.start
                 );
             }
         });
     }
-
 };
+
+/**
+ * Check if at least one keyword equals to passed name
+ * @param {String} name
+ * @param {Array} keywords
+ * @return {Boolean}
+ */
+function checkKeywords(name, keywords) {
+    for (var i = 0; i < keywords.length; i++) {
+        if (name === keywords[i]) {
+            return false;
+        }
+    }
+
+    return true;
+}
diff --git a/test/test.safe-context-keyword.js b/test/test.safe-context-keyword.js
@@ -4,48 +4,95 @@ var assert = require('assert');
 describe('rules/safe-context-keyword', function() {
     var checker;
 
-    beforeEach(function() {
-        checker = new Checker();
-        checker.registerDefaultRules();
-        checker.configure({ safeContextKeyword: 'that' });
-    });
+    describe('string argument', function() {
+        beforeEach(function() {
+            checker = new Checker();
+            checker.registerDefaultRules();
+            checker.configure({ safeContextKeyword: 'that' });
+        });
+
+        describe('var', function() {
+            it('should not report "var that = this"', function() {
+                assert(checker.checkString('var that = this;').isEmpty());
+            });
+
+            it('should report "var notthat = this"', function() {
+                assert(checker.checkString('var notthat = this;').getErrorCount() === 1);
+            });
 
-    describe('var', function() {
-        it('should not report "var that = this"', function() {
-            assert(checker.checkString('var that = this;').isEmpty());
+            it('should not report "var foo;"', function() {
+                assert(checker.checkString('var foo;').getErrorCount() === 0);
+            });
         });
 
-        it('should report "var notthat = this"', function() {
-            assert(checker.checkString('var notthat = this;').getErrorCount() === 1);
+        describe('without var', function() {
+            it('should not report "var that = this"', function() {
+                assert(checker.checkString('that = this;').isEmpty());
+            });
+
+            it('should report "var notthat = this"', function() {
+                assert(checker.checkString('notthat = this;').getErrorCount() === 1);
+            });
+
+            it('should not report propery assignment "foo.bar = this"', function() {
+                assert(checker.checkString('foo.bar = this').getErrorCount() === 0);
+            });
         });
 
-        it('should not report "var foo;"', function() {
-            assert(checker.checkString('var foo;').getErrorCount() === 0);
+        describe('multiple var decl', function() {
+            it('should not report "var that = this"', function() {
+                assert(checker.checkString('var a = 1, that = this;').isEmpty());
+            });
+
+            it('should report "var notthat = this"', function() {
+                assert(checker.checkString('var a = 1, notthat = this;').getErrorCount() === 1);
+            });
         });
     });
 
-    describe('withot var', function() {
-        it('should not report "var that = this"', function() {
-            assert(checker.checkString('that = this;').isEmpty());
+    describe('array argument', function() {
+        beforeEach(function() {
+            checker = new Checker();
+            checker.registerDefaultRules();
+            checker.configure({ safeContextKeyword: ['that', 'self'] });
         });
 
-        it('should report "var notthat = this"', function() {
-            assert(checker.checkString('notthat = this;').getErrorCount() === 1);
-        });
+        describe('var', function() {
+            it('should not report "var that = this, self = this"', function() {
+                assert(checker.checkString('var that = this, self = this;').isEmpty());
+            });
+
+            it('should report "var notthat = this"', function() {
+                assert(checker.checkString('var notthat = this;').getErrorCount() === 1);
+            });
 
-        it('should not report propery assignment "foo.bar = this"', function() {
-            assert(checker.checkString('foo.bar = this').getErrorCount() === 0);
+            it('should not report "var foo;"', function() {
+                assert(checker.checkString('var foo;').getErrorCount() === 0);
+            });
         });
-    });
 
-    describe('multiple var decl', function() {
-        it('should not report "var that = this"', function() {
-            assert(checker.checkString('var a = 1, that = this;').isEmpty());
+        describe('without var', function() {
+            it('should not report "var that = this"', function() {
+                assert(checker.checkString('that = this, self = this;').isEmpty());
+            });
+
+            it('should report "notthat = this, notself = this;"', function() {
+                assert(checker.checkString('notthat = this; notself = this;').getErrorCount() === 2);
+            });
+
+            it('should not report propery assignment "foo.bar = this; bar.foo = this"', function() {
+                assert(checker.checkString('foo.bar = this; bar.foo = this').getErrorCount() === 0);
+            });
         });
 
-        it('should report "var notthat = this"', function() {
-            assert(checker.checkString('var a = 1, notthat = this;').getErrorCount() === 1);
+        describe('multiple var decl', function() {
+            it('should not report "var that = this"', function() {
+                assert(checker.checkString('var a = 1, that = this, self = this;').isEmpty());
+            });
+
+            it('should report "var notthat = this"', function() {
+                assert(checker.checkString('var a = 1, notthat = this, notself = this;').getErrorCount() === 2);
+            });
         });
     });
-
 });