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: no-implicit-coercion using a negative/minus (-) symbol #7076

Closed
6 tasks done
NotWoods opened this issue May 31, 2023 · 7 comments · Fixed by #7390
Closed
6 tasks done

Rule proposal: no-implicit-coercion using a negative/minus (-) symbol #7076

NotWoods opened this issue May 31, 2023 · 7 comments · Fixed by #7390
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@NotWoods
Copy link
Contributor

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
  • My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

ESLint's no-implicit-coercion rule can't check for casting a string to a number by using - because it would disallow negating a regular number. By using type information, we can provide additional coercion checks.

Fail Cases

const foo = '32px'
var n = -foo;

Pass Cases

const bar = 32
var n = -bar;
var n = -Number(foo);
var n = -parseFloat(foo);
var n = -parseInt(foo, 10);

Additional Info

No response

@NotWoods NotWoods added enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels May 31, 2023
@bradzacher
Copy link
Member

This is a really interesting idea.
I'll be honest when I say that it's been a very long time since I've had to operate on numbers and needed to use the unary - to invert a sign.
I'd almost go as far as to ban it for non-literal numbers altogether (if you want to invert then use * -1 which will be correctly checked by TS).

But this does seem like a niche, but useful check to have to ensure safety.

@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Jun 7, 2023
@NotWoods
Copy link
Contributor Author

NotWoods commented Jun 9, 2023

It might be worthwhile for the documentation for this rule to show how to configure no-restricted-syntax to ban - altogether.

@samestep
Copy link
Contributor

I've created an ESLint plugin with this rule: https://www.npmjs.com/package/eslint-plugin-unary-minus

@tylerlaprade
Copy link

Thank you @samestep, looks good! Any reason not to submit it as a PR to the main repo?

@samestep
Copy link
Contributor

samestep commented Jul 26, 2023

Thanks @tylerlaprade! The only reason was this conversation I had on Discord with @Zamiell yesterday:

start by making your own custom rule yeah
and then get everything working and write some tests, then you can submit a PR afterward

Should I submit a PR with this now? How can I do that?

@bradzacher
Copy link
Member

bradzacher commented Jul 26, 2023

Yeah there is no need to publish a separate package for a rule! Doing that involves a lot of extra boilerplate and effort - we don't recommend doing that unless you've got an idea for a rule that we haven't marked as accepted.

Even if you want to try the rule locally on your project before submitting a PR we suggest local linking for that to save you time!

To submit your rule for us you'll just need to fork and clone the repo and then add your code to our plugin, then raise a PR. The pain here will be in adjusting your code and tests to adhere to our code style but should otherwise be straight-forward.

@samestep
Copy link
Contributor

Oh! Sorry about that, I guess I just misunderstood @Zamiell's message on Discord.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants