-
-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support autofix of numerical property access and ternary expressions in no-get
rule
#1865
Conversation
lib/utils/types.js
Outdated
@@ -399,6 +400,10 @@ function isStringLiteral(node) { | |||
return isLiteral(node) && typeof node.value === 'string'; | |||
} | |||
|
|||
function isNumber(node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid adding trivial helpers like this per the comment at the top of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed trivial helper
no-get
rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Great to improve this autofixer.
* master: (88 commits) Release 11.7.0 build(deps-dev): Bump @typescript-eslint/parser from 5.59.5 to 5.59.6 (ember-cli#1863) Account for only having template tag in `no-empty-glimmer-component-classes` rule (ember-cli#1866) Support autofix of numerical property access and ternary expressions in `no-get` rule (ember-cli#1865) Release 11.6.0 build(deps-dev): Bump prettier from 2.8.7 to 2.8.8 (ember-cli#1842) build(deps-dev): Bump @typescript-eslint/parser from 5.58.0 to 5.59.5 (ember-cli#1860) build(deps-dev): Bump @babel/eslint-parser from 7.21.3 to 7.21.8 (ember-cli#1856) build(deps-dev): Bump markdownlint-cli from 0.33.0 to 0.34.0 (ember-cli#1848) build(deps-dev): Bump jquery from 3.6.4 to 3.7.0 (ember-cli#1861) build(deps-dev): Bump release-it from 15.10.1 to 15.10.3 (ember-cli#1859) build(deps): Bump vm2 from 3.9.17 to 3.9.18 (ember-cli#1862) Support autofix in gts files (ember-cli#1853) Only show `no-undef` errors for templates in gts files (ember-cli#1852) Release 11.5.2 Fix a bug in autofixer and autofix additional cases with `firstObject and `lastObject` in `no-get` rule (ember-cli#1841) build(deps-dev): Bump eslint from 8.37.0 to 8.38.0 (ember-cli#1830) build(deps-dev): Bump @typescript-eslint/parser from 5.57.0 to 5.58.0 (ember-cli#1837) build(deps-dev): Bump typescript from 5.0.3 to 5.0.4 (ember-cli#1832) build(deps): Bump vm2 from 3.9.11 to 3.9.17 (ember-cli#1839) ...
This PR is a partial response to this issue (partially fixes #1840) regarding enhancements for the
no-get
rule, along with another enhancement I propose.this.get(5)
===>this[5]
andget(obj, 5)
===>obj[5]
foo1 === foo2 ? obj.get('bar') : obj.get('baz')
===>foo1 === foo2 ? obj.bar : obj.baz
already works as isthis.get(foo1 === foo2 ? 'foo' : 'bar')
===>foo1 === foo2 ? this.foo : this.bar
andget(foo, foo1 === foo2 ? 'bar' : 'baz')
===>foo1 === foo2 ? foo.bar : foo.baz
I attempted to fix cases with variables or other dynamic expressions as the property argument
obj.get(foo)
===>obj[foo]
, but I now recommend that these cases not be autofixed. The reason for this isfoo
or any other dynamic expression could resolve to a string representation of chained property access (ex.'bar.baz'
), in which caseobj[foo] !== obj.get(foo)
sinceobj.get(foo)
would resolve at runtime asobj.bar?.baz
whileobj[foo]
would resolve asobj['bar.baz']
. Hence, autofixing these cases could cause bugs in existing code.