Skip to content
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

Rule proposal: prefer-split-limit #2464

Open
gurgunday opened this issue Oct 3, 2024 · 3 comments
Open

Rule proposal: prefer-split-limit #2464

gurgunday opened this issue Oct 3, 2024 · 3 comments

Comments

@gurgunday
Copy link
Contributor

gurgunday commented Oct 3, 2024

Description

Whenever we use String.prototype.split with literals like so:

string.split('/')[0]
// or
string.split('/')[1]
// or
string.split('/')[2]

We benefit by limiting the search space, this allows split to exit early once it attains the number of splits and to not scan the whole string:

string.split('/', 1)[0]
// and
string.split('/', 2)[1]
// and
string.split('/', 3)[2]

// Once we get to the element we want, .split exits early (faster)

Fail

string.split('/')[0]
string.split('/').at(0)

Pass

string.split('/', 1)[0]
string.split('/', 1).at(0)
string.split('/').at(-1)
string.split('/').at(numberVar)
string.split('/')[numberVar]

Proposed rule name

specify-split-limit or prefer-split-limit

Additional Info

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split#limit

@fregante
Copy link
Collaborator

fregante commented Oct 3, 2024

string.split('/').at(numberVar)

Could also be:

string.split('/', numberVar).at(numberVar)

@gurgunday
Copy link
Contributor Author

Yeah

Just a small correction in case anyone starts implementing the rule -- since the second parameter is the length of the returned array instead of the number of splits to do:

string.split('/').at(numberVar)

// would be:

string.split('/', numberVar + 1).at(numberVar)

@fregante
Copy link
Collaborator

fregante commented Oct 5, 2024

The rule could potentially also warn for that:

s.split('/', 1).at(2);
// Error: This will always be undefined 

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

No branches or pull requests

2 participants