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

Associativity fails for intersections of unions #19055

Closed
tenedor opened this issue Oct 9, 2017 · 6 comments
Closed

Associativity fails for intersections of unions #19055

tenedor opened this issue Oct 9, 2017 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@tenedor
Copy link

tenedor commented Oct 9, 2017

TypeScript Version: 2.5.1-insiders.20170825

Code

interface A {
    identity: 'a';
    a: number;
}
interface B {
    identity: 'b';
    b: number;
}
interface C {}

// This function errors as written. It does not error if changed to `value: C & (A | B)`.
function f(value: ((A | B) & C) & (A | B)): number {
    switch (value.identity) {
        case 'a':
            return value.a; // Error: Property 'a' does not exist on 'B & C'.
        case 'b':
            return value.b; // Error: Property 'b' does not exist on 'A & C'.
    }
}

Expected behavior:
No error. This is a discriminated union and the switch statement should infer property a.

Furthermore, I expect that the following types are equivalent:

  1. ((A | B) & C) & (A | B)
  2. (A | B) & C & (A | B)
  3. C & (A | B)

However, 1 behaves differently from 2 and 3.

Actual behavior:

TS errors because it fails to infer that the type cannot be B & C in the first case.

@tenedor
Copy link
Author

tenedor commented Oct 9, 2017

Here is a practical example where this causes problems. This is analogous to the issue I ran into at my company.

If I have a union type (Animal) and a mixin (Domestic) and I define their composite type (Domestic Animal), I am unable to restrict this type to a subset of the union (Domestic Dog) by using an intersection. Instead I need to expose the mixin - Domestic & Dog instead of DomesticAnimal & Dog. I do not want to do this because I want to keep the mixin private.

interface Laborador {
    breed: 'laborador';
    bark: () => void;
}
interface Chihuahua {
    breed: 'chihuahua';
    yip: () => void;
}
interface Cat {
    breed: 'cat';
}
type Animal = Laborador | Chihuahua | Cat;
interface Domestic {
    wearsLeash: boolean;
}
type DomesticAnimal = Animal & Domestic;

function makeDogNoise(domesticDog: DomesticAnimal & (Laborador | Chihuahua)): void {
    switch (domesticDog.breed) {
        case 'laborador':
            domesticDog.bark(); // Error: Property 'bark' does not exist on `Chihuahua & Domestic`
        case 'chihuahua':
            domesticDog.yip(); // Error: Property 'bark' does not exist on `Laborador & Domestic`
    }
}

@jcalz
Copy link
Contributor

jcalz commented Oct 10, 2017

You could think of this as a failure of associativity (it looks like the type system is not collapsing nested intersections to a flat intersection before processing further). Or you could think of this as a failure of absorption (see #16386): since X & Y is a subtype of X, the intersection should collapse to X & Y (where X is A|B and Y is C in your case).

Anyway if I inspect what happens to ((A | B) & C) & (A | B) in the compiler, the intersections first get distributed across the unions, to yield (A & C) | (B & C) | (A & C & B) | (B & C & A); whereas (A | B) & C & (A | B) gets collapsed to (A | B) & C before the intersection is distributed across the union, to yield (A & C) | (B & C). The former confuses the control flow analysis, as you noted. If the absorption laws were implemented, the former would reduce to the latter.


For the problem you noted, there's the obvious workaround of assigning to the right type before doing case analysis:

function f(value: ((A | B) & C) & (A | B)): number {
  const v: (A & C) | (B & C) = value; // works, no problem
  switch (v.identity) {
    case 'a':
      return v.a; // okay
    case 'b':
      return v.b; // okay
    default:
      const exhaustivenessCheck: never = v;
      return exhaustivenessCheck;
  }
}

For the animal example, I agree there might not be a workaround that doesn't involve using the Domestic interface or an equivalent. You could ignore the DomesticAnimal part during case analysis, but I can imagine that would get annoying:

function makeDogNoise(domesticDog: DomesticAnimal & (Labrador | Chihuahua)): void {
  const dog: Labrador | Chihuahua = domesticDog; // forget domesticity
  switch (dog.breed) {
    case 'labrador':
      dog.bark();
      break;
    case 'chihuahua':
      if (domesticDog.wearsLeash) {
        dog.yip();
      }
  }
}

@tenedor
Copy link
Author

tenedor commented Oct 11, 2017

Thanks @jcalz, agreed on every point.

The reason this one matters to my team is that in our real code there are 31 "dogs" and 51 total "animals", and these combine with either of two modifying types like "domestic". We're using this fine-grained typing because we need to write N x N interleaving rules for every animal pairing and very much want discriminated unions with never checks to ensure we don't miss an interleaving. The set of animals occasionally changes, too, and we need to update all the interleavings:

type Dog = Laborador | Chihuahua | GreatDane | Poodle /* | ... x 31 */;
type Cat = Siamese | Tawny | Calico /* | ... x 12 */;
type OtherAnimal = Elephant | Giraffe /* | ... x 8 */;
type Animal = Dog | Cat | OtherAnimal;

interface Domestic { wearsLeash: boolean; }
type DomesticAnimal = Animal & Domestic;

function getsAlong(animal1: DomesticAnimal, animal2: UndomesticAnimal) {
  switch (animal1.breed) {
    case 'poodle':
    case 'chihuahua':
      return getsAlongWithSmallDomesticAnimal(animal2);
    // ...
    default:
      return assertNever(animal1);
  }
}

For now we've decided to expose Domestic and Undomestic as well as DomesticAnimal and UndomesticAnimal, but it's confusing for unfamiliar developers to track the difference. Our actual name for Domestic ends up more like DomesticAnimalMixin so it all ends up more esoteric than the metaphor suggests.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 11, 2017

The examples in this thread should be addressed by #18438. most of the cases with unit types (the discriminant) that were in the past an intersection (e.g. poodle & chiuahua), now reduce to never, which removes them from the union.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 11, 2017

Marking this issue as a duplicate of #16386 and #18210.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Oct 11, 2017
@tenedor
Copy link
Author

tenedor commented Oct 12, 2017

@mhegazy great, this seems correct. Thanks.

@mhegazy mhegazy closed this as completed Oct 12, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants