Fix issue #22923 #23050

akhomchenko · 2018-04-01T16:17:32Z

I'm still not sure that isPromiseLike should use isReferenceToType or something more smart (e.g. getPromisedTypeOfPromise to resolve actual types)

mhegazy · 2018-04-02T18:32:51Z

src/compiler/checker.ts

@@ -16409,6 +16409,9 @@ namespace ts {
            if (suggestion !== undefined) {
                errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2, declarationNameToString(propNode), typeToString(containingType), suggestion);
            }
+            else if (isPromiseLike(containingType)) {


call getPromisedTypeOfPromise instead. this will return you the type of the T in Promise<T>. this should handle unions and custom promises, etc.. if it is undefined then it was not a promise.

you also need to check if the property exists on the promisedType. in other words:

var p: Pomise<number>; p.toExponential; // should report error `Property toExponential does not exist on type 'Promise<number>' did you forget `await`?` var x: Promise<string>; p.toExponential; // should not report the `did you forget await` part.

akhomchenko · 2018-04-02T21:00:50Z

So, I've fixed a check plus have dropped tests for Union and Intersection and closed #23058. Should I add any extra tests?

mhegazy · 2018-04-03T20:35:15Z

src/compiler/diagnosticMessages.json

@@ -2000,7 +2000,10 @@
        "category": "Error",
        "code": 2569
    },
-
+    "Property '{0}' does not exist on type '{1}'. Did you forget to await the '{1}'?": {


how about Property '{0}' does not exist on type '{1}'. Did you forget 'await' ?

@DanielRosenwasser can i get your input on this error message as well.

Property '{0}' does not exist on type '{1}'. Did you forget to use 'await'? is probably the best one actually.

Or potentially

Property '{0}' does not exist on type '{1}', but does exist on type '{2}'. Did you forget to use 'await'?

I had this at #23058. So which message?

Property '{0}' does not exist on type '{1}'. Did you forget to use 'await'?

Property '{0}' does not exist on type '{1}', but does exist on type '{2}'. Did you forget to use 'await'?

mhegazy · 2018-04-03T20:36:43Z

src/compiler/checker.ts

            if (suggestion !== undefined) {
                errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2, declarationNameToString(propNode), typeToString(containingType), suggestion);
            }
+            else if (promisedType && getPropertyOfType(promisedType, propNode.escapedText)) {


nit, consider moving const promisedType = getPromisedTypeOfPromise(containingType); inside an else block for if (suggestion !== undefined). it just makes sure we only compute it if we need it.

DanielRosenwasser

Close to perfect. Can you also add the following to your tests?

try accessing a string property on string | Promise<string>
use an element access to access a property on the awaited type.

DanielRosenwasser · 2018-04-04T16:26:29Z

src/compiler/checker.ts

@@ -16410,7 +16410,13 @@ namespace ts {
                errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2, declarationNameToString(propNode), typeToString(containingType), suggestion);
            }
            else {
-                errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1, declarationNameToString(propNode), typeToString(containingType));
+                const promisedType = getPromisedTypeOfPromise(containingType);


I would say that you should do the promise check before any sort of spelling checks because if a property exists on the awaited type, that's almost always a preferable suggestion.

akhomchenko · 2018-04-04T20:08:21Z

try accessing a string property on string | Promise

this is clear

use an element access to access a property on the awaited type.

What do you mean by element access? Something like foo.bar?

mhegazy · 2018-04-04T21:51:23Z

element access is a[expression], property access is a.name

akhomchenko · 2018-04-05T19:55:14Z

Ok, I've updated a message and have added the case for string | Promise<string>. Latest results in:

!!! error TS2339: Property 'toLowerCase' does not exist on type 'string | Promise<string>'.
!!! error TS2339:   Property 'toLowerCase' does not exist on type 'Promise<string>'.

As for element access I can not get what should I test. E.g.

function f(x: Promise<string>) {
    return x["toString"]();
}

will not emit even an old error, as they are in different branches - checkPropertyAccessExpression and checkIndexedAccess. No?

Or do you mean something like this?

function f(x: Promise<string>, y: any) {
    return y[x.toString()];
}

DanielRosenwasser · 2018-04-06T01:56:25Z

I meant something like x["toLowerCase"]() which should use the same error handling branch if I'm not mistaken.

mhegazy · 2018-04-06T16:31:21Z

I meant something like x"toLowerCase" which should use the same error handling branch if I'm not mistaken

there is no error here for using a property that does not exist. the only error is under --noImplicitAny. so do not think this is an issue.

mhegazy · 2018-04-06T16:31:56Z

thanks @gagoman!

akhomchenko · 2018-04-06T16:40:22Z

Yes, I've tested this case. It is not covered by this branch.

akhomchenko force-pushed the fix/22923 branch 2 times, most recently from d789ce0 to 49a2599 Compare April 1, 2018 20:20

akhomchenko mentioned this pull request Apr 2, 2018

Fix issue #22923: more sofisticated #23058

Closed

mhegazy reviewed Apr 2, 2018

View reviewed changes

akhomchenko force-pushed the fix/22923 branch from 49a2599 to 828c00a Compare April 2, 2018 20:58

mhegazy reviewed Apr 3, 2018

View reviewed changes

akhomchenko force-pushed the fix/22923 branch from 828c00a to 457fb64 Compare April 3, 2018 22:19

DanielRosenwasser requested changes Apr 4, 2018

View reviewed changes

Fix issue microsoft#22923

134b341

akhomchenko force-pushed the fix/22923 branch from 457fb64 to 134b341 Compare April 5, 2018 19:39

mhegazy merged commit da31239 into microsoft:master Apr 6, 2018

akhomchenko deleted the fix/22923 branch April 6, 2018 16:40

microsoft locked and limited conversation to collaborators Jul 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue #22923 #23050

Fix issue #22923 #23050

akhomchenko commented Apr 1, 2018 •

edited

Loading

mhegazy Apr 2, 2018

akhomchenko commented Apr 2, 2018 •

edited

Loading

mhegazy Apr 3, 2018

mhegazy Apr 3, 2018

DanielRosenwasser Apr 4, 2018

DanielRosenwasser Apr 4, 2018

akhomchenko Apr 4, 2018

mhegazy Apr 3, 2018

DanielRosenwasser left a comment

DanielRosenwasser Apr 4, 2018

akhomchenko Apr 4, 2018

akhomchenko commented Apr 4, 2018 •

edited

Loading

mhegazy commented Apr 4, 2018

akhomchenko commented Apr 5, 2018 •

edited

Loading

DanielRosenwasser commented Apr 6, 2018 •

edited

Loading

mhegazy commented Apr 6, 2018

mhegazy commented Apr 6, 2018

akhomchenko commented Apr 6, 2018

Fix issue #22923 #23050

Fix issue #22923 #23050

Conversation

akhomchenko commented Apr 1, 2018 • edited Loading

Choose a reason for hiding this comment

akhomchenko commented Apr 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akhomchenko commented Apr 4, 2018 • edited Loading

mhegazy commented Apr 4, 2018

akhomchenko commented Apr 5, 2018 • edited Loading

DanielRosenwasser commented Apr 6, 2018 • edited Loading

mhegazy commented Apr 6, 2018

mhegazy commented Apr 6, 2018

akhomchenko commented Apr 6, 2018

akhomchenko commented Apr 1, 2018 •

edited

Loading

akhomchenko commented Apr 2, 2018 •

edited

Loading

akhomchenko commented Apr 4, 2018 •

edited

Loading

akhomchenko commented Apr 5, 2018 •

edited

Loading

DanielRosenwasser commented Apr 6, 2018 •

edited

Loading