Skip to content

Commit 7e82eb1

Browse files
ArtixZArtixZ
authored
Fix a bug in autofixer and autofix additional cases with firstObject and lastObject in no-get` rule (#1841)
1 parent 0b314ce commit 7e82eb1

File tree

2 files changed

+235
-9
lines changed

2 files changed

+235
-9
lines changed

‎lib/rules/no-get.js‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,13 @@ function fixGet({
4949
isInLeftSideOfAssignmentExpression,
5050
objectText,
5151
}) {
52-
if (path.includes('.') && !useOptionalChaining && !isInLeftSideOfAssignmentExpression) {
52+
const getResultIsChained = node.parent.type === 'MemberExpression' && node.parent.object === node;
53+
54+
// If the result of get is chained, we can safely autofix nests paths without using optional chaining.
55+
// In the left side of an assignment, we can safely autofix nested paths without using optional chaining.
56+
const shouldIgnoreOptionalChaining = getResultIsChained || isInLeftSideOfAssignmentExpression;
57+
58+
if (path.includes('.') && !useOptionalChaining && !shouldIgnoreOptionalChaining) {
5359
// Not safe to autofix nested properties because some properties in the path might be null or undefined.
5460
return null;
5561
}
@@ -59,19 +65,38 @@ function fixGet({
5965
return null;
6066
}
6167

62-
const getResultIsChained = node.parent.type === 'MemberExpression' && node.parent.object === node;
68+
if (path.match(/lastObject/g)?.length > 1) {
69+
// Do not autofix when multiple `lastObject` are chained.
70+
return null;
71+
}
6372

64-
// If the result of get is chained, we can safely autofix nests paths without using optional chaining.
65-
// In the left side of an assignment, we can safely autofix nested paths without using optional chaining.
66-
let replacementPath =
67-
getResultIsChained || isInLeftSideOfAssignmentExpression ? path : path.replace(/\./g, '?.');
73+
let replacementPath = shouldIgnoreOptionalChaining ? path : path.replace(/\./g, '?.');
6874

6975
// Replace any array element access (foo.1 => foo[1] or foo?.[1]).
7076
replacementPath = replacementPath
71-
.replace(/\.(\d+)/g, isInLeftSideOfAssignmentExpression ? '[$1]' : '.[$1]') // Usages in middle of path.
72-
.replace(/^(\d+)\??\./, isInLeftSideOfAssignmentExpression ? '[$1].' : '[$1]?.') // Usage at beginning of path.
77+
.replace(/\.(\d+)/g, shouldIgnoreOptionalChaining ? '[$1]' : '.[$1]') // Usages in middle of path.
78+
.replace(/^(\d+)\??\./, shouldIgnoreOptionalChaining ? '[$1].' : '[$1]?.') // Usage at beginning of path.
7379
.replace(/^(\d+)$/, '[$1]'); // Usage as entire string.
7480

81+
// Replace any array element access using `firstObject` and `lastObject` (foo.firstObject => foo[0] or foo?.[0]).
82+
replacementPath = replacementPath
83+
.replace(/\.firstObject/g, shouldIgnoreOptionalChaining ? '[0]' : '.[0]') // When `firstObject` is used in the middle of the path. e.g. foo.firstObject
84+
.replace(/^firstObject\??\./, shouldIgnoreOptionalChaining ? '[0].' : '[0]?.') // When `firstObject` is used at the beginning of the path. e.g. firstObject.bar
85+
.replace(/^firstObject$/, '[0]') // When `firstObject` is used as the entire path.
86+
.replace(
87+
/\??\.lastObject/, // When `lastObject` is used in the middle of the path. e.g. foo.lastObject
88+
(_, offset) =>
89+
`${shouldIgnoreOptionalChaining ? '' : '?.'}[${objectText}.${replacementPath.slice(
90+
0,
91+
offset
92+
)}.length - 1]`
93+
)
94+
.replace(
95+
/^lastObject\??\./, // When `lastObject` is used at the beginning of the path. e.g. lastObject.bar
96+
`[${objectText}.length - 1]${shouldIgnoreOptionalChaining ? '.' : '?.'}`
97+
)
98+
.replace(/^lastObject$/, `[${objectText}.length - 1]`); // When `lastObject` is used as the entire path.
99+
75100
// Add parenthesis around the object text in case of something like this: get(foo || {}, 'bar')
76101
const objectTextSafe = isValidJSPath(objectText) ? objectText : `(${objectText})`;
77102

‎tests/lib/rules/no-get.js‎

Lines changed: 202 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,9 @@ ruleTester.run('no-get', rule, {
226226
},
227227
{
228228
// useOptionalChaining = false
229+
// We can safely autofix nested paths when the result of get() is chained,
229230
code: "foo1.foo2.get('bar.bar').baz;",
230-
output: null,
231+
output: 'foo1.foo2.bar.bar.baz;',
231232
options: [{ catchUnsafeObjects: true, useOptionalChaining: false }],
232233
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
233234
},
@@ -618,6 +619,18 @@ ruleTester.run('no-get', rule, {
618619
},
619620
],
620621
},
622+
{
623+
// We can safely autofix nested paths when the result of get() is chained,
624+
code: "this.get('foo.0.bar')[123];",
625+
output: 'this.foo[0].bar[123];',
626+
options: [{ useOptionalChaining: true }],
627+
errors: [
628+
{
629+
message: ERROR_MESSAGE_GET,
630+
type: 'CallExpression',
631+
},
632+
],
633+
},
621634
{
622635
// Handle array element access (left side of an assignment, entire string).
623636
code: "this.get('0')[123] = 'hello world';",
@@ -643,6 +656,194 @@ ruleTester.run('no-get', rule, {
643656
],
644657
},
645658

659+
{
660+
code: `
661+
import { get } from '@ember/object';
662+
import { somethingElse } from '@ember/object';
663+
import { random } from 'random';
664+
get(this, 'foo.firstObject.bar');
665+
`,
666+
output: `
667+
import { get } from '@ember/object';
668+
import { somethingElse } from '@ember/object';
669+
import { random } from 'random';
670+
this.foo?.[0]?.bar;
671+
`,
672+
options: [{ useOptionalChaining: true }],
673+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
674+
},
675+
{
676+
code: `
677+
import { get as g } from '@ember/object';
678+
import { somethingElse } from '@ember/object';
679+
import { random } from 'random';
680+
g(obj.baz.qux, 'foo.firstObject.bar');
681+
`,
682+
output: `
683+
import { get as g } from '@ember/object';
684+
import { somethingElse } from '@ember/object';
685+
import { random } from 'random';
686+
obj.baz.qux.foo?.[0]?.bar;
687+
`,
688+
options: [{ useOptionalChaining: true }],
689+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
690+
},
691+
{
692+
// `firstObject` used in the middle of a path.
693+
// And the result of get() is chained (getResultIsChained=true).
694+
code: "this.get('foo.firstObject.bar')[123];",
695+
output: 'this.foo[0].bar[123];',
696+
options: [{ useOptionalChaining: true }],
697+
errors: [
698+
{
699+
message: ERROR_MESSAGE_GET,
700+
type: 'CallExpression',
701+
},
702+
],
703+
},
704+
{
705+
// `firstObject` used in the middle of a path.
706+
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
707+
code: "this.get('foo.firstObject.bar');",
708+
output: 'this.foo?.[0]?.bar;',
709+
options: [{ useOptionalChaining: true }],
710+
errors: [
711+
{
712+
message: ERROR_MESSAGE_GET,
713+
type: 'CallExpression',
714+
},
715+
],
716+
},
717+
{
718+
// `firstObject` used at the beginning of a path.
719+
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
720+
code: "this.get('firstObject.bar');",
721+
output: 'this[0]?.bar;',
722+
options: [{ useOptionalChaining: true }],
723+
errors: [
724+
{
725+
message: ERROR_MESSAGE_GET,
726+
type: 'CallExpression',
727+
},
728+
],
729+
},
730+
{
731+
// `firstObject` used as the entire path.
732+
// And the result of get() is chained (getResultIsChained=true).
733+
code: "this.get('firstObject')[123];",
734+
output: 'this[0][123];',
735+
options: [{ useOptionalChaining: true }],
736+
errors: [
737+
{
738+
message: ERROR_MESSAGE_GET,
739+
type: 'CallExpression',
740+
},
741+
],
742+
},
743+
{
744+
// `firstObject` used in the middle of a path.
745+
// And `get` is used in a left side of an assignment (isInLeftSideOfAssignmentExpression=true).
746+
code: "this.get('foo.firstObject.bar')[123] = 'hello world';",
747+
output: "this.foo[0].bar[123] = 'hello world';",
748+
options: [{ useOptionalChaining: true }],
749+
errors: [
750+
{
751+
message: ERROR_MESSAGE_GET,
752+
type: 'CallExpression',
753+
},
754+
],
755+
},
756+
{
757+
// `lastObject` used in the middle of a path.
758+
// And the result of get() is chained (getResultIsChained=true).
759+
code: "this.get('foo.lastObject.bar')[123];",
760+
output: 'this.foo[this.foo.length - 1].bar[123];',
761+
options: [{ useOptionalChaining: true }],
762+
errors: [
763+
{
764+
message: ERROR_MESSAGE_GET,
765+
type: 'CallExpression',
766+
},
767+
],
768+
},
769+
{
770+
// `lastObject` used at the beginning of a path.
771+
// And the result of get() is chained (getResultIsChained=true).
772+
code: "this.get('lastObject.bar')[123];",
773+
output: 'this[this.length - 1].bar[123];',
774+
options: [{ useOptionalChaining: true }],
775+
errors: [
776+
{
777+
message: ERROR_MESSAGE_GET,
778+
type: 'CallExpression',
779+
},
780+
],
781+
},
782+
{
783+
// `lastObject` used as the entire path.
784+
// And the result of get() is chained (getResultIsChained=true).
785+
code: "this.get('lastObject')[123];",
786+
output: 'this[this.length - 1][123];',
787+
options: [{ useOptionalChaining: true }],
788+
errors: [
789+
{
790+
message: ERROR_MESSAGE_GET,
791+
type: 'CallExpression',
792+
},
793+
],
794+
},
795+
{
796+
// `lastObject` used in the middle of a path.
797+
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
798+
code: "this.get('foo.lastObject.bar');",
799+
output: 'this.foo?.[this.foo.length - 1]?.bar;',
800+
options: [{ useOptionalChaining: true }],
801+
errors: [
802+
{
803+
message: ERROR_MESSAGE_GET,
804+
type: 'CallExpression',
805+
},
806+
],
807+
},
808+
{
809+
// `lastObject` used at the beginning of a path.
810+
// And the resolved path of `get` is NOT chained (getResultIsChained=false).
811+
code: "this.get('lastObject.bar');",
812+
output: 'this[this.length - 1]?.bar;',
813+
options: [{ useOptionalChaining: true }],
814+
errors: [
815+
{
816+
message: ERROR_MESSAGE_GET,
817+
type: 'CallExpression',
818+
},
819+
],
820+
},
821+
{
822+
// `lastObject` used in the middle of a path.
823+
// When multiple `lastObject` are chained, it won't auto-fix.
824+
code: "this.get('foo.lastObject.bar.lastObject')[123];",
825+
output: null,
826+
options: [{ useOptionalChaining: true }],
827+
errors: [
828+
{
829+
message: ERROR_MESSAGE_GET,
830+
type: 'CallExpression',
831+
},
832+
],
833+
},
834+
{
835+
// `lastObject` used at the beginning of a path.
836+
// And the result of get() is chained (getResultIsChained=true).
837+
code: "this.get('lastObject.bar.lastObject')[123];",
838+
output: null,
839+
options: [{ useOptionalChaining: true }],
840+
errors: [
841+
{
842+
message: ERROR_MESSAGE_GET,
843+
type: 'CallExpression',
844+
},
845+
],
846+
},
646847
{
647848
// Reports violation after (classic) proxy class.
648849
code: `

0 commit comments

Comments
 (0)