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

feat(eslint-plugin): [restrict-plus-operands] add allowAny option #901

Closed

Conversation

lonyele
Copy link
Contributor

@lonyele lonyele commented Aug 24, 2019

Fixes #386

I added allowAny option to the rule as described in the issue. Please take a look.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @lonyele!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint

@codecov
Copy link

codecov bot commented Aug 24, 2019

Codecov Report

Merging #901 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
+ Coverage   94.13%   94.13%   +<.01%     
==========================================
  Files         115      115              
  Lines        5011     5013       +2     
  Branches     1399     1400       +1     
==========================================
+ Hits         4717     4719       +2     
  Misses        166      166              
  Partials      128      128
Impacted Files Coverage Δ
.../eslint-plugin/src/rules/restrict-plus-operands.ts 97.36% <100%> (+0.14%) ⬆️

@bradzacher bradzacher added the enhancement: plugin rule option New rule option for an existing eslint-plugin rule label Aug 24, 2019
@@ -88,6 +106,10 @@ export default util.createRule({
const leftType = getNodeType(node.left);
const rightType = getNodeType(node.right);

if (option.allowAny && (leftType === 'any' || rightType === 'any')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not quite correct.
This shouldn't override all validation in the rule, it should only allow any as a valid type.

I.e.
any + boolean shouldn't be allowed - because boolean isn't a valid type for this rule without the option.
However any + (string | number | bigint) should be allowed, as those 3 are all valid types.

},
{
code: `
export const f = (a: boolean, b: any) => a + b;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as mentioned - this should be invalid


```ts
const x = (a: any, b: string) => a + b;
const y = (a: boolean, b: any) => a + b;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto - see comment in rule

Comment on lines +37 to +38
If the rule is too strict then making this option `true`
can be a help. Though It is not recommended since lint errors are potentially a real runtime errors in many cases.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion for stronger/clearer wording for this:


This option is not recommended, as it greatly reduces the type safety in your code.
It is provided as a stopgap to make it easier to migrate codebases onto the rule.

This option will cause the rule to accept any as a type for operands. any will be allowed to be summed with all other valid types.

Comment on lines +19 to 33
Options may be provided as an object with:

- `allowAny` to ignore this rule when either left or
right of plus operand is a type `any`

```json
{
"@typescript-eslint/restrict-plus-operands": "error"
"@typescript-eslint/estrict-plus-operands": [
"error",
{
"allowAny": true
}
]
}
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section can be changed to just this snippet.
We try to avoid putting the full config block in rule docs now.


type Options = {
  allowAny?: boolean;
}

const defaultOptions: Options = {
  allowAny: false,
};

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed enhancement: plugin rule option New rule option for an existing eslint-plugin rule labels Nov 13, 2019
@armano2 armano2 added the help wanted Extra attention is needed label Jan 1, 2020
@bradzacher bradzacher added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Apr 13, 2020
@bradzacher bradzacher closed this Apr 13, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party enhancement: plugin rule option New rule option for an existing eslint-plugin rule help wanted Extra attention is needed stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[restrict-plus-operands] Allowing any for summands
3 participants