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

Bad type checking for GenericType<boolean> vs GenericType<false> #33833

Closed
thomcc opened this issue Oct 6, 2019 · 5 comments
Closed

Bad type checking for GenericType<boolean> vs GenericType<false> #33833

thomcc opened this issue Oct 6, 2019 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@thomcc
Copy link

thomcc commented Oct 6, 2019

TypeScript Version: 3.7.0-dev.20191006

Search Terms: covariance, subtype, inference, value type (... honestly I have no idea what the right term for "false when used as a type" is, it's... probably not this though).

Code:

class Grid<T> {
    constructor(width: number, height: number, defaultFill: T) { }
    // some function which either behaves as a fill or an in-place equivalent of map, for example
    fillWith(valueOrCallback: T | ((current: T, x: number, y: number, idx: number) => T)) {}
}

function takesBoolGrid(grid: Grid<boolean>) { }
// This causes a type error
takesBoolGrid(new Grid(16, 16, false));

// But this doesn't
let g = new Grid(16, 16, false);
takesBoolGrid(g);

Expected behavior: The code above should typecheck. Or, at the very least, both cases should fail, instead of assigning to an intermediate variable fixing the issue.

Actual behavior: I get an error like the following on the first call to takesBoolGrid:

Argument of type 'Grid<false>' is not assignable to parameter of type 'Grid<boolean>'.
  Types of property 'fillWith' are incompatible.
    Type '(valueOrCallback: false | ((current: false, x: number, y: number, idx: number) => false)) => void' is not assignable to type '(valueOrCallback: boolean | ((current: boolean, x: number, y: number, idx: number) => boolean)) => void'.
      Types of parameters 'valueOrCallback' and 'valueOrCallback' are incompatible.
        Type 'boolean | ((current: boolean, x: number, y: number, idx: number) => boolean)' is not assignable to type 'false | ((current: false, x: number, y: number, idx: number) => false)'.
          Type 'true' is not assignable to type 'false | ((current: false, x: number, y: number, idx: number) => false)'.(2345)

Playground Link: https://www.typescriptlang.org/play/?ts=3.7-Beta&ssl=1&ssc=1&pln=14&pc=1#code/MYGwhgzhAEDiBOBLAJgHgCoD5oG8CwAUNMdMAPYB2EALvAK7DVnwAUA7itQBYBc0FdALYAjAKbwANNC6jEAcy7U+AkeKnJRAMzB0Q1AGKIQIPugCUuaAF9CJaAHp70CGUGjomuhUaJK0NlyIwFzQstzi0GJcYABuojCQ0GAeRiDQzEkU0IgUALQADuDA7qIAjnSIMWAgohTU6ZrQgmD5UpoZogAeYIKForYkmqkA6ojcLFUgdKIA8vAAwtUgwmDAANam0AA+0CwswHTw8LVK0OhSncpCYpLQAJ5Xqrcol-zX4hYAvNjmFjg2BABhE83moviy1DAa3iACEyGQQAgUCw5EhkHwkWhhPCamAKJg-tZCI4zoEYMAdBB4kloNQ7vkSkdmIRIdCIHCEZiWBRRGw4GiWABGABsUhFbWqVLMZgA3IRiU4YXR6txEDBkGR4hQAOTUQg1epyaCffi8-nI8XQS3aEBSuUEVmwnFcuSywhAA

Related Issues: I looked, but sadly my search terms quickly brought me to a bunch of issues that assume more familiarity with tsc (and type system jargon, probably) than I have. (This is probably because the only terminology I have for describing this problem is, well, type system jargon that I only barely can say I understand).


Some notes that don't fit above which might help, possibly: this code used to work in like, typescript 2.something (I could probably find out which version it was on Monday if it would help).

I updated the code to the new typescript, and got this error, and assumed it was just typescript getting updated with some stricter behavior around variance. I complained online, and was asked to produce an example, and it turned out that the issue was weirder than I had though, as:

  1. It only errored if the fillWith function was present
  2. It didn't error if I assign the Grid to an intermediate variable.

Point 2 seems weird, so I decided to file a bug.

Note that (IMO fairly unsurprisingly) the expression takesBoolGrid(new Grid<boolean>(16, 16, false)) type checks fine, e.g. if the type is listed explicitly, no intermediate variable is needed. So I guess it's some kind of type inference thing too? Who knows... (hopefully someone here!)

@jack-williams
Copy link
Collaborator

Could well be caused by #29478, and this may be a duplicate of #30390.

It seems that the argument type in the signature of takesBoolGrid is contextually typing the argument which prevents widening of false. The problem is that Grid is invariant in T, so restricting to false rather than widening to type boolean makes the types unrelated.

Not really sure how best to fix this. I wonder if variance markers can be used to control widening: a boolean type that instantiates an invariant type parameter should not produce a non-widening contextual type.

@xiaoxiangmoe
Copy link
Contributor

I think both cases should fail. Because Grid is invariant in T.

@thomcc
Copy link
Author

thomcc commented Oct 13, 2019

@xiaoxiangmoe Ideally the type would be inferred as Grid<boolean> and not Grid<false> here.

@ahejlsberg ahejlsberg added the Duplicate An existing issue was already created label Oct 16, 2019
@ahejlsberg
Copy link
Member

This is a duplicate of #30390.

@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants