Merge pull request #12826 from Microsoft/mappedTypeModifiers2

Improve propagation of modifiers in mapped types
microsoft · Dec 10, 2016 · 7c5c664 · 7c5c664
2 parents 57cb4ac + 7fdfcf1
commit 7c5c664
Show file tree

Hide file tree

Showing 9 changed files with 627 additions and 470 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -4521,12 +4521,11 @@ namespace ts {
             // Resolve upfront such that recursive references see an empty object type.
             setStructuredTypeMembers(type, emptySymbols, emptyArray, emptyArray, undefined, undefined);
             // In { [P in K]: T }, we refer to P as the type parameter type, K as the constraint type,
-            // and T as the template type. If K is of the form 'keyof S', the mapped type and S are
-            // homomorphic and we copy property modifiers from corresponding properties in S.
+            // and T as the template type.
             const typeParameter = getTypeParameterFromMappedType(type);
             const constraintType = getConstraintTypeFromMappedType(type);
-            const homomorphicType = getHomomorphicTypeFromMappedType(type);
             const templateType = getTemplateTypeFromMappedType(type);
+            const modifiersType = getModifiersTypeFromMappedType(type);
             const templateReadonly = !!type.declaration.readonlyToken;
             const templateOptional = !!type.declaration.questionToken;
             // First, if the constraint type is a type parameter, obtain the base constraint. Then,
@@ -4545,11 +4544,11 @@ namespace ts {
                 // Otherwise, for type string create a string index signature.
                 if (t.flags & TypeFlags.StringLiteral) {
                     const propName = (<LiteralType>t).text;
-                    const homomorphicProp = homomorphicType && getPropertyOfType(homomorphicType, propName);
-                    const isOptional = templateOptional || !!(homomorphicProp && homomorphicProp.flags & SymbolFlags.Optional);
+                    const modifiersProp = getPropertyOfType(modifiersType, propName);
+                    const isOptional = templateOptional || !!(modifiersProp && modifiersProp.flags & SymbolFlags.Optional);
                     const prop = <TransientSymbol>createSymbol(SymbolFlags.Property | SymbolFlags.Transient | (isOptional ? SymbolFlags.Optional : 0), propName);
                     prop.type = propType;
-                    prop.isReadonly = templateReadonly || homomorphicProp && isReadonlySymbol(homomorphicProp);
+                    prop.isReadonly = templateReadonly || modifiersProp && isReadonlySymbol(modifiersProp);
                     members[propName] = prop;
                 }
                 else if (t.flags & TypeFlags.String) {
@@ -4576,9 +4575,16 @@ namespace ts {
                     unknownType);
         }
 
-        function getHomomorphicTypeFromMappedType(type: MappedType) {
-            const constraint = getConstraintDeclaration(getTypeParameterFromMappedType(type));
-            return constraint.kind === SyntaxKind.TypeOperator ? instantiateType(getTypeFromTypeNode((<TypeOperatorNode>constraint).type), type.mapper || identityMapper) : undefined;
+        function getModifiersTypeFromMappedType(type: MappedType) {
+            if (!type.modifiersType) {
+                // If the mapped type was declared as { [P in keyof T]: X } or as { [P in K]: X }, where
+                // K is constrained to 'K extends keyof T', then we will copy property modifiers from T.
+                const declaredType = <MappedType>getTypeFromMappedTypeNode(type.declaration);
+                const constraint = getConstraintTypeFromMappedType(declaredType);
+                const extendedConstraint = constraint.flags & TypeFlags.TypeParameter ? getConstraintOfTypeParameter(<TypeParameter>constraint) : constraint;
+                type.modifiersType = extendedConstraint.flags & TypeFlags.Index ? instantiateType((<IndexType>extendedConstraint).type, type.mapper || identityMapper) : emptyObjectType;
+            }
+            return type.modifiersType;
         }
 
         function getErasedTemplateTypeFromMappedType(type: MappedType) {

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2933,6 +2933,7 @@ namespace ts {
         typeParameter?: TypeParameter;
         constraintType?: Type;
         templateType?: Type;
+        modifiersType?: Type;
         mapper?: TypeMapper;  // Instantiation mapper
     }
 

diff --git a/tests/baselines/reference/mappedTypeErrors.errors.txt b/tests/baselines/reference/mappedTypeErrors.errors.txt
@@ -26,9 +26,19 @@ tests/cases/conformance/types/mapped/mappedTypeErrors.ts(78,59): error TS2345: A
   Object literal may only specify known properties, and 'z' does not exist in type 'Readonly<{ x: number; y: number; }>'.
 tests/cases/conformance/types/mapped/mappedTypeErrors.ts(84,58): error TS2345: Argument of type '{ x: number; y: number; z: number; }' is not assignable to parameter of type 'Partial<{ x: number; y: number; }>'.
   Object literal may only specify known properties, and 'z' does not exist in type 'Partial<{ x: number; y: number; }>'.
+tests/cases/conformance/types/mapped/mappedTypeErrors.ts(106,15): error TS2345: Argument of type '{ a: undefined; }' is not assignable to parameter of type 'Pick<Foo, "a">'.
+  Types of property 'a' are incompatible.
+    Type 'undefined' is not assignable to type 'string'.
+tests/cases/conformance/types/mapped/mappedTypeErrors.ts(107,17): error TS2345: Argument of type '{ c: boolean; }' is not assignable to parameter of type 'Pick<Foo, "a" | "b">'.
+  Object literal may only specify known properties, and 'c' does not exist in type 'Pick<Foo, "a" | "b">'.
+tests/cases/conformance/types/mapped/mappedTypeErrors.ts(124,12): error TS2345: Argument of type '{ a: undefined; }' is not assignable to parameter of type 'Pick<Foo, "a">'.
+  Types of property 'a' are incompatible.
+    Type 'undefined' is not assignable to type 'string'.
+tests/cases/conformance/types/mapped/mappedTypeErrors.ts(125,14): error TS2345: Argument of type '{ c: boolean; }' is not assignable to parameter of type 'Pick<Foo, "a" | "b">'.
+  Object literal may only specify known properties, and 'c' does not exist in type 'Pick<Foo, "a" | "b">'.
 
 
-==== tests/cases/conformance/types/mapped/mappedTypeErrors.ts (17 errors) ====
+==== tests/cases/conformance/types/mapped/mappedTypeErrors.ts (21 errors) ====
 
     interface Shape {
         name: string;
@@ -158,4 +168,59 @@ tests/cases/conformance/types/mapped/mappedTypeErrors.ts(84,58): error TS2345: A
                                                              ~~~~
 !!! error TS2345: Argument of type '{ x: number; y: number; z: number; }' is not assignable to parameter of type 'Partial<{ x: number; y: number; }>'.
 !!! error TS2345:   Object literal may only specify known properties, and 'z' does not exist in type 'Partial<{ x: number; y: number; }>'.
-    }
+    }
+
+    // Verify use of Pick<T, K> for setState functions (#12793)
+
+    interface Foo {
+        a: string;
+        b?: number;
+    }
+
+    function setState<T, K extends keyof T>(obj: T, props: Pick<T, K>) {
+        for (let k in props) {
+            obj[k] = props[k];
+        }
+    }
+
+    let foo: Foo = { a: "hello", b: 42 };
+    setState(foo, { a: "test", b: 43 })
+    setState(foo, { a: "hi" });
+    setState(foo, { b: undefined });
+    setState(foo, { });
+    setState(foo, foo);
+    setState(foo, { a: undefined });  // Error
+                  ~~~~~~~~~~~~~~~~
+!!! error TS2345: Argument of type '{ a: undefined; }' is not assignable to parameter of type 'Pick<Foo, "a">'.
+!!! error TS2345:   Types of property 'a' are incompatible.
+!!! error TS2345:     Type 'undefined' is not assignable to type 'string'.
+    setState(foo, { c: true });  // Error
+                    ~~~~~~~
+!!! error TS2345: Argument of type '{ c: boolean; }' is not assignable to parameter of type 'Pick<Foo, "a" | "b">'.
+!!! error TS2345:   Object literal may only specify known properties, and 'c' does not exist in type 'Pick<Foo, "a" | "b">'.
+
+    class C<T> {
+        state: T;
+        setState<K extends keyof T>(props: Pick<T, K>) {
+            for (let k in props) {
+                this.state[k] = props[k];
+            }
+        }
+    }
+
+    let c = new C<Foo>();
+    c.setState({ a: "test", b: 43 });
+    c.setState({ a: "hi" });
+    c.setState({ b: undefined });
+    c.setState({ });
+    c.setState(foo);
+    c.setState({ a: undefined });  // Error
+               ~~~~~~~~~~~~~~~~
+!!! error TS2345: Argument of type '{ a: undefined; }' is not assignable to parameter of type 'Pick<Foo, "a">'.
+!!! error TS2345:   Types of property 'a' are incompatible.
+!!! error TS2345:     Type 'undefined' is not assignable to type 'string'.
+    c.setState({ c: true });  // Error
+                 ~~~~~~~
+!!! error TS2345: Argument of type '{ c: boolean; }' is not assignable to parameter of type 'Pick<Foo, "a" | "b">'.
+!!! error TS2345:   Object literal may only specify known properties, and 'c' does not exist in type 'Pick<Foo, "a" | "b">'.
+
diff --git a/tests/baselines/reference/mappedTypeErrors.js b/tests/baselines/reference/mappedTypeErrors.js
@@ -83,7 +83,48 @@ function f21() {
     let x1 = objAndPartial({ x: 0, y: 0 }, { x: 1 });
     let x2 = objAndPartial({ x: 0, y: 0 }, { x: 1, y: 1 });
     let x3 = objAndPartial({ x: 0, y: 0 }, { x: 1, y: 1, z: 1 });  // Error
-}
+}
+
+// Verify use of Pick<T, K> for setState functions (#12793)
+
+interface Foo {
+    a: string;
+    b?: number;
+}
+
+function setState<T, K extends keyof T>(obj: T, props: Pick<T, K>) {
+    for (let k in props) {
+        obj[k] = props[k];
+    }
+}
+
+let foo: Foo = { a: "hello", b: 42 };
+setState(foo, { a: "test", b: 43 })
+setState(foo, { a: "hi" });
+setState(foo, { b: undefined });
+setState(foo, { });
+setState(foo, foo);
+setState(foo, { a: undefined });  // Error
+setState(foo, { c: true });  // Error
+
+class C<T> {
+    state: T;
+    setState<K extends keyof T>(props: Pick<T, K>) {
+        for (let k in props) {
+            this.state[k] = props[k];
+        }
+    }
+}
+
+let c = new C<Foo>();
+c.setState({ a: "test", b: 43 });
+c.setState({ a: "hi" });
+c.setState({ b: undefined });
+c.setState({ });
+c.setState(foo);
+c.setState({ a: undefined });  // Error
+c.setState({ c: true });  // Error
+
 
 //// [mappedTypeErrors.js]
 function f1(x) {
@@ -124,6 +165,37 @@ function f21() {
     var x2 = objAndPartial({ x: 0, y: 0 }, { x: 1, y: 1 });
     var x3 = objAndPartial({ x: 0, y: 0 }, { x: 1, y: 1, z: 1 }); // Error
 }
+function setState(obj, props) {
+    for (var k in props) {
+        obj[k] = props[k];
+    }
+}
+var foo = { a: "hello", b: 42 };
+setState(foo, { a: "test", b: 43 });
+setState(foo, { a: "hi" });
+setState(foo, { b: undefined });
+setState(foo, {});
+setState(foo, foo);
+setState(foo, { a: undefined }); // Error
+setState(foo, { c: true }); // Error
+var C = (function () {
+    function C() {
+    }
+    C.prototype.setState = function (props) {
+        for (var k in props) {
+            this.state[k] = props[k];
+        }
+    };
+    return C;
+}());
+var c = new C();
+c.setState({ a: "test", b: 43 });
+c.setState({ a: "hi" });
+c.setState({ b: undefined });
+c.setState({});
+c.setState(foo);
+c.setState({ a: undefined }); // Error
+c.setState({ c: true }); // Error
 
 
 //// [mappedTypeErrors.d.ts]
@@ -168,3 +240,14 @@ declare function objAndReadonly<T>(primary: T, secondary: Readonly<T>): T;
 declare function objAndPartial<T>(primary: T, secondary: Partial<T>): T;
 declare function f20(): void;
 declare function f21(): void;
+interface Foo {
+    a: string;
+    b?: number;
+}
+declare function setState<T, K extends keyof T>(obj: T, props: Pick<T, K>): void;
+declare let foo: Foo;
+declare class C<T> {
+    state: T;
+    setState<K extends keyof T>(props: Pick<T, K>): void;
+}
+declare let c: C<Foo>;