Skip to content

Commit

Permalink
Merge pull request #913 from bmish/no-get-unsafe-objects
Browse files Browse the repository at this point in the history
Add `catchUnsafeObjects` option (default false) to `no-get` rule to catch `foo.get('bar')`
  • Loading branch information
bmish committed Aug 14, 2020
2 parents 50bdf60 + af59282 commit 66a27a8
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
1 change: 1 addition & 0 deletions docs/rules/no-get.md
Expand Up @@ -95,6 +95,7 @@ This rule takes an optional object containing:
* `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
Expand Down
12 changes: 9 additions & 3 deletions lib/rules/no-get.js
Expand Up @@ -106,6 +106,10 @@ module.exports = {
type: 'boolean',
default: false,
},
catchUnsafeObjects: {
type: 'boolean',
default: false,
},
},
additionalProperties: false,
},
Expand All @@ -117,6 +121,7 @@ module.exports = {
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(
Expand Down Expand Up @@ -199,21 +204,22 @@ 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 &&
types.isStringLiteral(node.arguments[0]) &&
(!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: 'this',
objectText: sourceCode.getText(node.callee.object),
});
}

Expand Down Expand Up @@ -247,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)
Expand Down
12 changes: 12 additions & 0 deletions tests/lib/rules/no-get.js
Expand Up @@ -190,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';
Expand Down Expand Up @@ -272,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,
Expand Down

0 comments on commit 66a27a8

Please sign in to comment.