From a06d2db784d2af858afa82e9762cff4396efeada Mon Sep 17 00:00:00 2001 From: Dominik Ferber Date: Mon, 7 Mar 2016 17:56:26 +0100 Subject: [PATCH] feat(rules): add rule prefer-session-equals closes #118 --- README.md | 1 + docs/rules/prefer-session-equals.md | 57 ++++++++++++++++ lib/index.js | 2 + lib/rules/prefer-session-equals.js | 68 +++++++++++++++++++ tests/lib/rules/prefer-session-equals.js | 84 ++++++++++++++++++++++++ 5 files changed, 212 insertions(+) create mode 100644 docs/rules/prefer-session-equals.md create mode 100644 lib/rules/prefer-session-equals.js create mode 100644 tests/lib/rules/prefer-session-equals.js diff --git a/README.md b/README.md index b0e4c39..f30c0d4 100755 --- a/README.md +++ b/README.md @@ -58,6 +58,7 @@ For a more thorough introduction, read the [setup guide](/docs/guides/setup.md). * [no-blaze-lifecycle-assignment](docs/rules/no-blaze-lifecycle-assignment.md): Prevent deprecated template lifecycle callback assignments * [no-zero-timeout](docs/rules/no-zero-timeout.md): Prevent usage of Meteor.setTimeout with zero delay * [blaze-consistent-eventmap-params](docs/rules/blaze-consistent-eventmap-params.md): Force consistent event handler parameters in event maps +* [prefer-session-equals](docs/prefer-session-equals.md): Prefer `Session.equals` in conditions ## Core API diff --git a/docs/rules/prefer-session-equals.md b/docs/rules/prefer-session-equals.md new file mode 100644 index 0000000..497a7e8 --- /dev/null +++ b/docs/rules/prefer-session-equals.md @@ -0,0 +1,57 @@ +# Prefer `Session.equals` in conditions (prefer-session-equals) + +Using `Session.equals('foo', bar)` toggles fewer invalidations compared to `Session.get('foo') === bar`. + + +## Rule Details + +While the above is only true for scalar types, this rule encourages use over `Session.equals` in all conditional statements. + +The following patterns are considered warnings: + +```js +if (Session.get("foo")) {/* ... */} + +if (Session.get("foo") == bar) {/* ... */} + +if (Session.get("foo") === bar) {/* ... */} + +Session.get("foo") ? true : false + +Session.get("foo") === bar ? true : false +``` + +The following patterns are not warnings: + +```js +if (Session.equals("foo", true)) {/* ... */} + +if (Session.equals("foo", 1)) {/* ... */} + +if (Session.equals("foo", "hello")) {/* ... */} + +if (Session.equals("foo", bar)) {/* ... */} + +if (_.isEqual(Session.get("foo"), otherValue)) {/* ... */} + +Session.equals("foo", true) ? true : false +``` + +```js +const foo = Session.get("foo") +if (foo === 'bar') {/* ... */} +``` + +## When Not To Use It + +Turn this rule off when you are comparing compound types, e.g. Arrays. + + +## Further Reading + +- http://docs.meteor.com/#/full/session_equals + + +## Possible Improvements + +* Track which variables were set through `Session.get` and warn when they are used in conditions diff --git a/lib/index.js b/lib/index.js index 5fdca89..0697227 100755 --- a/lib/index.js +++ b/lib/index.js @@ -5,6 +5,7 @@ module.exports = { 'no-blaze-lifecycle-assignment': require('./rules/no-blaze-lifecycle-assignment'), 'no-zero-timeout': require('./rules/no-zero-timeout'), 'blaze-consistent-eventmap-params': require('./rules/blaze-consistent-eventmap-params'), + 'prefer-session-equals': require('./rules/prefer-session-equals'), }, configs: { parserOptions: { @@ -19,6 +20,7 @@ module.exports = { 'meteor/no-blaze-lifecycle-assignment': 2, 'meteor/no-zero-timeout': 2, 'meteor/blaze-consistent-eventmap-params': 2, + 'meteor/prefer-session-equals': 0, }, }, }, diff --git a/lib/rules/prefer-session-equals.js b/lib/rules/prefer-session-equals.js new file mode 100644 index 0000000..ad07da9 --- /dev/null +++ b/lib/rules/prefer-session-equals.js @@ -0,0 +1,68 @@ +/** + * @fileoverview Prefer Session.equals in conditions + * @author Dominik Ferber + * @copyright 2016 Dominik Ferber. All rights reserved. + * See LICENSE file in root directory for full license. + */ + + +const isSessionGetCallExpression = node => ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + ( + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && node.callee.object.name === 'Session' && + ( + ( + !node.callee.computed && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'get' + ) || + ( + node.callee.computed && + node.callee.property.type === 'Literal' && + node.callee.property.value === 'get' + ) + ) + ) +) + + +// ----------------------------------------------------------------------------- +// Rule Definition +// ----------------------------------------------------------------------------- +module.exports = context => { + // --------------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------------- + const errorMessage = 'Use "Session.equals" instead' + + const checkTest = (node) => { + switch (node.type) { + case 'BinaryExpression': + case 'LogicalExpression': + checkTest(node.left) + checkTest(node.right) + break + case 'CallExpression': + if (isSessionGetCallExpression(node)) { + context.report(node.callee, errorMessage) + } + break + default: + return + } + } + + // --------------------------------------------------------------------------- + // Public + // --------------------------------------------------------------------------- + return { + ConditionalExpression: node => { + checkTest(node.test) + }, + IfStatement: node => checkTest(node.test), + } +} + +module.exports.schema = [] diff --git a/tests/lib/rules/prefer-session-equals.js b/tests/lib/rules/prefer-session-equals.js new file mode 100644 index 0000000..9ee722d --- /dev/null +++ b/tests/lib/rules/prefer-session-equals.js @@ -0,0 +1,84 @@ +/** + * @fileoverview Prefer Session.equals in conditions + * @author Dominik Ferber + * @copyright 2016 Dominik Ferber. All rights reserved. + * See LICENSE file in root directory for full license. + */ + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +const rule = require('../../../dist/rules/prefer-session-equals') +const RuleTester = require('eslint').RuleTester +const ruleTester = new RuleTester() + +ruleTester.run('prefer-session-equals', rule, { + valid: [ + 'if (Session.equals("foo", true)) {}', + 'if (Session.equals("foo", false)) {}', + 'if (Session.equals("foo", 1)) {}', + 'if (Session.equals("foo", "hello")) {}', + 'if (!Session.equals("foo", "hello")) {}', + 'if (_.isEqual(Session.get("foo"), otherValue)) {}', + 'Session.equals("foo", true) ? true : false', + 'if (Session.set("foo")) {}', + ], + + invalid: [ + { + code: 'if (Session.get("foo")) {}', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'if (Session.get("foo") == 3) {}', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'if (Session.get("foo") === 3) {}', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'if (Session.get("foo") === bar) {}', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'if (Session.get("foo") !== bar) {}', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'Session.get("foo") ? true : false', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'Session.get("foo") && false ? true : false', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'Session.get("foo") === 2 ? true : false', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + { + code: 'true || Session.get("foo") === 2 ? true : false', + errors: [ + { message: 'Use "Session.equals" instead', type: 'MemberExpression' }, + ], + }, + ], +})