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.

Sign up for GitHub

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

Jump to bottom

Ignore direct instantiation of `EmberArray` in `no-array-prototype-extensions` rule #1749

Merged

bmish merged 7 commits into ember-cli:master from canrozanes:canrozanes/add-condition-to-array-prototype-extension-rule

Jan 23, 2023

Contributor

canrozanes commented Jan 21, 2023 •

edited

Loading

Overview

Partially fixes #1561. Helps reduce false positives.

Direct instantiation of Ember arrays ie:

function myFunction(){
  const array = A([1, 2, 3])
  array.toArray()
}

are not deprecated so they should be allowed by the linter.


          fix: ignore ember array initialized variables in no-array-prototype-e…

f2daf32

…xtension-rule

bmish added the Bug label

bmish reviewed

View reviewed changes

tests/lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved

lib/rules/no-array-prototype-extensions.js Outdated

+                        node.type === 'CallExpression' &&
+                        nodeInitializedTo &&
+                        nodeInitializedTo.callee &&
+                        (nodeInitializedTo.callee.name === 'A' || nodeInitializedTo.callee.name === 'EmberArray')

Member

bmish Jan 21, 2023

We're going to have to check against the actual names these are imported under, if they're imported at all. See the ImportDeclaration visitor for example.

Contributor Author

canrozanes Jan 22, 2023

Updated the logic to fetch the actual import name and also updated the test cases to include an import statement.


          check actual import name, remove surrounding function from test case

9c71a65

bmish reviewed

View reviewed changes

Member

bmish left a comment

Can you also add a correct code sample in the rule doc showing that this case is allowed?

lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved

lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved

lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved

bmish changed the title ~~fix: ignore ember array initialized variables in no-array-prototype-extension~~ Ignore direct instantiation of EmberArray in no-array-prototype-extensions rule


          add new rule exception to doc, updated falsy checks

bmish reviewed

View reviewed changes

docs/rules/no-array-prototype-extensions.md Outdated Show resolved Hide resolved


          Update docs/rules/no-array-prototype-extensions.md

0730c13

bmish changed the title ~~Ignore direct instantiation of EmberArray in no-array-prototype-extensions rule~~ Ignore variable instantiated to an EmberArray in no-array-prototype-extensions rule

bmish approved these changes

View reviewed changes

Member

bmish left a comment

Thanks!

bmish reviewed

View reviewed changes

tests/lib/rules/no-array-prototype-extensions.js Show resolved Hide resolved


          Update tests/lib/rules/no-array-prototype-extensions.js

6262eff

bmish reviewed

View reviewed changes

tests/lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved


          Update tests/lib/rules/no-array-prototype-extensions.js

3b8b7b8

bmish reviewed

View reviewed changes

tests/lib/rules/no-array-prototype-extensions.js Outdated Show resolved Hide resolved


          Update tests/lib/rules/no-array-prototype-extensions.js

435ac1e

bmish changed the title ~~Ignore variable instantiated to an EmberArray in no-array-prototype-extensions rule~~ Ignore direct instantiation of EmberArray in no-array-prototype-extensions rule

bmish merged commit 9ded580 into ember-cli:master

canrozanes deleted the canrozanes/add-condition-to-array-prototype-extension-rule branch

January 23, 2023 20:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment