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

Expanded type guard suggestions #4868

Closed
RyanCavanaugh opened this issue Sep 18, 2015 · 29 comments
Closed

Expanded type guard suggestions #4868

RyanCavanaugh opened this issue Sep 18, 2015 · 29 comments
Labels
Effort: Difficult Good luck. Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

Potential "safe" type guards based on in-office discussion:

  • When guarded by typeof x === 'function', remove any types from the union that do not have call or construct signatures
  • When guarded by typeof x === 'object', remove all primitives from the union, as well as any types that have any call or construct signatures
@kitsonk
Copy link
Contributor

kitsonk commented Sep 19, 2015

Do you also need to account for:

  • When guarded by typeof x === 'symbol'

Also according to the spec, typeof functions only need to implement [[Call]] so something that has a construct but not [[Call]] would not be a function.

@rotemdan
Copy link

@RyanCavanaugh

In cases where the target type is not a is not a weak type, e.g. {} or { prop1?, prop2?, ..}, this is not a logically sound inference at runtime:

interface SomeType {
    func(arg: number): string;
}

let x: SomeType | number;

...

if (typeof x === 'object') {

    // Although 'x' can safely be deduced not to be a 'number' here, and to be a non-primitive
    // object capable of having properties, there is no guarantee that it has a property 'func' 
    // with a function type having an argument of type 'number' and return type 'string.
    x.func(99); // No guarantee this wouldn't throw an exception
}

This problem also occurs in the more general case when using input type constrained user-defined guards for type deductions (though it was more magnified at that particular case):

interface A {
    f1(): number;
}

interface B extends A {
    f2(): string;
}

function isA(obj: any): obj is A {
    return typeof obj === "object" && typeof obj.a === "function";
}

// This can also be stated in a class context, e.g. isB(obj:A): this is B
function isB(obj: A): obj is B { 
    return typeof obj.b === "function";
}

let obj: B;
...

if (isB(obj)) {
    obj.f1() // No guarantee this wouldn't throw an exception
}

To attain true logical soundness at runtime, more complex guard compositions would be needed:

function isTrulyB(obj: any) obj is B {
    return isA(obj) && isB(obj); // isA is not input type constrained so this is logically sound.
}

In the case of typeof obj === 'object' for a union type, there is no way to ensure run-time soundness in general (unless it is combined with a specific user-defined guard for the expected type of obj, but that would remove the need for the original deduction) .

I don't mean by this that achieving this level of soundness is absolutely necessary or must have been the design goal (though I would have personally strongly supported that). However, if guards are extended to be used automated tools like a run-time argument type validators or used in similar ways with hand-made decorators, these compositions would be strictly necessary (and unfortunately, not always possible if there is no way to extend a chain to a guard with an any input type).


What will this yield when x is of type any?

let x: any;
...
if (typeof x === 'object') {
  ...
}

Any plans on introducing the 'runtime' object type?

@rotemdan
Copy link

@RyanCavanaugh

I've thought more deeply about this issue so I think I should expand on this matter:

I find it generally reasonable, at places, to make compromises and accept compile-time assumptions on types as that is in the general spirit of TypeScript. However the point of guards is to provide run-time assertions on types. So if initial types are allowed to be assumed for guards at compile-time, then what is the need for guards in the first place? why not just accept arbitrary user assertions like:

let x: SomeType | string;

if (assert(x has SomeType)) {
   ...
}

Obviously that wouldn't be useful, because it doesn't provide any run-time guarantees..

But how is this different from?

function isB(obj: A): obj is B { 
    ..
}

This assumes obj is known to be A, which is a compile-time assumption for the context of a run-time assertion.

If this is a 'compromise' (with what?), then it doesn't seem justified in any way, including not on practical reasons, what it does in practice is encourage users to define weaker, and less sound implementation of guards, which is a bad practice in general. The solution is very simple, and even easier to implement than the original proposal:

Disallow input type constrained guards (or more precisely 'guards with compile-type assumptions for input types'),

function isB(obj: A): obj is B { // Error: this guard doesn't guarantee run-time soundness
    ...
}

function isB(obj: any): obj is B { // OK
    ...
}

and get run-time soundness. Simple as that!

@rotemdan
Copy link

@RyanCavanaugh

To address the original scenario of:

type SomeObjectType = { a: string; b: boolean }

let x: number | SomeObjectType;

...

if (typeof x === "object") { // Should this guard narrow the type of 'x' to 'SomeObjectType'?
    ...
}

I would say the following:

Narrowing the type here is somewhat reasonable at compile-time, as the guard does provide a sound negative assertion of x not being a number, however it doesn't provide a sound positive assertion of x being SomeObjectType. It could as well be a completely different type, altogether:

type SomeObjectType = { a: string; b: boolean }

let x: number | SomeObjectType;

