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

No apparent way to prevent redundant type checking against already-narrowed types? #15211

Closed
axefrog opened this issue Apr 16, 2017 · 13 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@axefrog
Copy link

axefrog commented Apr 16, 2017

TypeScript Version: 2.3.0-rc

I'm doing a whole lot of refactoring at the moment and going around trying to find places where, due to some structural changes in my application, I no longer need to be calling my isNotNull function, which I use to narrow the type to a non-null value. The narrowing part is fine, but I'd really like to only be allowed to call it if the argument could at least possibly be a nullable type, according to the type system. If the type system already knows the value is not null, I'd like it to prevent me from passing it to the type-narrowing function in the first place.

Code

// as per the return value, the compiler knows this function is designed to narrow the type:
function isNotNull<T>(value: T|null): value is T {
  return value !== null;
}

const x: string|null = getSomeStringOrNull();

// it's ok to try and narrow the type here because the compiler can't guarantee it's not null yet:
if(isNotNull(x)) {
  if(isNotNull(x)) { // how can I make this call a compile-time error?
    // ...
  }
}

// or an alternate example:

function printNumber(n: number): void {
  console.log(n);
}

const x = 3;

// I want an error or warning below. It's pointless narrowing an already-narrowed type:
if(isNotNull(x)) {
  printNumber(3);
}

Because this redundancy is apparently invisible to the compiler, I can't simply work through the error list to fix the places where it matters, and instead need to look through all calls to the function and manually analyse the surrounding code to determine if I still need the check.

@aluanhaddad
Copy link
Contributor

@axefrog Actually there is a way to do this. It is a bit of a hack but it works.

function isNotNull(value: {}): value is never;
function isNotNull<T>(value: T | null): value is T;
function isNotNull<T>(value: T | null): value is T {
  // tslint:disable-next-line:no-null-keyword
  return value !== null;
}

declare const value: string | null;

if (isNotNull(value)) {
  if (isNotNull(value)) {
    // error on usage of value.
  }
}

It is not exactly the same as receiving an error on the second call but practically it comes close.

@axefrog
Copy link
Author

axefrog commented Apr 16, 2017

@aluanhaddad Heh, thanks, I'll give that a go. Might make a suitable stopgap solution in the mean time.

@axefrog
Copy link
Author

axefrog commented Apr 17, 2017

Doesn't quite work in this example:

export function isNotNull(value: {}): value is never;
export function isNotNull<T>(value: T|null): value is T;
export function isNotNull<T>(value: T|null): value is T {
  return value !== null;
}

function printNumber(n: number): void {
  console.log(n);
}

const n = Math.random();
if(isNotNull(n)) {
  printNumber(n); // no error - apparently it's ok to use `never` in this way.
}

@axefrog
Copy link
Author

axefrog commented Apr 17, 2017

Related?: #15211

@axefrog
Copy link
Author

axefrog commented Apr 17, 2017

Update: Partial/interim workaround is to enable noImplicitAny. It doesn't solve the problem directly, but it can generally be inferred that anywhere that, due to refactored code, I'm redundantly checking for null, I'm probably going to be doing something with the narrowed type that contradicts the type's nature, and in doing so, causing the compiler to treat the behavior as implicit any, which will cause an error if the flag is enabled, thus highlighting at least some of the redundancies I wanted to eliminate.

@mhegazy mhegazy added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Apr 17, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Apr 17, 2017

Do not think there is a way to make this work. in a sense you want to specify an overload and then make it an error. the language has no constructs to do such a thing.

@axefrog
Copy link
Author

axefrog commented Apr 17, 2017

@mhegazy I don't want to specify an overload; not sure if you were responding to @aluanhaddad 's workaround or the question that preceded it? (the latter being what I was actually asking).

To recap, I'd like the compiler to recognise the redundancy of passing a value to a type-narrowing function where the narrowed type is already of the type being narrowed to.

// as per the return value, the compiler knows this function is designed to narrow the type:
function isNotNull<T>(value: T|null): value is T {
  return value !== null;
}

function printNumber(n: number): void {
  console.log(n);
}

const x = 3;

// I want an error or warning below. It's pointless narrowing an already-narrowed type:
if(isNotNull(x)) {
  printNumber(3);
}

Not sure how that relates to overloading?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 17, 2017

but you still want isNotNull to work on null | number, since number is a subtype of null| number, the only way i can think of "chose" one is overloads.

@fatcerberus
Copy link

I'm not too familiar with TypeScript internals, but from what I can see the compiler knows, when it encounters a call to isNotNull(), both the type being narrowed to (number), and the actual type of the argument passed in. So if the argument is known, at the call site, to be the same type as the target type (known-number as opposed to number | null), then the compiler could just generate a warning in that case, right? No overload logic needed.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 24, 2017

The isNotNull is just a function call. the compiler knows it does something extra, i.e. removing the null from the type of the input it it exists. not sure why this would be a warning, and other calls are not.

@axefrog
Copy link
Author

axefrog commented Apr 24, 2017

@mhegazy Could you clarify why you're not sure? As per the original question and what @fatcerberus said, the warning specifically relates to redundancy. The compiler can see that the return type is arg is number and has already resolved the type being passed in as having already been narrowed to the specific type that the function narrows to. If it were a different type being passed in, or a wider type, or the function is not a narrowing function, then the warning would of course not apply. The point is that TypeScript offers type narrowing through a very specific function syntax, and given that it's operating in the context of a statically-analysed input type, it stands to reason that a simple comparison between the function arg is whatever return type and the type of the passed-in argument could easily see that the type is already identical, and that, for a project written from the ground up in TypeScript, that the type check is redundant, given the surrounding context. These are the kinds of things that make TypeScript great, rather than just writing plain old JavaScript.

@mhegazy
Copy link
Contributor

mhegazy commented May 1, 2017

Looks like this the same request as #14977

@mhegazy mhegazy added Duplicate An existing issue was already created and removed Design Limitation Constraints of the existing architecture prevent this from being fixed labels May 1, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 19, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed May 19, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants