From efa490eb164fbb4addcf774c62eab9d811e9b975 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 16:20:39 -0700 Subject: [PATCH 1/4] Detect weak type errors with primitive sources --- src/compiler/checker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cb855969ea8fc..4485b7857a36e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9427,7 +9427,7 @@ namespace ts { function hasCommonProperties(source: Type, target: Type) { const isComparingJsxAttributes = !!(source.flags & TypeFlags.JsxAttributes); for (const prop of getPropertiesOfType(source)) { - if (isKnownProperty(target, prop.name, isComparingJsxAttributes)) { + if (isKnownProperty(target, prop.name, isComparingJsxAttributes, /*skipGlobalObject*/ true)) { return true; } } @@ -14221,12 +14221,12 @@ namespace ts { * @param name a property name to search * @param isComparingJsxAttributes a boolean flag indicating whether we are searching in JsxAttributesType */ - function isKnownProperty(targetType: Type, name: string, isComparingJsxAttributes: boolean): boolean { + function isKnownProperty(targetType: Type, name: string, isComparingJsxAttributes: boolean, skipGlobalObject?: boolean): boolean { if (targetType.flags & TypeFlags.Object) { const resolved = resolveStructuredTypeMembers(targetType); if (resolved.stringIndexInfo || resolved.numberIndexInfo && isNumericLiteralName(name) || - getPropertyOfType(targetType, name) || + (skipGlobalObject ? getPropertyOfObjectType(targetType, name) : getPropertyOfType(targetType, name)) || isComparingJsxAttributes && !isUnhyphenatedJsxName(name)) { // For JSXAttributes, if the attribute has a hyphenated name, consider that the attribute to be known. return true; From b509e681c1d2580c3c73093cbee1fd0a6db4853c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 16:20:58 -0700 Subject: [PATCH 2/4] Test weak type errors with primitives --- .../reference/generatorTypeCheck63.errors.txt | 26 ++++++++++--------- tests/baselines/reference/weakType.errors.txt | 21 ++++++++++++--- tests/baselines/reference/weakType.js | 11 +++++++- tests/cases/compiler/weakType.ts | 6 ++++- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/tests/baselines/reference/generatorTypeCheck63.errors.txt b/tests/baselines/reference/generatorTypeCheck63.errors.txt index ed2ed36a438da..c08635e8b730b 100644 --- a/tests/baselines/reference/generatorTypeCheck63.errors.txt +++ b/tests/baselines/reference/generatorTypeCheck63.errors.txt @@ -1,11 +1,12 @@ -tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(24,14): error TS2322: Type '(a: State | 1) => IterableIterator' is not assignable to type 'Strategy'. - Type 'IterableIterator' is not assignable to type 'IterableIterator'. - Type 'State | 1' is not assignable to type 'State'. - Type '1' is not assignable to type 'State'. +tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(24,61): error TS2345: Argument of type '(state: State) => IterableIterator' is not assignable to parameter of type '(a: StrategicState) => IterableIterator'. + Type 'IterableIterator' is not assignable to type 'IterableIterator'. + Type 'State | 1' is not assignable to type 'StrategicState'. + Type '1' has no properties in common with type 'StrategicState'. tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(29,70): error TS7025: Generator implicitly has type 'IterableIterator' because it does not yield any values. Consider supplying a return type. tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(32,42): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly. Type argument candidate 'State' is not a valid type argument because it is not a supertype of candidate '1'. -tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(36,14): error TS2322: Type '(a: State | 1) => IterableIterator' is not assignable to type 'Strategy'. +tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(36,62): error TS2345: Argument of type '(state: State) => IterableIterator' is not assignable to parameter of type '(a: StrategicState) => IterableIterator'. + Type 'IterableIterator' is not assignable to type 'IterableIterator'. ==== tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts (4 errors) ==== @@ -33,11 +34,11 @@ tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(36,14): err } export const Nothing: Strategy = strategy("Nothing", function* (state: State) { - ~~~~~~~ -!!! error TS2322: Type '(a: State | 1) => IterableIterator' is not assignable to type 'Strategy'. -!!! error TS2322: Type 'IterableIterator' is not assignable to type 'IterableIterator'. -!!! error TS2322: Type 'State | 1' is not assignable to type 'State'. -!!! error TS2322: Type '1' is not assignable to type 'State'. + ~~~~~~~~ +!!! error TS2345: Argument of type '(state: State) => IterableIterator' is not assignable to parameter of type '(a: StrategicState) => IterableIterator'. +!!! error TS2345: Type 'IterableIterator' is not assignable to type 'IterableIterator'. +!!! error TS2345: Type 'State | 1' is not assignable to type 'StrategicState'. +!!! error TS2345: Type '1' has no properties in common with type 'StrategicState'. yield 1; return state; }); @@ -55,8 +56,9 @@ tests/cases/conformance/es6/yieldExpressions/generatorTypeCheck63.ts(36,14): err }); export const Nothing3: Strategy = strategy("Nothing", function* (state: State) { - ~~~~~~~~ -!!! error TS2322: Type '(a: State | 1) => IterableIterator' is not assignable to type 'Strategy'. + ~~~~~~~~ +!!! error TS2345: Argument of type '(state: State) => IterableIterator' is not assignable to parameter of type '(a: StrategicState) => IterableIterator'. +!!! error TS2345: Type 'IterableIterator' is not assignable to type 'IterableIterator'. yield state; return 1; }); \ No newline at end of file diff --git a/tests/baselines/reference/weakType.errors.txt b/tests/baselines/reference/weakType.errors.txt index 7064ace5daaaa..441ef70ac1674 100644 --- a/tests/baselines/reference/weakType.errors.txt +++ b/tests/baselines/reference/weakType.errors.txt @@ -1,11 +1,14 @@ -tests/cases/compiler/weakType.ts(31,18): error TS2559: Type '{ error?: number; }' has no properties in common with type 'ChangeOptions'. -tests/cases/compiler/weakType.ts(56,5): error TS2322: Type '{ properties: { wrong: string; }; }' is not assignable to type 'Weak & Spoiler'. +tests/cases/compiler/weakType.ts(16,13): error TS2559: Type '12' has no properties in common with type 'Settings'. +tests/cases/compiler/weakType.ts(17,13): error TS2559: Type '"completely wrong"' has no properties in common with type 'Settings'. +tests/cases/compiler/weakType.ts(18,13): error TS2559: Type 'false' has no properties in common with type 'Settings'. +tests/cases/compiler/weakType.ts(35,18): error TS2559: Type '{ error?: number; }' has no properties in common with type 'ChangeOptions'. +tests/cases/compiler/weakType.ts(60,5): error TS2322: Type '{ properties: { wrong: string; }; }' is not assignable to type 'Weak & Spoiler'. Type '{ properties: { wrong: string; }; }' is not assignable to type 'Weak'. Types of property 'properties' are incompatible. Type '{ wrong: string; }' has no properties in common with type '{ b?: number; }'. -==== tests/cases/compiler/weakType.ts (2 errors) ==== +==== tests/cases/compiler/weakType.ts (5 errors) ==== interface Settings { timeout?: number; onError?(): void; @@ -16,10 +19,20 @@ tests/cases/compiler/weakType.ts(56,5): error TS2322: Type '{ properties: { wron } function doSomething(settings: Settings) { /* ... */ } - // forgot to call `getDefaultSettings` // but it is not caught because we don't check for call signatures doSomething(getDefaultSettings); + // same for arrow expressions: + doSomething(() => { }); + doSomething(12); + ~~ +!!! error TS2559: Type '12' has no properties in common with type 'Settings'. + doSomething('completely wrong'); + ~~~~~~~~~~~~~~~~~~ +!!! error TS2559: Type '"completely wrong"' has no properties in common with type 'Settings'. + doSomething(false); + ~~~~~ +!!! error TS2559: Type 'false' has no properties in common with type 'Settings'. // this is an oddly popular way of defining settings // this example is from services/textChanges.ts diff --git a/tests/baselines/reference/weakType.js b/tests/baselines/reference/weakType.js index 6ed3ff1814df9..5637271ccec3a 100644 --- a/tests/baselines/reference/weakType.js +++ b/tests/baselines/reference/weakType.js @@ -9,10 +9,14 @@ function getDefaultSettings() { } function doSomething(settings: Settings) { /* ... */ } - // forgot to call `getDefaultSettings` // but it is not caught because we don't check for call signatures doSomething(getDefaultSettings); +// same for arrow expressions: +doSomething(() => { }); +doSomething(12); +doSomething('completely wrong'); +doSomething(false); // this is an oddly popular way of defining settings // this example is from services/textChanges.ts @@ -65,6 +69,11 @@ function doSomething(settings) { } // forgot to call `getDefaultSettings` // but it is not caught because we don't check for call signatures doSomething(getDefaultSettings); +// same for arrow expressions: +doSomething(function () { }); +doSomething(12); +doSomething('completely wrong'); +doSomething(false); function del(options, error) { if (options === void 0) { options = {}; } if (error === void 0) { error = {}; } diff --git a/tests/cases/compiler/weakType.ts b/tests/cases/compiler/weakType.ts index f04f36c9c2f9e..ffe51205e53c5 100644 --- a/tests/cases/compiler/weakType.ts +++ b/tests/cases/compiler/weakType.ts @@ -8,10 +8,14 @@ function getDefaultSettings() { } function doSomething(settings: Settings) { /* ... */ } - // forgot to call `getDefaultSettings` // but it is not caught because we don't check for call signatures doSomething(getDefaultSettings); +// same for arrow expressions: +doSomething(() => { }); +doSomething(12); +doSomething('completely wrong'); +doSomething(false); // this is an oddly popular way of defining settings // this example is from services/textChanges.ts From 7797b1ddba2db6fa306dc15a56b532e7312d7db5 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 8 Jun 2017 09:01:40 -0700 Subject: [PATCH 3/4] Always use getPropertyOfObjectType in isKnownProperty It doesn't make sense to say that 'toString' is part of a weak type since having 'toString' would mean that the type isn't actually weak. --- src/compiler/checker.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4485b7857a36e..a193c01272adc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9427,7 +9427,7 @@ namespace ts { function hasCommonProperties(source: Type, target: Type) { const isComparingJsxAttributes = !!(source.flags & TypeFlags.JsxAttributes); for (const prop of getPropertiesOfType(source)) { - if (isKnownProperty(target, prop.name, isComparingJsxAttributes, /*skipGlobalObject*/ true)) { + if (isKnownProperty(target, prop.name, isComparingJsxAttributes)) { return true; } } @@ -14214,19 +14214,23 @@ namespace ts { /** * Check if a property with the given name is known anywhere in the given type. In an object type, a property - * is considered known if the object type is empty and the check is for assignability, if the object type has - * index signatures, or if the property is actually declared in the object type. In a union or intersection - * type, a property is considered known if it is known in any constituent type. + * is considered known if + * 1. the object type is empty and the check is for assignability, or + * 2. if the object type has index signatures, or + * 3. if the property is actually declared in the object type + * (this means that 'toString', for example, is not usually a known property). + * 4. In a union or intersection type, + * a property is considered known if it is known in any constituent type. * @param targetType a type to search a given name in * @param name a property name to search * @param isComparingJsxAttributes a boolean flag indicating whether we are searching in JsxAttributesType */ - function isKnownProperty(targetType: Type, name: string, isComparingJsxAttributes: boolean, skipGlobalObject?: boolean): boolean { + function isKnownProperty(targetType: Type, name: string, isComparingJsxAttributes: boolean): boolean { if (targetType.flags & TypeFlags.Object) { const resolved = resolveStructuredTypeMembers(targetType); if (resolved.stringIndexInfo || resolved.numberIndexInfo && isNumericLiteralName(name) || - (skipGlobalObject ? getPropertyOfObjectType(targetType, name) : getPropertyOfType(targetType, name)) || + getPropertyOfObjectType(targetType, name) || isComparingJsxAttributes && !isUnhyphenatedJsxName(name)) { // For JSXAttributes, if the attribute has a hyphenated name, consider that the attribute to be known. return true; From a5b68c0800e27ac898113b6a84969ee83def2332 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 8 Jun 2017 09:07:16 -0700 Subject: [PATCH 4/4] Update tests and baselines --- ...ralFunctionArgContextualTyping2.errors.txt | 24 +++++++++---------- .../tsxSpreadAttributesResolution1.js | 5 ++-- .../tsxSpreadAttributesResolution1.symbols | 3 +-- .../tsxSpreadAttributesResolution1.types | 7 +++--- .../jsx/tsxSpreadAttributesResolution1.tsx | 4 ++-- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/tests/baselines/reference/objectLiteralFunctionArgContextualTyping2.errors.txt b/tests/baselines/reference/objectLiteralFunctionArgContextualTyping2.errors.txt index b967b227809e2..0117d2c0c8c23 100644 --- a/tests/baselines/reference/objectLiteralFunctionArgContextualTyping2.errors.txt +++ b/tests/baselines/reference/objectLiteralFunctionArgContextualTyping2.errors.txt @@ -4,12 +4,12 @@ tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(9,4): error TS Property 'doStuff' is missing in type '{ value: string; }'. tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(10,17): error TS2345: Argument of type '{ value: string; what: number; }' is not assignable to parameter of type 'I2'. Object literal may only specify known properties, and 'what' does not exist in type 'I2'. -tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(11,4): error TS2345: Argument of type '{ toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. - Property 'value' is missing in type '{ toString: (s: any) => any; }'. -tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(12,4): error TS2345: Argument of type '{ toString: (s: string) => string; }' is not assignable to parameter of type 'I2'. - Property 'value' is missing in type '{ toString: (s: string) => string; }'. -tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(13,4): error TS2345: Argument of type '{ value: string; toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. - Property 'doStuff' is missing in type '{ value: string; toString: (s: any) => any; }'. +tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(11,6): error TS2345: Argument of type '{ toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. + Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. +tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(12,6): error TS2345: Argument of type '{ toString: (s: string) => string; }' is not assignable to parameter of type 'I2'. + Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. +tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(13,17): error TS2345: Argument of type '{ value: string; toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. + Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. ==== tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts (6 errors) ==== @@ -33,14 +33,14 @@ tests/cases/compiler/objectLiteralFunctionArgContextualTyping2.ts(13,4): error T !!! error TS2345: Argument of type '{ value: string; what: number; }' is not assignable to parameter of type 'I2'. !!! error TS2345: Object literal may only specify known properties, and 'what' does not exist in type 'I2'. f2({ toString: (s) => s }) - ~~~~~~~~~~~~~~~~~~~~~~ + ~~~~~~~~~~~~~~~~~~ !!! error TS2345: Argument of type '{ toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. -!!! error TS2345: Property 'value' is missing in type '{ toString: (s: any) => any; }'. +!!! error TS2345: Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. f2({ toString: (s: string) => s }) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ~~~~~~~~~~~~~~~~~~~~~~~~~~ !!! error TS2345: Argument of type '{ toString: (s: string) => string; }' is not assignable to parameter of type 'I2'. -!!! error TS2345: Property 'value' is missing in type '{ toString: (s: string) => string; }'. +!!! error TS2345: Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. f2({ value: '', toString: (s) => s.uhhh }) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ~~~~~~~~~~~~~~~~~~~~~~~ !!! error TS2345: Argument of type '{ value: string; toString: (s: any) => any; }' is not assignable to parameter of type 'I2'. -!!! error TS2345: Property 'doStuff' is missing in type '{ value: string; toString: (s: any) => any; }'. \ No newline at end of file +!!! error TS2345: Object literal may only specify known properties, and 'toString' does not exist in type 'I2'. \ No newline at end of file diff --git a/tests/baselines/reference/tsxSpreadAttributesResolution1.js b/tests/baselines/reference/tsxSpreadAttributesResolution1.js index f8c3cfbb5946d..7d83e9bd39067 100644 --- a/tests/baselines/reference/tsxSpreadAttributesResolution1.js +++ b/tests/baselines/reference/tsxSpreadAttributesResolution1.js @@ -7,11 +7,12 @@ class Poisoned extends React.Component<{}, {}> { } } -const obj: Object = {}; +const obj = {}; // OK let p = ; -let y = ; +let y = ; + //// [file.jsx] "use strict"; diff --git a/tests/baselines/reference/tsxSpreadAttributesResolution1.symbols b/tests/baselines/reference/tsxSpreadAttributesResolution1.symbols index 82a4e5cc82271..9b3f535f309b6 100644 --- a/tests/baselines/reference/tsxSpreadAttributesResolution1.symbols +++ b/tests/baselines/reference/tsxSpreadAttributesResolution1.symbols @@ -17,9 +17,8 @@ class Poisoned extends React.Component<{}, {}> { } } -const obj: Object = {}; +const obj = {}; >obj : Symbol(obj, Decl(file.tsx, 8, 5)) ->Object : Symbol(Object, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) // OK let p = ; diff --git a/tests/baselines/reference/tsxSpreadAttributesResolution1.types b/tests/baselines/reference/tsxSpreadAttributesResolution1.types index f8f1cb8d15ad8..26f83c740013d 100644 --- a/tests/baselines/reference/tsxSpreadAttributesResolution1.types +++ b/tests/baselines/reference/tsxSpreadAttributesResolution1.types @@ -18,9 +18,8 @@ class Poisoned extends React.Component<{}, {}> { } } -const obj: Object = {}; ->obj : Object ->Object : Object +const obj = {}; +>obj : {} >{} : {} // OK @@ -28,7 +27,7 @@ let p = ; >p : JSX.Element > : JSX.Element >Poisoned : typeof Poisoned ->obj : Object +>obj : {} let y = ; >y : JSX.Element diff --git a/tests/cases/conformance/jsx/tsxSpreadAttributesResolution1.tsx b/tests/cases/conformance/jsx/tsxSpreadAttributesResolution1.tsx index 6d5225df2d902..a14a7ffe59c88 100644 --- a/tests/cases/conformance/jsx/tsxSpreadAttributesResolution1.tsx +++ b/tests/cases/conformance/jsx/tsxSpreadAttributesResolution1.tsx @@ -11,8 +11,8 @@ class Poisoned extends React.Component<{}, {}> { } } -const obj: Object = {}; +const obj = {}; // OK let p = ; -let y = ; \ No newline at end of file +let y = ;