From 8680768478c5e6d2e526eb46f02aad3e1c8f2a3e Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 14 Jun 2017 11:26:15 -0700 Subject: [PATCH 1/4] Improve excess property check for spread property Fall back to the assignment's declaration; don't use the property's valueDeclaration because that is not useful when the property comes from a spread. The fallback now happens when the property's valueDeclaration does not have the object literal's valueDeclaration as an ancestor. --- src/compiler/checker.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c931fddb832..db7bfaf698c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8955,7 +8955,8 @@ namespace ts { reportError(Diagnostics.Property_0_does_not_exist_on_type_1, symbolToString(prop), typeToString(target)); } else { - if (prop.valueDeclaration) { + const objectLiteralDeclaration = source.symbol && source.symbol.valueDeclaration; + if (prop.valueDeclaration && findAncestor(prop.valueDeclaration, d => d === objectLiteralDeclaration)) { errorNode = prop.valueDeclaration; } reportError(Diagnostics.Object_literal_may_only_specify_known_properties_and_0_does_not_exist_in_type_1, From 19c564d48dafe9d2db0d0075ae9b85119de658f4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 14 Jun 2017 11:29:06 -0700 Subject: [PATCH 2/4] Test:error span for spread prop in excess prop check --- .../reference/objectSpreadNegative.errors.txt | 27 ++++++++++++++++++- .../reference/objectSpreadNegative.js | 16 +++++++++++ .../types/spread/objectSpreadNegative.ts | 10 +++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/baselines/reference/objectSpreadNegative.errors.txt b/tests/baselines/reference/objectSpreadNegative.errors.txt index d9106d651a3..1a8df3ed74c 100644 --- a/tests/baselines/reference/objectSpreadNegative.errors.txt +++ b/tests/baselines/reference/objectSpreadNegative.errors.txt @@ -17,9 +17,15 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(52,9): error TS2339 tests/cases/conformance/types/spread/objectSpreadNegative.ts(57,11): error TS2339: Property 'a' does not exist on type '{}'. tests/cases/conformance/types/spread/objectSpreadNegative.ts(61,14): error TS2698: Spread types may only be created from object types. tests/cases/conformance/types/spread/objectSpreadNegative.ts(64,14): error TS2698: Spread types may only be created from object types. +tests/cases/conformance/types/spread/objectSpreadNegative.ts(78,7): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. + Object literal may only specify known properties, and 'extra' does not exist in type 'A'. +tests/cases/conformance/types/spread/objectSpreadNegative.ts(81,7): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. + Object literal may only specify known properties, and 'extra' does not exist in type 'A'. +tests/cases/conformance/types/spread/objectSpreadNegative.ts(83,7): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. + Object literal may only specify known properties, and 'extra' does not exist in type 'A'. -==== tests/cases/conformance/types/spread/objectSpreadNegative.ts (16 errors) ==== +==== tests/cases/conformance/types/spread/objectSpreadNegative.ts (19 errors) ==== let o = { a: 1, b: 'no' } /// private propagates @@ -128,4 +134,23 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(64,14): error TS269 f({ a: 1 }, { a: 'mismatch' }) let overwriteId: { id: string, a: number, c: number, d: string } = f({ a: 1, id: true }, { c: 1, d: 'no' }) + + // excess property checks + type A = { a: string, b: string }; + type Extra = { a: string, b: string, extra: string }; + const extra1: A = { a: "a", b: "b", extra: "extra" }; + ~~~~~~ +!!! error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. +!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'A'. + const extra2 = { a: "a", b: "b", extra: "extra" }; + const a1: A = { ...extra1 }; // error spans should be here + const a2: A = { ...extra2 }; // not on the symbol declarations above + ~~ +!!! error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. +!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'A'. + const extra3: Extra = { a: "a", b: "b", extra: "extra" }; + const a3: A = { ...extra3 }; // same here + ~~ +!!! error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. +!!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'A'. \ No newline at end of file diff --git a/tests/baselines/reference/objectSpreadNegative.js b/tests/baselines/reference/objectSpreadNegative.js index fd834b6ef9c..0cc93692b59 100644 --- a/tests/baselines/reference/objectSpreadNegative.js +++ b/tests/baselines/reference/objectSpreadNegative.js @@ -72,6 +72,16 @@ let overlapConflict: { id:string, a: string } = f({ a: 1 }, { a: 'mismatch' }) let overwriteId: { id: string, a: number, c: number, d: string } = f({ a: 1, id: true }, { c: 1, d: 'no' }) + +// excess property checks +type A = { a: string, b: string }; +type Extra = { a: string, b: string, extra: string }; +const extra1: A = { a: "a", b: "b", extra: "extra" }; +const extra2 = { a: "a", b: "b", extra: "extra" }; +const a1: A = { ...extra1 }; // error spans should be here +const a2: A = { ...extra2 }; // not on the symbol declarations above +const extra3: Extra = { a: "a", b: "b", extra: "extra" }; +const a3: A = { ...extra3 }; // same here //// [objectSpreadNegative.js] @@ -152,3 +162,9 @@ var exclusive = f({ a: 1, b: 'yes' }, { c: 'no', d: false }); var overlap = f({ a: 1 }, { a: 2, b: 'extra' }); var overlapConflict = f({ a: 1 }, { a: 'mismatch' }); var overwriteId = f({ a: 1, id: true }, { c: 1, d: 'no' }); +var extra1 = { a: "a", b: "b", extra: "extra" }; +var extra2 = { a: "a", b: "b", extra: "extra" }; +var a1 = __assign({}, extra1); // error spans should be here +var a2 = __assign({}, extra2); // not on the symbol declarations above +var extra3 = { a: "a", b: "b", extra: "extra" }; +var a3 = __assign({}, extra3); // same here diff --git a/tests/cases/conformance/types/spread/objectSpreadNegative.ts b/tests/cases/conformance/types/spread/objectSpreadNegative.ts index c3b42b31aa9..fced7706c96 100644 --- a/tests/cases/conformance/types/spread/objectSpreadNegative.ts +++ b/tests/cases/conformance/types/spread/objectSpreadNegative.ts @@ -72,3 +72,13 @@ let overlapConflict: { id:string, a: string } = f({ a: 1 }, { a: 'mismatch' }) let overwriteId: { id: string, a: number, c: number, d: string } = f({ a: 1, id: true }, { c: 1, d: 'no' }) + +// excess property checks +type A = { a: string, b: string }; +type Extra = { a: string, b: string, extra: string }; +const extra1: A = { a: "a", b: "b", extra: "extra" }; +const extra2 = { a: "a", b: "b", extra: "extra" }; +const a1: A = { ...extra1 }; // error spans should be here +const a2: A = { ...extra2 }; // not on the symbol declarations above +const extra3: Extra = { a: "a", b: "b", extra: "extra" }; +const a3: A = { ...extra3 }; // same here From d719de5928692e90fa3655706f610b2f775a1358 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 14 Jun 2017 11:43:51 -0700 Subject: [PATCH 3/4] Use first declaration, not valueDeclaration valueDeclaration is frequently not set --- src/compiler/checker.ts | 2 +- tests/baselines/reference/objectSpreadNegative.errors.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index db7bfaf698c..67936dd64e4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8955,7 +8955,7 @@ namespace ts { reportError(Diagnostics.Property_0_does_not_exist_on_type_1, symbolToString(prop), typeToString(target)); } else { - const objectLiteralDeclaration = source.symbol && source.symbol.valueDeclaration; + const objectLiteralDeclaration = source.symbol && firstOrUndefined(source.symbol.declarations); if (prop.valueDeclaration && findAncestor(prop.valueDeclaration, d => d === objectLiteralDeclaration)) { errorNode = prop.valueDeclaration; } diff --git a/tests/baselines/reference/objectSpreadNegative.errors.txt b/tests/baselines/reference/objectSpreadNegative.errors.txt index 1a8df3ed74c..a9381a0dcb1 100644 --- a/tests/baselines/reference/objectSpreadNegative.errors.txt +++ b/tests/baselines/reference/objectSpreadNegative.errors.txt @@ -17,7 +17,7 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(52,9): error TS2339 tests/cases/conformance/types/spread/objectSpreadNegative.ts(57,11): error TS2339: Property 'a' does not exist on type '{}'. tests/cases/conformance/types/spread/objectSpreadNegative.ts(61,14): error TS2698: Spread types may only be created from object types. tests/cases/conformance/types/spread/objectSpreadNegative.ts(64,14): error TS2698: Spread types may only be created from object types. -tests/cases/conformance/types/spread/objectSpreadNegative.ts(78,7): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. +tests/cases/conformance/types/spread/objectSpreadNegative.ts(78,37): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. Object literal may only specify known properties, and 'extra' does not exist in type 'A'. tests/cases/conformance/types/spread/objectSpreadNegative.ts(81,7): error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. Object literal may only specify known properties, and 'extra' does not exist in type 'A'. @@ -139,7 +139,7 @@ tests/cases/conformance/types/spread/objectSpreadNegative.ts(83,7): error TS2322 type A = { a: string, b: string }; type Extra = { a: string, b: string, extra: string }; const extra1: A = { a: "a", b: "b", extra: "extra" }; - ~~~~~~ + ~~~~~~~~~~~~~~ !!! error TS2322: Type '{ a: string; b: string; extra: string; }' is not assignable to type 'A'. !!! error TS2322: Object literal may only specify known properties, and 'extra' does not exist in type 'A'. const extra2 = { a: "a", b: "b", extra: "extra" }; From ef70a6c61b935b2907f65a26a0227c63704b8bcd Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 14 Jun 2017 15:13:46 -0700 Subject: [PATCH 4/4] Add comment to new code in hasExcessProperties --- src/compiler/checker.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 67936dd64e4..58eec90204d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8955,6 +8955,7 @@ namespace ts { reportError(Diagnostics.Property_0_does_not_exist_on_type_1, symbolToString(prop), typeToString(target)); } else { + // use the property's value declaration if the property is assigned inside the literal itself const objectLiteralDeclaration = source.symbol && firstOrUndefined(source.symbol.declarations); if (prop.valueDeclaration && findAncestor(prop.valueDeclaration, d => d === objectLiteralDeclaration)) { errorNode = prop.valueDeclaration;