deno_lint icon indicating copy to clipboard operation
deno_lint copied to clipboard

feature: adding Lint rule in favor of arr.at(-1) (#779)

Open subhakundu opened this issue 3 years ago • 3 comments

I have created a pull request for the lining rule mentioned. Right now, the following things are considered.

  1. a basic regex is validating if the pattern is matching or not. It can get complicated based on the scope. It will be of great help if the reviewer can help me with other use cases.
  2. Some basic UTs are added. More UTs are needed to be added based on the scope of the rule.
  3. A copied documentation file is added. The contents are needed to be finalized.
  4. Message, code, and hint need to be finalized.
  5. We need to decide if the rule is "recommended" or not at this moment.

Please consider it a basic POC. Please let me know the pointers I need to incorporate to make the rule robust. I will work on those during revision.

Testing Details:

Input:

const fruits = ['Apple', 'Banana']
const _x = fruits[fruits.length-1];

Output:

error[ban-array-length-minus-one]: arr[arr.length - 1] is deprecated.
 --> /Users/subhakundu/JS_Examples/array_size.ts:2:19
  |
2 | const _x = fruits[fruits.length-1];
  |                   ^^^^^^^^^^^^^^^
  |
  = help: Please consider using arr.at(-1) instead of arr[arr.length - 1]
Found 1 problem

Note: There are failed checks. I saw other PRs with the same check failures. I am checking to see if this is a known issue.

subhakundu avatar Oct 06 '21 01:10 subhakundu

@magurotuna thanks for clarifying the usecase. I believe then following are possible as well

  1. arr.at(-2)
  2. arr.at(-1)
  3. arr.at(-length) length being any expression or identifier.

So, the evaluation will be simplified to this arr[arr.length - <any identifier/expression>]. Am I right?

It might be a very silly question, but just to clarify - due to static analysis nature, it is not required to evaluate the identifier/expression. So, we don't need to perform array out of bound check. Please let me know if I am wrong.

Also, I require other information like manual texts, error messages, recommended or not, and other details.

subhakundu avatar Oct 06 '21 17:10 subhakundu

In my opinion, it's better to target only arr[arr.length - <integer numeric literal>]. This is because Array#at trancates the argument to an integer value, unlike the usual [[Get]].

const arr = [1, 2, 3];
arr.at(0.5); // arr[0]
arr.at(NaN); // arr[0]

If some identifier or expression is mixed in with it, the behavior will be completely different.

petamoriken avatar Nov 05 '21 13:11 petamoriken

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Feb 18 '22 14:02 CLAassistant

Closing because stale

ry avatar Nov 22 '22 15:11 ry