Merge pull request #15036 from Microsoft/remove-readonly-from-spread-properties

Remove readonly from spread properties

Author: sandersn

src/compiler/checker.ts

@@ -5501,7 +5501,7 @@ namespace ts {
                     prop.checkFlags = templateReadonly || modifiersProp && isReadonlySymbol(modifiersProp) ? CheckFlags.Readonly : 0;
                     prop.type = propType;
                     if (propertySymbol) {
-                        prop.mappedTypeOrigin = propertySymbol;
+                        prop.syntheticOrigin = propertySymbol;
                     }
                     members.set(propName, prop);
                 }
@@ -7450,7 +7450,7 @@ namespace ts {
                     skippedPrivateMembers.set(rightProp.name, true);
                 }
                 else if (!isClassMethod(rightProp) && !isSetterWithoutGetter) {
-                    members.set(rightProp.name, rightProp);
+                    members.set(rightProp.name, getNonReadonlySymbol(rightProp));
                 }
             }
             for (const leftProp of getPropertiesOfType(left)) {
@@ -7466,7 +7466,6 @@ namespace ts {
                         const declarations: Declaration[] = concatenate(leftProp.declarations, rightProp.declarations);
                         const flags = SymbolFlags.Property | (leftProp.flags & SymbolFlags.Optional);
                         const result = createSymbol(flags, leftProp.name);
-                        result.checkFlags = isReadonlySymbol(leftProp) || isReadonlySymbol(rightProp) ? CheckFlags.Readonly : 0;
                         result.type = getUnionType([getTypeOfSymbol(leftProp), getTypeWithFacts(rightType, TypeFacts.NEUndefined)]);
                         result.leftSpread = leftProp;
                         result.rightSpread = rightProp;
@@ -7475,12 +7474,24 @@ namespace ts {
                     }
                 }
                 else {
-                    members.set(leftProp.name, leftProp);
+                    members.set(leftProp.name, getNonReadonlySymbol(leftProp));
                 }
             }
             return createAnonymousType(undefined, members, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo);
         }
 
+        function getNonReadonlySymbol(prop: Symbol) {
+            if (!isReadonlySymbol(prop)) {
+                return prop;
+            }
+            const flags = SymbolFlags.Property | (prop.flags & SymbolFlags.Optional);
+            const result = createSymbol(flags, prop.name);
+            result.type = getTypeOfSymbol(prop);
+            result.declarations = prop.declarations;
+            result.syntheticOrigin = prop;
+            return result;
+        }
+
         function isClassMethod(prop: Symbol) {
             return prop.flags & SymbolFlags.Method && find(prop.declarations, decl => isClassLike(decl.parent));
         }
