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

Support predicate types #34

Closed
sophiebits opened this issue Nov 18, 2014 · 67 comments
Closed

Support predicate types #34

sophiebits opened this issue Nov 18, 2014 · 67 comments

Comments

@sophiebits
Copy link
Contributor

Currently, if you do

if (typeof x === "function") {
  // ...
}

then Flow knows that the code within the if statement is a function. This doesn't work if you instead call a helper function like Underscore's _.isFunction. Similarly, utilities like Object.assign and React.PropTypes seem to have hardcoded behavior at the AST level, so they don't work when aliased or given alternative names.

Will it be possible to teach Flow about these sorts of functions so that it's possible to use these third-party libraries without changing code too much?

@gabelevi
Copy link
Contributor

Certainly we could hardcode this stuff in the typechecker, but I would love to come up with syntax to declare this kind of knowledge. We performed this kind of thought experiment with Hack without much success.

So for an isFunction(x) test, it would be great if we could declare that, if isFunction(x) returns true, then x is a function. Otherwise it is not a function. So maybe something like

declare function isFunction<T>(T x): true when T = function, false otherwise;

But that's kind of verbose. Maybe overloading can help?

declare function isFunction(x: function): true;
declare function isFunction(x: not function): false;

Ideally you'd also be able to declare when certain functions will throw, like in an assertFunction(x) function

declare function assertFunction<T extends function>(x: T): T;
declare function assertFunction(x: not function): explodes;

@sophiebits
Copy link
Contributor Author

I just make problems; I don't fix them. :) I agree this probably isn't super trivial to fix.

@c-spencer
Copy link

The way this is tackled in things like core.typed is through a shorthand syntax for predicates on types. For example (ann function? (Pred Function)) defines a boolean return type with type refinement implications for the function? declaration. The full annotation would be along the lines of:

(ann function? (All [x] (IFn [x -> Boolean
                                   :filters {:then (is Function 0) :else (! Function 0)}])))

Where 0 refers to the first argument (x), and specifies refinements in each branch based on the return value. I don't think you'd ever get annotations like this to be particularly clean, though making them available certainly reduces the amount of special casing/privileged forms.

(I'm not really sure how you'd translate this kind of syntax to Flow, though having functions from types-to-types would seem to be valuable, so that declare function isFunction<T>(T x): Predicate(function); could return the properly annotated Boolean return type)

@spion
Copy link

spion commented Nov 20, 2014

TypeScript developers and users are currently discussing a similar feature in issue 1007

@jareware
Copy link

On a related note, since:

typeof null === 'object'

constraining types with code such as:

if (typeof o === 'object') {
    o.someProperty;
}

is not sufficient, whereas a simple _.isObject(o) would be.

I was actually really surprised Flow didn't catch the null issue I had in code such as the above, and only found it during testing.

@leebyron
Copy link
Contributor

It looks like TypeScript ultimately decided on predicate function syntax, should flow try to use the same syntax?

function isB(p1: any): p1 is B;
(p1: any) => p1 is B;

Where the syntax in the return type could be Type | Predicate

Where Predicate := Identifier "is" Type

@samwgoldman
Copy link
Member

@leebyron Cool! Can you provide a link to this discussion on TypeScript's side of things?

It's possible that Flow might have different capabilities and constraints around refinements that could affect the syntax here. For example, refinements can be anded and ored together, and flow can refine specific keys within object types.

Also, it seems like a function would always carry a (maybe void) return type, and optionally a refinement. Not either/or. For example, _.isFunction(x) still returns a boolean, but if the return value is true, x is an array. If it's false, x is not an array, and flow can use that information, too.

@leebyron
Copy link
Contributor

@leebyron
Copy link
Contributor

TypeScript's behavior seems to be that functions which return these type predicate refinements actually always return booleans.

I would propose that within the function, flow actually checks to ensure any possible "true" return agrees with the annotated type refinement.

Example, this should typecheck:

function isFunction(value: any): value is Function {
  if (typeof value === 'function') {
    return true;
  }
  return false;
}

var value: any = () => {};
if (isFunction(value)) {
  value();
}

This should fail:

function isFunction(value: any): value is Function {
  if (typeof value === 'function') {
    return true;
  }
  // Error: isFunction must return boolean
}

var value: any = () => {};
if (isFunction(value)) {
  value();
}

And this should fail:

function isFunction(value: any): value is Function {
  return true; // flow doesn't think "value" is Function here.
}

var value: any = () => {};
if (isFunction(value)) {
  value();
}

@samwgoldman
Copy link
Member

Yeah, it seems like the x is y syntax is a boolean type with additional information (predicates that hold when the boolean is true).

This system is a good 80% solution, I think. We wouldn't be able to annotate the type of invariant, for example, because it returns void or blows up. I think that's OK.

I agree that flow should check that the return type is boolean, and it should also check that the truthiness of the returned value entails the correct refinement.

So, this would be bad:

function isFunction(x: any): x is Function {
  if (typeof value === 'function') {
    return false; // backwards
  }
  return true;
}

@leebyron
Copy link
Contributor

Totally, flow should catch that backwards predicate.

This system is a good 80% solution, I think. We wouldn't be able to annotate the type of invariant, for example, because it returns void or blows up. I think that's OK.

Yeah, I think this is okay too. Invariant does something a little bit different anyhow, it may make sense to be a different scope of work.

...
invariant(isFunction(value));
value();

flow should be able to type check this correctly as well.

@samwgoldman
Copy link
Member

Ah, it would also be very cool if we could infer a proof-carrying boolean. Consider this trivial example.

function isNull(val) { return val == null }

function safeFormatNumber(x: ?number) {
  if (isNull(x)) {
    return "";
  } else {
    return x.toFixed(2); // no error
  }
}

We didn't annotate isNull as carrying a refinement, but Flow could infer it. Maybe this is obvious behavior, but I don't think anyone has spelled it out yet :)

@leebyron
Copy link
Contributor

That's a great point. I imagine that would work similarly to flow's current return type annotation rules. If your isNull function is inline to the file, hopefully flow would just figure that out for you. If you export function isNull then you should be required to annotate.

@samwgoldman
Copy link
Member

I'm still interested in this feature, and it'd be great if @gabelevi could chime in about whether we want to move forward with this syntax. If we do, we'd need to build in support with Babel to strip the type. Currently x is y in the type position is a parse error.

@mroch mroch changed the title Interpret _.isFunction, $.extend, etc. more intelligently Support predicate types Jan 7, 2016
@kyldvs
Copy link

kyldvs commented Apr 20, 2016

It would be interesting if there were some way for the refinements to be associated across multiple functions. For example a has/get scenario:

class Wrapper<V> {
  ...
  get(): ?V {
    return this._v;
  }
  has(): boolean {
    return this._v != null;
  }
}
unwrap(wrapper: Wrapper<string>): string {
  return wrapper.has() ? wrapper.get() : 'default_value';
}

@nodkz
Copy link
Contributor

nodkz commented Aug 10, 2016

For me will be nice following syntax:

function isNeededThing(aaa: predicate): boolean {
  if(...) {
    aaa: SomeType = aaa;
    return true;
  }
  return false;
}

@nodkz
Copy link
Contributor

nodkz commented Aug 11, 2016

Right now at work and I can give more clarification.

More complex example:

function isNeededThing(testObject: some<T1 | T2 | T3>): boolean {
  if(...) {
    testObject: T2 = testObject;
    return true;
  } else if (...) {
    testObject: T1 = testObject;
    return true;
  }
  testObject: T3 = testObject;
}

if (isNeededThing(someObj)) {
   // here someObj may became one of the types  T1 | T2
} else {
   // here someObj became T3
}

I offer to add some keyword. It is new type likes mixed, but flow pointed that this type MAY be described below via self-assignment. Eg. testObject: T1 = testObject;, which have no sense for JS, and may be stripped out by optimizers.

In generally self-assignment gives the missing stuff. And it pins type of the variable from its definition to down in the scope.

And some keyword points to export derived type to upper scope.

Another example, only with self-assignment:

const callbackList: FunctionT[] = [];
if (isFunction(cb)) {
  cb: FunctionT = cb;
  callbackList.push(cb);
}

@Macil
Copy link
Contributor

