noUncheckedIndexedAccess should be enabled by default in strict mode

[a7madgamal]

Bug Report

const numbers: Array = [];
// this should fail, type of test should be number | undefined
const test = numbers[0].toFixed();

I was surprised that this is valid in strict mode. started writing issue about that then meanwhile found about noUncheckedIndexedAccess so now I'm changing the issue to enable it by default and make thousands of codebases safer, hopefully :D

🔎 Search Terms

"original" issue about unsafe array access:
array index undefined

after my realization:
noUncheckedIndexedAccess

🕗 Version & Regression Information

I think since ever, I did check the list anyway

"Bugs" that have existed in TS for a long time are very likely to be FAQs; refer to
https://github.com/Microsoft/TypeScript/wiki/FAQ#common-bugs-that-arent-bugs

If possible, please try testing the nightly version of TS to see if it's already been fixed.
For npm: typescript@next
This is also the 'Nightly' version in the playground: http://www.typescriptlang.org/play/?ts=Nightly

Note: The TypeScript Playground can be used to try older versions of TypeScript.

Please keep and fill in the line that best applies:

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about common bugs

⏯ Playground Link

Playground link with relevant code

💻 Code

const numbers: Array<number> = [];

const test = numbers[1] // should be number | undefined instead of number

🙁 Actual behavior

type of test is number and not number | undefined

🙂 Expected behavior

After a lot of research I found out about noUncheckedIndexedAccess which is great but would be awesome if it's enabled by default for strict

[MartinJohns]

It's intentionally not part of strict, see #39560.

[RyanCavanaugh]

In general when we add a new flag that adds new errors, the decision point for putting it in strict vs --noFoo comes down to whether we think a pre-existing, working codebase would find a reasonable mixture of bugs and false positives (-> strict), or just overwhelmingly false positives (-> noFoo).

This behavior is unambiguously in the latter camp on account of breaking every C-style loop in existence.

[styfle]

@RyanCavanaugh Would you consider changing it in a major version like TS 5.0?

I think we have plenty of loop options today: for...of, forEach, etc.

And C-style loops suffer from off-by-one errors.

So I think it would make sense to enable noUncheckedIndexedAccess by default when strict: true since that is what many users expect. Of course, anyone who uses lots of C-style loops can turn it off 🙂

[RyanCavanaugh]

Even if we decided to change some defaults in 5.0, which is certainly on the table, I don't think this one meets the bar. There are too many common sound-by-construction idioms that cause false positives with it. But this is probably the one that comes closest to wanting to be on by default. If we end up with a very robust configuration versioning story where there was an unambiguous opt-in to "latest and greatest as of right now", it might make sense there.

[typescript-bot]

This issue has been marked as "Declined" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[a7madgamal]

fun fact: I totally forgot about this ticket. Just today faced a similar situation (but I likely remembered about the flag and enabled it)
I kindly ask to reconsider.

Here is how I think about it as a user: when I enable the strict flag, I expect it to be, well ..., strict 😅
I like the fact that I can disable some parts of this strict mode by setting flags.
If I upgrade to a new major version of TypeScript and suddenly find a whole lot errors I will be happy actually that I can improve my code base and I will be informed about this important flag. Either I'm ready to fix or I will just disable it. Thats smooth enough for me tbh.

I think what we are misaligned on is how bad are those "false positives". Can someone kindly tell me some examples of how annoying such cases be?

[RyanCavanaugh]

I think what we are misaligned on is how bad are those "false positives". Can someone kindly tell me some examples of how annoying such cases be?

I would say we get 1-2 reports a week of "noUncheckedIndexedAccess is not smart enough". A sampling:

#55206
#55243
#53678
#54353
#54762
#53454
#51988
#42909
#38000

[marwankhalili]

Related to this, I'm wondering if there's a reason why noUncheckedIndexedAccess is not labeled as "recommended" in the TSConfig Reference?

In my opinion it would also be nice to add a non-null assertion code example there, as it is a useful escape hatch:

interface EnvironmentVars {
  NAME: string;
  OS: string;

  // Unknown properties are covered by this index signature.
  [propName: string]: string;
}

declare const env: EnvironmentVars;
 
// Declared as existing
const sysName = env.NAME;
const os = env.OS;
//    ^? const os: string

// Not declared, but because of the index
// signature, then it is considered a string
const nodeEnv = env.NODE_ENV;
//    ^? const nodeEnv: string | undefined

+ // The non-null assertion operator (!) can 
+ // still be used for unchecked index access
+ const nodeEnvString = env.NODE_ENV!;
//    ^? const nodeEnvString: string

[RyanCavanaugh]

Related to this, I'm wondering if there's a reason why noUncheckedIndexedAccess is not labeled as "recommended" in the TSConfig Reference?

The previous comment is why

[marwankhalili]

I agree that enabling the noUncheckedIndexedAccess option in an existing codebase would likely cause a lot of compilation errors and confusion. Therefore I understand it might not be a good fit for the strict option that developers already use.

But in my experience, noUncheckedIndexedAccess is still a great compilerOption to recommend for type safety and I would always manually enable it in a new codebase.

Yeah, some "false positives" do occur but the non-null assertion operator makes that a trivial issue if we can't write safer code. To illustrate this in e.g. #51988 that you mentioned:

const numbers = [1, 2, 3]; // Let's assume we only know this has type number[]

if (numbers.length > 0) {
  const first = numbers[0]!; // Non-null assertion, explicitly opting out of type safety
  const doubleFirst = first * 2;
}

// Better in this case to not rely on Array: length
const first = numbers[0];
if (first) {
  const doubleFirst = first * 2;
}