fix(eslint-plugin): [prefer-find] support ternary branches in prefer-…

…find (#8421) * 8379: Support ternary branches in prefer-find * fix codecov * lintfix * Update packages/eslint-plugin/tests/rules/prefer-find.test.ts Co-authored-by: Josh Goldberg ✨ <[email protected]> * add some straightforward tests * remove any * tweak tests slightly --------- Co-authored-by: Josh Goldberg ✨ <[email protected]>
typescript-eslint · Mar 11, 2024 · c0e3267 · c0e3267
1 parent 3fef9d6
commit c0e3267
Show file tree

Hide file tree

Showing 2 changed files with 190 additions and 19 deletions.
diff --git a/packages/eslint-plugin/src/rules/prefer-find.ts b/packages/eslint-plugin/src/rules/prefer-find.ts
@@ -41,20 +41,41 @@ export default createRule({
  filterNode: TSESTree.Node;
  }
 
- function parseIfArrayFilterExpression(
+ function parseArrayFilterExpressions(
  expression: TSESTree.Expression,
- ): FilterExpressionData | undefined {
+ ): FilterExpressionData[] {
  if (expression.type === AST_NODE_TYPES.SequenceExpression) {
  // Only the last expression in (a, b, [1, 2, 3].filter(condition))[0] matters
  const lastExpression = nullThrows(
  expression.expressions.at(-1),
  'Expected to have more than zero expressions in a sequence expression',
  );
- return parseIfArrayFilterExpression(lastExpression);
+ return parseArrayFilterExpressions(lastExpression);
  }
 
  if (expression.type === AST_NODE_TYPES.ChainExpression) {
- return parseIfArrayFilterExpression(expression.expression);
+ return parseArrayFilterExpressions(expression.expression);
+ }
+
+ // This is the only reason we're returning a list rather than a single value.
+ if (expression.type === AST_NODE_TYPES.ConditionalExpression) {
+ // Both branches of the ternary _must_ return results.
+ const consequentResult = parseArrayFilterExpressions(
+ expression.consequent,
+ );
+ if (consequentResult.length === 0) {
+ return [];
+ }
+
+ const alternateResult = parseArrayFilterExpressions(
+ expression.alternate,
+ );
+ if (alternateResult.length === 0) {
+ return [];
+ }
+
+ // Accumulate the results from both sides and pass up the chain.
+ return [...consequentResult, ...alternateResult];
  }
 
  // Check if it looks like <<stuff>>(...), but not <<stuff>>?.(...)
@@ -78,16 +99,19 @@ export default createRule({
  // As long as the object is a (possibly nullable) array,
  // this is an Array.prototype.filter expression.
  if (isArrayish(filteredObjectType)) {
- return {
- isBracketSyntaxForFilter,
- filterNode,
- };
+ return [
+ {
+ isBracketSyntaxForFilter,
+ filterNode,
+ },
+ ];
  }
  }
  }
  }
 
- return undefined;
+ // not a filter expression.
+ return [];
  }
 
  /**
@@ -223,8 +247,8 @@ export default createRule({
  CallExpression(node): void {
  const object = getObjectIfArrayAtZeroExpression(node);
  if (object) {
- const filterExpression = parseIfArrayFilterExpression(object);
- if (filterExpression) {
+ const filterExpressions = parseArrayFilterExpressions(object);
+ if (filterExpressions.length !== 0) {
  context.report({
  node,
  messageId: 'preferFind',
@@ -233,9 +257,11 @@ export default createRule({
  messageId: 'preferFindSuggestion',
  fix: (fixer): TSESLint.RuleFix[] => {
  return [
- generateFixToReplaceFilterWithFind(
- fixer,
- filterExpression,
+ ...filterExpressions.map(filterExpression =>
+ generateFixToReplaceFilterWithFind(
+ fixer,
+ filterExpression,
+ ),
  ),
  // Get rid of the .at(0) or ['at'](0).
  generateFixToRemoveArrayElementAccess(
@@ -261,8 +287,8 @@ export default createRule({
  ): void {
  if (isMemberAccessOfZero(node)) {
  const object = node.object;
- const filterExpression = parseIfArrayFilterExpression(object);
- if (filterExpression) {
+ const filterExpressions = parseArrayFilterExpressions(object);
+ if (filterExpressions.length !== 0) {
  context.report({
  node,
  messageId: 'preferFind',
@@ -271,9 +297,11 @@ export default createRule({
  messageId: 'preferFindSuggestion',
  fix: (fixer): TSESLint.RuleFix[] => {
  return [
- generateFixToReplaceFilterWithFind(
- fixer,
- filterExpression,
+ ...filterExpressions.map(filterExpression =>
+ generateFixToReplaceFilterWithFind(
+ fixer,
+ filterExpression,
+ ),
  ),
  // Get rid of the [0].
  generateFixToRemoveArrayElementAccess(

diff --git a/packages/eslint-plugin/tests/rules/prefer-find.test.ts b/packages/eslint-plugin/tests/rules/prefer-find.test.ts
@@ -70,6 +70,12 @@ ruleTester.run('prefer-find', rule, {
  `,
  "[1, 2, 3].filter(x => x)[Symbol('0')];",
  "[1, 2, 3].filter(x => x)[Symbol.for('0')];",
+ '(Math.random() < 0.5 ? [1, 2, 3].filter(x => true) : [1, 2, 3])[0];',
+ `
+ (Math.random() < 0.5
+ ? [1, 2, 3].find(x => true)
+ : [1, 2, 3].filter(x => true))[0];
+ `,
  ],
 
  invalid: [
@@ -584,5 +590,142 @@ arr.find(f, thisArg);
  },
  ],
  },
+ {
+ code: `
+(Math.random() < 0.5
+ ? [1, 2, 3].filter(x => false)
+ : [1, 2, 3].filter(x => true))[0];
+ `,
+ errors: [
+ {
+ line: 2,
+ messageId: 'preferFind',
+ suggestions: [
+ {
+ messageId: 'preferFindSuggestion',
+ output: `
+(Math.random() < 0.5
+ ? [1, 2, 3].find(x => false)
+ : [1, 2, 3].find(x => true));
+ `,
+ },
+ ],
+ },
+ ],
+ },
+ {
+ code: `
+Math.random() < 0.5
+ ? [1, 2, 3].find(x => true)
+ : [1, 2, 3].filter(x => true)[0];
+ `,
+ errors: [
+ {
+ line: 4,
+ messageId: 'preferFind',
+ suggestions: [
+ {
+ messageId: 'preferFindSuggestion',
+ output: `
+Math.random() < 0.5
+ ? [1, 2, 3].find(x => true)
+ : [1, 2, 3].find(x => true);
+ `,
+ },
+ ],
+ },
+ ],
+ },
+ {
+ code: `
+declare const f: (arg0: unknown, arg1: number, arg2: Array<unknown>) => boolean,
+ g: (arg0: unknown) => boolean;
+const nestedTernaries = (
+ Math.random() < 0.5
+ ? Math.random() < 0.5
+ ? [1, 2, 3].filter(f)
+ : []?.filter(x => 'shrug')
+ : [2, 3, 4]['filter'](g)
+).at(0.2);
+ `,
+ errors: [
+ {
+ line: 4,
+ messageId: 'preferFind',
+ suggestions: [
+ {
+ messageId: 'preferFindSuggestion',
+ output: `
+declare const f: (arg0: unknown, arg1: number, arg2: Array<unknown>) => boolean,
+ g: (arg0: unknown) => boolean;
+const nestedTernaries = (
+ Math.random() < 0.5
+ ? Math.random() < 0.5
+ ? [1, 2, 3].find(f)
+ : []?.find(x => 'shrug')
+ : [2, 3, 4]["find"](g)
+);
+ `,
+ },
+ ],
+ },
+ ],
+ },
+
+ {
+ code: `
+declare const f: (arg0: unknown) => boolean, g: (arg0: unknown) => boolean;
+const nestedTernariesWithSequenceExpression = (
+ Math.random() < 0.5
+ ? ('sequence',
+ 'expression',
+ Math.random() < 0.5 ? [1, 2, 3].filter(f) : []?.filter(x => 'shrug'))
+ : [2, 3, 4]['filter'](g)
+).at(0.2);
+ `,
+ errors: [
+ {
+ line: 3,
+ messageId: 'preferFind',
+ suggestions: [
+ {
+ messageId: 'preferFindSuggestion',
+ output: `
+declare const f: (arg0: unknown) => boolean, g: (arg0: unknown) => boolean;
+const nestedTernariesWithSequenceExpression = (
+ Math.random() < 0.5
+ ? ('sequence',
+ 'expression',
+ Math.random() < 0.5 ? [1, 2, 3].find(f) : []?.find(x => 'shrug'))
+ : [2, 3, 4]["find"](g)
+);
+ `,
+ },
+ ],
+ },
+ ],
+ },
+
+ {
+ code: `
+declare const spreadArgs: [(x: unknown) => boolean];
+[1, 2, 3].filter(...spreadArgs).at(0);
+ `,
+ errors: [
+ {
+ line: 3,
+ messageId: 'preferFind',
+ suggestions: [
+ {
+ messageId: 'preferFindSuggestion',
+ output: `
+declare const spreadArgs: [(x: unknown) => boolean];
+[1, 2, 3].find(...spreadArgs);
+ `,
+ },
+ ],
+ },
+ ],
+ },
  ],
 });