diff --git a/docs/rules/require-super-in-lifecycle-hooks.md b/docs/rules/require-super-in-lifecycle-hooks.md index c209e72106..27d856a19e 100644 --- a/docs/rules/require-super-in-lifecycle-hooks.md +++ b/docs/rules/require-super-in-lifecycle-hooks.md @@ -20,7 +20,7 @@ import Component from '@ember/component'; export default Component.extend({ init() { this.set('items', []); - } + }, }); ``` @@ -30,7 +30,7 @@ import Component from '@ember/component'; export default Component.extend({ didInsertElement() { // ... - } + }, }); ``` @@ -53,7 +53,7 @@ export default Component.extend({ init(...args) { this._super(...args); this.set('items', []); - } + }, }); ``` @@ -64,7 +64,7 @@ export default Component.extend({ didInsertElement(...args) { this._super(...args); // ... - } + }, }); ``` @@ -96,3 +96,4 @@ This rule takes an optional object containing: - `boolean` -- `checkInitOnly` -- whether the rule should only check the `init` lifecycle hook and not other lifecycle hooks (default `false`) - `boolean` -- `checkNativeClasses` -- whether the rule should check lifecycle hooks in native classes (in addition to classic classes) (default `true`) +- `boolean` -- `requireArgs` -- whether the rule should check if super() in init hook is called with args (default `false`) diff --git a/lib/rules/require-super-in-lifecycle-hooks.js b/lib/rules/require-super-in-lifecycle-hooks.js index 03d91c337d..4db2071181 100644 --- a/lib/rules/require-super-in-lifecycle-hooks.js +++ b/lib/rules/require-super-in-lifecycle-hooks.js @@ -6,15 +6,20 @@ const estraverse = require('estraverse'); function hasMatchingNode(node, matcher) { let foundMatch = false; + let foundNode = null; estraverse.traverse(node, { enter(child) { if (!foundMatch && matcher(child)) { foundMatch = true; + foundNode = child; } }, fallback: 'iteration', }); - return foundMatch; + return { + foundMatch, + foundNode, + }; } /** @@ -79,6 +84,10 @@ module.exports = { type: 'boolean', default: true, }, + requireArgs: { + type: 'boolean', + default: false, + }, }, additionalProperties: false, }, @@ -88,8 +97,9 @@ module.exports = { create(context) { const checkInitOnly = context.options[0] && context.options[0].checkInitOnly; const checkNativeClasses = !context.options[0] || context.options[0].checkNativeClasses; + const requireArgs = context.options[0] && context.options[0].requireArgs; - function report(node, isNativeClass, lifecycleHookName) { + function report(node, isNativeClass, lifecycleHookName, hasSuper) { context.report({ node, message: ERROR_MESSAGE, @@ -100,12 +110,23 @@ module.exports = { : '...arguments'; const replacement = isNativeClass - ? `super.${lifecycleHookName}(${replacementArgs});` - : `this._super(${replacementArgs});`; + ? `super.${lifecycleHookName}(${replacementArgs})` + : `this._super(${replacementArgs})`; + + // If super is already called, replace it with the correct call + if (hasSuper) { + const nodeToBeReplaced = hasMatchingNode(node, (bodyChild) => + isNativeClass + ? isNativeSuper(bodyChild, lifecycleHookName) + : isClassicSuper(bodyChild) + ).foundNode; + return fixer.replaceText(nodeToBeReplaced, replacement); + } + // Insert right after function curly brace. const sourceCode = context.getSourceCode(); const startOfBlockStatement = sourceCode.getFirstToken(node.value.body); - return fixer.insertTextAfter(startOfBlockStatement, `\n${replacement}`); + return fixer.insertTextAfter(startOfBlockStatement, `\n${replacement};`); }, }); } @@ -155,11 +176,20 @@ module.exports = { } const body = isNativeSuper ? node.value.body : node.body; - const hasSuper = hasMatchingNode(body, (bodyChild) => + const hasSuperMatchingNodeResult = hasMatchingNode(body, (bodyChild) => isNativeClass ? isNativeSuper(bodyChild, hookName) : isClassicSuper(bodyChild) ); - if (!hasSuper) { - report(node, isNativeClass, hookName); + const hasSuper = hasSuperMatchingNodeResult.foundMatch; + + if (hookName === 'init' && hasSuper && requireArgs) { + const superNode = hasSuperMatchingNodeResult.foundNode; + const hasSuperWithArguments = superNode.arguments && superNode.arguments.length > 0; + + if (!hasSuperWithArguments) { + report(node, isNativeClass, hookName, hasSuper); + } + } else if (!hasSuper) { + report(node, isNativeClass, hookName, hasSuper); } } diff --git a/tests/lib/rules/require-super-in-lifecycle-hooks.js b/tests/lib/rules/require-super-in-lifecycle-hooks.js index 02161c98f8..fe9f574d10 100644 --- a/tests/lib/rules/require-super-in-lifecycle-hooks.js +++ b/tests/lib/rules/require-super-in-lifecycle-hooks.js @@ -231,6 +231,36 @@ eslintTester.run('require-super-in-lifecycle-hooks', rule, { // Does not throw with node type (ClassProperty) not handled by estraverse. 'Component.extend({init() { class Foo { @someDecorator() someProp }; return this._super(...arguments); } });', + + // init hook should not be checked for arguments when requireArgs = false + `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(); + } + }`, + { + code: `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(); + } + }`, + options: [{ requireArgs: false }], + }, + `export default Component.extend({ + init() { + this._super(...arguments); + }, + });`, + { + code: `export default Component.extend({ + init() { + this._super(...arguments); + }, + });`, + options: [{ requireArgs: false }], + }, ], invalid: [ { @@ -858,5 +888,73 @@ super.didInsertElement(...arguments);} options: [{ checkNativeClasses: true, checkInitOnly: false }], errors: [{ message, line: 3 }], }, + + // init hooks shoudn't call super without ...arguments + { + code: `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(); + } + }`, + output: `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(...arguments); + } + }`, + options: [{ requireArgs: true }], + errors: [{ message, line: 3 }], + }, + { + code: `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(); + console.log(); + } + }`, + output: `import Service from '@ember/service'; + class Foo extends Service { + init() { + super.init(...arguments); + console.log(); + } + }`, + options: [{ requireArgs: true }], + errors: [{ message, line: 3 }], + }, + { + code: `export default Component.extend({ + init() { + this._super(); + }, + });`, + output: `export default Component.extend({ + init() { + this._super(...arguments); + }, + });`, + options: [{ requireArgs: true }], + errors: [{ message, line: 2 }], + }, + { + code: `export default Component.extend({ + init() { + this._super(); + this.set('prop', 'value'); + this.set('prop2', 'value2'); + }, + });`, + output: `export default Component.extend({ + init() { + this._super(...arguments); + this.set('prop', 'value'); + this.set('prop2', 'value2'); + }, + });`, + options: [{ requireArgs: true }], + errors: [{ message, line: 2 }], + }, ], });