hegel icon indicating copy to clipboard operation
hegel copied to clipboard

Inference from pattern matching

Open mindplay-dk opened this issue 4 years ago • 7 comments

Would it possible (and would it make sense) to add inference for pattern matching?

For example:

function update(state, action) {
  switch (action.type) {
    case "INCREMENT":
      return state + 1;
    case "DECREMENT":
      return state - 1;
    default:
      return state;
  }
}

let nextState = update(0, { type: "INCREMENT" });

Here, it's clear from reading the code that action.type has a defined set of expected values.

Hegel does infer from usage that action must have a type property.

But it does not infer that the type is a string - although it's clear to a person familiar with pattern matching that, not only is a string property expected, but there is also an expected set of constant string types.

I'm sure this is non-trivial, but I'm wondering if it's even possible and whether it would make sense? 🙂

mindplay-dk avatar Aug 09 '21 11:08 mindplay-dk

type for action.type is 'INCREMENT' | 'DECREMENT' and also it can be anything? since there is default: so correct type for action.type would be 'INCREMENT' | 'DECREMENT' | unknown which is same as unknown.

also this snippet fails to type check and produces this <_b: { 'type': _b0, ... }>(number, _b) => number as type for update function and _b0 is something that can't be satisfied when calling this function.

thecotne avatar Aug 09 '21 17:08 thecotne

type for action.type is 'INCREMENT' | 'DECREMENT' and also it can be anything? since there is default: so correct type for action.type would be 'INCREMENT' | 'DECREMENT' | unknown which is same as unknown.

It says default: return state - so it should be 'INCREMENT' | 'DECREMENT', but it's not.

also this snippet fails to type check

That's the point if this issue. It's not clear to me if you're debating or elaborating? 🙂

Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want action.type constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?

mindplay-dk avatar Aug 09 '21 18:08 mindplay-dk

That's the point if this issue. It's not clear to me if you're debating or elaborating? 🙂

elaborating

your issue seems to be about inferring type of action.type to union of string literals but that does not mean it's failing to type check in general

for example if action.type would be inferred to be unknown it would type check but not in a way that you wanted

It says default: return state - so it should be 'INCREMENT' | 'DECREMENT', but it's not.

so why do you have default case if only INCREMENT and DECREMENT is allowed?

Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want action.type constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?

that function can handle any object as action even one without type property since there is default: return state

so constraining action.type to something that would make default: return state unreachable branch would not make sense

thecotne avatar Aug 09 '21 18:08 thecotne

It says default: return state - so it should be 'INCREMENT' | 'DECREMENT', but it's not.

so why do you have default case if only INCREMENT and DECREMENT is allowed?

Because JavaScript? Type-checking is compile-time, so somebody will always be able to pass anything.

Inference from usage seems to be about "what could the developer possibly mean" - and in this particular case, it seems less likely you'd want action.type constrained to a type that the function can actually handle. Otherwise, you'd have declared a (wider) type, right?

that function can handle any object as action even one without type property since there is default: return state

so constraining action.type to something that would make default: return state unreachable branch would not make sense

But there's no such thing as an exhaustive switch/case in JS - so no matter what I do, handling default or not, you can always pass anything to this function. Even if for default there was a throw statement, it's still a branch, there's still a control flow, and therefore a type, in this case just an exception.

Reducer patterns like these are super common in JS now - is there some way we can handle them?

mindplay-dk avatar Aug 09 '21 19:08 mindplay-dk

Hey, @mindplay-dk. Thank you a lot for the proposal. Have an additional question: how in your case should we handle the next code:

function update(state, action) {
  switch (action.type) {
    case "INCREMENT":
      return state + 1;
    case "DECREMENT":
      return state - 1;
    // case "VALID_BUT_NOT_HANDLED_ACTION":
    default:
      return state;
  }
}

let nextState = update(0, { type: "VALID_BUT_NOT_HANDLED_ACTION" }); // should be a type error here?

Seems like, if a developer will have same logic for some action and default cases then it needs to add extra runtime code for the valid type inference, so we need a more complex heuristic for this type of inference 😞

JSMonk avatar Aug 10 '21 09:08 JSMonk

Yeah, default will make the function accept anything.

How about this then?

function update(state, action) {
  switch (action.type) {
    case "INCREMENT":
      return state + 1;
    case "DECREMENT":
      return state - 1;
    default:
      throw new Error(`unsupported action: ${action.type}`);
  }
}

Or just this:

function update(state, action) {
  switch (action.type) {
    case "INCREMENT":
      return state + 1;
    case "DECREMENT":
      return state - 1;
  }

  throw new Error(`unsupported action: ${action.type}`);
}

Any case not covered by the switch will generate an exception - effectively removing the possibility of passing any values/types other than those explicitly handled. Would that work?

mindplay-dk avatar Aug 15 '21 14:08 mindplay-dk

@mindplay-dk that sounds like a good idea

and would imply that this code

function concat(a, b) {
  if (typeof a !== "string") throw new TypeError("a must be a string!");
  if (typeof b !== "string") throw new TypeError("b must be a string!");

  return a + b;
}

will produce this interface

(string, string) => string

instead of this one

(unknown, unknown) => string throws TypeError

which would be very good for inferring types of js code instead of using poorly implemented .d.ts files from DefinitelyTyped

thecotne avatar Aug 15 '21 16:08 thecotne