Skip to content
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

Should/could typeguarding work on arrays/length #10272

Closed
normalser opened this issue Aug 11, 2016 · 7 comments
Closed

Should/could typeguarding work on arrays/length #10272

normalser opened this issue Aug 11, 2016 · 7 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@normalser
Copy link

TypeScript Version: @next

Code

//  --strictNullChecks
let args = process.argv.slice(2) // type: string[]
let test1 = args[Number.MAX_VALUE] // type: 'string' but should it be 'string | undefined' ?
if (args.length > 0) {
  let test2 = args.shift() // Type: 'string | undefined' - but should it be 'string' ?
}
@gcnew
Copy link
Contributor

gcnew commented Aug 11, 2016

Strictly speaking - no. It doesn't guard against sparse arrays, e.g:

const arr: number[] = [];
arr[1] = 5;
if (arr.length) {
    arr.shift(); // undefined
}
arr.shift(); // 5

Then again, the true type of indexing is also T | undefined.

@normalser
Copy link
Author

normalser commented Aug 11, 2016

@gcnew Thanks for the explanation, but then shouldn't accessing sparse arrays and having --strictNullChecks on - always return T | undefined

args[Number.MAX_VALUE] // type: 'string'

@gcnew
Copy link
Contributor

gcnew commented Aug 11, 2016

@wallverb Yes, well and truly. It was done for convenience as explained by the reply to #7140 (comment)

PS: To play devil's advocate, I guess the return type of shift() is written as T | undefined to accommodate for shifting an empty array, no so much about the sparse case. In this sense you are right, but it would be very hard to create a generic solution that takes into account all the hidden, intricate knowledge of an objects state and apply it to it's methods.

@normalser
Copy link
Author

wow, interesting, thank you so much for finding that comment for me. I wish such knowledge was more easily accessible/findable :)

@gcnew
Copy link
Contributor

gcnew commented Aug 11, 2016

BTW: as a workaround this works:

function isShiftable<T>(arr: T[]): arr is Array<T> & { shift(): T; } {
    return !!arr.length;
}

const arr: number[] = [5];
if (isShiftable(arr)) {
    const x = arr.shift(); // x is of type `number`
}

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Aug 11, 2016
@dlmanning
Copy link

dlmanning commented Sep 25, 2016

This workaround is not working for me.

function isNotEmpty<T>(arr: T[]): arr is Array<T> & { pop(): T; } {
    return arr.length > 0;
}

function doSomething () {
  const stack: Vertex[] = []
  const groups: string[] = []

  // put some things on the stack

  while (isNotEmpty(stack)) {
    const v = stack.pop() // v is still typed as Vertex | undefined
    groups.push(v.id) // [ts] Object is possibly 'undefined'
  }
}

did I miss something?

(tried this with typescript 2.0.3 and 2.1.0-dev.20160924)

Update: changed the order of the return type def like so:

function isNotEmpty<T>(arr: T[]): arr is { pop(): T; } & Array<T>

and it works as desired. Is the type intersection operator not commutative?

@gcnew
Copy link
Contributor

gcnew commented Sep 25, 2016

@dlmanning You are right, the suggested fix is the the way to go. It should be noted though that this type guard is a hack - I just found that it worked. About the type intersection operator I don't think there is a notion of commutativity, I suspect it's just the compiler picking the first matching declaration.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants