Skip to content

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

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

Closed
wants to merge 7 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ceb01e0
feat: add allowAny options to restric-plus-operands rule
lonyele Aug 23, 2019
40e6419
docs: add about allowAny option
lonyele Aug 24, 2019
30ebe76
Merge branch 'master' into feature/add-allow-any-option
lonyele Aug 24, 2019
5147a84
Merge branch 'master' into feature/add-allow-any-option
lonyele Aug 31, 2019
2e9c883
Merge branch 'master' into feature/add-allow-any-option
lonyele Sep 6, 2019
4531ce8
Merge branch 'master' into feature/add-allow-any-option
lonyele Sep 10, 2019
55b86ef
Merge branch 'master' into feature/add-allow-any-option
lonyele Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion packages/eslint-plugin/docs/rules/restrict-plus-operands.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,34 @@ var foo = 1n + 1;

## Options

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
}
]
}
```
Comment on lines +19 to 33
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,
};


### allowAny

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.
Comment on lines +37 to +38
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.


Examples of **correct** code for this rule with `{ allowAny: true`:

```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

```

## Compatibility

- TSLint: [restrict-plus-operands](https://palantir.github.io/tslint/rules/restrict-plus-operands/)
34 changes: 28 additions & 6 deletions packages/eslint-plugin/src/rules/restrict-plus-operands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@ import { TSESTree } from '@typescript-eslint/experimental-utils';
import ts from 'typescript';
import * as util from '../util';

export default util.createRule({
type Options = [
{
allowAny?: boolean;
},
];

type MessageId = 'notStrings' | 'notBigInts' | 'notNumbers';

export default util.createRule<Options, MessageId>({
name: 'restrict-plus-operands',
meta: {
type: 'problem',
Expand All @@ -20,15 +28,24 @@ export default util.createRule({
"Operands of '+' operation must either be both strings or both numbers. Consider using a template literal.",
notBigInts: "Operands of '+' operation must be both bigints.",
},
schema: [],
schema: [
{
type: 'object',
properties: {
allowAny: {
type: 'boolean',
},
},
},
],
},
defaultOptions: [],
create(context) {
defaultOptions: [{ allowAny: false }],
create(context, [option]) {
const service = util.getParserServices(context);

const typeChecker = service.program.getTypeChecker();

type BaseLiteral = 'string' | 'number' | 'bigint' | 'invalid';
type BaseLiteral = 'string' | 'number' | 'bigint' | 'invalid' | 'any';

/**
* Helper function to get base type of node
Expand Down Expand Up @@ -65,7 +82,8 @@ export default util.createRule({
if (
stringType === 'number' ||
stringType === 'string' ||
stringType === 'bigint'
stringType === 'bigint' ||
stringType === 'any'
) {
return stringType;
}
Expand All @@ -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.

return;
}

if (
leftType === 'invalid' ||
rightType === 'invalid' ||
Expand Down
47 changes: 47 additions & 0 deletions packages/eslint-plugin/tests/rules/restrict-plus-operands.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,36 @@ function foo<T extends 1>(a: T) {
return a + 1;
}
`,
{
code: `
export const f = (a: any, b: any) => a + b;
`,
options: [
{
allowAny: true,
},
],
},
{
code: `
export const f = (a: string, b: any) => a + b;
`,
options: [
{
allowAny: true,
},
],
},
{
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

`,
options: [
{
allowAny: true,
},
],
},
],
invalid: [
{
Expand Down Expand Up @@ -383,5 +413,22 @@ function foo<T extends 1>(a: T) {
},
],
},
{
code: `
export const f = (a: string, b: number) => a + b;
`,
errors: [
{
messageId: 'notStrings',
line: 2,
column: 44,
},
],
options: [
{
allowAny: true,
},
],
},
],
});