Skip to content

POC: Make assignment expressions and function calls safe #1

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

Closed
wants to merge 4 commits into from

Conversation

kevinbarabash
Copy link
Owner

@kevinbarabash kevinbarabash commented Jun 9, 2019

This prevents unsafe operations since mutable covariant arrays can no longer be assigned to their contravariant.

This refactored isRelatedTo and similar functions to accept an options object which made feeding the current node, errorNode, down to recursiveTypeRelatedTo before computing variances to override the default variance for type parameters which is now invariant to be covariant for array literals only.

It also updates getRelationKey to also take an errorNode which is used to differentiate the relation key when the source is an array literal. This allows isTypeRelatedTo to return different results depending on whether an array literal or a non-literal array with the same type are the source.

TODO:

  • make function calls work so that passing literals is covariant while passing variables (to mutable params) is invariant
  • place this new behavior behind a flag
  • run the existing test suite and make sure nothing's broken
  • add new tests for the new behavior

Test Plan:

  • yarn gulp build-min
  • node built/local/tsc.js --skipLibCheck --strict --noEmit foo.ts
  • see the following (expected) errors:
foo.ts:9:7 - error TS2322: Type 'Cat[]' is not assignable to type 'Animal[]'.

9 const badAnimals: Animal[] = cats; // error
        ~~~~~~~~~~

foo.ts:9:30 - error TS2322: Type 'Cat[]' is not assignable to type 'Animal[]'.
  Property 'purr' is missing in type 'Animal' but required in type 'Cat'.

9 const badAnimals: Animal[] = cats; // error
                               ~~~~

  foo.ts:3:28
    3 class Cat extends Animal { purr() {} }
                                 ~~~~
    'purr' is declared here.

foo.ts:13:5 - error TS2345: Argument of type 'Cat[]' is not assignable to parameter of type 'Animal[]'.

13 foo(cats); // error
       ~~~~


Found 3 errors.

foo.ts:

class Animal {}
class Dog extends Animal { bark() {} }
class Cat extends Animal { purr() {} }

const cats: Cat[] = [new Cat];
const goodAnimals: Animal[] = [new Cat, new Dog];
const readonlyAnimals: ReadonlyArray<Animal> = cats;
const badAnimals: Animal[] = cats; // error

const foo = (animals: Animal[]) => {};
foo(goodAnimals);
foo([new Cat]);
foo(cats); // error

const bar = (animals: ReadonlyArray<Animal>) => {};
bar(goodAnimals);
bar([new Cat]);
bar(cats);

badAnimals.push(new Dog);
// Unsound and bad
// This should never be possible, because `badAnimals = cats` will be flagged as an error.
cats.forEach(cat => cat.purr());

Summary:
This prevents unsafe operations since mutable covariant arrays
can no longer be assigned to their contravariant.

TODO: hide this new behavior behind a flag

Test Plan:
- yarn gulp build-min
- node built/local/tsc.js --skipLibCheck --strict --noEmit foo.ts
@kevinbarabash
Copy link
Owner Author

There's still an issue with object's and their properties always being covariant which makes aliasing assignments of objects unsafe:

type NumberNode = { id: number };
const node0: NumberNode = { id: 5 };
type MixedNode = { id: number | string };
const node1: MixedNode = { id: 5 };
const node2: MixedNode = node0; // This should fail, but doesn't
node2.id = "five"; // whoops, now node0 has a string for its "id"
const node3: Readonly<MixedNode> = node0; // This okay b/c node3 is readonly

@kevinbarabash
Copy link
Owner Author

const node2: MixedNode = node0;

now fails as expected, but other types of object properties need to be updated to behave the same way, i.e. invariant when assigning a variable but covariant when assigning an object literal or covariant when being assigned to Readonly type.

@kevinbarabash kevinbarabash changed the title Make alias assignments invariant POC: Make assignment expressions and function calls safe Jun 15, 2019
@kevinbarabash
Copy link
Owner Author

Closing in preference of #4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant