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

Compilation fails because tsc incorrectly believes "types 'A' and 'B' have no overlap" #31555

Closed
pineapplemachine opened this issue May 23, 2019 · 17 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@pineapplemachine
Copy link

TypeScript Version: 3.2.4

Search Terms: generics, strict equality, overlap

Code

Here is a function I wrote, to serve as a repro case.

export function arraysEqual<A, B>(a: A[], b: B[]): boolean {
    if(a.length !== b.length) return false;
    for(let i = 0; i < a.length; i++) {
        if(a[i] !== b[i]) return false;
    }
    return true;
}

Expected behavior:

The function compiles. It accepts two arrays of any two types and performs a strict equality check on their elements, whether A and B are the same type or not.

Actual behavior:

Compilation error on the line if(a[i] !== b[i]) return false; -

This condition will always return 'true' since the types 'A' and 'B' have no overlap.

A and B could very well be the same type. The error isn't logical.

I will probably just rewrite it to take a single generic parameter T and both arrays will be of type T[], since the function should work just as well for me in this case. But I still believe that this behavior qualifies as a bug.

@pineapplemachine pineapplemachine changed the title Compilation fail because tsc incorrectly believes "types 'A' and 'B' have no overlap" Compilation fails because tsc incorrectly believes "types 'A' and 'B' have no overlap" May 23, 2019
@jack-williams
Copy link
Collaborator

Duplicate of #17445

@fatcerberus
Copy link

fatcerberus commented May 23, 2019

This should no longer be an error now that #30637 has landed. With that change A and B are considered related by way of unknown.

A and B could very well be the same type. The error isn't logical.

True, but devil's advocate here: They are more likely (from the compiler's point of view) to be different types--because why have two type parameters otherwise--and this is why the compiler is producing an error. The code as written is not provably sound. The error message itself ("condition is always true") is fallacious, but the compiler is (IMO) within its rights to flag an error here.

I'm of mixed opinion here. I would personally want this to be an error:

let x: string[] = { "pig", "cow" };
let y: number[] = [ 812, 1208 ];
arraysEqual(x, y);  // can't possibly be true!

I would therefore probably declare arraysEqual as (note this also fixes the error in the OP):

function arraysEqual<A extends B, B>(a: A[], b: B[]): boolean
{
    /* magic goes here */
}

let strs: string[] = [ "pig", "cow" ];
let nums: number[] = [ 812, 1208 ];
let both: (string | number)[] = [ "pig", 812, "cow", 1208 ];
arraysEqual(strs, both);  // OK, types are related
arraysEqual(nums, both);  // also OK
arraysEqual(strs, nums);  // error, unrelated types

I don't think there's a way you can make A and B interchangeable and still keep the above error, however.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 23, 2019
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 23, 2019

If we think about generic functions as C++-style templates where we just rewrite the function body in terms of the template arguments and then see if that new instantiated body typechecks, then arraysEqual<string, string> would be a valid instantiation, but arraysEqual<string, number> would be an invalid instantiation.

But this is the whole point -- if you intended for the two type parameters to have the same-ish type, then you didn't need two type parameters in the first place. So your intent of having two type parameters must have been that arraysEqual<string, number> was an legal instantiation, therefore it's the body of the function that has an error.

@fatcerberus
Copy link

fatcerberus commented May 23, 2019

Using a single type parameter overall works the best from a type-checking POV too:

function arraysEqual<T>(a: T[], b: T[]): boolean {
    if(a.length !== b.length) return false;
    for(let i = 0; i < a.length; i++) {
        if(a[i] !== b[i]) return false;
    }
    return true;
}

let strs = [ "pig", "cow" ];
let nums = [ 812, 1208 ];
let both = [ "pig", 812, "cow", 1208 ];

arraysEqual(nums, both);  // OK
arraysEqual(strs, both);  // still OK
arraysEqual(both, strs);  // yep, that too
arraysEqual(nums, strs);  // this is an error, yay!

As long as the types are related, the call typechecks. If they are completely unrelated, the call is almost surely a mistake and the compiler flags it as such.

@pineapplemachine
Copy link
Author

Using a single type parameter overall works the best from a type-checking POV too

@fatcerberus This was the correct choice for my case, since it isn't important that compilation succeeds if the two arrays are of different types. I rewrote my function as you suggested, but I still created the issue because as you acknowledged, the error message is strictly speaking incorrect.

