Skip to content

noUncheckedIndexedAccess does not compile on incorrect possible undefined #51988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Magnuti opened this issue Dec 21, 2022 · 7 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@Magnuti
Copy link

Magnuti commented Dec 21, 2022

Bug Report

πŸ”Ž Search Terms

  • noUncheckedIndexedAccess

πŸ•— Version & Regression Information

  • v4.9.4
  • noUncheckedIndexedAccess: true
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries since v4.1.5

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

const numbers = [1, 2, 3];

if (numbers.length > 0) {
  const first = numbers[0];
  const doubleFirst = first * 2;
}

πŸ™ Actual behavior

The code does not compile since the compiler thinks first can be undefined.

image

πŸ™‚ Expected behavior

The compiler should notice that first must be of type number and not number | undefined since the if statement ensures that.

Of course the code compiles if we disable noUncheckedIndexedAccess, but I would like to keep it enabled.

@MartinJohns
Copy link
Contributor

MartinJohns commented Dec 21, 2022

Essentially a duplicate of #38000 and/or #42909. noUncheckedIndexedAccess does not change anything about this.

You can access the first element and check that one for undefined.

@Magnuti
Copy link
Author

Magnuti commented Dec 21, 2022

@MartinJohns it seems redundant having check the first element for undefined. The array cannot contain undefined elements and the array contains elements by the if(numbers.length > +) statement, therefore the first element first cannot be undefined.

@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Dec 21, 2022
@WesleyYue
Copy link

WesleyYue commented Dec 22, 2022

@MartinJohns it seems redundant having check the first element for undefined. The array cannot contain undefined elements and the array contains elements by the if(numbers.length > +) statement, therefore the first element first cannot be undefined.

if (numbers.length > x) doesn't check if the var is undefined, only the length, you can have [undefined, undefined, undefined].

Regardless, I would agree the error should not throw here since numbers is clearly defined. I'm running into a bunch of variations of this where the var is clearly defined but typescript appends undefined liberally.

@Magnuti
Copy link
Author

Magnuti commented Dec 22, 2022

@WesleyYue are you sure that the array can contain undefined values? If I explicitly specify the type on the array the code below won't compile.

const numbers: number[] = [1, 2, 3, undefined];

image

Even if I do not specify the type on the array the result is the same.

const numbers = [1, 2, 3];
numbers.push(undefined)

image

@Magnuti
Copy link
Author

Magnuti commented Dec 22, 2022

if (numbers.length > x) doesn't check if the var is undefined, only the length, you can have [undefined, undefined, undefined].

@WesleyYue I mean yes if the array was not clearly defined, but as you say the array is clearly defined. The error should not be thrown so we agree on that

@MartinJohns
Copy link
Contributor

Checking the length property does not narrow the type, that's what the issues I linked are about. I suggest checking the indexed value instead of the length property, or simply use a type assertion.

@Magnuti Magnuti closed this as completed Dec 22, 2022
@fatcerberus
Copy link

For the record, it's entirely possible for this to happen at runtime:

let arr: number[] = [];
arr[1] = 42;
console.log(arr.length);  // 2 (i.e. arr.length > 0)
console.log(arr[0]);  // undefined!

It's debatable whether most code needs to be hardened against that (sparse arrays are evil), but seems worth considering, at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants