Skip to content

[restrict-plus-operands] Allowing any for summands #386

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.

Sign up for GitHub

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

Jump to bottom
Closed
kachkaev opened this issue Mar 28, 2019 · 4 comments · Fixed by #4260
Closed

[restrict-plus-operands] Allowing any for summands #386

kachkaev opened this issue Mar 28, 2019 · 4 comments · Fixed by #4260
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@kachkaev
Copy link

kachkaev commented Mar 28, 2019 •
edited
Loading

Repro

  1. Enable the rule (see docs)

  2. Either

    • Create a js file where numbers or strings are added
    export const f = (a, b) => a + b;
    • Create a ts file, where arguments of type any are added
    export const f = (a: any, b: any) => a + b;

  3. Observe the following error in each case:

    Operands of '+' operation must either be both strings or both numbers.
eslint(@typescript-eslint/restrict-plus-operands)

.eslintrc.js

{
  "rules": {
    "@typescript-eslint/restrict-plus-operands": "error"
  }
}

Expected Result

The current result is fine, however too much strictness out of which you cannot opt out may get the rule challenging to use in existing codebases. I just tried enabling it in a monorepo with mixed js and ts code and got hundreds of errors. I understand the value behind this, just find it impossible to start using the rule to catch real issues (variable of known type number being added to a variable of known type string). Errors in js files are especially unfortunate.

Would it be possible to optionally allow any in the rule?

"@typescript-eslint/restrict-plus-operands":  ["error", { allowAny: true }]

or even

"@typescript-eslint/restrict-plus-operands":  ["error", { skipJs: true }]
"@typescript-eslint/restrict-plus-operands":  ["error", { allowAny: true, skipJs: true }]

Or should js files just be always skipped?

Versions

package version
@typescript-eslint/eslint-plugin 1.5.0
@typescript-eslint/parser 1.5.0
TypeScript 3.3.4000
ESLint 5.15.3
node 10.15.2
yarn 1.13.0

/cc @armano2 (author of #70)
@kachkaev kachkaev added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Mar 28, 2019
@bradzacher bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed triage Waiting for team members to take a look labels Mar 28, 2019
@bradzacher
Copy link
Member

bradzacher commented Mar 28, 2019 •
edited
Loading

Typescript will run over your JS code and try a little bit to give types to things based on obvious things like JSDoc comments and variable usage. Which means we can get some type information for them.

By default, eslint will apply every single configured rule to all files that match your cli command. I.e. if your codebase is mixed, and you have TS-specific rules enabled, eslint will run them over your JS code - it has no concept of JS vs TS.

If you don't want your JS files to be linted by this (or any TS specific) rule, you can use eslint's overrides config to disable the rule for certain globs: https://eslint.org/docs/user-guide/configuring#disabling-rules-only-for-a-group-of-files (i.e. `"*.js").

As for skipping any's, happy for it to be added - will accept PRs for it.

Personally, I'm not sure how much value there is in that as an option, as number + any is a potential problem in your codebase (which this rule is attempting to warn about).

@j-f1
Copy link
Contributor

j-f1 commented Mar 29, 2019

Shouldn’t any + any be allowed though?

@bradzacher
Copy link
Member

I would say no, not by the base rule - for the same reason that number + any isn't allowed.
Definitely an option to add a rule option to allow summing any (that's off by default, as it's an unsafe operation).

@bradzacher bradzacher added the good first issue Good for newcomers label Jul 27, 2019
@lonyele lonyele mentioned this issue Aug 24, 2019
Closed
@bradzacher bradzacher added the has pr there is a PR raised to close this label Nov 13, 2019
@bradzacher bradzacher removed the has pr there is a PR raised to close this label Apr 13, 2020
@papb
Copy link
Contributor

papb commented Sep 7, 2020

Hello, I would also like to see this option, and it should be added to restrict-template-expressions too. It is really weird to get a suggestion to consider using a template literal and when changing to it the error persists 😅

@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@lonyele lonyele mentioned this issue Dec 6, 2021
Merged
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
5 participants
@kachkaev @JoshuaKGoldberg @bradzacher @papb @j-f1