diff --git a/CHANGELOG.md b/CHANGELOG.md index e277ab7b2f..7e26e3c125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +## v8.11.0 (2020-08-14) + +#### :rocket: Enhancement +* [#912](https://github.com/ember-cli/eslint-plugin-ember/pull/912) Add `catchSafeObjects` option (default false) to [no-get](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-get.md) rule to catch `get(foo, 'bar')` ([@bmish](https://github.com/bmish)) +* [#913](https://github.com/ember-cli/eslint-plugin-ember/pull/913) Add `catchUnsafeObjects` option (default false) to [no-get](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-get.md) rule to catch `foo.get('bar')` ([@bmish](https://github.com/bmish)) + +#### :bug: Bug Fix +* [#911](https://github.com/ember-cli/eslint-plugin-ember/pull/911) Update [no-test-import-export](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-test-import-export.md) rule to allow importing from anything under `tests/helpers` path (when using relative path) ([@bmish](https://github.com/bmish)) +* [#909](https://github.com/ember-cli/eslint-plugin-ember/pull/909) Check imports when detecting computed properties in many rules ([@bmish](https://github.com/bmish)) + +#### Committers: 1 +- Bryan Mishkin ([@bmish](https://github.com/bmish)) + + ## v8.10.1 (2020-08-07) #### :bug: Bug Fix diff --git a/docs/rules/no-get.md b/docs/rules/no-get.md index 8ea9a9290a..391dcee467 100644 --- a/docs/rules/no-get.md +++ b/docs/rules/no-get.md @@ -94,6 +94,8 @@ This rule takes an optional object containing: * `boolean` -- `ignoreGetProperties` -- whether the rule should ignore `getProperties` (default `false`) * `boolean` -- `ignoreNestedPaths` -- whether the rule should ignore `this.get('some.nested.property')` (default `false`) * `boolean` -- `useOptionalChaining` -- whether the rule should use the [optional chaining operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining) `?.` to autofix nested paths such as `this.get('some.nested.property')` to `this.some?.nested?.property` (when this option is off, these nested paths won't be autofixed at all) (default `false`) +* `boolean` -- `catchSafeObjects` -- whether the rule should catch `get(foo, 'bar')` (default `false`, TODO: enable in next major release) +* `boolean` -- `catchUnsafeObjects` -- whether the rule should catch `foo.get('bar')` even though we don't know for sure if `foo` is an Ember object (default `false`) ## Related Rules diff --git a/lib/rules/computed-property-getters.js b/lib/rules/computed-property-getters.js index 28e9cfc6d4..18bbf55b12 100644 --- a/lib/rules/computed-property-getters.js +++ b/lib/rules/computed-property-getters.js @@ -2,6 +2,7 @@ const ember = require('../utils/ember'); const types = require('../utils/types'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // General rule - Prevent using a getter inside computed properties. @@ -94,9 +95,25 @@ module.exports = { } }; + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { - if (ember.isComputedProp(node) && node.arguments.length > 0) { + if ( + ember.isComputedProp(node, importedEmberName, importedComputedName) && + node.arguments.length > 0 + ) { if (requireGetters === 'always-with-setter') { requireGetterOnlyWithASetterInComputedProperty(node); } diff --git a/lib/rules/no-arrow-function-computed-properties.js b/lib/rules/no-arrow-function-computed-properties.js index 7a8e8d45a7..683135ec55 100644 --- a/lib/rules/no-arrow-function-computed-properties.js +++ b/lib/rules/no-arrow-function-computed-properties.js @@ -2,6 +2,7 @@ const types = require('../utils/types'); const emberUtils = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); const ERROR_MESSAGE = 'Do not use arrow functions in computed properties'; @@ -37,7 +38,20 @@ module.exports = { let isThisPresent = false; + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + ThisExpression() { isThisPresent = true; }, @@ -46,7 +60,9 @@ module.exports = { }, 'CallExpression:exit'(node) { const isComputedArrow = - emberUtils.isComputedProp(node) && + emberUtils.isComputedProp(node, importedEmberName, importedComputedName, { + includeMacro: true, + }) && node.arguments.length > 0 && types.isArrowFunctionExpression(node.arguments[node.arguments.length - 1]); diff --git a/lib/rules/no-deeply-nested-dependent-keys-with-each.js b/lib/rules/no-deeply-nested-dependent-keys-with-each.js index 51b006d2d1..050d1caf14 100644 --- a/lib/rules/no-deeply-nested-dependent-keys-with-each.js +++ b/lib/rules/no-deeply-nested-dependent-keys-with-each.js @@ -1,6 +1,7 @@ 'use strict'; const emberUtils = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // Rule Definition @@ -26,12 +27,22 @@ module.exports = { ERROR_MESSAGE, create(context) { + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { - if ( - emberUtils.isComputedProp(node) && - (!node.callee.property || node.callee.property.name === 'computed') - ) { + if (emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { emberUtils.parseDependentKeys(node).forEach((key) => { const parts = key.split('.'); const indexOfAtEach = parts.indexOf('@each'); diff --git a/lib/rules/no-duplicate-dependent-keys.js b/lib/rules/no-duplicate-dependent-keys.js index 1797c639c2..bf8220db28 100644 --- a/lib/rules/no-duplicate-dependent-keys.js +++ b/lib/rules/no-duplicate-dependent-keys.js @@ -4,6 +4,7 @@ const emberUtils = require('../utils/ember'); const types = require('../utils/types'); const fixerUtils = require('../utils/fixer'); const javascriptUtils = require('../utils/javascript'); +const { getImportIdentifier } = require('../utils/import'); const ERROR_MESSAGE = 'Dependent keys should not be repeated'; //------------------------------------------------------------------------------ @@ -27,9 +28,22 @@ module.exports = { ERROR_MESSAGE, create(context) { + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { - if (emberUtils.hasDuplicateDependentKeys(node)) { + if (emberUtils.hasDuplicateDependentKeys(node, importedEmberName, importedComputedName)) { context.report({ node, message: ERROR_MESSAGE, diff --git a/lib/rules/no-get.js b/lib/rules/no-get.js index 4643b28f56..994f2bef4a 100644 --- a/lib/rules/no-get.js +++ b/lib/rules/no-get.js @@ -23,18 +23,32 @@ function isValidJSPath(str) { return str.split('.').every((part) => isValidJSVariableName(part) || isValidJSArrayIndex(part)); } -function reportGet({ node, context, path, useOptionalChaining }) { +function reportGet({ node, context, path, useOptionalChaining, objectText }) { const isInLeftSideOfAssignmentExpression = utils.isInLeftSideOfAssignmentExpression(node); context.report({ node, message: ERROR_MESSAGE_GET, fix(fixer) { - return fixGet({ node, fixer, path, useOptionalChaining, isInLeftSideOfAssignmentExpression }); + return fixGet({ + node, + fixer, + path, + useOptionalChaining, + isInLeftSideOfAssignmentExpression, + objectText, + }); }, }); } -function fixGet({ node, fixer, path, useOptionalChaining, isInLeftSideOfAssignmentExpression }) { +function fixGet({ + node, + fixer, + path, + useOptionalChaining, + isInLeftSideOfAssignmentExpression, + objectText, +}) { if (path.includes('.') && !useOptionalChaining && !isInLeftSideOfAssignmentExpression) { // Not safe to autofix nested properties because some properties in the path might be null or undefined. return null; @@ -54,7 +68,10 @@ function fixGet({ node, fixer, path, useOptionalChaining, isInLeftSideOfAssignme isInLeftSideOfAssignmentExpression ? '[$1]' : '.[$1]' ); - return fixer.replaceText(node, `this.${replacementPath}`); + // Add parenthesis around the object text in case of something like this: get(foo || {}, 'bar') + const objectTextSafe = isValidJSPath(objectText) ? objectText : `(${objectText})`; + + return fixer.replaceText(node, `${objectTextSafe}.${replacementPath}`); } module.exports = { @@ -85,6 +102,14 @@ module.exports = { type: 'boolean', default: false, }, + catchSafeObjects: { + type: 'boolean', + default: false, + }, + catchUnsafeObjects: { + type: 'boolean', + default: false, + }, }, additionalProperties: false, }, @@ -95,6 +120,8 @@ module.exports = { const ignoreGetProperties = context.options[0] && context.options[0].ignoreGetProperties; const ignoreNestedPaths = context.options[0] && context.options[0].ignoreNestedPaths; const useOptionalChaining = context.options[0] && context.options[0].useOptionalChaining; + const catchSafeObjects = context.options[0] && context.options[0].catchSafeObjects; + const catchUnsafeObjects = context.options[0] && context.options[0].catchUnsafeObjects; if (ignoreNestedPaths && useOptionalChaining) { assert( @@ -177,7 +204,7 @@ module.exports = { if ( types.isMemberExpression(node.callee) && - types.isThisExpression(node.callee.object) && + (types.isThisExpression(node.callee.object) || catchUnsafeObjects) && types.isIdentifier(node.callee.property) && node.callee.property.name === 'get' && node.arguments.length === 1 && @@ -185,12 +212,14 @@ module.exports = { (!node.arguments[0].value.includes('.') || !ignoreNestedPaths) ) { // Example: this.get('foo'); + const sourceCode = context.getSourceCode(); reportGet({ node, context, path: node.arguments[0].value, isImportedGet: false, useOptionalChaining, + objectText: sourceCode.getText(node.callee.object), }); } @@ -198,17 +227,19 @@ module.exports = { types.isIdentifier(node.callee) && node.callee.name === importedGetName && node.arguments.length === 2 && - types.isThisExpression(node.arguments[0]) && + (types.isThisExpression(node.arguments[0]) || catchSafeObjects) && types.isStringLiteral(node.arguments[1]) && (!node.arguments[1].value.includes('.') || !ignoreNestedPaths) ) { // Example: get(this, 'foo'); + const sourceCode = context.getSourceCode(); reportGet({ node, context, path: node.arguments[1].value, isImportedGet: true, useOptionalChaining, + objectText: sourceCode.getText(node.arguments[0]), }); } @@ -222,7 +253,7 @@ module.exports = { if ( types.isMemberExpression(node.callee) && - types.isThisExpression(node.callee.object) && + (types.isThisExpression(node.callee.object) || catchUnsafeObjects) && types.isIdentifier(node.callee.property) && node.callee.property.name === 'getProperties' && validateGetPropertiesArguments(node.arguments, ignoreNestedPaths) @@ -234,7 +265,7 @@ module.exports = { if ( types.isIdentifier(node.callee) && node.callee.name === importedGetPropertiesName && - types.isThisExpression(node.arguments[0]) && + (types.isThisExpression(node.arguments[0]) || catchSafeObjects) && validateGetPropertiesArguments(node.arguments.slice(1), ignoreNestedPaths) ) { // Example: getProperties(this, 'foo', 'bar'); diff --git a/lib/rules/no-invalid-dependent-keys.js b/lib/rules/no-invalid-dependent-keys.js index bd7ff96eb2..6dae836525 100644 --- a/lib/rules/no-invalid-dependent-keys.js +++ b/lib/rules/no-invalid-dependent-keys.js @@ -2,6 +2,7 @@ const types = require('../utils/types'); const ember = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // General rule - Dependent keys used for computed properties have to be valid. @@ -56,9 +57,22 @@ module.exports = { ERROR_MESSAGE_INVALID_CHARACTER, create(context) { + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { - if (!ember.isComputedProp(node) || types.isMemberExpression(node.callee)) { + if (!ember.isComputedProp(node, importedEmberName, importedComputedName)) { return; } diff --git a/lib/rules/no-side-effects.js b/lib/rules/no-side-effects.js index 4eb83151d3..5258f07e9d 100644 --- a/lib/rules/no-side-effects.js +++ b/lib/rules/no-side-effects.js @@ -1,11 +1,11 @@ 'use strict'; const types = require('../utils/types'); -const ember = require('../utils/ember'); const computedPropertyUtils = require('../utils/computed-properties'); const propertySetterUtils = require('../utils/property-setter'); const emberUtils = require('../utils/ember'); const Traverser = require('../utils/traverser'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // General rule - Don't introduce side-effects in computed properties @@ -68,7 +68,8 @@ module.exports = { ERROR_MESSAGE, create(context) { - let emberImportAliasName; + let importedEmberName; + let importedComputedName; const report = function (node) { context.report(node, ERROR_MESSAGE); @@ -76,29 +77,33 @@ module.exports = { return { ImportDeclaration(node) { - if (!emberImportAliasName) { - emberImportAliasName = ember.getEmberImportAliasName(node); + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); } }, CallExpression(node) { - if (!emberUtils.isComputedProp(node)) { + if (!emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { return; } const computedPropertyBody = computedPropertyUtils.getComputedPropertyFunctionBody(node); - findSideEffects(computedPropertyBody, emberImportAliasName).forEach(report); + findSideEffects(computedPropertyBody, importedEmberName).forEach(report); }, Identifier(node) { - if (!emberUtils.isComputedProp(node)) { + if (!emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { return; } const computedPropertyBody = computedPropertyUtils.getComputedPropertyFunctionBody(node); - findSideEffects(computedPropertyBody, emberImportAliasName).forEach(report); + findSideEffects(computedPropertyBody, importedEmberName).forEach(report); }, }; }, diff --git a/lib/rules/no-test-import-export.js b/lib/rules/no-test-import-export.js index 625141cf06..e09ab1e105 100644 --- a/lib/rules/no-test-import-export.js +++ b/lib/rules/no-test-import-export.js @@ -4,6 +4,7 @@ 'use strict'; +const { dirname, resolve } = require('path'); const emberUtils = require('../utils/ember'); const NO_IMPORT_MESSAGE = @@ -45,7 +46,10 @@ module.exports = { ImportDeclaration(node) { const importSource = node.source.value; - if (importSource.endsWith('-test') && !isTestHelperImportSource(importSource)) { + if ( + importSource.endsWith('-test') && + !isTestHelperImportSource(resolve(dirname(context.getFilename()), importSource)) + ) { context.report({ message: NO_IMPORT_MESSAGE, node, diff --git a/lib/rules/no-volatile-computed-properties.js b/lib/rules/no-volatile-computed-properties.js index 71b6e5979f..ddeef979c0 100644 --- a/lib/rules/no-volatile-computed-properties.js +++ b/lib/rules/no-volatile-computed-properties.js @@ -2,6 +2,7 @@ const types = require('../utils/types'); const emberUtils = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); const ERROR_MESSAGE = 'Do not use volatile computed properties'; @@ -21,12 +22,25 @@ module.exports = { }, create(context) { + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { if ( types.isMemberExpression(node.callee) && types.isCallExpression(node.callee.object) && - emberUtils.isComputedProp(node.callee.object) && + emberUtils.isComputedProp(node.callee.object, importedEmberName, importedComputedName) && types.isIdentifier(node.callee.property) && node.callee.property.name === 'volatile' ) { diff --git a/lib/rules/require-computed-macros.js b/lib/rules/require-computed-macros.js index 78b5bb1058..196d9e0d57 100644 --- a/lib/rules/require-computed-macros.js +++ b/lib/rules/require-computed-macros.js @@ -5,6 +5,7 @@ const emberUtils = require('../utils/ember'); const propertyGetterUtils = require('../utils/property-getter'); const assert = require('assert'); const scopeReferencesThis = require('../utils/scope-references-this'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // Rule Definition @@ -227,10 +228,23 @@ module.exports = { }); } + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + // eslint-disable-next-line complexity CallExpression(node) { - if (!emberUtils.isComputedProp(node)) { + if (!emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { return; } diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js index e557240a9c..7f250c752f 100644 --- a/lib/rules/require-computed-property-dependencies.js +++ b/lib/rules/require-computed-property-dependencies.js @@ -47,19 +47,6 @@ function isMemberExpression(node, objectName, propertyName) { ); } -/** - * @param {ASTNode} node - * @param {string} importedEmberName - * @param {string} importedComputedName - * @returns {boolean} - */ -function isEmberComputed(node, importedEmberName, importedComputedName) { - return ( - isIdentifier(node, importedComputedName) || - isMemberExpression(node, importedEmberName, 'computed') - ); -} - /** * Checks if a node looks like: 'part1' + 'part2' * @@ -481,7 +468,7 @@ module.exports = { }, Identifier(node) { - if (isEmberComputed(node, importedEmberName, importedComputedName)) { + if (emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { checkComputedDependencies(node, [], { importedEmberName, importedGetName, @@ -492,7 +479,7 @@ module.exports = { }, CallExpression(node) { - if (isEmberComputed(node.callee, importedEmberName, importedComputedName)) { + if (emberUtils.isComputedProp(node, importedEmberName, importedComputedName)) { checkComputedDependencies(node, node.arguments, { importedEmberName, importedGetName, diff --git a/lib/rules/require-return-from-computed.js b/lib/rules/require-return-from-computed.js index d355d1afc1..14cd03831b 100644 --- a/lib/rules/require-return-from-computed.js +++ b/lib/rules/require-return-from-computed.js @@ -1,6 +1,7 @@ 'use strict'; const ember = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // General rule - Always return a value from computed properties @@ -46,12 +47,30 @@ module.exports = { } } + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + onCodePathStart(codePath) { funcInfo = { upper: funcInfo, codePath, - shouldCheck: context.getAncestors().findIndex(ember.isComputedProp) > -1, + shouldCheck: + context + .getAncestors() + .findIndex((node) => + ember.isComputedProp(node, importedEmberName, importedComputedName) + ) > -1, }; }, @@ -60,7 +79,10 @@ module.exports = { }, 'FunctionExpression:exit'(node) { - if (ember.isComputedProp(node.parent) || ember.isComputedProp(node.parent.parent.parent)) { + if ( + ember.isComputedProp(node.parent, importedEmberName, importedComputedName) || + ember.isComputedProp(node.parent.parent.parent, importedEmberName, importedComputedName) + ) { checkLastSegment(node); } }, diff --git a/lib/rules/use-brace-expansion.js b/lib/rules/use-brace-expansion.js index 39a2d800ca..6aaa528f0c 100644 --- a/lib/rules/use-brace-expansion.js +++ b/lib/rules/use-brace-expansion.js @@ -2,6 +2,7 @@ const types = require('../utils/types'); const ember = require('../utils/ember'); +const { getImportIdentifier } = require('../utils/import'); //------------------------------------------------------------------------------ // General rule - Use brace expansion @@ -26,9 +27,22 @@ module.exports = { ERROR_MESSAGE, create(context) { + let importedEmberName; + let importedComputedName; + return { + ImportDeclaration(node) { + if (node.source.value === 'ember') { + importedEmberName = importedEmberName || getImportIdentifier(node, 'ember'); + } + if (node.source.value === '@ember/object') { + importedComputedName = + importedComputedName || getImportIdentifier(node, '@ember/object', 'computed'); + } + }, + CallExpression(node) { - if (!ember.isComputedProp(node) || types.isMemberExpression(node.callee)) { + if (!ember.isComputedProp(node, importedEmberName, importedComputedName)) { return; } diff --git a/lib/utils/ember.js b/lib/utils/ember.js index 70a6554c41..2bb87f4b59 100644 --- a/lib/utils/ember.js +++ b/lib/utils/ember.js @@ -298,16 +298,70 @@ function isObserverProp(node) { return isPropOfType(node, 'observer'); } -function isComputedProp(node) { - const allowedMemberExpNames = new Set(['volatile', 'readOnly', 'property', 'meta']); - if (types.isMemberExpression(node.callee) && types.isCallExpression(node.callee.object)) { - return ( - isModule(node.callee.object, 'computed') && +const allowedMemberExpNames = new Set(['volatile', 'readOnly', 'property', 'meta']); +/** + * Checks if a node is a computed property. + * @param {node} node + * @param {string} importedEmberName name that `Ember` is imported under + * @param {string} importedComputedName name that `computed` is imported under + * @param {object} options + * @param {boolean} options.includeSuffix whether to consider something like computed().volatile() as a computed property + * @param {boolean} options.includeMacro whether to consider something like computed.and() as a computed property + */ +function isComputedProp( + node, + importedEmberName, + importedComputedName, + { includeSuffix, includeMacro } = {} +) { + return ( + // computed + (types.isIdentifier(node) && node.name === importedComputedName) || + // computed() + (types.isCallExpression(node) && + types.isIdentifier(node.callee) && + node.callee.name === importedComputedName) || + // Ember.computed() + (types.isCallExpression(node) && + types.isMemberExpression(node.callee) && + types.isIdentifier(node.callee.object) && + node.callee.object.name === importedEmberName && types.isIdentifier(node.callee.property) && - allowedMemberExpNames.has(node.callee.property.name) - ); - } - return isModule(node, 'computed'); + node.callee.property.name === 'computed') || + // Ember.computed().volatile() or computed().volatile() + (includeSuffix && + types.isCallExpression(node) && + types.isMemberExpression(node.callee) && + types.isIdentifier(node.callee.property) && + allowedMemberExpNames.has(node.callee.property.name) && + isComputedProp(node.callee.object, importedEmberName, importedComputedName)) || + (includeMacro && isComputedPropMacro(node, importedEmberName, importedComputedName)) + ); +} + +/** + * Checks if a node is a computed property macro such as: computed.and() + * @param {node} node + * @param {string} importedEmberName name that `Ember` is imported under + * @param {string} importedComputedName name that `computed` is imported under + */ +function isComputedPropMacro(node, importedEmberName, importedComputedName) { + return ( + // computed.someMacro() + (types.isCallExpression(node) && + types.isMemberExpression(node.callee) && + types.isIdentifier(node.callee.object) && + node.callee.object.name === importedComputedName && + types.isIdentifier(node.callee.property)) || + // Ember.computed.someMacro() + (types.isCallExpression(node) && + types.isMemberExpression(node.callee) && + types.isMemberExpression(node.callee.object) && + types.isIdentifier(node.callee.object.object) && + node.callee.object.object.name === importedEmberName && + types.isIdentifier(node.callee.object.property) && + node.callee.object.property.name === 'computed') + ); } function isArrayProp(node) { @@ -463,7 +517,8 @@ function isSingleLineFn(property) { types.isCallExpression(property.value) && utils.getSize(property.value) === 1 && !isObserverProp(property) && - (isComputedProp(property.value) || !types.isCallWithFunctionExpression(property.value))) + (isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) || + !types.isCallWithFunctionExpression(property.value))) ); } @@ -474,7 +529,8 @@ function isMultiLineFn(property) { types.isCallExpression(property.value) && utils.getSize(property.value) > 1 && !isObserverProp(property) && - (isComputedProp(property.value) || !types.isCallWithFunctionExpression(property.value))) + (isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) || + !types.isCallWithFunctionExpression(property.value))) ); } @@ -503,8 +559,8 @@ function isRelation(property) { * @param {CallExpression} callExp Given call expression * @return {Boolean} Flag whether dependent keys present. */ -function hasDuplicateDependentKeys(callExp) { - if (!isComputedProp(callExp)) { +function hasDuplicateDependentKeys(callExp, importedEmberName, importedComputedName) { + if (!isComputedProp(callExp, importedEmberName, importedComputedName)) { return false; } diff --git a/package.json b/package.json index 77eda61f94..6ee251083f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-ember", - "version": "8.10.1", + "version": "8.11.0", "description": "Eslint plugin for Ember.js apps", "main": "lib/index.js", "files": [ diff --git a/tests/helpers/test-case.js b/tests/helpers/test-case.js new file mode 100644 index 0000000000..35d243cc09 --- /dev/null +++ b/tests/helpers/test-case.js @@ -0,0 +1,26 @@ +module.exports = { + addPrefix, + addComputedImport, +}; + +function addPrefix(testCase, prefix) { + if (typeof testCase === 'string') { + return `${prefix}${testCase}`; + } + + const updated = { + ...testCase, + code: `${prefix}${testCase.code}`, + }; + + if (testCase.output) { + updated.output = `${prefix}${testCase.output}`; + } + + return updated; +} + +function addComputedImport(testCase) { + const importStatement = "import { computed } from '@ember/object';\n"; + return addPrefix(testCase, importStatement); +} diff --git a/tests/lib/rules/computed-property-getters.js b/tests/lib/rules/computed-property-getters.js index d8f0916822..bd9fa3d9ec 100644 --- a/tests/lib/rules/computed-property-getters.js +++ b/tests/lib/rules/computed-property-getters.js @@ -6,6 +6,7 @@ const rule = require('../../../lib/rules/computed-property-getters'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); //------------------------------------------------------------------------------ // Tests @@ -44,6 +45,11 @@ const codeWithRawComputed = [ foo: computed() }`, ` + import Ember from 'ember'; + { + foo: Ember.computed() + }`, + ` { foo: computed }`, @@ -105,6 +111,12 @@ const codeWithOnlyGetters = [ get() {} }) }`, + `import Ember from 'ember'; + { + foo: Ember.computed('model.foo', { + get() {} + }) + }`, ]; const codeWithOnlySetters = [ @@ -311,6 +323,10 @@ ruleTester.run('computed-property-getters', rule, { ...validWithAlwaysWithSetterOptions, ...validWithNeverOption, ...validWithAlwaysOption, - ], - invalid: [...inValidWithDefaultOptions, ...inValidWithNeverOption, ...inValidWithAlwaysOption], + ].map(addComputedImport), + invalid: [ + ...inValidWithDefaultOptions, + ...inValidWithNeverOption, + ...inValidWithAlwaysOption, + ].map(addComputedImport), }); diff --git a/tests/lib/rules/no-arrow-function-computed-properties.js b/tests/lib/rules/no-arrow-function-computed-properties.js index a740c24507..315f2af9be 100644 --- a/tests/lib/rules/no-arrow-function-computed-properties.js +++ b/tests/lib/rules/no-arrow-function-computed-properties.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/no-arrow-function-computed-properties'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE } = rule; @@ -50,13 +51,18 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { code: "computed.map('products', product => { return someFunction(product); });", options: [{ onlyThisContexts: true }], }, - ], + ].map(addComputedImport), invalid: [ { code: 'computed(() => { return 123; })', output: null, errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], }, + { + code: "import Ember from 'ember'; Ember.computed(() => { return 123; })", + output: null, + errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], + }, { code: "computed('prop', () => { return this.prop; })", output: null, @@ -85,5 +91,5 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], options: [{ onlyThisContexts: true }], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/no-deeply-nested-dependent-keys-with-each.js b/tests/lib/rules/no-deeply-nested-dependent-keys-with-each.js index 78029c1610..9f4990fd5e 100644 --- a/tests/lib/rules/no-deeply-nested-dependent-keys-with-each.js +++ b/tests/lib/rules/no-deeply-nested-dependent-keys-with-each.js @@ -21,72 +21,77 @@ const ruleTester = new RuleTester({ }); ruleTester.run('no-deeply-nested-dependent-keys-with-each', rule, { valid: [ - 'Ember.computed(function() {})', - "Ember.computed('foo', function() {})", - "Ember.computed('foo', function() {}).readOnly()", - "Ember.computed('foo.bar', function() {})", - "Ember.computed('foo.bar.@each.baz', function() {})", - "Ember.computed('foo.@each.bar', function() {})", - "Ember.computed('foo.@each.{bar,baz}', function() {})", - 'computed(function() {})', - "computed('foo', function() {})", - "computed('foo', function() {}).readOnly()", - "computed('foo.bar', function() {})", - "computed('foo.bar.@each.baz', function() {})", - "computed('foo.@each.bar', function() {})", - "computed('foo.@each.{bar,baz}', function() {})", + "import Ember from 'ember'; Ember.computed(function() {})", + "import Ember from 'ember'; Ember.computed('foo', function() {})", + "import Ember from 'ember'; Ember.computed('foo', function() {}).readOnly()", + "import Ember from 'ember'; Ember.computed('foo.bar', function() {})", + "import Ember from 'ember'; Ember.computed('foo.bar.@each.baz', function() {})", + "import Ember from 'ember'; Ember.computed('foo.@each.bar', function() {})", + "import Ember from 'ember'; Ember.computed('foo.@each.{bar,baz}', function() {})", + "import { computed } from '@ember/object'; computed(function() {})", + "import { computed } from '@ember/object'; computed('foo', function() {})", + "import { computed } from '@ember/object'; computed('foo', function() {}).readOnly()", + "import { computed } from '@ember/object'; computed('foo.bar', function() {})", + "import { computed } from '@ember/object'; computed('foo.bar.@each.baz', function() {})", + "import { computed } from '@ember/object'; computed('foo.@each.bar', function() {})", + "import { computed } from '@ember/object'; computed('foo.@each.{bar,baz}', function() {})", // Not Ember's `computed` function: "otherClass.computed('foo.@each.bar.baz', function() {})", "otherClass.myFunction('foo.@each.bar.baz', function() {})", "myFunction('foo.@each.bar.baz', function() {})", - "Ember.myFunction('foo.@each.bar.baz', function() {})", - "computed.unrelatedFunction('foo.@each.bar.baz', function() {})", - "Ember.computed.unrelatedFunction('foo.@each.bar.baz', function() {})", + "import Ember from 'ember'; Ember.myFunction('foo.@each.bar.baz', function() {})", + "import { computed } from '@ember/object'; computed.unrelatedFunction('foo.@each.bar.baz', function() {})", + "import Ember from 'ember'; Ember.computed.unrelatedFunction('foo.@each.bar.baz', function() {})", ], invalid: [ { - code: "Ember.computed('foo.@each.bar.baz', function() {})", + code: "import Ember from 'ember'; Ember.computed('foo.@each.bar.baz', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "Ember.computed('foo.@each.bar.baz', function() {}).readOnly()", + code: + "import Ember from 'ember'; Ember.computed('foo.@each.bar.baz', function() {}).readOnly()", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "Ember.computed('foo.@each.bar.[]', function() {})", + code: "import Ember from 'ember'; Ember.computed('foo.@each.bar.[]', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "Ember.computed('foo.@each.bar.@each.baz', function() {})", + code: "import Ember from 'ember'; Ember.computed('foo.@each.bar.@each.baz', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "computed('foo.@each.bar.baz', function() {})", + code: + "import { computed } from '@ember/object'; computed('foo.@each.bar.baz', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "computed('foo.@each.bar.baz', function() {}).readOnly()", + code: + "import { computed } from '@ember/object'; computed('foo.@each.bar.baz', function() {}).readOnly()", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "computed('foo.@each.bar.[]', function() {})", + code: "import { computed } from '@ember/object'; computed('foo.@each.bar.[]', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "computed('foo.@each.bar.@each.baz', function() {})", + code: + "import { computed } from '@ember/object'; computed('foo.@each.bar.@each.baz', function() {})", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, { - code: "class Test { @computed('foo.@each.bar.baz') get someProp() { return true; } }", + code: + "import { computed } from '@ember/object'; class Test { @computed('foo.@each.bar.baz') get someProp() { return true; } }", output: null, errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }], }, diff --git a/tests/lib/rules/no-duplicate-dependent-keys.js b/tests/lib/rules/no-duplicate-dependent-keys.js index e4a30acb8b..46679ee343 100644 --- a/tests/lib/rules/no-duplicate-dependent-keys.js +++ b/tests/lib/rules/no-duplicate-dependent-keys.js @@ -6,6 +6,7 @@ const rule = require('../../../lib/rules/no-duplicate-dependent-keys'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); //------------------------------------------------------------------------------ // Tests @@ -79,7 +80,7 @@ ruleTester.run('no-duplicate-dependent-keys', rule, { }).volatile() } `, - ], + ].map(addComputedImport), invalid: [ { code: ` @@ -98,6 +99,17 @@ ruleTester.run('no-duplicate-dependent-keys', rule, { }, ], }, + { + code: ` + import Ember from 'ember'; + { foo: Ember.computed('model.foo', 'model.bar', 'model.baz', 'model.foo', function() {}) } + `, + output: ` + import Ember from 'ember'; + { foo: Ember.computed('model.foo', 'model.bar', 'model.baz', function() {}) } + `, + errors: [{ message: ERROR_MESSAGE }], + }, { code: ` { @@ -177,5 +189,5 @@ ruleTester.run('no-duplicate-dependent-keys', rule, { }, ], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js index 200b63b4dd..217232c502 100644 --- a/tests/lib/rules/no-get.js +++ b/tests/lib/rules/no-get.js @@ -89,6 +89,7 @@ ruleTester.run('no-get', rule, { // Not `this`. "myObject.getProperties('prop1', 'prop2');", + "import { getProperties } from '@ember/object'; getProperties(myObject, 'prop1', 'prop2');", // Not `getProperties`. "this.foo('prop1', 'prop2');", @@ -189,6 +190,12 @@ ruleTester.run('no-get', rule, { output: 'this.foo;', errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }], }, + { + code: "foo1.foo2.get('bar');", + options: [{ catchUnsafeObjects: true }], + output: 'foo1.foo2.bar;', + errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }], + }, { code: ` import { get } from '@ember/object'; @@ -204,6 +211,20 @@ ruleTester.run('no-get', rule, { `, errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }], }, + { + // Calling the imported function on an unknown object (without `this`). + code: "import { get } from '@ember/object'; get(foo1.foo2, 'bar');", + options: [{ catchSafeObjects: true }], + output: "import { get } from '@ember/object'; foo1.foo2.bar;", + errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }], + }, + { + // Calling the imported function on an unknown object (without `this`) with an object argument that needs parenthesis. + code: "import { get } from '@ember/object'; get(foo || {}, 'bar');", + options: [{ catchSafeObjects: true }], + output: "import { get } from '@ember/object'; (foo || {}).bar;", + errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }], + }, { // With renamed import: code: "import { get as g } from '@ember/object'; g(this, 'foo');", @@ -257,6 +278,12 @@ ruleTester.run('no-get', rule, { output: null, errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }], }, + { + code: "foo.getProperties('prop1', 'prop2');", + options: [{ catchUnsafeObjects: true }], + output: null, + errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }], + }, { code: "this.getProperties(['prop1', 'prop2']);", // With parameters in array. output: null, @@ -272,6 +299,13 @@ ruleTester.run('no-get', rule, { output: null, errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }], }, + { + // Calling the imported function on an unknown object (without `this`). + code: "import { getProperties } from '@ember/object'; getProperties(foo, 'prop1', 'prop2');", + options: [{ catchSafeObjects: true }], + output: null, + errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }], + }, { // With renamed import: code: "import { getProperties as gp } from '@ember/object'; gp(this, 'prop1', 'prop2');", diff --git a/tests/lib/rules/no-invalid-dependent-keys.js b/tests/lib/rules/no-invalid-dependent-keys.js index 38ab829111..40a8030643 100644 --- a/tests/lib/rules/no-invalid-dependent-keys.js +++ b/tests/lib/rules/no-invalid-dependent-keys.js @@ -1,4 +1,5 @@ const rule = require('../../../lib/rules/no-invalid-dependent-keys'); +const { addComputedImport } = require('../../helpers/test-case'); const RuleTester = require('eslint').RuleTester; const { @@ -53,7 +54,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, { // Unrelated functions 'myFunction("{ key: string }, saving,test}", "b}", false)', 'myFunction("A string containing curly braces {}}")', - ], + ].map(addComputedImport), invalid: [ // Unbalanced braces { @@ -62,19 +63,11 @@ eslintTester.run('no-invalid-dependent-keys', rule, { errors: [ { message: ERROR_MESSAGE_UNBALANCED_BRACES, - line: 1, - column: 18, type: 'Literal', - endLine: 1, - endColumn: 35, }, { message: ERROR_MESSAGE_UNBALANCED_BRACES, - line: 1, - column: 37, type: 'Literal', - endLine: 1, - endColumn: 54, }, ], }, @@ -84,11 +77,18 @@ eslintTester.run('no-invalid-dependent-keys', rule, { errors: [ { message: ERROR_MESSAGE_UNBALANCED_BRACES, - line: 1, - column: 18, type: 'Literal', - endLine: 1, - endColumn: 68, + }, + ], + }, + { + code: + "import Ember from 'ember'; { test: Ember.computed('foo.{bar.{name,place},qux.[],{thing,@each.stuff}', function() {}) }", + output: null, + errors: [ + { + message: ERROR_MESSAGE_UNBALANCED_BRACES, + type: 'Literal', }, ], }, @@ -99,11 +99,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, { errors: [ { message: ERROR_MESSAGE_UNBALANCED_BRACES, - line: 1, - column: 19, type: 'Literal', - endLine: 1, - endColumn: 71, }, ], }, @@ -114,11 +110,7 @@ eslintTester.run('no-invalid-dependent-keys', rule, { errors: [ { message: ERROR_MESSAGE_UNBALANCED_BRACES, - line: 1, - column: 19, type: 'Literal', - endLine: 1, - endColumn: 142, }, ], }, @@ -209,5 +201,5 @@ eslintTester.run('no-invalid-dependent-keys', rule, { output: null, errors: [{ message: ERROR_MESSAGE_UNBALANCED_BRACES, type: 'Literal' }], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/no-side-effects.js b/tests/lib/rules/no-side-effects.js index 8c88ba7b5e..0b78dfb485 100644 --- a/tests/lib/rules/no-side-effects.js +++ b/tests/lib/rules/no-side-effects.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/no-side-effects'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE } = rule; @@ -69,7 +70,7 @@ eslintTester.run('no-side-effects', rule, { 'this.setProperties({ x: 123 });', 'this.x = 123;', 'this.x.y = 123;', - ], + ].map(addComputedImport), invalid: [ { code: 'prop: computed("test", function() {this.set("testAmount", test.length); return "";})', @@ -320,5 +321,5 @@ eslintTester.run('no-side-effects', rule, { }, ], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/no-test-import-export.js b/tests/lib/rules/no-test-import-export.js index 582d146fc8..9638560355 100644 --- a/tests/lib/rules/no-test-import-export.js +++ b/tests/lib/rules/no-test-import-export.js @@ -40,10 +40,20 @@ ruleTester.run('no-test-file-importing', rule, { }, // Importing anything from tests/helpers is allowed. - "import setupApplicationTest from 'tests/helpers/setup-application-test.js';", + "import setupApplicationTest from 'tests/helpers/setup-application-test';", "import { setupApplicationTest } from 'tests/helpers';", - "import setupApplicationTest from 'my-app-name/tests/helpers/setup-application-test.js';", + "import setupApplicationTest from 'my-app-name/tests/helpers/setup-application-test';", "import { setupApplicationTest } from 'my-app-name/tests/helpers';", + + // Importing anything from test/helpers is allowed (using relative path) + { + filename: 'my-app-name/tests/helpers/foo.js', + code: "import setupApplicationTest from './setup-application-test';", + }, + { + filename: 'my-app-name/tests/helpers/nested/foo.js', + code: "import setupApplicationTest from '../setup-application-test';", + }, ], invalid: [ { @@ -91,6 +101,13 @@ ruleTester.run('no-test-file-importing', rule, { }, ], }, + { + // Importing from a test file outside test/helpers is disallowed. + filename: 'my-app-name/tests/helpers/foo.js', + code: "import testModule from '../../test-dir/another-test';", + output: null, + errors: [{ message: NO_IMPORT_MESSAGE }], + }, { filename: 'tests/some-test.js', code: ` diff --git a/tests/lib/rules/no-volatile-computed-properties.js b/tests/lib/rules/no-volatile-computed-properties.js index 66103a001a..b247be2865 100644 --- a/tests/lib/rules/no-volatile-computed-properties.js +++ b/tests/lib/rules/no-volatile-computed-properties.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/no-volatile-computed-properties'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE } = rule; @@ -11,7 +12,7 @@ const { ERROR_MESSAGE } = rule; // Tests //------------------------------------------------------------------------------ -const ruleTester = new RuleTester(); +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018, sourceType: 'module' } }); ruleTester.run('no-volatile-computed-properties', rule, { valid: [ 'computed()', @@ -31,7 +32,7 @@ ruleTester.run('no-volatile-computed-properties', rule, { ecmaFeatures: { legacyDecorators: true }, }, }, - ], + ].map(addComputedImport), invalid: [ { code: 'computed().volatile()', @@ -39,6 +40,12 @@ ruleTester.run('no-volatile-computed-properties', rule, { errors: [{ message: ERROR_MESSAGE, type: 'Identifier' }], }, + { + code: "import Ember from 'ember'; Ember.computed().volatile()", + output: null, + errors: [{ message: ERROR_MESSAGE, type: 'Identifier' }], + }, + { code: "computed('prop', function() { return this.prop; }).volatile()", output: null, @@ -57,5 +64,5 @@ ruleTester.run('no-volatile-computed-properties', rule, { ecmaFeatures: { legacyDecorators: true }, }, }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/require-computed-macros.js b/tests/lib/rules/require-computed-macros.js index 16305b0eb8..3f0a0eeb57 100644 --- a/tests/lib/rules/require-computed-macros.js +++ b/tests/lib/rules/require-computed-macros.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/require-computed-macros'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE_READS, @@ -119,6 +120,11 @@ ruleTester.run('require-computed-macros', rule, { output: "computed.reads('x')", errors: [{ message: ERROR_MESSAGE_READS, type: 'CallExpression' }], }, + { + code: "import Ember from 'ember'; Ember.computed(function() { return this.x; })", + output: "import Ember from 'ember'; computed.reads('x')", + errors: [{ message: ERROR_MESSAGE_READS, type: 'CallExpression' }], + }, { code: 'computed(function() { return this.x.y; })', // Nested path. output: "computed.reads('x.y')", @@ -281,5 +287,5 @@ ruleTester.run('require-computed-macros', rule, { output: "class Test { @computed.mapBy('children', 'age') someProp }", errors: [{ message: ERROR_MESSAGE_MAP_BY, type: 'MethodDefinition' }], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/require-return-from-computed.js b/tests/lib/rules/require-return-from-computed.js index 34e11917c3..b6ad7180a7 100644 --- a/tests/lib/rules/require-return-from-computed.js +++ b/tests/lib/rules/require-return-from-computed.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/require-return-from-computed'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE } = rule; @@ -26,10 +27,7 @@ eslintTester.run('require-return-from-computed', rule, { { // This rule intentionally does not apply to native classes / decorator usage. // ESLint already has its own recommended rules `getter-return` and `no-setter-return` for this. - code: ` - import { computed } from '@ember/object'; - class Test { @computed() get someProp() {} set someProp(val) {} } - `, + code: 'class Test { @computed() get someProp() {} set someProp(val) {} }', parser: require.resolve('babel-eslint'), parserOptions: { ecmaVersion: 6, @@ -37,7 +35,7 @@ eslintTester.run('require-return-from-computed', rule, { ecmaFeatures: { legacyDecorators: true }, }, }, - ], + ].map(addComputedImport), invalid: [ { code: 'let foo = computed("test", function() { })', @@ -49,6 +47,16 @@ eslintTester.run('require-return-from-computed', rule, { }, ], }, + { + code: "import Ember from 'ember'; let foo = Ember.computed('test', function() { })", + output: null, + errors: [ + { + message: ERROR_MESSAGE, + type: 'FunctionExpression', + }, + ], + }, { code: 'let foo = computed("test", function() { if (true) { return ""; } })', output: null, @@ -93,5 +101,5 @@ eslintTester.run('require-return-from-computed', rule, { }, ], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/rules/use-brace-expansion.js b/tests/lib/rules/use-brace-expansion.js index 7733cf5e96..bcb25b617e 100644 --- a/tests/lib/rules/use-brace-expansion.js +++ b/tests/lib/rules/use-brace-expansion.js @@ -4,6 +4,7 @@ const rule = require('../../../lib/rules/use-brace-expansion'); const RuleTester = require('eslint').RuleTester; +const { addComputedImport } = require('../../helpers/test-case'); const { ERROR_MESSAGE } = rule; @@ -35,7 +36,7 @@ eslintTester.run('use-brace-expansion', rule, { // Decorator: "class Test { @computed('a.{test1,test2}') get someProp() { return true; } }", - ], + ].map(addComputedImport), invalid: [ { code: @@ -45,10 +46,17 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 73, + }, + ], + }, + { + code: + "import Ember from 'ember'; { test: Ember.computed('foo.{name,place}', 'foo.[]', 'foo.{thing,@each.stuff}', function() {}) }", + output: null, + errors: [ + { + message: ERROR_MESSAGE, + type: 'CallExpression', }, ], }, @@ -59,10 +67,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 37, }, ], }, @@ -73,10 +77,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 52, }, ], }, @@ -87,10 +87,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 45, }, ], }, @@ -101,10 +97,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 56, }, ], }, @@ -115,10 +107,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 45, }, ], }, @@ -129,10 +117,6 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 18, - endColumn: 37, }, ], }, @@ -143,12 +127,8 @@ eslintTester.run('use-brace-expansion', rule, { { message: ERROR_MESSAGE, type: 'CallExpression', - line: 1, - endLine: 1, - column: 24, - endColumn: 33, }, ], }, - ], + ].map(addComputedImport), }); diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js index 9f7123d91b..f686f3b73b 100644 --- a/tests/lib/utils/ember-test.js +++ b/tests/lib/utils/ember-test.js @@ -882,40 +882,111 @@ describe('isComputedProp', () => { it("should check if it's an computed prop", () => { node = parse('computed()'); - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeTruthy(); node = parse('Ember.computed()'); - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeTruthy(); + + node = parse('foo()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + + node = parse('Ember.foo()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + + node = parse('Foo.computed()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + + // Non-standard names: + node = parse('c()'); + expect(emberUtils.isComputedProp(node, 'E', 'c')).toBeTruthy(); + + // Non-standard names: + node = parse('E.computed()'); + expect(emberUtils.isComputedProp(node, 'E', 'c')).toBeTruthy(); }); it('should detect allow-listed computed props with MemberExpressions', () => { ['volatile', 'meta', 'readOnly', 'property'].forEach((prop) => { + // With includeSuffix + node = parse(`computed().${prop}()`); - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect( + emberUtils.isComputedProp(node, 'Ember', 'computed', { includeSuffix: true }) + ).toBeTruthy(); node = parse(`Ember.computed().${prop}()`); - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect( + emberUtils.isComputedProp(node, 'Ember', 'computed', { includeSuffix: true }) + ).toBeTruthy(); + + // Without includeSuffix + + node = parse(`computed().${prop}()`); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + + node = parse(`Ember.computed().${prop}()`); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); }); }); it("shouldn't allow other MemberExpressions", () => { node = parse('computed().foo()'); - expect(emberUtils.isComputedProp(node)).not.toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).not.toBeTruthy(); node = parse('Ember.computed().foo()'); - expect(emberUtils.isComputedProp(node)).not.toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).not.toBeTruthy(); + + node = parse('computed.foo()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).not.toBeTruthy(); + + node = parse('Ember.computed.foo()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).not.toBeTruthy(); }); it('should detect the computed annotation', () => { const program = babelEslint.parse('class Object { @computed() get foo() {} }'); node = program.body[0].body.body[0].decorators[0].expression; - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeTruthy(); }); it('should detect the computed annotation without parentheses', () => { const program = babelEslint.parse('class Object { @computed get foo() {} }'); node = program.body[0].body.body[0].decorators[0].expression; - expect(emberUtils.isComputedProp(node)).toBeTruthy(); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeTruthy(); + }); + + it('should not detect a sub-module decorator', () => { + const program = babelEslint.parse('class Object { @computed.foo() get foo() {} }'); + node = program.body[0].body.body[0].decorators[0].expression; + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + }); + + it('should not detect the wrong decorator', () => { + const program = babelEslint.parse('class Object { @foo() get foo() {} }'); + node = program.body[0].body.body[0].decorators[0].expression; + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + }); + + it('should detect macros', () => { + // With includeMacro + + node = parse('computed.someMacro()'); + expect( + emberUtils.isComputedProp(node, 'Ember', 'computed', { includeMacro: true }) + ).toBeTruthy(); + + node = parse('Ember.computed.someMacro()'); + expect( + emberUtils.isComputedProp(node, 'Ember', 'computed', { includeMacro: true }) + ).toBeTruthy(); + + // Without includeMacro + + node = parse('computed.someMacro()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); + + node = parse('Ember.computed.someMacro()'); + expect(emberUtils.isComputedProp(node, 'Ember', 'computed')).toBeFalsy(); }); }); @@ -1312,30 +1383,23 @@ describe('unwrapBraceExpressions', () => { describe('hasDuplicateDependentKeys', () => { it('reports duplicate dependent keys in computed calls', () => { let node = parse("computed('model.{foo,bar}', 'model.bar')"); - expect(emberUtils.hasDuplicateDependentKeys(node)).toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).toBeTruthy(); node = parse("Ember.computed('model.{foo,bar}', 'model.bar')"); - expect(emberUtils.hasDuplicateDependentKeys(node)).toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).toBeTruthy(); }); it('ignores not repeated dependentKeys', () => { let node = parse("computed('model.{foo,bar}', 'model.qux')"); - expect(emberUtils.hasDuplicateDependentKeys(node)).not.toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).not.toBeTruthy(); node = parse("Ember.computed('model.{foo,bar}', 'model.qux')"); - expect(emberUtils.hasDuplicateDependentKeys(node)).not.toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).not.toBeTruthy(); node = parse("computed('model.{foo,bar}', 'model.qux').volatile()"); - expect(emberUtils.hasDuplicateDependentKeys(node)).not.toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).not.toBeTruthy(); }); it('ignores non-computed calls', () => { const node = parse("foo('model.{foo,bar}', 'model.bar')"); - expect(emberUtils.hasDuplicateDependentKeys(node)).not.toBeTruthy(); - }); - - it('reports duplicate dependent keys in computed calls with MemberExp', () => { - let node = parse("Ember.computed('model.{foo,bar}', 'model.bar').volatile()"); - expect(emberUtils.hasDuplicateDependentKeys(node)).toBeTruthy(); - node = parse("computed('model.{foo,bar}', 'model.bar').volatile()"); - expect(emberUtils.hasDuplicateDependentKeys(node)).toBeTruthy(); + expect(emberUtils.hasDuplicateDependentKeys(node, 'Ember', 'computed')).not.toBeTruthy(); }); });