From 8683d075db69a7c7c4b2b8def453682b6c2b303a Mon Sep 17 00:00:00 2001 From: webschik Date: Sat, 23 Feb 2019 20:26:41 +0100 Subject: [PATCH 1/3] feat(eslint-plugin): support type assertions in no-extra-parens rule --- .../docs/rules/no-extra-parens.md | 245 ++++++++++++++++ .../src/rules/no-extra-parens.ts | 33 +++ .../tests/rules/no-extra-parens.test.ts | 266 ++++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 23 ++ packages/eslint-plugin/typings/ts-eslint.d.ts | 6 +- 5 files changed, 572 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/docs/rules/no-extra-parens.md create mode 100644 packages/eslint-plugin/src/rules/no-extra-parens.ts create mode 100644 packages/eslint-plugin/tests/rules/no-extra-parens.test.ts diff --git a/packages/eslint-plugin/docs/rules/no-extra-parens.md b/packages/eslint-plugin/docs/rules/no-extra-parens.md new file mode 100644 index 000000000000..ba394d8d1ef1 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-extra-parens.md @@ -0,0 +1,245 @@ +# disallow unnecessary parentheses (no-extra-parens) + +This rule restricts the use of parentheses to only where they are necessary. + +## Rule Details + +This rule always ignores extra parentheses around the following: + +- RegExp literals such as `(/abc/).test(var)` to avoid conflicts with the [wrap-regex](wrap-regex.md) rule +- immediately-invoked function expressions (also known as IIFEs) such as `var x = (function () {})();` and `((function foo() {return 1;})())` to avoid conflicts with the [wrap-iife](wrap-iife.md) rule +- arrow function arguments to avoid conflicts with the [arrow-parens](arrow-parens.md) rule + +## Options + +```cjson +{ + // note you must disable the base rule as it can report incorrect errors + "no-extra-parens": "off", + "@typescript-eslint/no-extra-parens": ["error"] +} +``` + +This rule has a string option: + +- `"all"` (default) disallows unnecessary parentheses around _any_ expression +- `"functions"` disallows unnecessary parentheses _only_ around function expressions + +This rule has an object option for exceptions to the `"all"` option: + +- `"conditionalAssign": false` allows extra parentheses around assignments in conditional test expressions +- `"returnAssign": false` allows extra parentheses around assignments in `return` statements +- `"nestedBinaryExpressions": false` allows extra parentheses in nested binary expressions +- `"ignoreJSX": "none|all|multi-line|single-line"` allows extra parentheses around no/all/multi-line/single-line JSX components. Defaults to `none`. +- `"enforceForArrowConditionals": false` allows extra parentheses around ternary expressions which are the body of an arrow function + +### all + +Examples of **incorrect** code for this rule with the default `"all"` option: + +```js +/* eslint @typescript-eslint/camelcase: "error" */ + +a = b * c; + +a * b + c; + +for (a in (b, c)); + +for (a in b); + +for (a of b); + +typeof a; + +(function() {} ? a() : b()); +``` + +Examples of **correct** code for this rule with the default `"all"` option: + +```js +/* eslint @typescript-eslint/camelcase: "error" */ + +(0).toString(); + +Object.prototype.toString.call(); + +({}.toString.call()); + +(function() {} ? a() : b()); + +/^a$/.test(x); + +for (a of (b, c)); + +for (a of b); + +for (a in (b, c)); + +for (a in b); +``` + +### conditionalAssign + +Examples of **correct** code for this rule with the `"all"` and `{ "conditionalAssign": false }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { "conditionalAssign": false }] */ + +while ((foo = bar())) {} + +if ((foo = bar())) { +} + +do; +while ((foo = bar())); + +for (; (a = b); ); +``` + +### returnAssign + +Examples of **correct** code for this rule with the `"all"` and `{ "returnAssign": false }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { "returnAssign": false }] */ + +function a(b) { + return (b = 1); +} + +function a(b) { + return b ? (c = d) : (c = e); +} + +b => (b = 1); + +b => (b ? (c = d) : (c = e)); +``` + +### nestedBinaryExpressions + +Examples of **correct** code for this rule with the `"all"` and `{ "nestedBinaryExpressions": false }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { "nestedBinaryExpressions": false }] */ + +x = a || (b && c); +x = a + b * c; +x = (a * b) / c; +``` + +### ignoreJSX + +Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "all" }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "all" }] */ +const Component =
; +const Component =
; +``` + +Examples of **incorrect** code for this rule with the `all` and `{ "ignoreJSX": "multi-line" }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "multi-line" }] */ +const Component =
; +const Component = ( +
+

+

+); +``` + +Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "multi-line" }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "multi-line" }] */ +const Component = ( +
+

+

+); +const Component =
; +``` + +Examples of **incorrect** code for this rule with the `all` and `{ "ignoreJSX": "single-line" }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "single-line" }] */ +const Component = ( +
+

+

+); +const Component =
; +``` + +Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "single-line" }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "single-line" }] */ +const Component =
; +const Component = ( +
+

+

+); +``` + +### enforceForArrowConditionals + +Examples of **correct** code for this rule with the `"all"` and `{ "enforceForArrowConditionals": false }` options: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "all", { "enforceForArrowConditionals": false }] */ + +const b = a => (1 ? 2 : 3); +const d = c => (1 ? 2 : 3); +``` + +### functions + +Examples of **incorrect** code for this rule with the `"functions"` option: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "functions"] */ + +(function foo() {})(); + +var y = function() { + return 1; +}; +``` + +Examples of **correct** code for this rule with the `"functions"` option: + +```js +/* eslint @typescript-eslint/camelcase: ["error", "functions"] */ + +(0).toString(); + +Object.prototype.toString.call(); + +({}.toString.call()); + +(function() {} ? a() : b()); + +/^a$/.test(x); + +a = b * c; + +a * b + c; + +typeof a; +``` + +## Further Reading + +- [MDN: Operator Precedence](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence) + +## Related Rules + +- [arrow-parens](arrow-parens.md) +- [no-cond-assign](no-cond-assign.md) +- [no-return-assign](no-return-assign.md) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts new file mode 100644 index 000000000000..46d663857a23 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -0,0 +1,33 @@ +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; +import baseRule from 'eslint/lib/rules/no-extra-parens'; +import * as util from '../util'; + +type Options = util.InferOptionsTypeFromRule; +type MessageIds = util.InferMessageIdsTypeFromRule; + +export default util.createRule({ + name: 'no-extra-parens', + meta: { + type: 'layout', + docs: { + description: 'disallow unnecessary parentheses', + category: 'Possible Errors', + recommended: false, + }, + fixable: 'code', + schema: baseRule.meta.schema, + messages: baseRule.meta.messages, + }, + defaultOptions: ['all'], + create(context) { + const rules = baseRule.create(context); + + return Object.assign({}, rules, { + MemberExpression(node: TSESTree.MemberExpression) { + if ((node.object as any).type !== AST_NODE_TYPES.TSAsExpression) { + return rules.MemberExpression(node); + } + }, + }); + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts new file mode 100644 index 000000000000..154179c192c6 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts @@ -0,0 +1,266 @@ +import rule from '../../src/rules/no-extra-parens'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-extra-parens', rule, { + valid: [ + { + code: ` + (0).toString(); + (function(){}) ? a() : b(); + (/^a$/).test(x); + for (a of (b, c)); + for (a of b); + for (a in b, c); + for (a in b); + `, + }, + { + code: `t.true((me.get as SinonStub).calledWithExactly('/foo', other));`, + }, + { + code: `while ((foo = bar())) {}`, + options: ['all', { conditionalAssign: false }], + }, + { + code: `if ((foo = bar())) {}`, + options: ['all', { conditionalAssign: false }], + }, + { + code: `do; while ((foo = bar()))`, + options: ['all', { conditionalAssign: false }], + }, + { + code: `for (;(a = b););`, + options: ['all', { conditionalAssign: false }], + }, + { + code: ` + function a(b) { + return (b = 1); + } + `, + options: ['all', { returnAssign: false }], + }, + { + code: ` + function a(b) { + return b ? (c = d) : (c = e); + } + `, + options: ['all', { returnAssign: false }], + }, + { + code: `b => (b = 1);`, + options: ['all', { returnAssign: false }], + }, + { + code: `b => b ? (c = d) : (c = e);`, + options: ['all', { returnAssign: false }], + }, + { + code: `x = a || (b && c);`, + options: ['all', { nestedBinaryExpressions: false }], + }, + { + code: `x = a + (b * c);`, + options: ['all', { nestedBinaryExpressions: false }], + }, + { + code: `x = (a * b) / c;`, + options: ['all', { nestedBinaryExpressions: false }], + }, + { + code: ` + const Component = (
) + const Component = ( +
+ ) + `, + options: ['all', { ignoreJSX: 'all' }], + }, + { + code: ` + const Component = ( +
+

+

+ ) + const Component = ( +
+ ) + `, + options: ['all', { ignoreJSX: 'multi-line' }], + }, + { + code: ` + const Component = (
) + const Component = (

) + `, + options: ['all', { ignoreJSX: 'single-line' }], + }, + { + code: ` + const b = a => 1 ? 2 : 3; + const d = c => (1 ? 2 : 3); + `, + options: ['all', { enforceForArrowConditionals: false }], + }, + { + code: ` + (0).toString(); + (Object.prototype.toString.call()); + ({}.toString.call()); + (function(){} ? a() : b()); + (/^a$/).test(x); + a = (b * c); + (a * b) + c; + typeof (a); + `, + options: ['functions'], + }, + ], + + invalid: [ + { + code: `a = (b * c);`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 5, + }, + ], + }, + { + code: `(a * b) + c;`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 1, + }, + ], + }, + { + code: `for (a in (b, c));`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 11, + }, + ], + }, + { + code: `for (a in (b));`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 11, + }, + ], + }, + { + code: `for (a of (b));`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 11, + }, + ], + }, + { + code: `typeof (a);`, + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 8, + }, + ], + }, + { + code: ` + const Component = (
) + const Component = (

) + `, + options: ['all', { ignoreJSX: 'multi-line' }], + errors: [ + { + messageId: 'unexpected', + line: 2, + column: 27, + }, + { + messageId: 'unexpected', + line: 3, + column: 27, + }, + ], + }, + { + code: ` + const Component = ( +
+

+

+ ) + const Component = ( +
+ ) + `, + options: ['all', { ignoreJSX: 'single-line' }], + errors: [ + { + messageId: 'unexpected', + line: 2, + column: 27, + }, + { + messageId: 'unexpected', + line: 7, + column: 27, + }, + ], + }, + { + code: `((function foo() {}))();`, + options: ['functions'], + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 2, + }, + ], + }, + { + code: `var y = (function () {return 1;});`, + options: ['functions'], + errors: [ + { + messageId: 'unexpected', + line: 1, + column: 9, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index b4094c05474f..aca4c8d8638c 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -330,3 +330,26 @@ declare module 'eslint/lib/rules/no-useless-constructor' { >; export = rule; } + +declare module 'eslint/lib/rules/no-extra-parens' { + import { TSESTree } from '@typescript-eslint/typescript-estree'; + import RuleModule from 'ts-eslint'; + + const rule: RuleModule< + 'unexpected', + ( + | 'all' + | 'functions' + | { + conditionalAssign?: boolean; + returnAssign?: boolean; + nestedBinaryExpressions?: boolean; + ignoreJSX?: 'none' | 'all' | 'multi-line' | 'single-line'; + enforceForArrowConditionals?: boolean; + })[], + { + MemberExpression(node: TSESTree.MemberExpression): void; + } + >; + export = rule; +} diff --git a/packages/eslint-plugin/typings/ts-eslint.d.ts b/packages/eslint-plugin/typings/ts-eslint.d.ts index 998c69583428..6567bfb4542d 100644 --- a/packages/eslint-plugin/typings/ts-eslint.d.ts +++ b/packages/eslint-plugin/typings/ts-eslint.d.ts @@ -209,7 +209,11 @@ declare module 'ts-eslint' { /** * The general category the rule falls within */ - category: 'Best Practices' | 'Stylistic Issues' | 'Variables'; + category: + | 'Best Practices' + | 'Stylistic Issues' + | 'Variables' + | 'Possible Errors'; /** * Concise description of the rule */ From 8ffb94f00400bc1654a2a6a53b73f96c606d9a5f Mon Sep 17 00:00:00 2001 From: webschik Date: Mon, 25 Feb 2019 21:03:38 +0100 Subject: [PATCH 2/3] fix(eslint-plugin): support type assertions in LeftHandSideExpression --- packages/eslint-plugin/src/rules/no-extra-parens.ts | 2 +- packages/typescript-estree/src/ts-estree/ts-estree.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index 46d663857a23..3ad963974c2f 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -24,7 +24,7 @@ export default util.createRule({ return Object.assign({}, rules, { MemberExpression(node: TSESTree.MemberExpression) { - if ((node.object as any).type !== AST_NODE_TYPES.TSAsExpression) { + if (node.object.type !== AST_NODE_TYPES.TSAsExpression) { return rules.MemberExpression(node); } }, diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index 4a9c7b91ae10..e384af6d2d3e 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -322,7 +322,8 @@ export type LeftHandSideExpression = | MemberExpression | PrimaryExpression | TaggedTemplateExpression - | TSNonNullExpression; + | TSNonNullExpression + | TSAsExpression; export type LiteralExpression = BigIntLiteral | Literal | TemplateLiteral; export type Modifier = | TSAbstractKeyword From 94ada35ebf57caf0e12ae4d0ec9c1d1c49c8368c Mon Sep 17 00:00:00 2001 From: webschik Date: Mon, 25 Feb 2019 22:15:11 +0100 Subject: [PATCH 3/3] docs(eslint-plugin): updated no-extra-parens docs --- .../docs/rules/no-extra-parens.md | 233 +----------------- 1 file changed, 5 insertions(+), 228 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-extra-parens.md b/packages/eslint-plugin/docs/rules/no-extra-parens.md index ba394d8d1ef1..d7942ec1b169 100644 --- a/packages/eslint-plugin/docs/rules/no-extra-parens.md +++ b/packages/eslint-plugin/docs/rules/no-extra-parens.md @@ -4,13 +4,10 @@ This rule restricts the use of parentheses to only where they are necessary. ## Rule Details -This rule always ignores extra parentheses around the following: +This rule extends the base [eslint/no-extra-parens](https://eslint.org/docs/rules/no-extra-parens) rule. +It supports all options and features of the base rule plus TS type assertions. -- RegExp literals such as `(/abc/).test(var)` to avoid conflicts with the [wrap-regex](wrap-regex.md) rule -- immediately-invoked function expressions (also known as IIFEs) such as `var x = (function () {})();` and `((function foo() {return 1;})())` to avoid conflicts with the [wrap-iife](wrap-iife.md) rule -- arrow function arguments to avoid conflicts with the [arrow-parens](arrow-parens.md) rule - -## Options +## How to use ```cjson { @@ -20,226 +17,6 @@ This rule always ignores extra parentheses around the following: } ``` -This rule has a string option: - -- `"all"` (default) disallows unnecessary parentheses around _any_ expression -- `"functions"` disallows unnecessary parentheses _only_ around function expressions - -This rule has an object option for exceptions to the `"all"` option: - -- `"conditionalAssign": false` allows extra parentheses around assignments in conditional test expressions -- `"returnAssign": false` allows extra parentheses around assignments in `return` statements -- `"nestedBinaryExpressions": false` allows extra parentheses in nested binary expressions -- `"ignoreJSX": "none|all|multi-line|single-line"` allows extra parentheses around no/all/multi-line/single-line JSX components. Defaults to `none`. -- `"enforceForArrowConditionals": false` allows extra parentheses around ternary expressions which are the body of an arrow function - -### all - -Examples of **incorrect** code for this rule with the default `"all"` option: - -```js -/* eslint @typescript-eslint/camelcase: "error" */ - -a = b * c; - -a * b + c; - -for (a in (b, c)); - -for (a in b); - -for (a of b); - -typeof a; - -(function() {} ? a() : b()); -``` - -Examples of **correct** code for this rule with the default `"all"` option: - -```js -/* eslint @typescript-eslint/camelcase: "error" */ - -(0).toString(); - -Object.prototype.toString.call(); - -({}.toString.call()); - -(function() {} ? a() : b()); - -/^a$/.test(x); - -for (a of (b, c)); - -for (a of b); - -for (a in (b, c)); - -for (a in b); -``` - -### conditionalAssign - -Examples of **correct** code for this rule with the `"all"` and `{ "conditionalAssign": false }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { "conditionalAssign": false }] */ - -while ((foo = bar())) {} - -if ((foo = bar())) { -} - -do; -while ((foo = bar())); - -for (; (a = b); ); -``` - -### returnAssign - -Examples of **correct** code for this rule with the `"all"` and `{ "returnAssign": false }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { "returnAssign": false }] */ - -function a(b) { - return (b = 1); -} - -function a(b) { - return b ? (c = d) : (c = e); -} - -b => (b = 1); - -b => (b ? (c = d) : (c = e)); -``` - -### nestedBinaryExpressions - -Examples of **correct** code for this rule with the `"all"` and `{ "nestedBinaryExpressions": false }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { "nestedBinaryExpressions": false }] */ - -x = a || (b && c); -x = a + b * c; -x = (a * b) / c; -``` - -### ignoreJSX - -Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "all" }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "all" }] */ -const Component =
; -const Component =
; -``` - -Examples of **incorrect** code for this rule with the `all` and `{ "ignoreJSX": "multi-line" }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "multi-line" }] */ -const Component =
; -const Component = ( -
-

-

-); -``` - -Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "multi-line" }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "multi-line" }] */ -const Component = ( -
-

-

-); -const Component =
; -``` - -Examples of **incorrect** code for this rule with the `all` and `{ "ignoreJSX": "single-line" }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "single-line" }] */ -const Component = ( -
-

-

-); -const Component =
; -``` - -Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "single-line" }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { ignoreJSX: "single-line" }] */ -const Component =
; -const Component = ( -
-

-

-); -``` - -### enforceForArrowConditionals - -Examples of **correct** code for this rule with the `"all"` and `{ "enforceForArrowConditionals": false }` options: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "all", { "enforceForArrowConditionals": false }] */ - -const b = a => (1 ? 2 : 3); -const d = c => (1 ? 2 : 3); -``` - -### functions - -Examples of **incorrect** code for this rule with the `"functions"` option: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "functions"] */ - -(function foo() {})(); - -var y = function() { - return 1; -}; -``` - -Examples of **correct** code for this rule with the `"functions"` option: - -```js -/* eslint @typescript-eslint/camelcase: ["error", "functions"] */ - -(0).toString(); - -Object.prototype.toString.call(); - -({}.toString.call()); - -(function() {} ? a() : b()); - -/^a$/.test(x); - -a = b * c; - -a * b + c; - -typeof a; -``` - -## Further Reading - -- [MDN: Operator Precedence](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence) - -## Related Rules +## Options -- [arrow-parens](arrow-parens.md) -- [no-cond-assign](no-cond-assign.md) -- [no-return-assign](no-return-assign.md) +See [eslint/no-extra-parens options](https://eslint.org/docs/rules/no-extra-parens#options).