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

prefer-string-starts-ends-with ignores /i case insensitive modifier #1687

Closed
AndyA opened this issue Dec 15, 2023 · 2 comments · Fixed by #1689 or #1688
Closed

prefer-string-starts-ends-with ignores /i case insensitive modifier #1687

AndyA opened this issue Dec 15, 2023 · 2 comments · Fixed by #1689 or #1688
Assignees

Comments

@AndyA
Copy link
Contributor

AndyA commented Dec 15, 2023

The (most welcome) prefer-string-starts-ends-with doesn't consider whether the RegExp it proposes replacing has an i modifier to make it case insensitive.

eslint-plugin-unicorn(prefer-string-starts-ends-with): Prefer String#startsWith over a regex with a caret.
     ╭─[lib/model/setup.ts:103:1]
 103 │               const uri = rec["target-uri"] || "";
 104 │               if (/^http/i.test(uri)) {
     ·                   ──────────────────
     ╰────

Arguably it could recommend str.toLowerCase().startsWith() but for long strings it's presumably more efficient to run a case insensitive RegExp on the first few characters rather than down casing the whole string.

Thanks for oxlint - it's awesome.

@camc314 camc314 self-assigned this Dec 15, 2023
@camc314
Copy link
Collaborator

camc314 commented Dec 15, 2023

Thanks for the detailed issue!

I think it's reasonable in this case, to not report an error.

Will fix shortly!

@AndyA
Copy link
Contributor Author

AndyA commented Dec 15, 2023

Here's a PR: #1688 :)

camc314 added a commit that referenced this issue Dec 15, 2023
…rt more test cases (#1689)

closes #1687 - uses `intersection` instead of `||`:
 - this improves performance as it combines the flags into a single bitmask instead of doing two seperate checks
 - 

Adds missing test cases from eslint-plugin-unicorn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants