infra(unicorn): prefer-string-slice

[ST-DDT]

Ref: #2439

• Check eslint-plugin-unicorn rules to enable/configure/disable #2439

---

Enables the unicorn/prefer-string-slice lint rule.

[netlify]

✅ Deploy Preview for fakerjs ready!

Name	Link
🔨 Latest commit	36f85bd
🔍 Latest deploy log	https://app.netlify.com/sites/fakerjs/deploys/6648f6633e649000086edd91
😎 Deploy Preview	https://deploy-preview-2814.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (7dc8a18) to head (36f85bd).

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2814      +/-   ##
==========================================
- Coverage   99.96%   99.96%   -0.01%     
==========================================
  Files        2977     2977              
  Lines      215422   215424       +2     
  Branches      950      952       +2     
==========================================
- Hits       215351   215342       -9     
- Misses         71       82      +11     

Files	Coverage Δ
src/modules/finance/index.ts	100.00% <100.00%> (ø)
src/modules/helpers/eval.ts	96.56% <100.00%> (ø)
src/modules/helpers/index.ts	98.38% <100.00%> (+<0.01%)	⬆️
src/modules/internet/index.ts	100.00% <100.00%> (ø)
src/modules/lorem/index.ts	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[ST-DDT]

Please pay extra attention when reviewing the files changed by f48dfe4 because I had to convert them by hand and I don't know why.

[xDivisionByZerox]

From the docs:

String#substr() and String#substring() are the two lesser known legacy ways to slice a string.

Have to disagree on the String#substring part. Without having prove of it, I would say that most JS developers would know String#substring while being unaware of String#slice 🤔

[ST-DDT]

I can also disable the rule permanently.

[xDivisionByZerox]

While I have some doubts about the previously explained statement, this rule would at least unify the way string slicing would be performed in the project. This might be a DX improvement (for faker developers) in itself because one has not wondered why X is used in one place while Y is used in another.

[Shinigami92]

My personal rules:

• never use substr 🙅 its the most deprecated one
• use substring in specific cases (as far as I know it has some specific behavior, but I would need always to look these up)
• mostly use slice
  • pros: it is well known, acts like for arrays so some can imagine exactly how it works by thinking that each char is just an array element, and some can use shortways like -1 to address elements from the end
  • cons: in minor cases it is confusing if there is currently a string or an array, however in todays world using TypeScript, this is not a problem anymore as you have highlighting and code tooltips

---

Examples:

- filePath.substring(pathLocales.length + 1, filePath.length - 3)
+ filePath.slice(pathLocales.length + 1, -3)

this is a really good change, and I like it

- fileContent.substring(0, compareIndex)
+ fileContent.slice(0, Math.max(0, compareIndex))

this is not so good, because it complexifies the code by using Math.max and some need to more think about what's going on
-> Looks like the thing is that substring(_, negativeValue) results always in empty string.

TBH I personally like use substring for such cases then... 👀
Does the lint-rule has some options to configure it?

[matthewmayer]

This only really makes sense if we manually go through cases like this and remove the ugly Math.max in places where we know compareIndex must be positive.

fileContent.slice(0, Math.max(0, compareIndex))