From 6255e882818431a47fbc2d9637e563a1f9fc2b90 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 6 Mar 2020 20:51:20 +0900 Subject: [PATCH 1/5] fix(eslint-plugin): [no-extra-parens] false positive with arrow generic --- .../src/rules/no-extra-parens.ts | 37 ++++++++++++ .../tests/rules/no-extra-parens.test.ts | 58 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index e3ab2395279..25391fe1c5e 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -5,6 +5,7 @@ import { AST_NODE_TYPES, TSESTree, TSESLint, + AST_TOKEN_TYPES, } from '@typescript-eslint/experimental-utils'; import baseRule from 'eslint/lib/rules/no-extra-parens'; import * as util from '../util'; @@ -62,11 +63,47 @@ export default util.createRule({ return rule(node); } + + function countsWrappingParens(node: TSESTree.Node, max: number): number { + const sourceCode = context.getSourceCode(); + let count = 0; + let left = sourceCode.getTokenBefore(node); + let right = sourceCode.getTokenAfter(node); + + while ( + count < max && + left?.type === AST_TOKEN_TYPES.Punctuator && + left.value === '(' && + right?.type === AST_TOKEN_TYPES.Punctuator && + right.value === ')' + ) { + count++; + left = sourceCode.getTokenBefore(left); + right = sourceCode.getTokenAfter(right); + } + + return count; + } + function callExp( node: TSESTree.CallExpression | TSESTree.NewExpression, ): void { const rule = rules.CallExpression as (n: typeof node) => void; + // ESLint core cannot check parens well when there is function type in type parameters with only one argument. + // e.g. foo<() => void>(a); + if ( + node.typeParameters?.params.some(util.isTSFunctionType) && + node.arguments.length === 1 + ) { + if (countsWrappingParens(node.arguments[0], 2) < 2) { + return rule({ + ...node, + arguments: [], + }); + } + } + if (util.isTypeAssertion(node.callee)) { // reduces the precedence of the node so the rule thinks it needs to be wrapped return rule({ diff --git a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts index 29afab88ec5..b6930dac229 100644 --- a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts +++ b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts @@ -14,6 +14,17 @@ ruleTester.run('no-extra-parens', rule, { valid: [ ...batchedSingleLineTests({ code: ` +foo<() => void>(a); +foo<() => void>(a = b); +foo<() => void>(a, b); +foo<() => void>((a, b)); +foo<() => () => void>(a); +foo<() => void>(()=> undefined); +foo void>(a); + `, + }), + ...batchedSingleLineTests({ + code: ` (0).toString(); (function(){}) ? a() : b(); (/^a$/).test(x); @@ -52,6 +63,10 @@ for (;(a = b);); code: `b => (b = 1);`, options: ['all', { returnAssign: false }], }, + { + code: `foo<() => void>(b => (b = 1));`, + options: ['all', { returnAssign: false }], + }, { code: `b => b ? (c = d) : (c = e);`, options: ['all', { returnAssign: false }], @@ -61,6 +76,7 @@ for (;(a = b);); x = a || (b && c); x = a + (b * c); x = (a * b) / c; +foo<() => void>(x = (a * b) / c); `, options: ['all', { nestedBinaryExpressions: false }], }), @@ -222,6 +238,48 @@ switch (foo) { case 1: case (<2>2): break; default: break; } invalid: [ ...batchedSingleLineTests({ code: ` +foo<() => void>((a)); +foo<() => void>((a = b)); +foo<() => void>((a), b); +foo<() => () => void>((a)); +foo<() => void>((()=> undefined)); +foo void>((a)); + `, + errors: [ + { + messageId: 'unexpected', + line: 2, + column: 17, + }, + { + messageId: 'unexpected', + line: 3, + column: 17, + }, + { + messageId: 'unexpected', + line: 4, + column: 17, + }, + { + messageId: 'unexpected', + line: 5, + column: 23, + }, + { + messageId: 'unexpected', + line: 6, + column: 17, + }, + { + messageId: 'unexpected', + line: 7, + column: 20, + }, + ], + }), + ...batchedSingleLineTests({ + code: ` a = (b * c); (a * b) + c; for (a in (b, c)); From 41f14e18b47c1a0f0e059156670fb5a0ae11d530 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 6 Mar 2020 21:47:32 +0900 Subject: [PATCH 2/5] fix: change checking logic --- .../src/rules/no-extra-parens.ts | 37 ++++--------------- .../tests/rules/no-extra-parens.test.ts | 12 ++++++ 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index 25391fe1c5e..5139b1abe86 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -64,44 +64,23 @@ export default util.createRule({ return rule(node); } - function countsWrappingParens(node: TSESTree.Node, max: number): number { - const sourceCode = context.getSourceCode(); - let count = 0; - let left = sourceCode.getTokenBefore(node); - let right = sourceCode.getTokenAfter(node); - - while ( - count < max && - left?.type === AST_TOKEN_TYPES.Punctuator && - left.value === '(' && - right?.type === AST_TOKEN_TYPES.Punctuator && - right.value === ')' - ) { - count++; - left = sourceCode.getTokenBefore(left); - right = sourceCode.getTokenAfter(right); - } - - return count; - } - function callExp( node: TSESTree.CallExpression | TSESTree.NewExpression, ): void { const rule = rules.CallExpression as (n: typeof node) => void; - + const sourceCode = context.getSourceCode(); // ESLint core cannot check parens well when there is function type in type parameters with only one argument. // e.g. foo<() => void>(a); if ( node.typeParameters?.params.some(util.isTSFunctionType) && - node.arguments.length === 1 + node.arguments.length === 1 && + sourceCode.getTokenAfter(node.typeParameters) === + sourceCode.getTokenBefore(node.arguments[0]) ) { - if (countsWrappingParens(node.arguments[0], 2) < 2) { - return rule({ - ...node, - arguments: [], - }); - } + return rule({ + ...node, + arguments: [], + }); } if (util.isTypeAssertion(node.callee)) { diff --git a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts index b6930dac229..4b47df5deda 100644 --- a/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts +++ b/packages/eslint-plugin/tests/rules/no-extra-parens.test.ts @@ -244,6 +244,8 @@ foo<() => void>((a), b); foo<() => () => void>((a)); foo<() => void>((()=> undefined)); foo void>((a)); +foo void>((a),); +foo void>(((a))); `, errors: [ { @@ -276,6 +278,16 @@ foo void>((a)); line: 7, column: 20, }, + { + messageId: 'unexpected', + line: 8, + column: 20, + }, + { + messageId: 'unexpected', + line: 9, + column: 21, + }, ], }), ...batchedSingleLineTests({ From b4510f31d0fab5922257c02b14cde667b1ba68c6 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 6 Mar 2020 21:49:57 +0900 Subject: [PATCH 3/5] chore: formatting --- packages/eslint-plugin/src/rules/no-extra-parens.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index 5139b1abe86..cbecd529db5 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -5,7 +5,6 @@ import { AST_NODE_TYPES, TSESTree, TSESLint, - AST_TOKEN_TYPES, } from '@typescript-eslint/experimental-utils'; import baseRule from 'eslint/lib/rules/no-extra-parens'; import * as util from '../util'; @@ -68,14 +67,15 @@ export default util.createRule({ node: TSESTree.CallExpression | TSESTree.NewExpression, ): void { const rule = rules.CallExpression as (n: typeof node) => void; - const sourceCode = context.getSourceCode(); + + const { getTokenBefore, getTokenAfter } = context.getSourceCode(); + // ESLint core cannot check parens well when there is function type in type parameters with only one argument. // e.g. foo<() => void>(a); if ( node.typeParameters?.params.some(util.isTSFunctionType) && node.arguments.length === 1 && - sourceCode.getTokenAfter(node.typeParameters) === - sourceCode.getTokenBefore(node.arguments[0]) + getTokenAfter(node.typeParameters) === getTokenBefore(node.arguments[0]) ) { return rule({ ...node, From b8394465d6f7409684a9891c7531695c914dff30 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 6 Mar 2020 22:04:17 +0900 Subject: [PATCH 4/5] fix: logic --- .../src/rules/no-extra-parens.ts | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index cbecd529db5..9d75dc0fb51 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -68,19 +68,27 @@ export default util.createRule({ ): void { const rule = rules.CallExpression as (n: typeof node) => void; - const { getTokenBefore, getTokenAfter } = context.getSourceCode(); - // ESLint core cannot check parens well when there is function type in type parameters with only one argument. // e.g. foo<() => void>(a); if ( node.typeParameters?.params.some(util.isTSFunctionType) && - node.arguments.length === 1 && - getTokenAfter(node.typeParameters) === getTokenBefore(node.arguments[0]) + node.arguments.length === 1 ) { - return rule({ - ...node, - arguments: [], - }); + const sourceCode = context.getSourceCode(); + const tokenAfterTypeParam = sourceCode.getTokenAfter( + node.typeParameters, + ); + const tokenBeforeArg = sourceCode.getTokenBefore(node.arguments[0]); + + if ( + tokenAfterTypeParam === tokenBeforeArg && + tokenBeforeArg?.value === '(' + ) { + return rule({ + ...node, + arguments: [], + }); + } } if (util.isTypeAssertion(node.callee)) { From 2a31734e45f4a26720a2fb0f6f1f1d72cdbd9bdf Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 7 Mar 2020 02:35:54 +0900 Subject: [PATCH 5/5] fix: remove useless check --- packages/eslint-plugin/src/rules/no-extra-parens.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-extra-parens.ts b/packages/eslint-plugin/src/rules/no-extra-parens.ts index 9d75dc0fb51..653d2fa74d1 100644 --- a/packages/eslint-plugin/src/rules/no-extra-parens.ts +++ b/packages/eslint-plugin/src/rules/no-extra-parens.ts @@ -80,10 +80,7 @@ export default util.createRule({ ); const tokenBeforeArg = sourceCode.getTokenBefore(node.arguments[0]); - if ( - tokenAfterTypeParam === tokenBeforeArg && - tokenBeforeArg?.value === '(' - ) { + if (tokenAfterTypeParam === tokenBeforeArg) { return rule({ ...node, arguments: [],