From 439da04fc371e467212a21c6a3c44e716481d86d Mon Sep 17 00:00:00 2001 From: "Azat S." Date: Mon, 3 Nov 2025 03:19:34 +0300 Subject: [PATCH 1/4] feat(eslint-plugin): [no-unnecessary-condition] detect unnecessary `Array.isArray` calls --- .../src/rules/no-unnecessary-condition.ts | 99 +++++++++++++++- .../rules/no-unnecessary-condition.test.ts | 111 ++++++++++++++++++ 2 files changed, 209 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index b9a1ebb9a4bf..2753d2ea8475 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -288,6 +288,71 @@ export default createRule({ .some(part => checker.isTupleType(part)); } + function typeIsExclusivelyArrayOrTuple(type: ts.Type): boolean { + return tsutils + .unionConstituents(type) + .every(part => checker.isArrayType(part) || checker.isTupleType(part)); + } + + function getDeclaredTypesOfExpression( + node: TSESTree.Expression, + ): ts.Type[] { + const tsNode = services.esTreeNodeToTSNodeMap.get(node); + const symbol = checker.getSymbolAtLocation(tsNode); + + if (!symbol) { + return []; + } + + const declarations = symbol.getDeclarations(); + + if (!declarations || declarations.length === 0) { + return []; + } + + function getTypeNodeFromDeclaration( + declaration: ts.Declaration, + ): ts.TypeNode | undefined { + if ( + ts.isParameter(declaration) || + ts.isPropertySignature(declaration) || + ts.isPropertyDeclaration(declaration) || + ts.isVariableDeclaration(declaration) + ) { + return declaration.type ?? undefined; + } + + if (ts.isBindingElement(declaration)) { + const bindingPattern = declaration.parent; + const parentDeclaration = bindingPattern.parent; + + if ( + ts.isVariableDeclaration(parentDeclaration) || + ts.isParameter(parentDeclaration) || + ts.isPropertyDeclaration(parentDeclaration) || + ts.isPropertySignature(parentDeclaration) + ) { + return parentDeclaration.type ?? undefined; + } + } + + return undefined; + } + + const types: ts.Type[] = []; + + for (const declaration of declarations) { + const typeNode = getTypeNodeFromDeclaration(declaration); + if (!typeNode) { + continue; + } + + types.push(checker.getTypeFromTypeNode(typeNode)); + } + + return types; + } + function isArrayIndexExpression(node: TSESTree.Expression): boolean { return ( // Is an index signature @@ -580,6 +645,31 @@ export default createRule({ checkNode(node.test); } + function isArrayIsArrayCall(node: TSESTree.CallExpression): boolean { + if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { + return false; + } + + const { computed, object, property } = node.callee; + + if (!isIdentifier(object) || object.name !== 'Array') { + return false; + } + + if (computed) { + return ( + node.callee.property.type === AST_NODE_TYPES.Literal && + node.callee.property.value === 'isArray' + ); + } + + return ( + isIdentifier(property) && + property.name === 'isArray' && + !node.callee.optional + ); + } + function checkCallExpression(node: TSESTree.CallExpression): void { if (checkTypePredicates) { const truthinessAssertedArgument = findTruthinessAssertedArgument( @@ -599,7 +689,14 @@ export default createRule({ services, typeGuardAssertedArgument.argument, ); - if (typeOfArgument === typeGuardAssertedArgument.type) { + if ( + typeOfArgument === typeGuardAssertedArgument.type || + (isArrayIsArrayCall(node) && + typeIsExclusivelyArrayOrTuple(typeOfArgument) && + getDeclaredTypesOfExpression( + typeGuardAssertedArgument.argument, + ).every(typeIsExclusivelyArrayOrTuple)) + ) { context.report({ node: typeGuardAssertedArgument.argument, messageId: 'typeGuardAlreadyIsType', diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 22bb2f46c152..4d4badf4cf3f 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -1138,6 +1138,55 @@ isString('falafel'); `, options: [{ checkTypePredicates: true }], }, + + { + code: ` +const items: number[] | null = [1, 2, 3]; +if (Array.isArray(items)) { + console.log('items is an array'); +} + `, + options: [{ checkTypePredicates: true }], + }, + { + code: ` +const items: number[] | string = [1, 2, 3]; +if (Array.isArray(items)) { + console.log('items is an array'); +} + `, + options: [{ checkTypePredicates: true }], + }, + { + code: ` +const items: unknown = [1, 2, 3]; +if (Array.isArray(items)) { + console.log('items is an array'); +} + `, + options: [{ checkTypePredicates: true }], + }, + { + code: ` +function process(value: string | number[]) { + if (Array.isArray(value)) { + return value.length; + } + return value.length; +} + `, + options: [{ checkTypePredicates: true }], + }, + { + code: ` +const items: number[] = [1, 2, 3]; +if (Array.isArray(items)) { + console.log('items is an array'); +} + `, + options: [{ checkTypePredicates: false }], + }, + ` type A = { [name in Lowercase]?: A }; declare const a: A; @@ -3518,6 +3567,68 @@ isString('fa' + 'lafel'); ], options: [{ checkTypePredicates: true }], }, + { + code: ` +const items: number[] = [1, 2, 3]; +if (Array.isArray(items)) { + console.log('items is an array'); +} + `, + errors: [ + { + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, + { + code: ` +const items: string[] = ['a', 'b']; +Array.isArray(items); + `, + errors: [ + { + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, + { + code: ` +const matrix: number[][] = [ + [1, 2], + [3, 4], +]; +if (Array.isArray(matrix)) { + console.log('matrix is an array'); +} + `, + errors: [ + { + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, + { + code: ` +function processArray(arr: readonly string[]) { + if (Array.isArray(arr)) { + return arr.length; + } +} + `, + errors: [ + { + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, // "branded" types unnecessaryConditionTest('"" & {}', 'alwaysFalsy'), From 6157a5a52824118233dd61adae3101f755ab86e9 Mon Sep 17 00:00:00 2001 From: "Azat S." Date: Mon, 3 Nov 2025 13:28:38 +0300 Subject: [PATCH 2/4] refactor(eslint-plugin): [no-unnecessary-condition] use `isTypeAssignableTo` API for `Array.isArray` checks --- .../src/rules/no-unnecessary-condition.ts | 82 ++----------------- .../rules/no-unnecessary-condition.test.ts | 33 ++------ 2 files changed, 16 insertions(+), 99 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 2753d2ea8475..4dbfa2be3ac3 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -288,71 +288,6 @@ export default createRule({ .some(part => checker.isTupleType(part)); } - function typeIsExclusivelyArrayOrTuple(type: ts.Type): boolean { - return tsutils - .unionConstituents(type) - .every(part => checker.isArrayType(part) || checker.isTupleType(part)); - } - - function getDeclaredTypesOfExpression( - node: TSESTree.Expression, - ): ts.Type[] { - const tsNode = services.esTreeNodeToTSNodeMap.get(node); - const symbol = checker.getSymbolAtLocation(tsNode); - - if (!symbol) { - return []; - } - - const declarations = symbol.getDeclarations(); - - if (!declarations || declarations.length === 0) { - return []; - } - - function getTypeNodeFromDeclaration( - declaration: ts.Declaration, - ): ts.TypeNode | undefined { - if ( - ts.isParameter(declaration) || - ts.isPropertySignature(declaration) || - ts.isPropertyDeclaration(declaration) || - ts.isVariableDeclaration(declaration) - ) { - return declaration.type ?? undefined; - } - - if (ts.isBindingElement(declaration)) { - const bindingPattern = declaration.parent; - const parentDeclaration = bindingPattern.parent; - - if ( - ts.isVariableDeclaration(parentDeclaration) || - ts.isParameter(parentDeclaration) || - ts.isPropertyDeclaration(parentDeclaration) || - ts.isPropertySignature(parentDeclaration) - ) { - return parentDeclaration.type ?? undefined; - } - } - - return undefined; - } - - const types: ts.Type[] = []; - - for (const declaration of declarations) { - const typeNode = getTypeNodeFromDeclaration(declaration); - if (!typeNode) { - continue; - } - - types.push(checker.getTypeFromTypeNode(typeNode)); - } - - return types; - } - function isArrayIndexExpression(node: TSESTree.Expression): boolean { return ( // Is an index signature @@ -689,14 +624,15 @@ export default createRule({ services, typeGuardAssertedArgument.argument, ); - if ( - typeOfArgument === typeGuardAssertedArgument.type || - (isArrayIsArrayCall(node) && - typeIsExclusivelyArrayOrTuple(typeOfArgument) && - getDeclaredTypesOfExpression( - typeGuardAssertedArgument.argument, - ).every(typeIsExclusivelyArrayOrTuple)) - ) { + const isArrayCheck = isArrayIsArrayCall(node); + const typesMatch = isArrayCheck + ? checker.isTypeAssignableTo( + typeOfArgument, + typeGuardAssertedArgument.type, + ) + : typeOfArgument === typeGuardAssertedArgument.type; + + if (typesMatch) { context.report({ node: typeGuardAssertedArgument.argument, messageId: 'typeGuardAlreadyIsType', diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 4d4badf4cf3f..48ded2c2d6bc 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -1141,7 +1141,7 @@ isString('falafel'); { code: ` -const items: number[] | null = [1, 2, 3]; +declare const items: number[] | null; if (Array.isArray(items)) { console.log('items is an array'); } @@ -1150,7 +1150,7 @@ if (Array.isArray(items)) { }, { code: ` -const items: number[] | string = [1, 2, 3]; +declare const items: number[] | string; if (Array.isArray(items)) { console.log('items is an array'); } @@ -1159,7 +1159,7 @@ if (Array.isArray(items)) { }, { code: ` -const items: unknown = [1, 2, 3]; +declare const items: unknown; if (Array.isArray(items)) { console.log('items is an array'); } @@ -1179,7 +1179,7 @@ function process(value: string | number[]) { }, { code: ` -const items: number[] = [1, 2, 3]; +declare const items: number[]; if (Array.isArray(items)) { console.log('items is an array'); } @@ -3569,7 +3569,7 @@ isString('fa' + 'lafel'); }, { code: ` -const items: number[] = [1, 2, 3]; +declare const items: number[]; if (Array.isArray(items)) { console.log('items is an array'); } @@ -3584,7 +3584,7 @@ if (Array.isArray(items)) { }, { code: ` -const items: string[] = ['a', 'b']; +declare const items: string[]; Array.isArray(items); `, errors: [ @@ -3597,10 +3597,7 @@ Array.isArray(items); }, { code: ` -const matrix: number[][] = [ - [1, 2], - [3, 4], -]; +declare const matrix: number[][]; if (Array.isArray(matrix)) { console.log('matrix is an array'); } @@ -3613,22 +3610,6 @@ if (Array.isArray(matrix)) { ], options: [{ checkTypePredicates: true }], }, - { - code: ` -function processArray(arr: readonly string[]) { - if (Array.isArray(arr)) { - return arr.length; - } -} - `, - errors: [ - { - line: 3, - messageId: 'typeGuardAlreadyIsType', - }, - ], - options: [{ checkTypePredicates: true }], - }, // "branded" types unnecessaryConditionTest('"" & {}', 'alwaysFalsy'), From c13f7b73b398eac9c7b80041bad11b3b2bdd5cc3 Mon Sep 17 00:00:00 2001 From: "Azat S." Date: Mon, 3 Nov 2025 23:19:51 +0300 Subject: [PATCH 3/4] refactor(no-unnecessary-condition): use assignability check for all type predicates --- .../src/rules/no-unnecessary-condition.ts | 40 +++------------- .../rules/no-unnecessary-condition.test.ts | 46 +++++++++++-------- 2 files changed, 34 insertions(+), 52 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 4dbfa2be3ac3..7c78b4842a68 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -580,31 +580,6 @@ export default createRule({ checkNode(node.test); } - function isArrayIsArrayCall(node: TSESTree.CallExpression): boolean { - if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { - return false; - } - - const { computed, object, property } = node.callee; - - if (!isIdentifier(object) || object.name !== 'Array') { - return false; - } - - if (computed) { - return ( - node.callee.property.type === AST_NODE_TYPES.Literal && - node.callee.property.value === 'isArray' - ); - } - - return ( - isIdentifier(property) && - property.name === 'isArray' && - !node.callee.optional - ); - } - function checkCallExpression(node: TSESTree.CallExpression): void { if (checkTypePredicates) { const truthinessAssertedArgument = findTruthinessAssertedArgument( @@ -624,15 +599,12 @@ export default createRule({ services, typeGuardAssertedArgument.argument, ); - const isArrayCheck = isArrayIsArrayCall(node); - const typesMatch = isArrayCheck - ? checker.isTypeAssignableTo( - typeOfArgument, - typeGuardAssertedArgument.type, - ) - : typeOfArgument === typeGuardAssertedArgument.type; - - if (typesMatch) { + if ( + checker.isTypeAssignableTo( + typeOfArgument, + typeGuardAssertedArgument.type, + ) + ) { context.report({ node: typeGuardAssertedArgument.argument, messageId: 'typeGuardAlreadyIsType', diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 48ded2c2d6bc..e2dbc3fafd63 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -1122,23 +1122,6 @@ isString(a); `, options: [{ checkTypePredicates: false }], }, - { - // Technically, this has type 'falafel' and not string. - code: ` -declare function assertString(x: unknown): asserts x is string; -assertString('falafel'); - `, - options: [{ checkTypePredicates: true }], - }, - { - // Technically, this has type 'falafel' and not string. - code: ` -declare function isString(x: unknown): x is string; -isString('falafel'); - `, - options: [{ checkTypePredicates: true }], - }, - { code: ` declare const items: number[] | null; @@ -3610,7 +3593,34 @@ if (Array.isArray(matrix)) { ], options: [{ checkTypePredicates: true }], }, - + { + code: ` +declare function isString(x: unknown): x is string; +isString('falafel'); + `, + errors: [ + { + column: 10, + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, + { + code: ` +declare function assertString(x: unknown): asserts x is string; +assertString('falafel'); + `, + errors: [ + { + column: 14, + line: 3, + messageId: 'typeGuardAlreadyIsType', + }, + ], + options: [{ checkTypePredicates: true }], + }, // "branded" types unnecessaryConditionTest('"" & {}', 'alwaysFalsy'), unnecessaryConditionTest('"" & { __brand: string }', 'alwaysFalsy'), From e1abd4560d5abfe025ba352fe1fcf0e99c05114c Mon Sep 17 00:00:00 2001 From: "Azat S." Date: Sun, 9 Nov 2025 23:14:42 +0300 Subject: [PATCH 4/4] fix(eslint-plugin): refine type guard assignability handling --- .../src/rules/no-unnecessary-condition.ts | 42 ++++++++- .../rules/no-unnecessary-condition.test.ts | 87 ++++++++++++------- .../src/utils/flat-config-schema.ts | 2 +- 3 files changed, 93 insertions(+), 38 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 7c78b4842a68..a08097906968 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -70,6 +70,30 @@ function toStaticValue( return undefined; } +function typeContainsAnyOrUnknown(type: ts.Type): boolean { + return tsutils + .unionConstituents(type) + .some(part => isTypeAnyType(part) || isTypeUnknownType(part)); +} + +function isArrayIsArrayCall(node: TSESTree.CallExpression): boolean { + if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { + return false; + } + + const memberExpr = node.callee; + if (memberExpr.computed || memberExpr.optional) { + return false; + } + + return ( + memberExpr.object.type === AST_NODE_TYPES.Identifier && + memberExpr.object.name === 'Array' && + memberExpr.property.type === AST_NODE_TYPES.Identifier && + memberExpr.property.name === 'isArray' + ); +} + const BOOL_OPERATORS = new Set([ '<', '>', @@ -599,7 +623,19 @@ export default createRule({ services, typeGuardAssertedArgument.argument, ); - if ( + if (typeOfArgument === typeGuardAssertedArgument.type) { + context.report({ + node: typeGuardAssertedArgument.argument, + messageId: 'typeGuardAlreadyIsType', + data: { + typeGuardOrAssertionFunction: typeGuardAssertedArgument.asserts + ? 'assertion function' + : 'type guard', + }, + }); + } else if ( + isArrayIsArrayCall(node) && + !typeContainsAnyOrUnknown(typeOfArgument) && checker.isTypeAssignableTo( typeOfArgument, typeGuardAssertedArgument.type, @@ -609,9 +645,7 @@ export default createRule({ node: typeGuardAssertedArgument.argument, messageId: 'typeGuardAlreadyIsType', data: { - typeGuardOrAssertionFunction: typeGuardAssertedArgument.asserts - ? 'assertion function' - : 'type guard', + typeGuardOrAssertionFunction: 'type guard', }, }); } diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index e2dbc3fafd63..65f6b6d87810 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -1122,11 +1122,27 @@ isString(a); `, options: [{ checkTypePredicates: false }], }, + { + // Technically, this has type 'falafel' and not string. + code: ` +declare function assertString(x: unknown): asserts x is string; +assertString('falafel'); + `, + options: [{ checkTypePredicates: true }], + }, + { + // Technically, this has type 'falafel' and not string. + code: ` +declare function isString(x: unknown): x is string; +isString('falafel'); + `, + options: [{ checkTypePredicates: true }], + }, { code: ` declare const items: number[] | null; if (Array.isArray(items)) { - console.log('items is an array'); + console.log(items.length); } `, options: [{ checkTypePredicates: true }], @@ -1135,7 +1151,7 @@ if (Array.isArray(items)) { code: ` declare const items: number[] | string; if (Array.isArray(items)) { - console.log('items is an array'); + console.log(items.length); } `, options: [{ checkTypePredicates: true }], @@ -1144,32 +1160,49 @@ if (Array.isArray(items)) { code: ` declare const items: unknown; if (Array.isArray(items)) { - console.log('items is an array'); + console.log(items.length); } `, options: [{ checkTypePredicates: true }], }, { code: ` -function process(value: string | number[]) { - if (Array.isArray(value)) { - return value.length; - } - return value.length; +declare const items: any; +if (Array.isArray(items)) { + console.log(items.length); } `, options: [{ checkTypePredicates: true }], }, { code: ` +declare const MaybeArray: typeof Array | undefined; declare const items: number[]; -if (Array.isArray(items)) { - console.log('items is an array'); +if (MaybeArray?.isArray(items)) { + console.log(items.length); } `, - options: [{ checkTypePredicates: false }], + options: [{ checkTypePredicates: true }], + }, + { + code: ` +declare const items: number[]; +if (Array['isArray'](items)) { + console.log(items.length); +} + `, + options: [{ checkTypePredicates: true }], + }, + { + code: ` +function process(value: T) { + if (Array.isArray(value)) { + console.log(value.length); + } +} + `, + options: [{ checkTypePredicates: true }], }, - ` type A = { [name in Lowercase]?: A }; declare const a: A; @@ -3554,7 +3587,7 @@ isString('fa' + 'lafel'); code: ` declare const items: number[]; if (Array.isArray(items)) { - console.log('items is an array'); + console.log(items.length); } `, errors: [ @@ -3580,9 +3613,9 @@ Array.isArray(items); }, { code: ` -declare const matrix: number[][]; -if (Array.isArray(matrix)) { - console.log('matrix is an array'); +const tuple: [string, number] = ['a', 1]; +if (Array.isArray(tuple)) { + console.log(tuple[0]); } `, errors: [ @@ -3595,32 +3628,20 @@ if (Array.isArray(matrix)) { }, { code: ` -declare function isString(x: unknown): x is string; -isString('falafel'); - `, - errors: [ - { - column: 10, - line: 3, - messageId: 'typeGuardAlreadyIsType', - }, - ], - options: [{ checkTypePredicates: true }], - }, - { - code: ` -declare function assertString(x: unknown): asserts x is string; -assertString('falafel'); +declare const items: string[] | number[]; +if (Array.isArray(items)) { + console.log(items.length); +} `, errors: [ { - column: 14, line: 3, messageId: 'typeGuardAlreadyIsType', }, ], options: [{ checkTypePredicates: true }], }, + // "branded" types unnecessaryConditionTest('"" & {}', 'alwaysFalsy'), unnecessaryConditionTest('"" & { __brand: string }', 'alwaysFalsy'), diff --git a/packages/rule-tester/src/utils/flat-config-schema.ts b/packages/rule-tester/src/utils/flat-config-schema.ts index 1b631070a3e0..9e6e9342d94e 100644 --- a/packages/rule-tester/src/utils/flat-config-schema.ts +++ b/packages/rule-tester/src/utils/flat-config-schema.ts @@ -423,7 +423,7 @@ const processorSchema: ObjectPropertySchema = { }, }; -type ConfigRules = Record; +type ConfigRules = Record; const rulesSchema = { merge(first: ConfigRules = {}, second: ConfigRules = {}): ConfigRules {