x = callSomeUnsafeFunction(); // returned value { d: [1,2,3,4], e: "Hello World!" }

if (typeof x === "object") { // Should this guard narrow the type to 'SomeObjectType'?
    ...
}

So this narrowing, although somewhat reasonable at compile-time, would be an exception to existing built in guards, that it applies a limited run-time check and then uses the result in conjuction with a compile-time assumption to derive another compile-time assumption, rather than soundly inferring an actual type.

Is it better than none at all? I'm not sure, the problem with this is that it like user-defined guards with compile-time assumptions on input types, it encourages "cutting-edges" in runtime type checking, so the type would be narrowed and that would lower the motivation of the programmer to apply an "actual", sound, guard (why apply a guard if the type has already been narrowed?).

It is very similar to the following user defined guard:

type A = { a: string; b: boolean };

function isA(x: number | A): x is A {
    return typeof x === "object";
}

Which in terms of run-time soundness would probably be described as, well, "poor".

So, is this reasonable at compile-time? possibly yes.. Conservative?.. no. Encourages good practices?.. probably not..

@DanielRosenwasser
Copy link
Member

Narrowing the type here is somewhat reasonable at compile-time, as the guard does provide a sound negative assertion of x not being a number, however it doesn't provide a sound positive assertion of x being SomeObjectType. It could as well be a completely different type, altogether:

@rotemdan you can say this about any portion of the type system.

var x: {a: { b: number } } = {a: { b: 100 } };
var y: any = "bad";
x = y;
x.a.b += 100;

x's static type can be incorrect at any point in the program.

The purpose of type guards is to narrow with respect to what we already know about the type. If that assumption is violated (i.e. if someone "lied" or made an error with a type annotation) then there's nothing we can do. That's always a possibility throughout the type system, but we're not going to error at any property access with a message like:

foo.ts(1,6): error TS1234: Property access not safe. Static type may not match dynamic type.

@rotemdan
Copy link

@DanielRosenwasser

My comments were directed to several subjects, one was about the typeof x === 'object' guard, which I mentioned I did not find strongly objectionable at compile time, but somewhat problematic at run-time.

The other one was a strong criticism of user-defined guards with compile-time assumptions for input types, which differ by being primarily run-time instruments. So I will concentrate on this subject now, so now please comment on this statement of my view on them (and please read previous ones as well):

I understand the general design approach, but these weak and unsound assumptions are not particularly useful when writing guards destined for real world applications. I would personally never recommend anyone writing:

function isB(x: A): x is B {
  ..
}

function example(a: A) {
   if (isB(a)) {
   ...
   }
}

The entry to the function example does not provide any way to guarantee that argument a is of type A, so usage of a guard of the kind of isB, which relies on a very weak compile-time assumption, is very questionable and should probably be prohibited in general.

@DanielRosenwasser
Copy link
Member

The entry to the function example does not provide any way to guarantee that argument a is of type A

That's no different than the example I just gave. These are pathological cases where the user usually has to really try to pass in incorrect data, or is being called by an external library, in which case, bets are off anyway.

I understand the general design approach, but these [...] are not particularly useful when writing guards destined for real world applications.

I disagree. I think real world users have a general intent that they are interested in expressing. Specifically, I suggest you look at No. 3 on TypeScript's non-goals:

  1. Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.

@rotemdan
Copy link

@DanielRosenwasser

There is a difference between:

  1. Not guaranteeing run-time soundness for types.
  2. Intentionally encouraging programmers to apply unsound assumptions at run-time assertions and thus reduce the safety of running code.

It's like I'm telling some TS dev:

"Hi, it's nice there are guards that give compile-time soundness, but you can also ensure run-time soundness for those guards if only you required all user-defined guards to have input type 'any'. Really, that's all you need to do :)",

And then they respond with "NO.", "TOO COMPLEX", "OUT OF SCOPE", "DECLINED". "PLEASE READ OUR RULE BOOK. WE DONT CARE ABOUT RUNTIME TYPE SAFETY SO WE INTENTIONALLY DEGRADE IT. GO AWAY".

The magic of TypeScript discussions... I'm starting to get used to this.. and that's sad :(

@rotemdan
Copy link

@DanielRosenwasser

If the goal was to provide some convenient way of 'nesting', or reusing assertions:

function isA(x: any): x is A {
 ...
}

function isB(x: A): x is B {
 ...
}

if (isA(x)) {
 ...
  if (isB(x)) {
  }
}

Then the same can be easily achieved with plain-old structured programming (and provide even better reuse as the dependencies are embedded into the guard(s) so only need to be stated once):

function isA(x: any): x is A {
 ...
}

function isB(x: any): x is B {
  if (!isA(x))
    return false;
 ... 
}

if (isA(x)) { 
  ...
}
...
if (isB(x)) {
  ...
}

I guess now the TS team will frown upon me that this does not maximize run-time performance! :) and the rule-book says that TS should always sacrifice safety for performance :) (there are probably also ways to conditionally disable the check in isB(), using some sort of additional function argument, assuming that really has any impact for performance).

@rotemdan
Copy link

@DanielRosenwasser

I think there needs to be a conceptual separation here:

  1. Built-in, or implicit guards: Detect existing run-time checks and use them to indirectly, or implicitly, narrow or infer static types at compile-time.
  2. User-defined, or explicit guards: Allow the programmer to define custom run-time assertions on types of values, and explicitly mark them as acting as positive affirmations for a particular type.

In 1. There is no evidence of an intention of the programmer of using the guard as a way to perform run-time validation for any particular type, as they never explicitly stated that effect. So as I see it, a narrowing such as proposed for typeof x === "object" is reasonable (though not very conservative).

In 2. The user explicitly states the assertion would verify that particular type at run-time, e.g. x is A. That is the prompt intention of the programmer here, and should be taken at full seriousness. Again take note, this is a run-time check, not just a compile-time assertion. There are no reasonable justifications at allowing "partial" or "speculative" tests here. If that is needed, as I've demonstrated, these guards can internally call other guards, which would also improve code reuse in general.

@rotemdan
Copy link

@jbondc

You're right (though I admit this looks like a very strange design decision, since all types are nullable - not just the portion included in object - but this kind of weirdness is very common in Javascript). However, that is not a necessarily an issue for the guard (though the reasoning involves an additional assumption based on a heuristic):

x: number | { a: string }
...

x = null;

...

if (typeof x === "object") {
  ...
}

If the value of x is null, the executing code will pass the guard, and now Typescript would narrow the type to { a: string }. So the question needs to be asked: what was the intention of programmer here? because if the value was an actual number, then it would have never passed the guard. It would only have passed if it was an object, or null.

So unless the programmer used that runtime check in a very strange way, say, to only check if the value was null (assuming somehow they knew it was not an object) - there aren't really any other options left - the test was done to check if it was a runtime object of some sort, so the compiler can reasonably narrow its type assumption.

If the guard was user-defined, it is up to the user to explicitly state if the type accepts null objects or properties:

function isSomeObj(x: any): x is { a: string } {
    return x !== null && typeof x === "object" && typeof x.a === "string";
}

@rotemdan
Copy link

I mentioned that for weak types:

let x: number | { a?: number, b?: string ...}, 

if (typeof x === "object") { // narrows to '{ a?: number, b?: string ...}'
    ...
}