@@ -22144,10 +22155,10 @@ namespace ts {
             else if (symbol.flags & SymbolFlags.Transient) {
                 if ((symbol as SymbolLinks).leftSpread) {
                     const links = symbol as SymbolLinks;
-                    return [links.leftSpread, links.rightSpread];
+                    return [...getRootSymbols(links.leftSpread), ...getRootSymbols(links.rightSpread)];
                 }
-                if ((symbol as SymbolLinks).mappedTypeOrigin) {
-                    return getRootSymbols((symbol as SymbolLinks).mappedTypeOrigin);
+                if ((symbol as SymbolLinks).syntheticOrigin) {
+                    return getRootSymbols((symbol as SymbolLinks).syntheticOrigin);
                 }
 
                 let target: Symbol;

src/compiler/types.ts

@@ -2852,7 +2852,7 @@ namespace ts {
         containingType?: UnionOrIntersectionType;  // Containing union or intersection type for synthetic property
         leftSpread?: Symbol;                // Left source for synthetic spread property
         rightSpread?: Symbol;               // Right source for synthetic spread property
-        mappedTypeOrigin?: Symbol;          // For a property on a mapped type, points back to the orignal 'T' from 'keyof T'.
+        syntheticOrigin?: Symbol;           // For a property on a mapped or spread type, points back to the original property
         isDiscriminantProperty?: boolean;   // True if discriminant synthetic property
         resolvedExports?: SymbolTable;      // Resolved exports of module
         exportsChecked?: boolean;           // True if exports of external module have been checked

tests/baselines/reference/objectSpread.types

@@ -211,7 +211,7 @@ let getter: { a: number, c: number } =
 >c : number
 
     { ...op, c: 7 }
->{ ...op, c: 7 } : { c: number; readonly a: number; }
+>{ ...op, c: 7 } : { c: number; a: number; }
 >op : { readonly a: number; }
 >c : number
 >7 : 7

tests/baselines/reference/spreadTypeRemovesReadonly.js

@@ -0,0 +1,22 @@
+//// [spreadTypeRemovesReadonly.ts]
+interface ReadonlyData {
+    readonly value: string;
+}
+
+const data: ReadonlyData = { value: 'foo' };
+const clone = { ...data };
+clone.value = 'bar';
+
+
+//// [spreadTypeRemovesReadonly.js]
+var __assign = (this && this.__assign) || Object.assign || function(t) {
+    for (var s, i = 1, n = arguments.length; i < n; i++) {
+        s = arguments[i];
+        for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
+            t[p] = s[p];
+    }
+    return t;
+};
+var data = { value: 'foo' };
+var clone = __assign({}, data);
+clone.value = 'bar';

tests/baselines/reference/spreadTypeRemovesReadonly.symbols

@@ -0,0 +1,22 @@
+=== tests/cases/compiler/spreadTypeRemovesReadonly.ts ===
+interface ReadonlyData {
+>ReadonlyData : Symbol(ReadonlyData, Decl(spreadTypeRemovesReadonly.ts, 0, 0))
+
+    readonly value: string;
+>value : Symbol(ReadonlyData.value, Decl(spreadTypeRemovesReadonly.ts, 0, 24))
+}
+
+const data: ReadonlyData = { value: 'foo' };
+>data : Symbol(data, Decl(spreadTypeRemovesReadonly.ts, 4, 5))
+>ReadonlyData : Symbol(ReadonlyData, Decl(spreadTypeRemovesReadonly.ts, 0, 0))
+>value : Symbol(value, Decl(spreadTypeRemovesReadonly.ts, 4, 28))
+
+const clone = { ...data };
+>clone : Symbol(clone, Decl(spreadTypeRemovesReadonly.ts, 5, 5))
+>data : Symbol(data, Decl(spreadTypeRemovesReadonly.ts, 4, 5))
+
+clone.value = 'bar';
+>clone.value : Symbol(value, Decl(spreadTypeRemovesReadonly.ts, 0, 24))
+>clone : Symbol(clone, Decl(spreadTypeRemovesReadonly.ts, 5, 5))
+>value : Symbol(value, Decl(spreadTypeRemovesReadonly.ts, 0, 24))
+

tests/baselines/reference/spreadTypeRemovesReadonly.types

@@ -0,0 +1,27 @@
+=== tests/cases/compiler/spreadTypeRemovesReadonly.ts ===
+interface ReadonlyData {
+>ReadonlyData : ReadonlyData
+
+    readonly value: string;
+>value : string
+}
+
+const data: ReadonlyData = { value: 'foo' };
+>data : ReadonlyData
+>ReadonlyData : ReadonlyData
+>{ value: 'foo' } : { value: string; }
+>value : string
+>'foo' : "foo"
+
+const clone = { ...data };
+>clone : { value: string; }
+>{ ...data } : { value: string; }
+>data : ReadonlyData
+
+clone.value = 'bar';
+>clone.value = 'bar' : "bar"
+>clone.value : string
+>clone : { value: string; }
+>value : string
+>'bar' : "bar"
+

tests/cases/compiler/spreadTypeRemovesReadonly.ts

@@ -0,0 +1,7 @@
+interface ReadonlyData {
+    readonly value: string;
+}
+
+const data: ReadonlyData = { value: 'foo' };
+const clone = { ...data };
+clone.value = 'bar';

tests/cases/fourslash/findAllRefsForObjectSpread.ts

@@ -1,21 +1,24 @@
 /// <reference path='fourslash.ts'/>
 
-////interface A1 { [|{| "isWriteAccess": true, "isDefinition": true |}a|]: string };
+////interface A1 { readonly [|{| "isWriteAccess": true, "isDefinition": true |}a|]: string };
 ////interface A2 { [|{| "isWriteAccess": true, "isDefinition": true |}a|]?: number };
 ////let a1: A1;
 ////let a2: A2;
 ////let a12 = { ...a1, ...a2 };
 ////a12.[|a|];
+////a1.[|a|];
 const ranges = test.ranges();
-const [r0, r1, r2] = ranges;
+const [r0, r1, r2, r3] = ranges;
 
 // members of spread types only refer to themselves and the resulting property
-verify.referenceGroups(r0, [{ definition: "(property) A1.a: string", ranges: [r0, r2] }]);
+verify.referenceGroups(r0, [{ definition: "(property) A1.a: string", ranges: [r0, r2, r3] }]);
 verify.referenceGroups(r1, [{ definition: "(property) A2.a: number", ranges: [r1, r2] }]);
 
 // but the resulting property refers to everything
 verify.referenceGroups(r2, [
-    { definition: "(property) A1.a: string", ranges: [r0] },
+    { definition: "(property) A1.a: string", ranges: [r0, r3] },
     { definition: "(property) A2.a: number", ranges: [r1] },
     { definition: "(property) a: string | number", ranges: [r2] }
 ]);
+
+verify.referenceGroups(r3, [{ definition: "(property) A1.a: string", ranges: [r0, r2, r3] }]);