Macil commented Aug 11, 2016

@nodkz If I'm understanding your examples right, then I don't think that idea adds anything that the x is Type-return type idea couldn't do. You could possibly write isNeededThing this way instead:

function isNeededThing(testObject: T1|T2|T3): testObject is T1|T2 {
  return testObject instanceof T1 || testObject instanceof T2;
}

if (isNeededThing(someObj)) {
   // here someObj is T1 | T2
} else {
   // here someObj is T3
}

Also consider that your way can't have the function be described from a declaration, unlike the x is Type-return type idea. There's not enough information here to be usable:

declare function isNeededThing(testObject: some<T1 | T2 | T3>): boolean;

@dralletje
Copy link

dralletje commented Aug 14, 2016

I really like this proposal, and read all the comments but may not understand everything, so apologies for stupid things in advance 😁

Best would ofcourse be to have it infer it automatically, but if that doesn't always work out (and for other things like exits or errors?), what about making special wrapper types for that that work with Tagged Unions?

declare function isObject<T>(value: T): $TypeCast<T: Object, true> | $TypeCast<T: any, false>;

where TypeCast's definition would be something like $TypeCast< (an actual type cast that is true when this type is returned ), (the type that it is bound to) >.
This would benefit even more from a negate-type, that would allow to communicate something like "If this type is returned, T is not an Object"

This would also make it (maybe something-ish) possible to have invariant working like

declare function invariant(predicate: $Throw<Error, false> | true): void;

although I am not sure if the $Throw in the arguments are clear enough that it could return it...
also would need a Truthy or Falsy type to fully describe invariant :-/

And process.exit would be

...
declare function exit(status: number): $Exit<void>;
...

EDIT:
Actually, Falsy is just type Falsy = '' | null | void | 0 | false; (Okay without NaN)

EDIT EDIT:
Invariant could also type it's effect in the return value using overloading

declare function invariant(predicate: Falsy, message: string): $Throw<Error, void>;
declare function invariant(predicate: any, message: string): void;

@apsavin
Copy link
Contributor

apsavin commented Jul 28, 2017

@loganfsmyth

If you want to use Flow, you need to make sure your code is written so that Flow can work on it.

Agree, but I think it's my responsibility to tell Flow developes about such cases (when I can't use Flow).

knowledge of an API

I can declare types for 3-rd party API and Flow will understand it perfectly

@vkurchatkin

How is Flow supposed to work with this?

Sure, there's no way right now. But, maybe, ideas like

Having flow consume an ESTree-compatible AST

can help us in future.

@noppa

Isn't this conversation deviating quite far from the purpose of this issue?

Totally agree. I reopened my original issue. Everyone who want to discuss - welcome to #4057

@philipp-sorin
Copy link

Any progress here?

@zaygraveyard
Copy link
Contributor

@philipp-sorin %checks is now supported in flow, see the documentation of functions.
But still no support for functions that throw like assert 😢

@futpib
Copy link

futpib commented Dec 13, 2017

@zaygraveyard this helps only with the most basic custom type predicates/refinement use-cases.

What I believe this issue is really about is implementing something like what is called user-defined type guards in typescript, that is a way to define a type predicate isFoo(x) which is trusted by flow to be true only when x is of type Foo, thus allowing flow to refine x to Foo type wherever isFoo(x) is true.

// Pretend this is an `opaque type Email = string` imported from another module
class Email {}

// Here you really want to tell flow that `isEmail(x) === true` if and only if
// `x` is of type `Email`, and this is not what `%checks` currently does
function isEmail(x: mixed): boolean %checks {
  return typeof x === 'string' && x.includes('@');
}

function spam(e: Email) {}

function main(s: string) {
  if (isEmail(s)) {
    spam(s);
    // ERROR: `string` type is incompatible with the expected param type of `Email`
  } else {
    throw new TypeError(`Invalid email: ${s}`);
  }
}

flow.org/try

EDIT:

No doubt this was already mentioned above, but this comment makes it seem like this issue has been finally addressed. I actually jumped to such conclusion after reading it and was then disillusioned, so I had to reply to at least make it clear to others that this is not really the case.

