JSMonk / hegel

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

Unknown shout be a generic type #206

Open artalar opened 4 years ago

artalar commented 4 years ago

tidr: for lazy type refinement we need a parameterized unknown

A problem: we can't define a expected type for future refinement:

const data: Array<number> | unknown = JSON.parse(some)

function head(list: Array<string> | unknown, fallback: ?string): string | undefined {
  if (list instanceof Array) return list[0]
  return fallback
}

// Expected: Type "Array<number>" is incompatible with type "Array<string>"
// Actual: OK!
const result = head(data)

If we define a head as function thats accept only Array<number> without unknown - we need to provide a type-garanted data for that function, thats may be an overhead for current case (walk of all array elements to refine it type).

I expecting thats unknown type may be a generic: unknown</* meta type: */Array<number>>.

As an option we could review a workaround: unknown union should not lose an other not unknown types. In other words the top ^ example just should work as a expected comment, but I think it is not intuitive because unknown as a part of union, technically, may be used for any refine, thats superfluously (не знаю как слова подобрать - кароч мы должны пресекать возможность передавать параметризированный unknown в функции принимающие простой unknown). May be, as a solution, we should use intersection for expected unknown: MyType & unknown, but I think container type is will be better: $Unknown<MyType>.

ikabirov commented 4 years ago

As i understood it should be something like this:

const data: $Unknown<Array<number>> = JSON.parse(some)

function head(list: $Unknown<Array<string>>, fallback: string = ''): string {
  if (typeof list == 'number') // Type $Unknown<Array<string>> can't be "number" type
  {
  }
  if (list instanceof Array) {
      // Expected: unknown
      const item = list[0] 
      if (typeof item == 'string')
      {
        return item
      }

  }
  return fallback
}

// Expected: Type "Array<number>" is incompatible with type "Array<string>"
// Actual: OK!
const result = head(data)
raveclassic commented 4 years ago

IMO T | unknown is just unknown. So both Array<string> | unknown and Array<number> | unknown are just unknown and therefore they are assignable to each other.

JSMonk commented 4 years ago

@raveclassic The behavior is needed for the try..catch statement. To show, that in catch may be placed as well unknown and your own defined errors from called functions.

raveclassic commented 4 years ago

How is that different? Looking at example from the docs:

function assertIsTrue(arg) {
    if (!arg) {
        throw new TypeError("arg is invalid")
    }
}
try {
    assertIsTrue(false);
} catch (e) {
    // Type of "e" variable is "TypeError | unknown"
}

it's not clear to me how should I work with e in the catch block and what advantages does | unknown provide in the type? In TS error is usually checked with:

if (e instanceof TypeError) {
  // e is `TypeError`
} else {
  // e is unknown
}

But it's absolutely the same for catch(e: unknown) without that TypeError | part. Keeping that in mind I would expect the error to be plain e: TypeError without unknown because assertIsTrue only throws TypeError. That would make sense.

JSMonk commented 4 years ago

The difference is in the fast understanding of which errors can be caught by the try..catch block. If you will see only unknown type, it gives you nothing, but, if you see that it will be UserNotFoundError | DataBaseError | unknown you can understand which errors you can catch, and find some problems related to that. As an example (UserNotFoundError | DataBaseError | unknown), you can understand, that you call method/function which throws low-level error DataBaseError, and you can find it and rethrow DataBaseError as other top-level error.

raveclassic commented 4 years ago

Seems we're talking about different things. I'm absolutely ok with declaring throwed errors in function signature (this reminds me checked exceptions in Java). What I don't get is:

Seems Hegel encodes unknown in a different way than TypeScript. If so, could you please clarify why is T | unknown not equal to unknown?

JSMonk commented 4 years ago

I understand your position and also I agree with that. (Actually unknown | T should be unknown). But I don't know which way is better for informative hints in try..catch block. If you have some advice or interesting solution for the case - I will the first one who change the semantic of union with unknown type 😄

raveclassic commented 4 years ago

That's a very good question. I'm pretty sure I don't see the whole picture but at least I'd suggest not using unknown in unions because it ultimately defeats the purpose of such unions for a person not familiar with these highly specific semantics.

Back to the original issue: if we both agree that adding unknown to a type (not in try/catch case) effectively turns such type to unknown, then I'd suggest using tuples+spreads (I don't know if Hegel supports them ATM):

declare const head: <A>(list: [A, ...unknown[]]) => A
artalar commented 4 years ago

Ok, here another case. You write a library for npm and type an argument of public function. You can't control a user env and don't know which type system they used (or no one at all). So u in same time want to describe a type of argument and what to mark it as unknown for internal contract provement. How to describe it better in code? May be we need a two kindest type: with two different signature, for publick and internal usage. Because if mark an argument of a function as generic unknown we tell a user of function that they may to provide anything by argument too, which is wrong.

I think we need a more investigation for this...

raveclassic commented 4 years ago

In TS I'd just declare it as unknown because it's the "smallest" possible type for a function argument, I mean when checking this argument in the body of that function. While also the "widest" type to pass to that function from outside. This reflects how outer-world interop can be encoded. Even if you add something more specific to the union you can't guarantee that in runtime the argument will adhere the type since Hegel is just a typechecker.

artalar commented 4 years ago

@raveclassic do you really think it is safety?? image

A type system, with this case, is useless and getting false garanties

raveclassic commented 4 years ago

I don't get the example. You declare a function which returns any (ehw), and then redefine that function. Could you clarify what do you mean?

raveclassic commented 4 years ago

Here's how I see it:

// this code requires runtime checks to guarantee that calls to `foreign` always return `number`
export declare const foreign: (data: string) => number;
export function process(): number {
    const foo = foreign('foo');
    const bar = foreign('bar');
    return foo + bar; // UNSAFE!
}

export declare const safeForeign: (data: string) => unknown;
// note the `| undefined` here
export function safeProcess(): number | undefined {
    const foo = safeForeign('foo');
    const bar = safeForeign('bar');
    // return foo + bar; // Error! Object is of type `unknown`

    // we need to check
    if (typeof foo === 'number' && typeof bar === 'number') {
        return foo + bar;
    }
    // here, foo and bar are still of type `unknown`
}