From e4ed7f904e43424ea9fac3e2e8f3ac6435949daf Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 30 Mar 2016 15:01:16 -0700 Subject: [PATCH] Address PR comments --- src/compiler/checker.ts | 28 +++++----- src/compiler/commandLineParser.ts | 1 + src/compiler/diagnosticMessages.json | 10 +++- .../noImplicitThisFunctions.errors.txt | 8 +-- .../thisTypeInFunctionsNegative.errors.txt | 53 +++++++++++-------- .../reference/thisTypeInFunctionsNegative.js | 11 ++++ .../thisType/thisTypeInFunctionsNegative.ts | 5 ++ 7 files changed, 76 insertions(+), 40 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9a2a6e1f8e7..e5a57cd96a9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5379,11 +5379,8 @@ namespace ts { function isContextSensitiveFunctionLikeDeclaration(node: FunctionLikeDeclaration) { const areAllParametersUntyped = !forEach(node.parameters, p => p.type); - if (node.kind === SyntaxKind.ArrowFunction) { - return !node.typeParameters && node.parameters.length && areAllParametersUntyped; - } - const hasThisType = node.parameters.length && (node.parameters[0].name).text === "this" && node.parameters[0].type; - return !node.typeParameters && areAllParametersUntyped && !hasThisType; + const isNullaryArrow = node.kind === SyntaxKind.ArrowFunction && !node.parameters.length; + return !node.typeParameters && areAllParametersUntyped && !isNullaryArrow; } function getTypeWithoutSignatures(type: Type): Type { @@ -8072,20 +8069,20 @@ namespace ts { if (signature.thisType) { return signature.thisType; } + if (container.parent && container.parent.kind === SyntaxKind.ObjectLiteralExpression) { + // Note: this works because object literal methods are deferred, + // which means that the type of the containing object literal is already known. + const type = checkExpressionCached(container.parent); + if (type) { + return type; + } + } } if (isClassLike(container.parent)) { const symbol = getSymbolOfNode(container.parent); const type = container.flags & NodeFlags.Static ? getTypeOfSymbol(symbol) : (getDeclaredTypeOfSymbol(symbol)).thisType; return getNarrowedTypeOfReference(type, node); } - if (container.parent && container.parent.kind === SyntaxKind.ObjectLiteralExpression) { - // Note: this works because object literal methods are deferred, - // which means that the type of the containing object literal is already known. - const type = checkExpressionCached(container.parent); - if (type) { - return type; - } - } if (isInJavaScriptFile(node)) { const type = getTypeForThisExpressionFromJSDoc(container); @@ -12401,9 +12398,12 @@ namespace ts { if (indexOf(func.parameters, node) !== 0) { error(node, Diagnostics.this_parameter_must_be_the_first_parameter); } - if (func.kind === SyntaxKind.Constructor) { + if (func.kind === SyntaxKind.Constructor || func.kind === SyntaxKind.ConstructSignature) { error(node, Diagnostics.A_constructor_cannot_have_a_this_parameter); } + if (func.kind === SyntaxKind.SetAccessor) { + error(node, Diagnostics.A_setter_cannot_have_a_this_parameter); + } } // Only check rest parameter type if it's not a binding pattern. Since binding patterns are diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index e418a529386..81d5d190fc7 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -125,6 +125,7 @@ namespace ts { { name: "noImplicitThis", type: "boolean", + description: Diagnostics.Raise_error_on_this_expressions_with_an_implied_any_type, }, { name: "noLib", diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index f0df9c4f9cb..0da7cb72932 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1887,10 +1887,14 @@ "category": "Error", "code": 2680 }, - "'this' implicitly has type 'any' because it does not have a type annotation.": { + "A setter cannot have a 'this' parameter.": { "category": "Error", "code": 2681 }, + "'this' implicitly has type 'any' because it does not have a type annotation.": { + "category": "Error", + "code": 2682 + }, "Import declaration '{0}' is using private name '{1}'.": { "category": "Error", "code": 4000 @@ -2636,6 +2640,10 @@ "category": "Message", "code": 6113 }, + "Raise error on 'this' expressions with an implied 'any' type.": { + "category": "Message", + "code": 6114 + }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", diff --git a/tests/baselines/reference/noImplicitThisFunctions.errors.txt b/tests/baselines/reference/noImplicitThisFunctions.errors.txt index f1742bc758a..c80552c6b37 100644 --- a/tests/baselines/reference/noImplicitThisFunctions.errors.txt +++ b/tests/baselines/reference/noImplicitThisFunctions.errors.txt @@ -1,5 +1,5 @@ -tests/cases/compiler/noImplicitThisFunctions.ts(14,12): error TS2681: 'this' implicitly has type 'any' because it does not have a type annotation. -tests/cases/compiler/noImplicitThisFunctions.ts(18,38): error TS2681: 'this' implicitly has type 'any' because it does not have a type annotation. +tests/cases/compiler/noImplicitThisFunctions.ts(14,12): error TS2682: 'this' implicitly has type 'any' because it does not have a type annotation. +tests/cases/compiler/noImplicitThisFunctions.ts(18,38): error TS2682: 'this' implicitly has type 'any' because it does not have a type annotation. ==== tests/cases/compiler/noImplicitThisFunctions.ts (2 errors) ==== @@ -18,11 +18,11 @@ tests/cases/compiler/noImplicitThisFunctions.ts(18,38): error TS2681: 'this' imp // error: this is implicitly any return this.a + z; ~~~~ -!!! error TS2681: 'this' implicitly has type 'any' because it does not have a type annotation. +!!! error TS2682: 'this' implicitly has type 'any' because it does not have a type annotation. } // error: `this` is `window`, but is still of type `any` let f4: (b: number) => number = b => this.c + b; ~~~~ -!!! error TS2681: 'this' implicitly has type 'any' because it does not have a type annotation. +!!! error TS2682: 'this' implicitly has type 'any' because it does not have a type annotation. \ No newline at end of file diff --git a/tests/baselines/reference/thisTypeInFunctionsNegative.errors.txt b/tests/baselines/reference/thisTypeInFunctionsNegative.errors.txt index c888f4e5ecb..305208ac7d7 100644 --- a/tests/baselines/reference/thisTypeInFunctionsNegative.errors.txt +++ b/tests/baselines/reference/thisTypeInFunctionsNegative.errors.txt @@ -75,29 +75,31 @@ tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(146,1): er tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(148,1): error TS2322: Type '(this: Base2) => number' is not assignable to type '(this: Base1) => number'. tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(154,16): error TS2678: A function that is called with the 'new' keyword cannot have a 'this' type that is void. tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(158,17): error TS2680: A constructor cannot have a 'this' parameter. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(161,30): error TS2679: 'this' parameter must be the first parameter. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(164,26): error TS1003: Identifier expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(164,30): error TS1005: ',' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(165,20): error TS2370: A rest parameter must be of an array type. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(165,23): error TS1003: Identifier expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(165,27): error TS1005: ',' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(166,23): error TS1005: ',' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(166,24): error TS1138: Parameter declaration expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(167,28): error TS1003: Identifier expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(167,32): error TS1005: ',' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,30): error TS1005: ',' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,32): error TS1138: Parameter declaration expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,39): error TS1005: ';' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,40): error TS1128: Declaration or statement expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,42): error TS2304: Cannot find name 'number'. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(168,49): error TS1005: ';' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,1): error TS7027: Unreachable code detected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,29): error TS2304: Cannot find name 'm'. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,32): error TS1005: ';' expected. -tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,35): error TS2304: Cannot find name 'm'. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(160,11): error TS2681: A setter cannot have a 'this' parameter. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(164,9): error TS2680: A constructor cannot have a 'this' parameter. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(166,30): error TS2679: 'this' parameter must be the first parameter. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(169,26): error TS1003: Identifier expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(169,30): error TS1005: ',' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(170,20): error TS2370: A rest parameter must be of an array type. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(170,23): error TS1003: Identifier expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(170,27): error TS1005: ',' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,23): error TS1005: ',' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,24): error TS1138: Parameter declaration expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(172,28): error TS1003: Identifier expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(172,32): error TS1005: ',' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,30): error TS1005: ',' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,32): error TS1138: Parameter declaration expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,39): error TS1005: ';' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,40): error TS1128: Declaration or statement expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,42): error TS2304: Cannot find name 'number'. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(173,49): error TS1005: ';' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(176,1): error TS7027: Unreachable code detected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(176,29): error TS2304: Cannot find name 'm'. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(176,32): error TS1005: ';' expected. +tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(176,35): error TS2304: Cannot find name 'm'. -==== tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts (63 errors) ==== +==== tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts (65 errors) ==== class C { n: number; explicitThis(this: this, m: number): number { @@ -377,6 +379,15 @@ tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts(171,35): e ~~~~~~~~~~~~~~~~~~~~~ !!! error TS2680: A constructor cannot have a 'this' parameter. } + set p(this: void) { + ~~~~~~~~~~ +!!! error TS2681: A setter cannot have a 'this' parameter. + } + } + interface ThisConstructorInterface { + new(this: ThisConstructor, n: number); + ~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2680: A constructor cannot have a 'this' parameter. } function notFirst(a: number, this: C): number { return this.n; } ~~~~~~~ diff --git a/tests/baselines/reference/thisTypeInFunctionsNegative.js b/tests/baselines/reference/thisTypeInFunctionsNegative.js index 8fa5aba6ad2..1cfb38bb100 100644 --- a/tests/baselines/reference/thisTypeInFunctionsNegative.js +++ b/tests/baselines/reference/thisTypeInFunctionsNegative.js @@ -158,6 +158,11 @@ let voidThis = new VoidThis(); class ThisConstructor { constructor(this: ThisConstructor, private n: number) { } + set p(this: void) { + } +} +interface ThisConstructorInterface { + new(this: ThisConstructor, n: number); } function notFirst(a: number, this: C): number { return this.n; } @@ -326,6 +331,12 @@ var ThisConstructor = (function () { function ThisConstructor(n) { this.n = n; } + Object.defineProperty(ThisConstructor.prototype, "p", { + set: function () { + }, + enumerable: true, + configurable: true + }); return ThisConstructor; }()); function notFirst(a, this) { return this.n; } diff --git a/tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts b/tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts index 394b1867490..1b049aeec50 100644 --- a/tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts +++ b/tests/cases/conformance/types/thisType/thisTypeInFunctionsNegative.ts @@ -157,6 +157,11 @@ let voidThis = new VoidThis(); class ThisConstructor { constructor(this: ThisConstructor, private n: number) { } + set p(this: void) { + } +} +interface ThisConstructorInterface { + new(this: ThisConstructor, n: number); } function notFirst(a: number, this: C): number { return this.n; }