Hegel: require explicit check for undefined?

Created on 20 May 2020  路  11Comments  路  Source: JSMonk/hegel

This code has a runtime error. It will not use the 0 value.

Should Hegel force us to explicitly check undefined to avoid the runtime error?

enhancement invalid

Most helpful comment

I think it would make sense to warn on Nullish | PossblyFalsy used in if, while, ternary, etc.
(where Nullish is either null or undefined or union of both, and PossiblyFalsy is any combination of string, number, bigint and boolean)

This is what Flow does (with sketchy-null lint enabled), and there's strict-boolean-expressions for typescript-eslint which also does that.

All 11 comments

This is the goal "Don't have a runtime error". But, I actually don't understand the example, because, I run it and didn't get an error. Could you please explain the problem?

Sorry, not runtime error, but a silent error: the n && useNum(n) does not pass the 0 into useNum because 0 is falsy. So if the dev meant to pass all numbers into the function, it will pass them all except zero.

If it forces us to check for undefined, then it could prevent such a mistake.

Maybe n && useNum(n) would still have a type error, and we would need to write something like n !== undefined && useNum(n). Not sure though.

I mean, maybe the fact that undefined was not checked could somehow be a type error?

Or, maybe numbers shouldn't be treated as booleans? But I can see how this will make some code more verbose.

Maybe https://github.com/JSMonk/hegel/issues/227 would help with eliminating verbosity.

Then

isNumber(n) && useNum(n)

would lead to no type error, because the type system would know that the check for typeof n === 'number' means that n can't possibly be undefined.

Maybe n && useNum(n) would still have a type error, and we would need to write something like n !== undefined && useNum(n). Not sure though.

No, actually, if you will hove at the left part of the && you will see which values will be not passed into the function.

Oh yeah, the tooltip shows 0 | undefined, but it isn't a guarantee people are going to look at that.

So TLDR, this type of error checking is out of scope?

It seems like I can't show the error in this case. And I don't sure, that this is Type System responsibility (Actually, I think it's linter responsibility, because of this case is about coding style neither the type system issue).
What do you think about it?

i think that this would report error

const nums: Array<number> = [0,1,2,3,4,5]

for (let i=0; i<nums.length; i++) {
  const n = nums[i]
  n && useNum(n)// should report: Type "number | undefined" is incompatible with type "boolean"
}

function useNum(n: number) {
  n
}

like this does

const nums: Array<number> = [0,1,2,3,4,5]

for (let i=0; i<nums.length; i++) {
  const n = nums[i]
  if (n) useNum(n)// reports: Type "number | undefined" is incompatible with type "boolean"
}

function useNum(n: number) {
  n
}

i think && and || are boolean operators (and you should not be allowed to use it otherwise)

I think it would make sense to warn on Nullish | PossblyFalsy used in if, while, ternary, etc.
(where Nullish is either null or undefined or union of both, and PossiblyFalsy is any combination of string, number, bigint and boolean)

This is what Flow does (with sketchy-null lint enabled), and there's strict-boolean-expressions for typescript-eslint which also does that.

I don't think a linter could do a good job checking these things, otherwise the linter would become something like Hegel already is.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

trusktr picture trusktr  路  5Comments

mohsen1 picture mohsen1  路  5Comments

nanot1m picture nanot1m  路  4Comments

jarble picture jarble  路  4Comments

devhammed picture devhammed  路  3Comments