Commenters on #17445 made a much better case than I could for why this shouldn't be an error, but the workarounds are straightforward enough that the mere fact that this is produces an error doesn't particularly bother me. If it was up to me, the original snippet would compile without errors, but this isn't a hill I would die on. I do think that, at the very least, the error message needs to be changed to no longer be inconsistent. Incorrect error messages should certainly qualify as buggy behavior!

If we think about generic functions as C++-style templates

@RyanCavanaugh While I am not going to debate the basic point of whether this code should produce an error or not - my primary concern is that really just that I think it should not be producing this error in particular - I don't think that your argument is especially applicable to this situation. Generics are not templates, and TypeScript is not really a strongly-typed language. Such a comparison between two generic types might be clearly invalid in a statically-typed language such as C++, but in JavaScript it's all fine, and the reason that TypeScript gives for the compilation failure is simply not productive.

@fatcerberus
Copy link

Based on your description, it sounds like you don't actually care what the types of the arrays are, just that they're both arrays - at which point you gain nothing from the generic and could get the same result by declaring the parameters as any[] (or unknown[]).

@pineapplemachine
Copy link
Author

pineapplemachine commented May 23, 2019

@fatcerberus I won't argue with that! I still think the error message should be improved, though.

@RyanCavanaugh
Copy link
Member

We're open to suggestions on error message wording.

Just curious, do you think === should be allowed with any non-primitive operands? e.g. cat === mortgage should be OK because both sides might be an aliased reference to a CatMortgage?

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

@pineapplemachine
Copy link
Author

pineapplemachine commented May 27, 2019

@RyanCavanaugh

We're open to suggestions on error message wording.

Perhaps,

Comparison between 'A' and 'B' is not allowed because the operation cannot be proven to be meaningful.

Just curious, do you think === should be allowed with any non-primitive operands?

It's my opinion that strict equality/inequality comparison should only produce an error like this when it's absolutely guaranteed that the comparison is a no-op, assuming the type annotations accurately reflect the values being operated upon.

@fatcerberus
Copy link

fatcerberus commented May 27, 2019

I don't think that's ever absolutely 100% guaranteed because we have intersection types. A & B is assignable to either A or B, without a cast. So there's always a chance they could be the same object, regardless of what the types involved are. I don't think I'd want to give up type safety on equality comparisons just because of that small uncertainty.

I consider it the same case with generics, just at a higher level - two type parameters could be the same type, but it's more likely they're different, so treating them as though they're always the same isn't sound.

@pineapplemachine
Copy link
Author

pineapplemachine commented May 27, 2019 via email

@fatcerberus
Copy link

It can be guaranteed when A and B are not generic types, such as when
comparing a number to a string. Again, assuming that the typing is accurate.

Only when A and B are primitives (and even then string & number is a valid type for some reason). If they are object types then either one could really be A & B. See Ryan's CatMortgage example above.

@fatcerberus
Copy link

fatcerberus commented May 27, 2019

For what it's worth I kind of see where you're coming from with generics, but considering that we already have ways to tell the compiler that two parameters are related:

function fn<A, B extends A>(a: A, b: B) {
   // now a and b can be compared
}

function fn2<T>(a: T, b: T) {
    // call site is only valid if a and b are related
}

I would rather that <A, B> continue to be treated as though they are different types. The uncertainty factor at the type level is IMO the same as the possibility of an A & B occuring at the value level.

@jack-williams
Copy link
Collaborator

(and even then string & number is a valid type for some reason)

Type 'number' is not assignable to type 'string & number'.

is a better error message than

Type 'number' is not assignable to type 'never'.

@pineapplemachine
Copy link
Author

I would rather that <A, B> continue to be treated as though they are different types. The uncertainty factor at the type level is IMO the same as the possibility of an A & B occuring at the value level.

That's ok with me, but still - the current error message is not great.

@HaraldKi
Copy link

HaraldKi commented Aug 6, 2022

The "why use two different type parameters" argument is only half valid:

type Size = 'big' | 'small';
class Shirt<T extends Size> {
    constructor(readonly size: T) {}
    is<O extends Size>(size: O): this is Shirt<O> {
        return size === this.size;
    }
}

The is cannot be written without a new type argument. The workaround here is to add | T to the type argument of is, i.e. is<O extends Size | T>.

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

6 participants