Skip to content

TS2367: This condition will always return 'false' #29822

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

Closed
lonereck opened this issue Feb 8, 2019 · 7 comments
Closed

TS2367: This condition will always return 'false' #29822

lonereck opened this issue Feb 8, 2019 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@lonereck
Copy link

lonereck commented Feb 8, 2019

TypeScript Version: 3.3.3

Search Terms: TS2367

class Scanner {
    private _index: number;
    private _input: string;

    public get current() { 
        return this._input[this._index];
    }

    constructor(input: string) {
        this._input = input;
        this._index = 0;
    }

    public Scan() {
        if (this.current == "A") {
            this._index++;
            // error TS2367: This condition will always return 'false' since the types '"A"' and '"B"' have no overlap.
            if (this.current == "B") {
                // do something
            }
        }
    }
}

Expected behavior:
No compilation errors

Actual behavior:
error TS2367: This condition will always return 'false' since the types '"A"' and '"B"' have no overlap.

Playground Link:
Here

@jack-williams
Copy link
Collaborator

Relevant comment and issue here.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 8, 2019
@fatcerberus
Copy link

Hmm, this is a bit different than #9998 (comment) though since .current is a getter and that should be known by the compiler. It’s surprising that it performs any narrowing on it in the first place.

@jack-williams
Copy link
Collaborator

I don't know if TypeScript distinguishes properties obtained literally or via getters, but I'm not sure it would help much. Even if that information was made available the choices are between performing some inline effect analysis, assuming all getters are mutable, or assuming all getters are immutable.
The first two seem impractical in the general case.

@fatcerberus
Copy link

That’s true, I guess it comes down to real-world tradeoffs as discussed in #9998 (is it bad I know this issue number by heart?). I guess what I find surprising is that this.getterProp is assumed to be stable and is therefore narrowed, while this.getterFunc() will never be narrowed. Since they are both function calls in the end, the inconsistency strikes me as jarring.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@slaneyrw
Copy link

slaneyrw commented Aug 26, 2019

sorry, but this is a mistake. Getter properties should not be narrowed. How do you "convert" to a function call if I can't invoke the property as a function

Adding a simple local function wrapper around the call to the property "tricks" the compile and it now compiles... insane!

class SomeIterator {
   get current(): string { ... }
   moveNext(): boolean { ... }
}

function iterateOver( iterator : SomeIterator ) {

  if ( iterator.current !=== "A" ) {
    return;
  }
  iterator.moveNext();
  // this throw compiler error TS2367
  if (iterator.current === "B") {
  }

  // All case statements compiler error TS2678
  switch(iterator.current) {
    case "C":
      break;
  }
}

if I replace with the following, the compiler is now happy

class SomeIterator {
   get current(): string { ... }
   moveNext(): boolean { ... }
}

function iterateOver( iterator : SomeIterator ) {

  let getCurrent = () => iterator.current;
  if ( getCurrent() === "A" ) {
    return;
  }
  iterator.moveNext();
  // this throw compiler error TS2367
  if (getCurrent () === "B") {
  }

  // All case statements compiler error TS2678
  switch(getCurrent()) {
    case "C":
      break;
  }
}

As properties are function under the hood, #9998 should function behaviour to properties.

@RyanCavanaugh
Copy link
Member

Trade-offs exist. Trying to act as if things don't have a more common and less common behavior is just nihilism and doesn't make the type system more usable.

For any given value that might be stable or might not be, TS has to make an assumption one way or the other. For functions, assuming instability is going to be correct much more often than it's wrong. For properties, assuming stability is going to be correct much more often than it's wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

7 participants