The guard is completely sound at runtime in this case (this assumes that weak types are not assignable from primitives here, so would require #3842 - [Edit: I'm rethinking this, I might not have been correct here, perhaps that is not needed]). But now I notice another subtlety. If the narrowing results in a union of several types:

let x: number | Date | RegExp

if (typeof x === "object") { // narrows to 'Date | RegExp'
    ...
}

then the guard can be said to be effectively sound at runtime, because the programmer still cannot perform any potentially unsafe operation on 'x' (i.e. an operation that would presuppose the unsound compile-time assumption of the type being any specific member of the union) before further disambiguating between those two remaining types (and hopefully that would be done with sound guards), and all types that have been eliminated from the union were removed by a sound negative assertion.

@rotemdan
Copy link

So the problem seems to be isolated to cases where there is only one remaining type, thus it is possible to introduce a more "conservative" variant, that would not resolve the guard in this case, or alternatively resolve it to something like RemainingType | object, that would effectively force the user to apply a "real" guard to disambiguate it further (note this would require introducing the runtime object type). I think this could be a reasonable solution.

This would render the guard effectively sound at run-time, for all cases.

(Discussion about user-defined guards is continued at #4898)

@RyanCavanaugh
Copy link
Member Author

@rotemdan

that would effectively force the user to apply a "real" guard to disambiguate it further

Available data here says this isn't something most programmers want. The relevant issue is #1719 -- we looked at how people treat runtime checks using instanceof, and the data says that people treat checks as being exhaustive.

If you can find data to the contrary, that would be very informative. Remember that TypeScript's goal is to apply static typing to JavaScript code, not to rewrite JavaScript code to conform to a sound runtime-enforced type system.

@rotemdan
Copy link

@jbondc

I'm not sure exactly what {object: 1} means. It doesn't compile as 1 is not a valid type for the property named object in the anonymous interface?

In any case, whenever the typeof o == "object" check is successfully passed, the value can either be a null or a run-time object type, so it is reasonable to narrow the type based on the assumption that the programmer didn't use the check only as a null-check and then, say, intends to assign a number to it. If that's the intention of the programmer, then the compiler would simply disallow that at compile-time, essentially enforcing a better coding style (somewhat like a linter would do).

@rotemdan
Copy link

@RyanCavanaugh

In light of the discussions at #4898, which started to reassure me that TS team members do actually care about run-time soundness in guards. I'm interested in hearing your detailed view on the particular suggestion of narrowing to RemainingType | object when there is only one remaining type, to ensure effective run-time soundness. If that (or a similar solution) would not be implemented as a behavior of the guard, this is going to be the first built-in guard (at least that I know of) that doesn't provide reliable results at run-time, and that would be unfortunate.

@rotemdan
Copy link

@jbondc

This is a programming language, not an application or a library. There should be a clear separation between design issues and implementation issues. The implications, use cases, theoretical aspects and problems of a particular feature should be investigated before it is put to code. If a feature ends up as too difficult to implement with reasonable standards, then it might be better off not to implement it all.

If it is found out that that a feature may be more accurate or reliable when particular aspects of it are modified, then that should be considered with all seriousness. Having run-time soundness for a guard is no joke material, and I would personally consider it an essential feature. If anyone finds the idea that a run-time check should have run-time correctness silly or amusing, then they may have lost connection to the necessities of real-world coding or became too absorbed with the fragile static type system TypeScript provides, that although does a great job at compile-time, is mostly out of context at run-time.

I've spent a lot of time trying to analyze and formulate the minimum amount of work that would be needed to get an effective level of run-time soundness for this particular guard (this includes implementation effort). It turned out that it was only needed to reduce a union to RemainingType | object when there is one remaining type to achieve that major effect, which would also nicely capture the semantic effect of the guard. It may be possible there are cases I've missed or an alternative solution. That's what this discussion is for.

@rotemdan
Copy link

@jbondc

Thanks for the link, though this feature is particularly designated to be a run-time type check, so validity at run-time is a naturally an important part of it. It is not about achieving soundness in general - only assuring some level of it through the guard (to be honest I'm not that interested in adding run-time checking to TypeScript in general, with the possible exception of argument types).

I'm also interested in others' views on the matter. My views are based on what I know and investigated. Without actual participation, it is difficult to yield fruitful effects and to enrich our understanding of the subject.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Effort: Difficult Good luck. and removed In Discussion Not yet reached consensus labels Oct 5, 2015
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 5, 2015
@RyanCavanaugh
Copy link
Member Author

If anyone's still here, we'd happily accept a PR for the type guards mentioned in the OP.

@cjbarre
Copy link

cjbarre commented Oct 25, 2015

@RyanCavanaugh I'm going to attempt this one.

@zpdDG4gta8XKpMCd
Copy link

sorry for being noisy, just curious, is a type guard something with a clean interface that can be written and tested separately or it's something baked in the code in many different places which cannot be easily abstracted/isolated?

@DanielRosenwasser
Copy link
Member

@Aleksey-Bykov what do you mean specifically? Like in the compiler itself?

@zpdDG4gta8XKpMCd
Copy link

Yes, what you call a type guard in the compiler, is it something with an interface that can implemented outside of the TypeScript code (provided all *.d.ts) and later plugged into some type guard registry of some sort to be immediately available for use.

@DanielRosenwasser
Copy link
Member

instanceof type guards are based on the type of both sides of the expression. User-defined type-guards are based on whatever the resolved signature of the called function is.

typeof type guards are hard-coded because they originally were there only to support primitives, and primitives themselves are hard-coded.

@dead-claudia
Copy link

By the way, I would like to note a few things:

  1. typeof x === "object" should narrow x to an object or literally null (the latter part I think was missed).
  2. typeof x === "undefined" should only invalidate x as undefined.
  3. x != null should make x non-nullable (c.f. Non-nullable types #7140).
  4. a(x) && b(x), where a(x): x is A and b(x): x is B, should constrain x to A & B.
  5. instanceof should not narrow a type, because of ES6's Symbol.hasInstance. An explicit cast should be required.

@RyanCavanaugh
Copy link
Member Author

@isiahmeadows regarding point 5 - why? This would break a huge amount of code in exchange for what?

@DanielRosenwasser
Copy link
Member

There's no need to break instanceof unless the Symbol.hasInstance method has been sabotaged.
Though internally, instanceof type guards could be dependent on a Symbol.hasInstance method if present:

class C {
    static [Symbol.hasInstance](x): x is C {
        return x instanceof C;
    }
}

var x: {};

if (C[Symbol.hasInstance](x)) {
    x // 'x' already has type 'C' here today.
}

@dead-claudia
Copy link

@DanielRosenwasser I wasn't certain how feasible that would be, or I would've probably said it.

@ahejlsberg
Copy link
Member

#7140 and #8010 implement the original suggestion in this thread plus numerous other improvements to type guards so I'm going to close this issue. Let's create new issues for remaining suggestions, if any.

@ahejlsberg ahejlsberg modified the milestones: TypeScript 2.0, Community Apr 26, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Apr 26, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Effort: Difficult Good luck. Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants