typeguards only narrow types when used with unary not

[ssube]

Bug Report

🔎 Search Terms

• negation
• typeguard
• unary not

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about narrowing/typeguards

The FAQ entry with the most promising title is a TODO: https://github.com/Microsoft/TypeScript/wiki/FAQ#why-doesnt-isfoox-narrow-x-to-foo-when-isfoo-is-a-type-guard

This occurs on every version of Typescript available in the playground, including nightly.

⏯ Playground Link

Playground Link

💻 Code

interface Entity {
  type: string;
}

const ACTOR_TYPE = 'actor';

interface Actor extends Entity {
  type: typeof ACTOR_TYPE;
}

function isActor(entity: Entity): entity is Actor {
  return entity.type === ACTOR_TYPE;
}

const foo: Actor = {type: 'actor'}; // valid, as expected
const bar: Actor = {type: 'not-that'}; // invalid, as expected
const bin: Entity = {type: 'item'}; // valid, not an actor

if (isActor(bin) === false) {
  typeof bin; // is Entity, as expected
} else {
  typeof bin; // is Entity, not as expected
}

if (isActor(bin) == false) {
  typeof bin;
} else {
  typeof bin; // unexpected Entity, same as above
}

if (!isActor(bin)) {
  typeof bin; // is Entity, as expected
} else {
  typeof bin; // is Actor, as expected
}

🙁 Actual behavior

Comparing the result of a typeguard with guard(it) === false or == false does not narrow the type, but the unary operator with !guard(it) does narrow it to Actor.

🙂 Expected behavior

This may be due to a subtlety with the entity is Actor return type not being a proper boolean, but I would expect all forms of negation and comparison to false to narrow the type, and https://www.typescriptlang.org/docs/handbook/2/narrowing.html#equality-narrowing seems to support that. For a typeguard that returns a boolean, the comparisons should be equivalent. Is there some reason that is not the case here?

I'm working with lint rules that discourage unary not (easy to miss) and discovered this while doing some refactoring.

[MartinJohns]

Related: #32819 (comment)

Narrowing forms have to be identified syntactically, so we pay a perf penalty for every possible form that's checked for. Since there's no real reason to write it this way, we don't consider === true forms

[ssube]

That makes sense, and I agree the === true forms are redundant and do not make sense to check, which is why I focused on === false in the example above. The lint rules that lead to this discovery ban equality with true as well as unary not ("UnaryExpression[operator='!']", "BinaryExpression[operator='!=='][right.value=true]"). My reasoning is the same as #32819 (comment)

[MartinJohns]

My reasoning is the same as #32819 (comment)

It's always fascinating to read statements like this. To me it's the exact other way around. Having === false makes head stumble, whereas the ! operator just feels naturally. I never overread it (perhaps a different font would be helpful to you?), and even if I would forget it: The type system generally protects me from such mistakes. Tho I tend to prefer wrapping it in a negated function as well, e.g. isNotFoo(..) instead of !isFoo(..).

[ssube]

I looked into providing negated typeguards, which could be an acceptable workaround if correctly typed, but I have not found a way to represent entity is not Entity. The list of possible types is not known ahead of time (hence Entity.type being a string), so writing an exhaustive list is not possible.

If looking for these cases is a performance problem, putting it behind a --strictTypeGuards type flag would also be more than sufficient for my purposes - I would rather have consistent behavior across the various forms of comparison, since they are equivalent for flow-control purposes.

[RyanCavanaugh]

I'm much more sympathetic to === false (which at least does something) than === true, my own personal tastes notwithstanding. I'd be inclined to support this form if we heard more feedback on it.

[nmain]

I'm working with lint rules that discourage unary not

Is this a generally available eslint rule? Google searching "eslint unary not" isn't bringing back much.

[Craigfis]

I just ran into a similar (perhaps the same?) issue.
Given:

const isA = (x: A | B): x is A => Object.prototype.hasOwnProperty.call(x, 'kind');
const isB = (x: A | B): x is B => !isA(x);

isA() narrows to A (when true) or B (when false).
isB() narrows to A | B (when true) or never (when false).

Playground

[ssube]

@nmain yes, I'm using https://eslint.org/docs/rules/no-restricted-syntax and the relevant selectors are:

    "no-restricted-syntax": [
      "IfStatement[alternate.type='IfStatement']",
      "UnaryExpression[operator='!']",
      "BinaryExpression[operator='!==']",
      "BinaryExpression[operator='==='][right.value=true]"
    ]

[clshortfuse]

Another example of lack of code clarity:

if (!event.target || event.target instanceof Element === false) throw TypeError('Expected Element');
else if (event.target instanceof HTMLElement === false)) handleElement();
else if (event.target instanceof HTMLAnchorElement) handleAnchor();
else if (event.target instanceof HTMLButtonElement) handleButton();
else if (event.target instanceof HTMLElement) handleHTMLElement();

vs

if (!event.target || !(event.target instanceof Element)) throw TypeError('Expected Element');
else if (!(event.target instanceof HTMLElement)) handleElement();
else if (event.target instanceof HTMLAnchorElement) handleAnchor();
else if (event.target instanceof HTMLButtonElement) handleButton();
else if (event.target instanceof HTMLElement) handleHTMLElement();

I think the change would be around here

TypeScript/src/compiler/binder.ts

Lines 929 to 933 in f84a67f

	case SyntaxKind.ExclamationEqualsEqualsToken:
	return isNarrowableOperand(expr.left) || isNarrowableOperand(expr.right) ||
	isNarrowingTypeofOperands(expr.right, expr.left) || isNarrowingTypeofOperands(expr.left, expr.right);
	case SyntaxKind.InstanceOfKeyword:
	return isNarrowableOperand(expr.left);

mimicking how typeof works but I could be wrong. I can work through a PR for this if the team is interested.

[ngregory-rbi]

This should absolutely be fixed. It does not allow for type guards on false, so it forces developers to indent their code inside of an if. That pattern may not be preferable to some developers. It also just doesn't work as expected.

[ljharb]

I just ran across this with #53005. It'd be great to handle this.

[ljharb]

Yay! Will this be in v5.3 or v5.4?

[jakebailey]

5.3, #55486 for the schedule.