From 42a2d9e568b4c6ba74eaf7438b55dd4710a69c03 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 11 Jul 2018 11:24:40 -0700 Subject: [PATCH] Excess property understands conditional types (#25584) Previously it did not, causing misleading excess property errors. Note that assignability errors with conditional types are still usually confusing. This PR doesn't address that. Also, make sure that exact matches in getSpellingSuggestion are skipped. --- src/compiler/checker.ts | 8 +++- src/compiler/core.ts | 3 ++ ...onditionalTypesExcessProperties.errors.txt | 24 ++++++++++ .../conditionalTypesExcessProperties.js | 18 +++++++ .../conditionalTypesExcessProperties.symbols | 41 ++++++++++++++++ .../conditionalTypesExcessProperties.types | 47 +++++++++++++++++++ .../conditionalTypesExcessProperties.ts | 10 ++++ 7 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/conditionalTypesExcessProperties.errors.txt create mode 100644 tests/baselines/reference/conditionalTypesExcessProperties.js create mode 100644 tests/baselines/reference/conditionalTypesExcessProperties.symbols create mode 100644 tests/baselines/reference/conditionalTypesExcessProperties.types create mode 100644 tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index fe8d32647c..04729d7b83 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -17406,7 +17406,7 @@ namespace ts { */ function isKnownProperty(targetType: Type, name: __String, isComparingJsxAttributes: boolean): boolean { if (targetType.flags & TypeFlags.Object) { - const resolved = resolveStructuredTypeMembers(targetType); + const resolved = resolveStructuredTypeMembers(targetType as ObjectType); if (resolved.stringIndexInfo || resolved.numberIndexInfo && isNumericLiteralName(name) || getPropertyOfObjectType(targetType, name) || @@ -17416,12 +17416,16 @@ namespace ts { } } else if (targetType.flags & TypeFlags.UnionOrIntersection) { - for (const t of (targetType).types) { + for (const t of (targetType as UnionOrIntersectionType).types) { if (isKnownProperty(t, name, isComparingJsxAttributes)) { return true; } } } + else if (targetType.flags & TypeFlags.Conditional) { + return isKnownProperty((targetType as ConditionalType).root.trueType, name, isComparingJsxAttributes) || + isKnownProperty((targetType as ConditionalType).root.falseType, name, isComparingJsxAttributes); + } return false; } diff --git a/src/compiler/core.ts b/src/compiler/core.ts index c2fd1eed63..da72ebe7a3 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1866,6 +1866,9 @@ namespace ts { if (candidateName !== undefined && Math.abs(candidateName.length - nameLowerCase.length) <= maximumLengthDifference) { const candidateNameLowerCase = candidateName.toLowerCase(); if (candidateNameLowerCase === nameLowerCase) { + if (candidateName === name) { + continue; + } return candidate; } if (justCheckExactMatches) { diff --git a/tests/baselines/reference/conditionalTypesExcessProperties.errors.txt b/tests/baselines/reference/conditionalTypesExcessProperties.errors.txt new file mode 100644 index 0000000000..a01cb8c898 --- /dev/null +++ b/tests/baselines/reference/conditionalTypesExcessProperties.errors.txt @@ -0,0 +1,24 @@ +tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts(8,5): error TS2322: Type '{ test: string; arg: A; }' is not assignable to type 'Something'. + Type '{ test: string; arg: A; }' is not assignable to type 'A extends object ? { arg: A; } : { arg?: undefined; }'. +tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts(9,33): error TS2322: Type 'A' is not assignable to type 'Something["arr"]'. + Type 'object' is not assignable to type 'Something["arr"]'. + + +==== tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts (2 errors) ==== + type Something = { test: string } & (T extends object ? { + arg: T + } : { + arg?: undefined + }); + + function testFunc2(a: A, sa: Something) { + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) + ~~ +!!! error TS2322: Type '{ test: string; arg: A; }' is not assignable to type 'Something'. +!!! error TS2322: Type '{ test: string; arg: A; }' is not assignable to type 'A extends object ? { arg: A; } : { arg?: undefined; }'. + sa = { test: 'bye', arg: a, arr: a } // excess + ~~~ +!!! error TS2322: Type 'A' is not assignable to type 'Something["arr"]'. +!!! error TS2322: Type 'object' is not assignable to type 'Something["arr"]'. + } + \ No newline at end of file diff --git a/tests/baselines/reference/conditionalTypesExcessProperties.js b/tests/baselines/reference/conditionalTypesExcessProperties.js new file mode 100644 index 0000000000..2dc9a0161a --- /dev/null +++ b/tests/baselines/reference/conditionalTypesExcessProperties.js @@ -0,0 +1,18 @@ +//// [conditionalTypesExcessProperties.ts] +type Something = { test: string } & (T extends object ? { + arg: T +} : { + arg?: undefined + }); + +function testFunc2(a: A, sa: Something) { + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) + sa = { test: 'bye', arg: a, arr: a } // excess +} + + +//// [conditionalTypesExcessProperties.js] +function testFunc2(a, sa) { + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) + sa = { test: 'bye', arg: a, arr: a }; // excess +} diff --git a/tests/baselines/reference/conditionalTypesExcessProperties.symbols b/tests/baselines/reference/conditionalTypesExcessProperties.symbols new file mode 100644 index 0000000000..807330df85 --- /dev/null +++ b/tests/baselines/reference/conditionalTypesExcessProperties.symbols @@ -0,0 +1,41 @@ +=== tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts === +type Something = { test: string } & (T extends object ? { +>Something : Symbol(Something, Decl(conditionalTypesExcessProperties.ts, 0, 0)) +>T : Symbol(T, Decl(conditionalTypesExcessProperties.ts, 0, 15)) +>test : Symbol(test, Decl(conditionalTypesExcessProperties.ts, 0, 21)) +>T : Symbol(T, Decl(conditionalTypesExcessProperties.ts, 0, 15)) + + arg: T +>arg : Symbol(arg, Decl(conditionalTypesExcessProperties.ts, 0, 61)) +>T : Symbol(T, Decl(conditionalTypesExcessProperties.ts, 0, 15)) + +} : { + arg?: undefined +>arg : Symbol(arg, Decl(conditionalTypesExcessProperties.ts, 2, 5)) + + }); + +function testFunc2(a: A, sa: Something) { +>testFunc2 : Symbol(testFunc2, Decl(conditionalTypesExcessProperties.ts, 4, 7)) +>A : Symbol(A, Decl(conditionalTypesExcessProperties.ts, 6, 19)) +>a : Symbol(a, Decl(conditionalTypesExcessProperties.ts, 6, 37)) +>A : Symbol(A, Decl(conditionalTypesExcessProperties.ts, 6, 19)) +>sa : Symbol(sa, Decl(conditionalTypesExcessProperties.ts, 6, 42)) +>Something : Symbol(Something, Decl(conditionalTypesExcessProperties.ts, 0, 0)) +>A : Symbol(A, Decl(conditionalTypesExcessProperties.ts, 6, 19)) + + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) +>sa : Symbol(sa, Decl(conditionalTypesExcessProperties.ts, 6, 42)) +>test : Symbol(test, Decl(conditionalTypesExcessProperties.ts, 7, 10)) +>arg : Symbol(arg, Decl(conditionalTypesExcessProperties.ts, 7, 22)) +>a : Symbol(a, Decl(conditionalTypesExcessProperties.ts, 6, 37)) + + sa = { test: 'bye', arg: a, arr: a } // excess +>sa : Symbol(sa, Decl(conditionalTypesExcessProperties.ts, 6, 42)) +>test : Symbol(test, Decl(conditionalTypesExcessProperties.ts, 8, 10)) +>arg : Symbol(arg, Decl(conditionalTypesExcessProperties.ts, 8, 23)) +>a : Symbol(a, Decl(conditionalTypesExcessProperties.ts, 6, 37)) +>arr : Symbol(arr, Decl(conditionalTypesExcessProperties.ts, 8, 31)) +>a : Symbol(a, Decl(conditionalTypesExcessProperties.ts, 6, 37)) +} + diff --git a/tests/baselines/reference/conditionalTypesExcessProperties.types b/tests/baselines/reference/conditionalTypesExcessProperties.types new file mode 100644 index 0000000000..7bbab8bfa1 --- /dev/null +++ b/tests/baselines/reference/conditionalTypesExcessProperties.types @@ -0,0 +1,47 @@ +=== tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts === +type Something = { test: string } & (T extends object ? { +>Something : Something +>T : T +>test : string +>T : T + + arg: T +>arg : T +>T : T + +} : { + arg?: undefined +>arg : undefined + + }); + +function testFunc2(a: A, sa: Something) { +>testFunc2 : (a: A, sa: Something) => void +>A : A +>a : A +>A : A +>sa : Something +>Something : Something +>A : A + + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) +>sa = { test: 'hi', arg: a } : { test: string; arg: A; } +>sa : Something +>{ test: 'hi', arg: a } : { test: string; arg: A; } +>test : string +>'hi' : "hi" +>arg : A +>a : A + + sa = { test: 'bye', arg: a, arr: a } // excess +>sa = { test: 'bye', arg: a, arr: a } : { test: string; arg: A; arr: A; } +>sa : Something +>{ test: 'bye', arg: a, arr: a } : { test: string; arg: A; arr: A; } +>test : string +>'bye' : "bye" +>arg : A +>a : A +>arr : A +>a : A +} + diff --git a/tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts b/tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts new file mode 100644 index 0000000000..136231d654 --- /dev/null +++ b/tests/cases/conformance/types/conditional/conditionalTypesExcessProperties.ts @@ -0,0 +1,10 @@ +type Something = { test: string } & (T extends object ? { + arg: T +} : { + arg?: undefined + }); + +function testFunc2(a: A, sa: Something) { + sa = { test: 'hi', arg: a }; // not excess (but currently still not assignable) + sa = { test: 'bye', arg: a, arr: a } // excess +}