requireDotNotation: require dots for es3 keywords when not in es3 mode

Fixes #161 Closes gh-825
jscs-dev · Dec 31, 2014 · 97f30d4 · 97f30d4
1 parent 6a6c4ec
commit 97f30d4
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 67 deletions.
diff --git a/README.md b/README.md
@@ -2917,18 +2917,19 @@ var d = new e();
 
 ### requireDotNotation
 
-Requires member expressions to use dot notation when possible
+Requires member expressions to use dot notation when possible. Note, if you specify the --es3 option to JSCS, ES3 keywords and future reserved words MUST remain quoted.
 
 Type: `Boolean`
 
 Values: `true`
 
 JSHint: [`sub`](http://www.jshint.com/docs/options/#sub)
 
-#### Example
+#### Example for `"es3": true`
 
 ```js
-"requireDotNotation": true
+"requireDotNotation": true,
+"es3": true
 ```
 
 ##### Valid
@@ -2947,6 +2948,30 @@ var a = b['while']; //reserved word
 var a = b['c'];
 ```
 
+#### Example for `"es3": false` or `"es3": null`
+
+```js
+"requireDotNotation": true,
+"es3": false
+```
+
+##### Valid
+
+```js
+var a = b[c];
+var a = b.c;
+var a = b[c.d];
+var a = b[1];
+var a = b.while;
+```
+
+##### Invalid
+
+```js
+var a = b['c'];
+var a = b['while']; //reserved words can be property names in ES5
+```
+
 ### requireYodaConditions
 
 Requires the variable to be the right hand operator when doing a boolean comparison

diff --git a/bin/jscs b/bin/jscs
@@ -15,7 +15,7 @@ program
     .usage('[options] <file ...>')
     .option('-c, --config [path]', 'configuration file path')
     .option('-e, --esnext', 'attempts to parse esnext code (currently es6)')
-    .option('-t, --es3', 'validates code as es3')
+    .option('--es3', 'validates code as es3')
     .option('-s, --esprima <path>', 'attempts to use a custom version of Esprima')
     .option('-n, --no-colors', 'clean output without colors')
     .option('-p, --preset <preset>', 'preset config')

diff --git a/lib/js-file.js b/lib/js-file.js
@@ -368,20 +368,20 @@ JsFile.prototype = {
         });
     },
     /**
-     * Returns whether this file supports es3.
+     * Returns which dialect of JS this file supports.
      *
-     * @returns {Boolean}
-     */
-    isES3Enabled: function() {
-        return this._es3;
-    },
-    /**
-     * Returns whether this file supports es6.
-     *
-     * @returns {Boolean}
+     * @returns {String}
      */
-    isES6Enabled: function() {
-        return this._es6;
+    getDialect: function() {
+        if (this._es6) {
+            return 'es6';
+        }
+
+        if (this._es3) {
+            return 'es3';
+        }
+
+        return 'es5';
     },
     /**
      * Returns string representing contents of the file.

diff --git a/lib/rules/require-dot-notation.js b/lib/rules/require-dot-notation.js
@@ -21,6 +21,10 @@ module.exports.prototype = {
     },
 
     check: function(file, errors) {
+        function isES3Allowed(value) {
+            return file.getDialect() === 'es3' && (utils.isEs3Keyword(value) || utils.isEs3FutureReservedWord(value));
+        }
+
         file.iterateNodesByType('MemberExpression', function(node) {
             if (!node.computed || node.property.type !== 'Literal') {
                 return;
@@ -36,8 +40,7 @@ module.exports.prototype = {
                 value === 'null' ||
                 value === 'true' ||
                 value === 'false' ||
-                utils.isEs3Keyword(value) ||
-                utils.isEs3FutureReservedWord(value)
+                isES3Allowed(value)
             ) {
                 return;
             }

diff --git a/test/js-file.js b/test/js-file.js
@@ -690,43 +690,41 @@ describe('modules/js-file', function() {
         });
     });
 
-    describe('isES3Enabled', function() {
-        it('should return false when unspecified', function() {
+    describe('getDialect', function() {
+        it('should return es5 with no options specified', function() {
             var sources = 'var x = 1;\nvar y = 2;';
             var file = createJsFile(sources);
-            assert.equal(file.isES3Enabled(), false);
+            assert.equal(file.getDialect(), 'es5');
         });
 
-        it('should return false when specified', function() {
+        it('should return es6 when es6 is specified as true', function() {
             var sources = 'var x = 1;\nvar y = 2;';
-            var file = createJsFile(sources, {es3: false});
-            assert.equal(file.isES3Enabled(), false);
+            var file = createJsFile(sources, {es6: true});
+            assert.equal(file.getDialect(), 'es6');
         });
 
-        it('should return true when specified', function() {
+        it('should return es5 when es6 is specified as false', function() {
             var sources = 'var x = 1;\nvar y = 2;';
-            var file = createJsFile(sources, {es3: true});
-            assert.equal(file.isES3Enabled(), true);
+            var file = createJsFile(sources, {es6: false});
+            assert.equal(file.getDialect(), 'es5');
         });
-    });
 
-    describe('isES6Enabled', function() {
-        it('should return false when unspecified', function() {
+        it('should return es3 when es3 is specified as true', function() {
             var sources = 'var x = 1;\nvar y = 2;';
-            var file = createJsFile(sources);
-            assert.equal(file.isES6Enabled(), false);
+            var file = createJsFile(sources, {es3: true});
+            assert.equal(file.getDialect(), 'es3');
         });
 
-        it('should return false when specified', function() {
+        it('should return es5 when es3 is specified as false', function() {
             var sources = 'var x = 1;\nvar y = 2;';
-            var file = createJsFile(sources, {es6: false});
-            assert.equal(file.isES6Enabled(), false);
+            var file = createJsFile(sources, {es3: false});
+            assert.equal(file.getDialect(), 'es5');
         });
 
-        it('should return true when specified', function() {
+        it('should return es6 when es3 and es6 are both specified as true', function() {
             var sources = 'var x = 1;\nvar y = 2;';
-            var file = createJsFile(sources, {es6: true});
-            assert.equal(file.isES6Enabled(), true);
+            var file = createJsFile(sources, {es3: true, es6: true});
+            assert.equal(file.getDialect(), 'es6');
         });
     });
 

diff --git a/test/rules/require-dot-notation.js b/test/rules/require-dot-notation.js
@@ -7,44 +7,71 @@ describe('rules/require-dot-notation', function() {
     beforeEach(function() {
         checker = new Checker();
         checker.registerDefaultRules();
-        checker.configure({ requireDotNotation: true });
     });
 
-    it('should report literal subscription', function() {
-        assert(checker.checkString('var x = a[\'b\']').getErrorCount() === 1);
-    });
+    describe('true value', function() {
+        beforeEach(function() {
+            checker.configure({ requireDotNotation: true });
+        });
 
-    it('should not report literal subscription for reserved words', function() {
-        assert(checker.checkString('var x = a[\'while\']').isEmpty());
-        assert(checker.checkString('var x = a[null]').isEmpty());
-        assert(checker.checkString('var x = a[true]').isEmpty());
-        assert(checker.checkString('var x = a[false]').isEmpty());
-        assert(checker.checkString('var x = a["null"]').isEmpty());
-        assert(checker.checkString('var x = a["true"]').isEmpty());
-        assert(checker.checkString('var x = a["false"]').isEmpty());
-    });
+        it('should report literal subscription', function() {
+            assert.equal(checker.checkString('var x = a[\'b\']').getErrorCount(), 1);
+        });
 
-    it('should not report number subscription', function() {
-        assert(checker.checkString('var x = a[1]').isEmpty());
-    });
+        it('should not report literal subscription for reserved words', function() {
+            assert(checker.checkString('var x = a[\'while\']').isEmpty());
+            assert(checker.checkString('var x = a[null]').isEmpty());
+            assert(checker.checkString('var x = a[true]').isEmpty());
+            assert(checker.checkString('var x = a[false]').isEmpty());
+            assert(checker.checkString('var x = a["null"]').isEmpty());
+            assert(checker.checkString('var x = a["true"]').isEmpty());
+            assert(checker.checkString('var x = a["false"]').isEmpty());
+        });
 
-    it('should not report variable subscription', function() {
-        assert(checker.checkString('var x = a[c]').isEmpty());
-    });
+        it('should not report number subscription', function() {
+            assert(checker.checkString('var x = a[1]').isEmpty());
+        });
+
+        it('should not report variable subscription', function() {
+            assert(checker.checkString('var x = a[c]').isEmpty());
+        });
 
-    it('should not report object property subscription', function() {
-        assert(checker.checkString('var x = a[b.c]').isEmpty());
+        it('should not report object property subscription', function() {
+            assert(checker.checkString('var x = a[b.c]').isEmpty());
+        });
+
+        it('should not report dot notation', function() {
+            assert(checker.checkString('var x = a.b').isEmpty());
+        });
+
+        it('should not report for string that can\'t be identifier', function() {
+            assert(checker.checkString('x["a-b"]').isEmpty());
+            assert(checker.checkString('x["a.b"]').isEmpty());
+            assert(checker.checkString('x["a b"]').isEmpty());
+            assert(checker.checkString('x["1a"]').isEmpty());
+            assert(checker.checkString('x["*"]').isEmpty());
+        });
     });
 
-    it('should not report dot notation', function() {
-        assert(checker.checkString('var x = a.b').isEmpty());
+    describe('true value with es3 explicitly enabled', function() {
+        beforeEach(function() {
+            checker.configure({ es3: true, requireDotNotation: true });
+        });
+
+        it('should not report literal subscription for es3 keywords or future reserved words', function() {
+            assert(checker.checkString('var x = a[\'while\']').isEmpty());
+            assert(checker.checkString('var x = a[\'abstract\']').isEmpty());
+        });
     });
 
-    it('should not report for string that can\'t be identifier', function() {
-        assert(checker.checkString('x["a-b"]').isEmpty());
-        assert(checker.checkString('x["a.b"]').isEmpty());
-        assert(checker.checkString('x["a b"]').isEmpty());
-        assert(checker.checkString('x["1a"]').isEmpty());
-        assert(checker.checkString('x["*"]').isEmpty());
+    describe('true value with es3 explicitly disabled', function() {
+        beforeEach(function() {
+            checker.configure({ es3: false, requireDotNotation: true });
+        });
+
+        it('should not report literal subscription for es3 keywords or future reserved words', function() {
+            assert.equal(checker.checkString('var x = a[\'while\']').getErrorCount(), 1);
+            assert.equal(checker.checkString('var x = a[\'abstract\']').getErrorCount(), 1);
+        });
     });
 });