JSMonk / hegel

An advanced static type checker
https://hegel.js.org
MIT License
2.09k stars 59 forks source link

The concept is wrong (or I don't get it) #265

Closed Finesse closed 4 years ago

Finesse commented 4 years ago

There is an example in the readme:

const numbers: Array<number> = [];

// HegelError: Type "Array<number>" is incompatible with type "Array<number | string>"
const numbersOrStrings: Array<string | number> = numbers;

The error is completely wrong and here is why. The type Array<number> means «an array which every element is a number». The type Array<string | number> means «an array which every element is a string or a number». The first type is assignable to the second because «a number» is «a string or a number». For example, array [1, 2, 3] matches the description «an array which every element is a string or a number». Thus numbers CAN be assigned to numbersOrStrings.

The second part of the example is correct. The example gives only 1 error in runtime (the one in the second part).

This lack of flexibility is useless and bring inconveniences.

srg-kostyrko commented 4 years ago

the error here is not about a type mismatch the problem with this example is that with such assignment we have 2 variables with different types pointing to the same array

as a result, it is easy to get a runtime error

const numbers: Array<number> = [];

const numbersOrStrings: Array<string | number> = numbers;

numbersOrStrings.push('a');

numbers.map(number => number.toFixed()) // TypeError: number.toFixed is not a function

Hegel tries to prevent runtime errors that is why here we get an error even though types are compatible.

If you create a copy of array on assignment - the error is gone

const numbersOrStrings: Array<string | number> = numbers.slice();
Finesse commented 4 years ago

Sounds reasonable, my bad.

JSMonk commented 4 years ago

Thank you for the issue ^_^

thecotne commented 4 years ago

@Finesse in some cases this type of assignment can be useful

for example i have function that will count sum of values in array and i want to support parsing numbers from strings for convenience reasons

function countSum(arr: Array<string | number>) {
  let sum = 0

  for (var i = 0; i < arr.length; i++) {
    const item = arr[i]
    if (typeof item === 'string') {
      const num = parseFloat(item)

      if (!isNaN(num)) {
        sum += num
      }
    } else if (typeof item === 'number') {
      sum += item
    }
  }

  return sum;
}

const numbers: Array<number> = [1,2,4]

const sum = countSum(numbers)// Type "Array<number>" is incompatible with type "Array<number | string>"

try

in this case there will be no problem in runtime and it may be confusing as to way you get this error and problem is as mentioned that Array is passed by reference and having mutable array meaning not $Immutable<Array<string | number>> means that countSum is allowed to add new items in array and if it did then you may end up with strings in numbers array

to fix this snippet you need to make arr in countSum Immutable like so

function countSum(arr: $Immutable<Array<string | number>>) {
  let sum = 0

  for (var i = 0; i < arr.length; i++) {
    const item = arr[i]
    if (typeof item === 'string') {
      const num = parseFloat(item)

      if (!isNaN(num)) {
        sum += num
      }
    } else if (typeof item === 'number') {
      sum += item
    }
  }

  return sum;
}

const numbers: Array<number> = [1,2,4]

const sum = countSum(numbers)// ok

try

Finesse commented 4 years ago

@thecotne This is pretty clear, thank you.

Just in case, there is a syntax in TypeScript for immutable arrays:

function countSum(arr: readonly <Array<string | number>>) {

It means that a value can't be add/changed/removed from the array through square brackets or the Array prototype methods. This syntax looks more pleasant than $Immutable<>.