This was arguably a bug fix in the existing test case, because it shouldn't be reported either.

This actually should be reported because the fixture config doesn't usenoUncheckedIndexAccess!

I think we should probably guard the new code around whether or not the flag is on.

@bradzacher This code is the exact same principle asdictionary[key] ??= defaultVal. If we want to ignore that, we have to ignore others as well.

They are and they aren't. You're correct in that they're both record index access in a logical position - so they're semantically the same case. However TS reports the types differently for the two cases.

In this instance TS will report the read type, meaning that ifnoUncheckedIndexAccess is turned on then TS will report the type asT | undefimed to us.

However inx ?? y, TS will report the write type, meaning it reports justT - no matter whether or notnoUncheckedIndexAccess is on or not.

This is the fundamental difference between the two cases and why we are false positiving on the assignment operators, butnot on this case.

However, we already ignore array index access, even withoutnoUncheckedIndexAccess. See this test case:

declareconstarr:{foo:number}[];constbar=arr[42]?.foo??0;

I find it quite natural to extend the scope to all index signatures, since there isn't something special about array indices.

The reason we did it for arrays and not objects is because defining arrays as(T | undefined)[] is similar toRecord<string, T | undefined>, but far, far less ergonomic syntactically and is arguably worse given how common it is to iterate arrays directly using an iterator method or for/of.

For objects it's pretty natural to union inundefined because most usecases for reading is an index signature - the direct iteration methods (Object.entries /Object.values) are both rarer usecases so theundefined doesn't get in the way as much.

Now that we havenoUncheckedIndexAccess - we don't really need the logic at all, but I understand that some people think the compiler option goes too far and can get in the way (which is true to a degree).

The motivation behind this change isn't bringing the two sets up to parity (that's a much larger change that we should discuss as a group to decide if we think we should treat objects the same as arrays in this regard) - instead it's just fixing the current false positives we have due to the logical operator handling we added in#6594

This actually should be reported because the fixture config doesn't usenoUncheckedIndexAccess!

I think we should probably guard the new code around whether or not the flag is on.

This check is actually no longer sound with modern TS.

Tuples can be variadic now - (eg[T1, ...T2[]],[...T1[], T2],[T1, ...T2[], T3],[...T1[]]) - so they're no more safe than an array.

I.e. forconst foo: [T1, ...T2[]] -foo[1] is possiblyundefined

playground

question: do we want to handle thenever case here?
I think it's okay not to just figured I'd flag it for completeness sake

This doens't handle "nominal" number types here.
EGtype T = number & { __brand: unknown } would work in the signature but we would not match it here.

I think it's okay that we ignore it here - it's a super niche usecase.
Just commenting for completeness's sake.

we should probably also handlesymbol indexes as well.
They're super rare but it should just be one extraif

declareconstobj:{[k:symbol]:string};declareconstkey:symbol;constres=obj[key];res?.length// should not report