Skip to content

Commit cdeec47

Browse files
committed
fix(no-unnecessary-condition): false positives for overlap and indexed ??= (#697)
1 parent dbab01b commit cdeec47

File tree

2 files changed

+58
-2
lines changed

2 files changed

+58
-2
lines changed

internal/rules/no_unnecessary_condition/no_unnecessary_condition.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,19 @@ var NoUnnecessaryConditionRule = rule.Rule{
13531353
}
13541354
}
13551355

1356+
// With noUncheckedIndexedAccess enabled, element accesses through index
1357+
// signatures can still be absent at runtime (e.g. Record<string, T>[key]).
1358+
// Treat these as potentially nullish for ??=.
1359+
if ctx.Program.Options().NoUncheckedIndexedAccess.IsTrue() && isElementAccess(leftSkip) {
1360+
elemAccess := leftSkip.AsElementAccessExpression()
1361+
baseType := getResolvedType(elemAccess.Expression)
1362+
if baseType != nil &&
1363+
(ctx.TypeChecker.GetStringIndexType(baseType) != nil ||
1364+
ctx.TypeChecker.GetNumberIndexType(baseType) != nil) {
1365+
return
1366+
}
1367+
}
1368+
13561369
// Skip optional property access - with exactOptionalPropertyTypes,
13571370
// the type doesn't include undefined but the property can still be absent
13581371
// Also skip private properties - they have complex semantics
@@ -1500,7 +1513,7 @@ var NoUnnecessaryConditionRule = rule.Rule{
15001513
if isEqualityOp {
15011514
// For equality operators, check if types can ever be equal
15021515
// Only skip if BOTH sides are nullish OR one side is a union that includes nullish
1503-
hasOverlap := typesHaveOverlap(leftType, rightType)
1516+
hasOverlap := typesHaveOverlap(ctx.TypeChecker, leftType, rightType)
15041517

15051518
if !hasOverlap {
15061519
// Check if this is a valid nullish check (e.g., `a: string | null` with `a === null`)
@@ -1958,7 +1971,7 @@ func isAllowedConstantLiteral(node *ast.Node) bool {
19581971
// - Nullish types: null/undefined/void overlap with each other (treated as interchangeable in some contexts)
19591972
// - Literals and base types: overlap (e.g., "hello" overlaps with string)
19601973
// - Union types: checked part by part (e.g., string | number overlaps with "hello")
1961-
func typesHaveOverlap(left, right *checker.Type) bool {
1974+
func typesHaveOverlap(typeChecker *checker.Checker, left, right *checker.Type) bool {
19621975
// Handle any/unknown types - they overlap with everything
19631976
leftFlags := checker.Type_flags(left)
19641977
rightFlags := checker.Type_flags(right)
@@ -1986,6 +1999,14 @@ func typesHaveOverlap(left, right *checker.Type) bool {
19861999
for _, rightPart := range rightParts {
19872000
rightPartFlags := checker.Type_flags(rightPart)
19882001

2002+
// Use TypeScript assignability first. This catches subtype/alias
2003+
// relationships (e.g. Window extends EventTarget, template literals,
2004+
// branded intersections, unique symbols) that raw TypeFlags miss.
2005+
if checker.Checker_isTypeAssignableTo(typeChecker, leftPart, rightPart) ||
2006+
checker.Checker_isTypeAssignableTo(typeChecker, rightPart, leftPart) {
2007+
return true
2008+
}
2009+
19892010
// Check if both are the same primitive type
19902011
primitiveFlags := checker.TypeFlagsString | checker.TypeFlagsNumber |
19912012
checker.TypeFlagsBoolean | checker.TypeFlagsBigInt |

internal/rules/no_unnecessary_condition/no_unnecessary_condition_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,24 @@ function foo<T extends object>(arg: T, key: keyof T): void {
349349
arg[key] == null;
350350
}
351351
`},
352+
// Issue #695 regressions
353+
{Code: `function repro1(e: FocusEvent) { if (e.target !== window) return; }`},
354+
{Code: `
355+
function repro2(flag: string & {}, arg: string) {
356+
if (arg === flag) return true;
357+
return false;
358+
}
359+
`},
360+
{Code: `
361+
const flagPresent = Symbol();
362+
type FlagPresent = typeof flagPresent;
363+
function repro3(queryFlag: string[] | FlagPresent) {
364+
if (Array.isArray(queryFlag) && queryFlag.length === 0) return true;
365+
if (queryFlag === flagPresent) return true;
366+
return false;
367+
}
368+
`},
369+
{Code: "type PersonalizationId = `${string}_6_main` | `${string}_7_main`;\nfunction repro4(a: PersonalizationId, b: string) {\n return a === b;\n}\n"},
352370

353371
// Predicate functions
354372
{Code: `
@@ -993,6 +1011,23 @@ function foo<T extends object>(arg: T, key: keyof T): void {
9931011
arg[key] ??= 'default';
9941012
}
9951013
`},
1014+
{Code: `
1015+
function repro5() {
1016+
const obj: Record<string, { name: string }> = {};
1017+
const key: string = 'foo';
1018+
obj[key] ??= { name: key };
1019+
}
1020+
`},
1021+
{
1022+
Code: `
1023+
function repro5WithNoUncheckedIndexedAccess() {
1024+
const obj: Record<string, { name: string }> = {};
1025+
const key: string = 'foo';
1026+
obj[key] ??= { name: key };
1027+
}
1028+
`,
1029+
TSConfig: "tsconfig.noUncheckedIndexedAccess.json",
1030+
},
9961031

9971032
// Generic indexed access
9981033
{Code: `

0 commit comments

Comments
 (0)