[Suggestion] Lint rule to disallow `arr[arr.length - 1]`

[grian32]

With the new arr.at(-1) syntax introduced V8 9.2, perhaps it would be a good idea to have a rule to disallow arr[arr.length - 1] syntax, ass arr.at(-1) is imo cleaner and was made for this purpose more or less.

This would of course have to wait for 1.12.1 or whatever version fixes the issue that doesn't let ts recognize .at

[bartlomieju]

This sounds good to me 👍

[crowlKats]

performance wise though .at() is quite slower

[bartlomieju]

performance wise though .at() is quite slower

Good to know, I'm sure V8 team will work on the performance in the coming release if this API becomes prevalent.

[grian32]

On the topic of performance, I've made a rudimentary JSBench here that displays the difference: https://jsbench.me/x1krswau85

I will also attempt to implement this rule and PR if I am succesful.

[lucacasonato]

99.999% of people will not notice the difference between 10 billion ops/s vs 100 billion ops/s. Don't think perf is a constraint here.

[grian32]

I will also attempt to implement this rule and PR if I am succesful.

I was not succesfull, if someone else would like to implement this feel free to.

[subhakundu]

Is it okay, if I start working on this one? It will give me good opportunity to learn.

[magurotuna]

@subhakundu Sure! Please do, and as always feel free to ask about anything. We're sorry that there's no comprehensive documentation about implementing a new lint rule right now (of course I wanna do though). I'd recommend looking at other rules implementation.

[subhakundu]

@magurotuna is there any routine to print ASTs (from top level to lower level) so that I can identify the type of node to use for VisitAll? I have created a barebone rule for learning how to create one. Next steps are to identify type of node (which is potentially Expr I guess) and traversing down from there till we can make sure it is arr.length, binary operation '-' and '1'. It requires more AST information. I am new to SWC, so, this will be really helpful. I am now using dbg! macro.

I will require name, code, message, hint and doc text in future, but we can discuss that later I guess. I am focusing on creating the rule now.

[magurotuna]

@subhakundu To explore AST, I usually use dbg! macro for the root AST node, i.e. Program.

Next steps are to identify type of node (which is potentially Expr I guess) and traversing down from there till we can make sure it is arr.length, binary operation '-' and '1'.

This approach sounds great 👍 I'm sure you can make it in that way.

By the way, it seems like you are referring to one of the existing rules that are implemented in a somewhat "old" way. I mean, the existing lint rules are implemented in two ways, one is using swc directly, while the other is using dprint-swc-ecma-ast-view which is a wrapper of swc, providing us with a couple of features useful for writing complex lint rules. We are under refactoring from swc to dprint-swc-ecma-ast-view, so both ways are mixed now. I recommend you learn from the rules implemented in the new way. Maybe no-await-in-loop is a good example.

[subhakundu]

Hi @magurotuna I require sometime to work on it. I am in some kind of personal emergency. Once that is sorted, I will start again. Is it okay? Else if someone wants to work on it, please go ahead.

[bartlomieju]

@subhakundu thanks for the heads up, don't worry, it's not super urgent. Please take care of your stuff first.

[magurotuna]

@subhakundu No problem, take your time 😉

[subhakundu]

Hi all, I have started working on it again after resolving issues at my end. Thanks for your patience. I have taken the following example

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

I have understood the syntax tree. For fruits[fruits.length - 1], the following is the syntax tree (each line denotes a level) MemberExpr (obj: fruits, prop: fruits.length-1)

MemberExpr(obj: fruits, prop: length), BinOp(-), Ident(Number:1)

Now, I am working on a POC based on traversing this subset of AST (in a hardcoded manner). But that will break with complex test cases. MemberExpr can go deep. So, it will be helpful if you can guide me here a bit on what are the constructs possible. Based on that our algorithm might be different. It will also help me writing tests as well. Also, the hint, message, and text for this page - https://lint.deno.land/.

[subhakundu]

Hi all, I have added raised a PR for the issue. Please let me know your feedback on it. Some things are needed before the changes are finalized. I have added the needed information in the last comment and comment in PR.

[bartlomieju]

@subhakundu please open a PR in this repo, it seems you only referenced this issue in commit message in your own repo.

[subhakundu]

@bartlomieju I am very sorry for that mistake. I am still getting habituated with the flow. I have opened a PR: https://github.com/denoland/deno_lint/pull/932

[dandv]

Any progress on this, ~3 years later? CC @ry

[magurotuna]

@dandv Not really, but the suggestion does sound reasonable, so PR is always welcome