@StreetStrider
Copy link

@futpib if you briefly look through the thread above, you could see that this idea was already mentioned.

@lukeapage
Copy link
Contributor

It seems like the %checks doesn't work for libraries like lodash which was in the original case.

The flow-typed definition might look like this:

     isString(value: string): true;
    isString(value: any): false;

which looks like a strong enough definition for flow to infer, but it isn't.

I found the definitions for Array.isArray and they just define boolean -

static isArray(obj: any): bool;

and it looks like you special case it within the code.

Wouldn't it be possible to look at the typed value and then infer the types in an if so we could do

const s: string | number = '1';
if (_.isString(s)) {
     // s is a string
}

without needing any syntax changes?

@benadamstyles
Copy link

@lukeapage See #4196 for why true | false is currently incompatible with boolean.

@StreetStrider
Copy link

StreetStrider commented Jun 4, 2018

@lukeapage your

  isString(value: string): true;
  isString(value: any): false;

looks lovely. It's like another way to say isString(value: any): value is String without involving additional syntax, like TS do. That's interesting.

@sibelius
Copy link

what is the right way of doing this on Flow?

@gajus
Copy link

gajus commented Jul 14, 2018

what is the right way of doing this on Flow?

There is no current way to write % asserts conditions.

Regarding non-throwing predicate functions, refer to %checks in the https://flow.org/en/docs/types/functions/ documentation.

@vicapow
Copy link
Contributor

vicapow commented Feb 1, 2019

Going through some issue backlogs so I apologies if I'm closing this issue prematurely but I believe at least the original issue has been addressed. Specifically,

function isFunction(a): boolean %checks {
  return typeof a === 'function';
}

function concat(a: (() => null) | number) {
  if (isFunction(a)) {
    a();
  }
}

@vicapow vicapow closed this as completed Feb 1, 2019
@bluepnume
Copy link

bluepnume commented Feb 1, 2019

Not really fixed, because it's still impossible to write meaningful type-checking functions that don't or can't use typeof or instanceof:

function isRegex(thing : mixed): boolean %checks {
  return toString.call(thing) === '[object RegExp]';
}

function match(regex : mixed, str : string) {
  if (isRegex(regex)) {
    regex.test(str);
  }
}
7:     regex.test(str);
             ^ Cannot call `regex.test` because property `test` is missing in mixed [1].
References:
5: function match(regex : mixed, str : string) {
                          ^ [1]

This encourages instanceof to be overused, which is often the wrong choice.

Here's a real-world example from React: https://overreacted.io/how-does-react-tell-a-class-from-a-function/

One caveat to the instanceof solution is that it doesn’t work when there are multiple copies of React on the page, and the component we’re checking inherits from another React copy’s React.Component.

Ideally flow should have a way to type a isReactComponent function.

@StreetStrider
Copy link

@vicapow if such a function's body may contain only single expression and must be based on typeof that is not a resolution. The whole purpose of this feature is to indicate to Flow your specific type approaches, they may not rely on prototype chain or primitives.

Like, for instance, I'm using Symbols for my types and they often have MyType.is(value: any): boolean which does the check, and not relying on typeof or instanceof.

Other examples by @bluepnume in topic above.

@TrySound
Copy link
Contributor

TrySound commented Feb 1, 2019

Agree, %checks is half working solution.

@TrySound TrySound reopened this Feb 1, 2019
@montogeek
Copy link
Contributor

It would be great to see this implemented, hopefully we can get it soon.

@goodmind
Copy link
Contributor

This also would allow to remove hardcoded invariant

declare function invariant(cond: boolean, message: string): empty %asserts(cond)

/cc @panagosg7

@leebyron
Copy link
Contributor

Just to continue documenting TypeScript's progress on this front, the recent release (3.7) includes an extension to the syntax they added in 2015 to include throwing functions like assertIsString() which is really similar to @gabelevi's suggested "explodes" idea in one of the first comments in this issue.

microsoft/TypeScript#32695

I'd still love to see both features in Flow!

@gajus
Copy link

gajus commented Nov 28, 2019

Would be nice to see this prioritised.

@SamChou19815
Copy link
Contributor

We now have type guards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests