Conversation
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
Implementation looks good. One question about a test and a whole bunch of ideas for more tests.
We're close enough to the 4.3 release that I want to hold this one for 4.4.
tests/baselines/reference/initializerReferencingConstructorParameters.errors.txt
Outdated
Show resolved
Hide resolved
|
|
||
| var str: typeof this.this = ''; | ||
| } | ||
| } |
There was a problem hiding this comment.
please add tests of
- implicitly any usage, like
function f() { let x: typeof this.no = 1 }. (x should have typeany) - implicit any errors for the same code, but when noImplicitThis: true
- The same usage in
function f (this: { no: number }) - usage inside arrow functions, to make sure that the emit is correct. It would be good to have an arrow function inside a container that binds this, like a class, and in one that doesn't, like a module.
There was a problem hiding this comment.
Ooops, this type can not be used in plain functions. 🤷
function Test1() {
let x: typeof this.no = 1
~~~~
!!! error TS2526: A 'this' type is available only in a non-static member of a class or interface.
}
There was a problem hiding this comment.
That error is wrong, then, because this.no is an expression, not a type. The type would be like let x: this['no'].
There was a problem hiding this comment.
I did not notice the differences between this type and this expression. Changing getThisType to checkThisExpression fixed it.
src/compiler/checker.ts
Outdated
|
|
||
| function checkQualifiedName(node: QualifiedName, checkMode: CheckMode | undefined) { | ||
| return checkPropertyAccessExpressionOrQualifiedName(node, node.left, checkNonNullExpression(node.left), node.right, checkMode); | ||
| const leftType = isTypeQueryNode(node.parent) && isThisIdentifier(node.left) ? checkNonNullType(checkThisExpression(node.left), node.left) : checkNonNullExpression(node.left); |
There was a problem hiding this comment.
does this work when the QualifiedName is nested, like this.x.y? I think you need a test for this.
There was a problem hiding this comment.
It doesn't work for this.x.y. I've replaced isTypeQueryNode with isPartOfTypeQuery
src/compiler/utilities.ts
Outdated
| export function isThisInTypeQuery(node: Node): boolean { | ||
| return isThisIdentifier(node) && | ||
| (node.parent.kind === SyntaxKind.TypeQuery || | ||
| (node.parent.kind === SyntaxKind.QualifiedName && (node.parent as QualifiedName).left === node)); |
There was a problem hiding this comment.
the QualifiedName case doesn't check that its ultimate parent is a TypeQuery, so I think you could maybe fool this with this.x in a weird location, or o.this.x The latter should be easy to add a test for; I don't know what test you need for the former.
3 participants
Fixes #1554