From 375402dd25fd454a1edea6f5051a42e4513a7172 Mon Sep 17 00:00:00 2001 From: Mike Sherov Date: Mon, 8 Dec 2014 11:07:03 -0500 Subject: [PATCH 1/2] Configuration: ability to specify and query es3/es6 support in files. --- README.md | 17 ++++++++++++ bin/jscs | 1 + lib/config/configuration.js | 20 ++++++++++++++ lib/config/node-configuration.js | 3 ++- lib/js-file.js | 24 ++++++++++++++++- lib/string-checker.js | 7 ++++- test/config/configuration.js | 22 +++++++++++++++ test/config/node-configuration.js | 4 ++- test/js-file.js | 45 +++++++++++++++++++++++++++++-- 9 files changed, 137 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 79890b641..12c42d5a2 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,9 @@ But you also can specify your own reporter, since this flag accepts relative or jscs path[ path[...]] --reporter=./some-dir/my-reporter.js ``` +### `--es3` +Validates your code as targetted for an ES3 environment. This effects the behavior of certain rules, e.g., requireDotNotation. + ### `--esnext` Attempts to parse your code as ES6 using the harmony version of the esprima parser. Please note that this is currently experimental, and will improve over time. @@ -226,6 +229,20 @@ Default: Infinity "maxErrors": 10 ``` +### es3 + +Validates your code as targetted for an ES3 environment. This effects the behavior of certain rules, e.g., requireDotNotation. + +Type: `Boolean` + +Value: `true` + +#### Example + +```js +"es3": true +``` + ### esnext Attempts to parse your code as ES6 using the harmony version of the esprima parser. diff --git a/bin/jscs b/bin/jscs index 926346086..37f96653d 100755 --- a/bin/jscs +++ b/bin/jscs @@ -15,6 +15,7 @@ program .usage('[options] ') .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('-s, --esprima ', 'attempts to use a custom version of Esprima') .option('-n, --no-colors', 'clean output without colors') .option('-p, --preset ', 'preset config') diff --git a/lib/config/configuration.js b/lib/config/configuration.js index de2e6caf2..858aa60aa 100644 --- a/lib/config/configuration.js +++ b/lib/config/configuration.js @@ -11,6 +11,7 @@ var BUILTIN_OPTIONS = { maxErrors: true, configPath: true, esnext: true, + es3: true, esprima: true, errorFilter: true }; @@ -35,6 +36,7 @@ function Configuration() { this._overrides = {}; this._presetName = null; this._esnextEnabled = false; + this._es3Enabled = true; this._esprima = null; this._errorFilter = null; } @@ -91,6 +93,7 @@ Configuration.prototype.getProcessedConfig = function() { result.maxErrors = this._maxErrors; result.preset = this._presetName; result.esnext = this._esnextEnabled; + result.es3 = this._es3Enabled; result.esprima = this._esprima; result.errorFilter = this._errorFilter; return result; @@ -163,6 +166,15 @@ Configuration.prototype.isESNextEnabled = function() { return this._esnextEnabled; }; +/** + * Returns `true` if `es3` option is enabled. + * + * @returns {Boolean} + */ +Configuration.prototype.isES3Enabled = function() { + return this._es3Enabled; +}; + /** * Returns `true` if `esprima` option is not null. * @@ -292,6 +304,14 @@ Configuration.prototype._processConfig = function(config) { this._esnextEnabled = Boolean(config.esnext); } + if (config.hasOwnProperty('es3')) { + assert( + typeof config.es3 === 'boolean' || config.es3 === null, + '`es3` option requires boolean or null value' + ); + this._es3Enabled = Boolean(config.es3); + } + if (config.hasOwnProperty('esprima')) { this._loadEsprima(config.esprima); } diff --git a/lib/config/node-configuration.js b/lib/config/node-configuration.js index 05fdd3762..eea6cb7cf 100644 --- a/lib/config/node-configuration.js +++ b/lib/config/node-configuration.js @@ -8,7 +8,8 @@ var OVERRIDE_OPTIONS = [ 'preset', 'maxErrors', 'errorFilter', - 'esprima' + 'esprima', + 'es3' ]; /** diff --git a/lib/js-file.js b/lib/js-file.js index f5bac07f1..266c8e7c2 100644 --- a/lib/js-file.js +++ b/lib/js-file.js @@ -13,10 +13,16 @@ var KEYWORD_OPERATORS = { * * @name JsFile */ -var JsFile = function(filename, source, tree) { +var JsFile = function(filename, source, tree, options) { + options = options || {}; + this._filename = filename; this._source = source; this._tree = tree || {tokens: []}; + + this._es3 = options.es3 || false; + this._es6 = options.es6 || false; + this._lines = source.split(/\r\n|\r|\n/); this._tokenRangeStartIndex = null; this._tokenRangeEndIndex = null; @@ -361,6 +367,22 @@ JsFile.prototype = { } }); }, + /** + * Returns whether this file supports es3. + * + * @returns {Boolean} + */ + isES3Enabled: function() { + return this._es3; + }, + /** + * Returns whether this file supports es6. + * + * @returns {Boolean} + */ + isES6Enabled: function() { + return this._es6; + }, /** * Returns string representing contents of the file. * diff --git a/lib/string-checker.js b/lib/string-checker.js index 55e029c1a..4a746eab7 100644 --- a/lib/string-checker.js +++ b/lib/string-checker.js @@ -93,7 +93,12 @@ StringChecker.prototype = { } catch (e) { parseError = e; } - var file = new JsFile(filename, str, tree); + + var file = new JsFile(filename, str, tree, { + es3: this._configuration.isES3Enabled(), + es6: this._configuration.isESNextEnabled() + }); + var errors = new Errors(file, this._verbose); var errorFilter = this._configuration.getErrorFilter(); diff --git a/test/config/configuration.js b/test/config/configuration.js index d8df629a0..14360667d 100644 --- a/test/config/configuration.js +++ b/test/config/configuration.js @@ -101,6 +101,28 @@ describe('modules/config/configuration', function() { }); }); + describe('isES3Enabled', function() { + it('should return false when null is specified', function() { + configuration.load({es3: null}); + assert.equal(configuration.isES3Enabled(), false); + }); + + it('should return false when false is specified', function() { + configuration.load({es3: false}); + assert.equal(configuration.isES3Enabled(), false); + }); + + it('should return true when true is specified', function() { + configuration.load({es3: true}); + assert.equal(configuration.isES3Enabled(), true); + }); + + it('should return true when unspecified', function() { + configuration.load({}); + assert.equal(configuration.isES3Enabled(), true); + }); + }); + describe('getRegisteredPresets', function() { it('should return registered presets object', function() { var preset = {maxErrors: 5}; diff --git a/test/config/node-configuration.js b/test/config/node-configuration.js index 810d22378..fd37910fc 100644 --- a/test/config/node-configuration.js +++ b/test/config/node-configuration.js @@ -24,7 +24,8 @@ describe('modules/config/node-configuration', function() { preset: 'jquery', maxErrors: '2', errorFilter: path.resolve(__dirname, '../data/error-filter.js'), - esprima: 'esprima-harmony-jscs' + esprima: 'esprima-harmony-jscs', + es3: true }); configuration.registerPreset('jquery', {}); @@ -32,6 +33,7 @@ describe('modules/config/node-configuration', function() { assert.equal(configuration.getProcessedConfig().preset, 'jquery'); assert.equal(configuration.getMaxErrors(), 2); + assert.equal(configuration.isES3Enabled(), true); assert.equal(typeof configuration.getErrorFilter, 'function'); assert.equal(configuration.hasCustomEsprima(), true); }); diff --git a/test/js-file.js b/test/js-file.js index 054bd1ea8..ac6a1f4cb 100644 --- a/test/js-file.js +++ b/test/js-file.js @@ -6,11 +6,12 @@ var sinon = require('sinon'); describe('modules/js-file', function() { - function createJsFile(sources) { + function createJsFile(sources, options) { return new JsFile( 'example.js', sources, - esprima.parse(sources, {loc: true, range: true, comment: true, tokens: true}) + esprima.parse(sources, {loc: true, range: true, comment: true, tokens: true}), + options || {} ); } @@ -677,6 +678,46 @@ describe('modules/js-file', function() { }); }); + describe('isES3Enabled', function() { + it('should return false when unspecified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources); + assert.equal(file.isES3Enabled(), false); + }); + + it('should return false when specified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources, {es3: false}); + assert.equal(file.isES3Enabled(), false); + }); + + it('should return true when specified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources, {es3: true}); + assert.equal(file.isES3Enabled(), true); + }); + }); + + describe('isES6Enabled', function() { + it('should return false when unspecified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources); + assert.equal(file.isES6Enabled(), false); + }); + + it('should return false when specified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources, {es6: false}); + assert.equal(file.isES6Enabled(), false); + }); + + it('should return true when specified', function() { + var sources = 'var x = 1;\nvar y = 2;'; + var file = createJsFile(sources, {es6: true}); + assert.equal(file.isES6Enabled(), true); + }); + }); + describe('getLines', function() { it('should return specified source code lines', function() { var sources = ['var x = 1;', 'var y = 2;']; From cab9f35eaac221265badaa1949a7d0dc45477d1f Mon Sep 17 00:00:00 2001 From: Mike Sherov Date: Mon, 8 Dec 2014 11:26:35 -0500 Subject: [PATCH 2/2] requireDotNotation: Require dots for es3 keywords when not in es3 mode. Fixes gh-161 --- README.md | 31 +++++++++-- lib/rules/require-dot-notation.js | 7 ++- test/rules/require-dot-notation.js | 85 ++++++++++++++++++++---------- 3 files changed, 89 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 12c42d5a2..1db8f4bf7 100644 --- a/README.md +++ b/README.md @@ -2913,7 +2913,7 @@ 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` @@ -2921,10 +2921,11 @@ Values: `true` JSHint: [`sub`](http://www.jshint.com/docs/options/#sub) -#### Example +#### Example for `"es3": true` ```js -"requireDotNotation": true +"requireDotNotation": true, +"es3": true ``` ##### Valid @@ -2943,6 +2944,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/lib/rules/require-dot-notation.js b/lib/rules/require-dot-notation.js index 544080c5d..6bf348582 100644 --- 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.isES3Enabled() && (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/rules/require-dot-notation.js b/test/rules/require-dot-notation.js index ae040d53f..7b255cf2e 100644 --- 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); + }); }); });