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

[3.9.0-beta] OR'd types seem to get ANDed because of assertions #37659

Closed
eamodio opened this issue Mar 28, 2020 · 1 comment · Fixed by #37762
Closed

[3.9.0-beta] OR'd types seem to get ANDed because of assertions #37659

eamodio opened this issue Mar 28, 2020 · 1 comment · Fixed by #37762
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@eamodio
Copy link

eamodio commented Mar 28, 2020

TypeScript Version: 3.9.0-dev.20200327

Search Terms:

Expected behavior:
No error -- this worked fine in 3.8 (and should)

Actual behavior:
Error:

Argument of type 'CommitFileNode | ResultsFileNode | (ResultsFileNode & StashFileNode)' is not assignable to parameter of type 'CommitFileNode | ResultsFileNode | StashFileNode | StatusFileNode'.
  Type 'ResultsFileNode & StashFileNode' is not assignable to type 'CommitFileNode | ResultsFileNode | StashFileNode | StatusFileNode'.
    Type 'ResultsFileNode & StashFileNode' is not assignable to type 'StashFileNode'.
      Property '_foo1' has conflicting declarations and is inaccessible in type 'ResultsFileNode & StashFileNode'.(2345)

Related Issues:

Code

abstract class ViewNode { }
abstract class ViewRefNode extends ViewNode { }
abstract class ViewRefFileNode extends ViewRefNode { }

class CommitFileNode extends ViewRefFileNode {
  private _id: any;
}

class ResultsFileNode extends ViewRefFileNode {
  private _id: any;
}

class StashFileNode extends CommitFileNode { 
  private _id2: any;
}

class StatusFileNode extends ViewNode {
  private _id: any;
}

class Foo {
  private async foo(node: CommitFileNode | ResultsFileNode | StashFileNode) {
		if (
			!(node instanceof CommitFileNode) &&
			!(node instanceof StashFileNode) &&
			!(node instanceof ResultsFileNode)
		) {
			return;
		}

		await this.bar(node);
	}

  private async bar(node: CommitFileNode | ResultsFileNode | StashFileNode | StatusFileNode, options?: {}) {
    return Promise.resolve(undefined);
  }
}
Output
"use strict";
class ViewNode {
}
class ViewRefNode extends ViewNode {
}
class ViewRefFileNode extends ViewRefNode {
}
class CommitFileNode extends ViewRefFileNode {
}
class ResultsFileNode extends ViewRefFileNode {
}
class StashFileNode extends CommitFileNode {
}
class StatusFileNode extends ViewNode {
}
class Foo {
    async foo(node) {
        if (!(node instanceof CommitFileNode) &&
            !(node instanceof StashFileNode) &&
            !(node instanceof ResultsFileNode)) {
            return;
        }
        await this.bar(node);
    }
    async bar(node, options) {
        return Promise.resolve(undefined);
    }
}
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

@ahejlsberg ahejlsberg self-assigned this Mar 31, 2020
@ahejlsberg ahejlsberg added the Bug A bug in TypeScript label Mar 31, 2020
@ahejlsberg
Copy link
Member

Here's a simpler repro:

class A { private x: unknown }
class B { private x: unknown }

function foo2(node: A | B) {
  if (node instanceof A || node instanceof A) {
    node;  // A | A & B, should be just A
  }
  else {
    node;  // B
  }
  node;  // A | B | A & B, should be just A | B
}

In the second instanceof check we form the intersection A & B (because we have a B and need a type that is also an A). This intersection used to be assignable to both A and B, but it now isn't because with #37195 we check the whole object for assignability and (correctly) detect the conflicting private fields.

We can fix this by ensuring that every intersection we create in a CFA operation is actually assignable to the individual constituents and otherwise resolve to never. Currently we implicitly assume this to be true, but there are plenty of intersections for which it isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
2 participants