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

the types 'T' and (anything else) have no overlap #32768

Closed
davidje13 opened this issue Aug 8, 2019 · 13 comments
Closed

the types 'T' and (anything else) have no overlap #32768

davidje13 opened this issue Aug 8, 2019 · 13 comments
Labels
Duplicate An existing issue was already created

Comments

@davidje13
Copy link

TypeScript Version: 3.5.3

Search Terms:

  • Generic "has no overlap"

Code

const num = 1;
function check<T>(x: T) {
  return x === num;
}
check(num);

Expected behavior:

No error; check returns true if given num (or an alias of it), and false otherwise.

Actual behavior:

index.ts:3:10 - error TS2367: This condition will always return 'false' since the types 'T' and '1' have no overlap.

3   return x === num;
           ~~~~~~~~~


Found 1 error.

I observed this in a slightly more complex case but with the same idea:

const makeNum = () => 1;
function check<T>(f: () => T) {
  return f === makeNum;
}
check(makeNum);

Playground Link:
https://www.typescriptlang.org/play/#code/MYewdgzgLgBGCuBbGBeGBGA3AKAGbzGCgEtwZgALAU2AGsAeAFQD4AKADwC4ZGBKGAN7YYMAE5Uo8UWBjtUKNAkQ4Avtko1arJb0xA

@davidje13
Copy link
Author

My use-case approximates to the following:

const isNum = (v: unknown) => {
  if (typeof v !== 'number') throw new Error('Not a number');
  return v;
};
const isString = (v: unknown) => {
  if (typeof v !== 'string') throw new Error('Not a string');
  return v;
};

function makeExampleValue<T>(validator: (v: unknown) => T): T {
  if (parser === isNum) return 42;
  if (parser === isString) return 'something';
  throw new Error('unknown type');
}

I'm considering restructuring to avoid the need for what is effectively a switch in a monolith function, but the reported error is incorrect in any case.

@AnyhowStep
Copy link
Contributor

T extends unknown?

@davidje13
Copy link
Author

@AnyhowStep that works, so I guess I'm confused why <T> and <T extends unknown> are different (i.e. why isn't extends unknown the default assumption?)

@AnyhowStep
Copy link
Contributor

If it can't infer T, it should default to unknown in your case (since you didn't specify a constraint type).

As for why you need to explicitly add extends unknown for your equality check to work, even though extends unknown should be implicit...

¯\(ツ)

@RyanCavanaugh
Copy link
Member

@DanielRosenwasser what's the reasoning behind the current comparability rules with type parameters?

@jack-williams
Copy link
Collaborator

jack-williams commented Aug 9, 2019

@AnyhowStep

As for why you need to explicitly add extends unknown for your equality check to work, even though extends unknown should be implicit

Without an explicit constraint I think the default constraint used is the empty object type (if you do extends {} you get the same result as with no constraint).

const constraint = getConstraintOfType(<TypeVariable>source);
if (!constraint || (source.flags & TypeFlags.TypeParameter && constraint.flags & TypeFlags.Any)) {
    // A type variable with no constraint is not related to the non-primitive object type.
    if (result = isRelatedTo(emptyObjectType, extractTypesOfKind(target, ~TypeFlags.NonPrimitive))) {
        errorInfo = saveErrorInfo;
        return result;
    }
}

I'm wondering if this was just a missed case in #30637?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 9, 2019

@RyanCavanaugh number is not comparable to T, and T isn't comparable to number (because unknown - T's constraint - isn't comparable to number).

We have another issue open about this, but if you tried to "fix" it, you might end up with weird behavior where two unrelated type parameters T and U can be checked for equality. While it's not unsound, it allows questionable code.

@RyanCavanaugh
Copy link
Member

@DanielRosenwasser

because unknown - T's constraint - isn't comparable to number

unknown is comparable to number, and the example works if T is explicit constrained to unknown (which should (???) be a no-op by my understanding).

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 12, 2019

unknown is comparable to number

It's actually the other way around - number is comparable to unknown. Comparability is still a unidirectional relationship that we happen to apply in two directions.

and the example works if T is explicit constrained to unknown (which should (???) be a no-op by my understanding).

I would consider that a bug, and I've opened #32814 to track it. I would guess it's likely either a bug in our getApparentType logic, or in the way we get implicit constraints, or we have some weird special unknown type we're not doing the right thing with.

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 12, 2019
@RyanCavanaugh RyanCavanaugh modified the milestone: Backlog Aug 12, 2019
@RyanCavanaugh RyanCavanaugh added Duplicate An existing issue was already created and removed Bug A bug in TypeScript labels Aug 12, 2019
@RyanCavanaugh
Copy link
Member

We're going to track this at #32814 for clarity

@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.

@weswigham
Copy link
Member

Without an explicit constraint I think the default constraint used is the empty object type

FYI, the default base constraint is unknown nowadays - we swapped a few releases ago.

@jack-williams
Copy link
Collaborator

Yeah I knew the design had changed, but I wasn't sure if a case had been missed. That code looked related, though tbh, I'm not really sure what's going on.

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

7 participants