Skip to content

Commit

Permalink
updates
Browse files Browse the repository at this point in the history
  • Loading branch information
Dongyue Zhu committed May 17, 2023
1 parent 208e6b6 commit 63e0286
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 35 deletions.
1 change: 1 addition & 0 deletions docs/rules/no-get.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ This rule takes an optional object containing:
- `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 `true`)
- `boolean` -- `catchSafeObjects` -- whether the rule should catch non-`this` imported usages like `get(foo, 'bar')` (default `true`)
- `boolean` -- `catchUnsafeObjects` -- whether the rule should catch non-`this` usages like `foo.get('bar')` even though we don't know for sure if `foo` is an Ember object (default `false`)
- `boolean` -- `useAt` -- whether the rule should use `at(-1)` to replace `lastObject` (default `false`)
## Related Rules
Expand Down
52 changes: 32 additions & 20 deletions lib/rules/no-get.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function isValidJSPath(str) {
return str.split('.').every((part) => isValidJSVariableName(part) || isValidJSArrayIndex(part));
}

function reportGet({ node, context, path, useOptionalChaining, objectText }) {
function reportGet({ node, context, path, useOptionalChaining, useAt, objectText }) {
const isInLeftSideOfAssignmentExpression = utils.isInLeftSideOfAssignmentExpression(node);
context.report({
node,
Expand All @@ -34,6 +34,7 @@ function reportGet({ node, context, path, useOptionalChaining, objectText }) {
fixer,
path,
useOptionalChaining,
useAt,
isInLeftSideOfAssignmentExpression,
objectText,
});
Expand All @@ -46,6 +47,7 @@ function fixGet({
fixer,
path,
useOptionalChaining,
useAt,
isInLeftSideOfAssignmentExpression,
objectText,
}) {
Expand All @@ -64,12 +66,10 @@ function fixGet({
// Do not autofix since the path would not be a valid JS path.
return null;
}

if (path.match(/lastObject/g)?.length > 1) {
// Do not autofix when multiple `lastObject` are chained.
if (path.match(/lastObject/g)?.length > 1 && !useAt) {
// Do not autofix when multiple `lastObject` are chained, and use `at(-1)` is not allowed.
return null;
}

let replacementPath = shouldIgnoreOptionalChaining ? path : path.replace(/\./g, '?.');

// Replace any array element access (foo.1 => foo[1] or foo?.[1]).
Expand All @@ -82,20 +82,25 @@ function fixGet({
replacementPath = replacementPath
.replace(/\.firstObject/g, shouldIgnoreOptionalChaining ? '[0]' : '.[0]') // When `firstObject` is used in the middle of the path. e.g. foo.firstObject
.replace(/^firstObject\??\./, shouldIgnoreOptionalChaining ? '[0].' : '[0]?.') // When `firstObject` is used at the beginning of the path. e.g. firstObject.bar
.replace(/^firstObject$/, '[0]') // When `firstObject` is used as the entire path.
.replace(
/\??\.lastObject/, // When `lastObject` is used in the middle of the path. e.g. foo.lastObject
(_, offset) =>
`${shouldIgnoreOptionalChaining ? '' : '?.'}[${objectText}.${replacementPath.slice(
0,
offset
)}.length - 1]`
)
.replace(
/^lastObject\??\./, // When `lastObject` is used at the beginning of the path. e.g. lastObject.bar
`[${objectText}.length - 1]${shouldIgnoreOptionalChaining ? '.' : '?.'}`
)
.replace(/^lastObject$/, `[${objectText}.length - 1]`); // When `lastObject` is used as the entire path.
.replace(/^firstObject$/, '[0]'); // When `firstObject` is used as the entire path.

Check failure on line 86 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Delete `····`

Check failure on line 86 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Delete `····`

Check failure on line 86 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Delete `····`
if(useAt) {

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

This `if` statement can be replaced by a ternary expression

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Insert `·`

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

This `if` statement can be replaced by a ternary expression

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Insert `·`

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

This `if` statement can be replaced by a ternary expression

Check failure on line 87 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Insert `·`
replacementPath= replacementPath.replace(/lastObject/g, 'at(-1)');

Check failure on line 88 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Insert `·`

Check failure on line 88 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Insert `·`

Check failure on line 88 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Insert `·`
} else {
replacementPath= replacementPath.replace(

Check failure on line 90 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Replace `=·replacementPath` with `·=·replacementPath⏎······`

Check failure on line 90 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Replace `=·replacementPath` with `·=·replacementPath⏎······`

Check failure on line 90 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Replace `=·replacementPath` with `·=·replacementPath⏎······`
/\??\.lastObject/, // When `lastObject` is used in the middle of the path. e.g. foo.lastObject
(_, offset) =>
`${shouldIgnoreOptionalChaining ? '' : '?.'}[${objectText}.${replacementPath.slice(
0,
offset
)}.length - 1]`
)
.replace(
/^lastObject\??\./, // When `lastObject` is used at the beginning of the path. e.g. lastObject.bar
`[${objectText}.length - 1]${shouldIgnoreOptionalChaining ? '.' : '?.'}`
)
.replace(/^lastObject$/, `[${objectText}.length - 1]`); // When `lastObject` is used as the entire path.
}

Check failure on line 103 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Delete `··`

Check failure on line 103 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Delete `··`

Check failure on line 103 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Delete `··`

// Add parenthesis around the object text in case of something like this: get(foo || {}, 'bar')
const objectTextSafe = isValidJSPath(objectText) ? objectText : `(${objectText})`;
Expand Down Expand Up @@ -137,7 +142,7 @@ function fixGetProperties({ fixer, node, objectText, propertyNames }) {
// When destructuring assignment is in the left side of "=".
// Example: const { foo, bar } = getProperties(this.obj, "foo", "bar");
// Expectation:
// const const { foo, bar } = this.obj;
// const { foo, bar } = this.obj;

return fixer.replaceText(node, `${objectText}`);
}
Expand Down Expand Up @@ -183,6 +188,10 @@ module.exports = {
type: 'boolean',
default: false,
},
useAt: {
type: 'boolean',
default: false,
}

Check failure on line 194 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 14.x) 

Insert `,`

Check failure on line 194 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 16.x) 

Insert `,`

Check failure on line 194 in lib/rules/no-get.js

View workflow job for this annotation

GitHub Actions / build (ubuntu, 18.x) 

Insert `,`
},
additionalProperties: false,
},
Expand All @@ -195,6 +204,7 @@ module.exports = {
const useOptionalChaining = context.options[0] && context.options[0].useOptionalChaining;
const catchSafeObjects = context.options[0] ? context.options[0].catchSafeObjects : true;
const catchUnsafeObjects = context.options[0] && context.options[0].catchUnsafeObjects;
const useAt = context.options[0] && context.options[0].useAt;

if (ignoreNestedPaths && useOptionalChaining) {
assert(
Expand Down Expand Up @@ -295,6 +305,7 @@ module.exports = {
path: node.arguments[0].value,
isImportedGet: false,
useOptionalChaining,
useAt,
objectText: sourceCode.getText(node.callee.object),
});
}
Expand All @@ -315,6 +326,7 @@ module.exports = {
path: node.arguments[1].value,
isImportedGet: true,
useOptionalChaining,
useAt,
objectText: sourceCode.getText(node.arguments[0]),
});
}
Expand Down
100 changes: 85 additions & 15 deletions tests/lib/rules/no-get.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,19 @@ ruleTester.run('no-get', rule, {
},
],
},
{
// `firstObject` used in multiple places in a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('firstObject.foo.firstObject.bar')[123];",
output: 'this[0].foo[0].bar[123];',
options: [{ useOptionalChaining: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
type: 'CallExpression',
},
],
},
{
// `firstObject` used in the middle of a path.
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
Expand Down Expand Up @@ -757,8 +770,8 @@ ruleTester.run('no-get', rule, {
// `lastObject` used in the middle of a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('foo.lastObject.bar')[123];",
output: 'this.foo[this.foo.length - 1].bar[123];',
options: [{ useOptionalChaining: true }],
output: 'this.foo.at(-1).bar[123];',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -770,8 +783,8 @@ ruleTester.run('no-get', rule, {
// `lastObject` used at the beginning of a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('lastObject.bar')[123];",
output: 'this[this.length - 1].bar[123];',
options: [{ useOptionalChaining: true }],
output: 'this.at(-1).bar[123];',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -783,8 +796,8 @@ ruleTester.run('no-get', rule, {
// `lastObject` used as the entire path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('lastObject')[123];",
output: 'this[this.length - 1][123];',
options: [{ useOptionalChaining: true }],
output: 'this.at(-1)[123];',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -796,8 +809,8 @@ ruleTester.run('no-get', rule, {
// `lastObject` used in the middle of a path.
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
code: "this.get('foo.lastObject.bar');",
output: 'this.foo?.[this.foo.length - 1]?.bar;',
options: [{ useOptionalChaining: true }],
output: 'this.foo?.at(-1)?.bar;',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -809,8 +822,8 @@ ruleTester.run('no-get', rule, {
// `lastObject` used at the beginning of a path.
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
code: "this.get('lastObject.bar');",
output: 'this[this.length - 1]?.bar;',
options: [{ useOptionalChaining: true }],
output: 'this.at(-1)?.bar;',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -819,11 +832,11 @@ ruleTester.run('no-get', rule, {
],
},
{
// `lastObject` used in the middle of a path.
// When multiple `lastObject` are chained, it won't auto-fix.
// multiple `lastObject` used in the middle of a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('foo.lastObject.bar.lastObject')[123];",
output: null,
options: [{ useOptionalChaining: true }],
output: 'this.foo.at(-1).bar.at(-1)[123];',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand All @@ -835,8 +848,65 @@ ruleTester.run('no-get', rule, {
// `lastObject` used at the beginning of a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('lastObject.bar.lastObject')[123];",
output: 'this.at(-1).bar.at(-1)[123];',
options: [{ useOptionalChaining: true, useAt: true }],
errors: [
{
message: ERROR_MESSAGE_GET,
type: 'CallExpression',
},
],
},

{
// useAt = false.
// `lastObject` used at the beginning of a path.
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
code: "this.get('lastObject.bar');",
output: 'this[this.length - 1]?.bar;',
options: [{ useOptionalChaining: true, useAt: false }],
errors: [
{
message: ERROR_MESSAGE_GET,
type: 'CallExpression',
},
],
},
{
// useAt = false.
// `lastObject` used as the entire path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('lastObject')[123];",
output: 'this[this.length - 1][123];',
options: [{ useOptionalChaining: true, useAt: false }],
errors: [
{
message: ERROR_MESSAGE_GET,
type: 'CallExpression',
},
],
},
{
// useAt = false.
// multiple `lastObject` used in the middle of a path.
// And the result of get() is chained (getResultIsChained=true).
code: "this.get('foo.lastObject.bar.lastObject')[123];",
output: null,
options: [{ useOptionalChaining: true }],
options: [{ useOptionalChaining: true, useAt: false }],
errors: [
{
message: ERROR_MESSAGE_GET,
type: 'CallExpression',
},
],
},
{
// useAt = false.
// `lastObject` used in the middle of a path.
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
code: "this.get('foo.lastObject.bar');",
output: 'this.foo?.[this.foo.length - 1]?.bar;',
options: [{ useOptionalChaining: true, useAt: false }],
errors: [
{
message: ERROR_MESSAGE_GET,
Expand Down

0 comments on commit 63e0286

Please sign in to comment.