From ed2bd6067f74ae33e36a084719bb91efedfba599 Mon Sep 17 00:00:00 2001 From: sinyovercosy Date: Sun, 26 Apr 2020 20:23:46 -0500 Subject: [PATCH] fix(eslint-plugin): [prefer-string-starts-ends-with] check for negative start index in slice (#1920) --- .../rules/prefer-string-starts-ends-with.ts | 31 +++++++++++++++++-- .../prefer-string-starts-ends-with.test.ts | 8 ++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts b/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts index 217eada2e85..795c877a8e3 100644 --- a/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts +++ b/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts @@ -166,6 +166,27 @@ export default createRule({ ); } + /** + * Check if a given node is a negative index expression + * + * E.g. `s.slice(- )`, `s.substring(s.length - )` + * + * @param node The node to check. + * @param expectedIndexedNode The node which is expected as the receiver of index expression. + */ + function isNegativeIndexExpression( + node: TSESTree.Node, + expectedIndexedNode: TSESTree.Node, + ): boolean { + return ( + (node.type === AST_NODE_TYPES.UnaryExpression && + node.operator === '-') || + (node.type === AST_NODE_TYPES.BinaryExpression && + node.operator === '-' && + isLengthExpression(node.left, expectedIndexedNode)) + ); + } + /** * Check if a given node is the expression of the last index. * @@ -538,9 +559,10 @@ export default createRule({ } const isEndsWith = - callNode.arguments.length === 1 || - (callNode.arguments.length === 2 && - isLengthExpression(callNode.arguments[1], node.object)); + (callNode.arguments.length === 1 || + (callNode.arguments.length === 2 && + isLengthExpression(callNode.arguments[1], node.object))) && + isNegativeIndexExpression(callNode.arguments[0], node.object); const isStartsWith = !isEndsWith && callNode.arguments.length === 2 && @@ -564,6 +586,9 @@ export default createRule({ ) { return null; } + // code being checked is likely mistake: + // unequal length of strings being checked for equality + // or reliant on behavior of substring (negative indices interpreted as 0) if (isStartsWith) { if (!isLengthExpression(callNode.arguments[1], eqNode.right)) { return null; diff --git a/packages/eslint-plugin/tests/rules/prefer-string-starts-ends-with.test.ts b/packages/eslint-plugin/tests/rules/prefer-string-starts-ends-with.test.ts index 0022c37cb2f..e97fa912df5 100644 --- a/packages/eslint-plugin/tests/rules/prefer-string-starts-ends-with.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-string-starts-ends-with.test.ts @@ -209,6 +209,12 @@ ruleTester.run('prefer-string-starts-ends-with', rule, { s.slice(-4, -1) === "bar" } `, + // https://github.com/typescript-eslint/typescript-eslint/issues/1690 + ` + function f(s: string) { + s.slice(1) === "bar" + } + `, ` function f(s: string) { pattern.test(s) @@ -872,7 +878,7 @@ ruleTester.run('prefer-string-starts-ends-with', rule, { { code: ` function f(s: string) { - s.slice(startIndex) === needle // 'startIndex' can be different + s.slice(-length) === needle // 'length' can be different } `, output: null,