From 73f3ea6505a1b0ae09cd232be302fba3c9ee1751 Mon Sep 17 00:00:00 2001 From: Santosh Sutar Date: Thu, 11 Oct 2018 23:51:05 -0700 Subject: [PATCH] Make the rule configurable to the usage of utility methods For details: https://github.com/ssutar/eslint-plugin-ember-object-update/issues/1 --- lib/index.js | 4 +- lib/rules/no-object-update.js | 122 ++++++++++++++++++++++--- tests/rules/no-object-update-test.js | 132 +++++++++++++++++++++++++++ 3 files changed, 242 insertions(+), 16 deletions(-) diff --git a/lib/index.js b/lib/index.js index d2843ea..2f04444 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,6 +1,6 @@ /** - * @fileoverview Eslint plugin for enforcing usages of ember es6 classes. - * @author scalvert + * @fileoverview Eslint plugin to prevent the record update after removal + * @author ssutar */ "use strict"; diff --git a/lib/rules/no-object-update.js b/lib/rules/no-object-update.js index c390e05..ce04af3 100644 --- a/lib/rules/no-object-update.js +++ b/lib/rules/no-object-update.js @@ -1,14 +1,46 @@ /** - * Prevent the code to use `EmberObject.extend` favoring the ES6 native class `extends` + * Prevent the record update after removal */ "use strict"; + //------------------------------------------------------------------------------ -// Rule Definition +// Utility methods //------------------------------------------------------------------------------ -function getReportMessage(objName = "obj") { - return `Please wrap '${objName}.set' in 'if (!${objName}.isDestroying) { ${objName}.set(...) }'`; + +/** + * Construct the error message to be report when rule fails + * + * @param {String} objName + * @param {String[]} utilityMethods + */ +function getReportMessage(objName = "obj", utilityMethods = []) { + let msg = `Wrap '${objName}.set' in 'if (!${objName}.isDestroying) { ${objName}.set(...) }'`; + if (utilityMethods.length) { + msg = `Use the utility method(s) '${utilityMethods.join( + "' OR '" + )}' to verify the '${objName}' is not being destroyed before calling '${objName}.set(...)'`; + } + return msg; } +/** + * Get a property from and object, useful to get nested props without checking for null values + * + * @param {Object} obj + * @param {String} path + * @returns {Any} + */ +function get(obj, path) { + return path.split(".").reduce(function(currentObject, pathSegment) { + return typeof currentObject == "undefined" || currentObject === null + ? currentObject + : currentObject[pathSegment]; + }, obj); +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ module.exports = { meta: { docs: { @@ -18,36 +50,98 @@ module.exports = { }, reportMessage: getReportMessage() }, + create: function create(context) { + /** + * Iterates recursively over the test node of conditional and extracts the nodes matching + * either call expressions or member expressions + * @param {Expression} testNode + * @returns {Expression[]} + */ + function getConditionalParams(testNode) { + const type = get(testNode, "type"); + if (type === "LogicalExpression") { + return [] + .concat(getConditionalParams(get(testNode, "left"))) + .concat(getConditionalParams(get(testNode, "right"))); + } + if (type === "UnaryExpression") { + return [].concat(getConditionalParams(get(testNode, "argument"))); + } + if (type === "MemberExpression" || type === "CallExpression") { + return [testNode]; + } + return []; + } + + /** + * Checks if the `statement` is wrapped in the destroy conditional + * + * @param {Scope} scope + * @param {String} objName + * @returns boolean + */ + function isWrappedInDestroyConditional(scope, objName) { + if (get(scope, "block.parent.type") !== "IfStatement") { + return false; + } + return getConditionalParams(get(scope, "block.parent.test")).some( + param => { + const type = get(param, "type"); + if (type === "MemberExpression") { + return ( + get(param, "object.name") === objName && + get(param, "property.name") === "isDestroying" + ); + } else { + return ( + context.options.includes(get(param, "callee.name")) && + get(param, "arguments").some(arg => get(arg, "name") === objName) + ); + } + } + ); + } + + /** + * Main entry point for rule + * + * Parses all the `set` Identifier expressions in the code and report if it can fail in tests + * + * @param {Identifier} node + */ function checkForObjectUpdate(node) { if ( - node.name === "set" && - node.parent.type !== "CallExpression" && - node.parent.object.type !== "ThisExpression" + get(node, "name") === "set" && + get(node, "parent.type") !== "CallExpression" && + get(node, "parent.object.type") !== "ThisExpression" ) { + const objName = get(node, "parent.object.name"); let scope = context.getScope(node); - while (scope && scope.type !== "function") { - scope = scope.upper; + while (scope && get(scope, "type") !== "function") { + if (isWrappedInDestroyConditional(scope, objName)) { + return; + } + scope = get(scope, "upper"); } - const pNodeScope = scope && scope.block.parent; + const pNodeScope = get(scope, "block.parent"); const pNodeScopeCallee = - pNodeScope && pNodeScope.type === "CallExpression" + get(pNodeScope, "type") === "CallExpression" ? pNodeScope.callee : null; if (!pNodeScopeCallee) { return; } else { - const propName = pNodeScopeCallee.property.name; + const propName = get(pNodeScopeCallee, "property.name"); const isSetWrappedInPromise = propName === "then" || propName === "catch" || propName === "finally"; if (isSetWrappedInPromise) { - const objName = node.parent.object.name; - context.report(node, getReportMessage(objName)); + context.report(node, getReportMessage(objName, context.options)); } } } diff --git a/tests/rules/no-object-update-test.js b/tests/rules/no-object-update-test.js index 14ca1e4..9674b14 100644 --- a/tests/rules/no-object-update-test.js +++ b/tests/rules/no-object-update-test.js @@ -8,6 +8,8 @@ const ruleTester = new RuleTester({ } }); +const MESSAGE_WITH_OPTIONS = `Use the utility method(s) 'isAlive' OR 'isRemoved' to verify the 'obj' is not being destroyed before calling 'obj.set(...)'`; + ruleTester.run("no-object-update", rule, { valid: [ { @@ -23,6 +25,63 @@ ruleTester.run("no-object-update", rule, { this.set('prop3', false); }); ` + }, + { + code: ` + somePromise + .then(() => { + if(!obj.isDestroying) { + obj.set('prop1', true); + } + }) + .catch(() => { + if(!obj.isDestroying) { + if(someOtherCondition()) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }) + .finally(() => { + if(someOtherCondition()) { + if(!obj.isDestroying) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }); + ` + }, + { + code: ` + somePromise + .then(() => { + if(!isRemoved(obj)) { + obj.set('prop1', true); + } + }) + .catch(() => { + if(isAlive(obj)) { + if(someOtherCondition()) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }) + .finally(() => { + if(someOtherCondition()) { + if(!isMarkedForDelete(obj)) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }); + `, + options: ["isAlive", "isRemoved", "isMarkedForDelete"] } ], invalid: [ @@ -50,6 +109,79 @@ ruleTester.run("no-object-update", rule, { message: MESSAGE } ] + }, + { + code: ` + somePromise + .then(() => { + if(!isRemoved(obj)) { + obj.set('prop1', true); + } + }) + .catch(() => { + if(isAlive(obj)) { + if(someOtherCondition()) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }) + .finally(() => { + if(someOtherCondition()) { + if(!isMarkedForDelete(obj)) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + } + }); + `, + errors: [ + { + message: MESSAGE + }, + { + message: MESSAGE + }, + { + message: MESSAGE + } + ] + }, + { + code: ` + somePromise + .then(() => { + obj.set('prop1', true); + }) + .catch(() => { + if(someOtherCondition()) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + }) + .finally(() => { + if(someOtherCondition()) { + if(someOtherNestedCondition) { + obj.set('prop2', true); + } + } + }); + `, + options: ["isAlive", "isRemoved"], + errors: [ + { + message: MESSAGE_WITH_OPTIONS + }, + { + message: MESSAGE_WITH_OPTIONS + }, + { + message: MESSAGE_WITH_OPTIONS + } + ] } ] });