VS Code API compatibility of ProviderResult

[planger]

The vscode-theia-comparator report shows ProviderResult as incompatible on master, but considered it as compatible in the last Theia release.

This seems to be related to change eclipse-theia/theia@e6b57ba, after which it is defined as follows in Theia:

export type ProviderResult<T> = T | undefined | null | Thenable<T | undefined | null>; 

In VS Code it is defined as follows (ie equivalent to how it is now in Theia):

export type ProviderResult<T> = T | undefined | null | Thenable<T | undefined | null>;

Before the change linked above, it was defined as follows in Theia and was deemed OK:

export type ProviderResult<T> = T | undefined | PromiseLike<T | undefined>;

[sgraband]

I can take a look.

[sgraband]

After some investigation i found out that there are some problems with the Thenable/PromiseLike checking in the code.
Currently Thenable is changed to PromiseLike in the parsing code to equal them when comparing later on. For some reason this does not work for the line mentioned above (and probably other instances as well).

In the end the current Theia main version of this line is parsed like it has no types, while all of the other verisons return two types T and PromiseLike (since all Thenables are PromiseLikes at that point). When changing the main line to PromiseLike everything works as expected.
So either:

• There are some problems parsing Thenable from Theia files
  or:
• The parsing is done correctly, but there are some issues while transforming from Thenable to PromiseLike

I guess a fix for this should not be too complicated, but maybe someone with more understanding of the parsing/comparison code (@lucas-koehler or @benoitf) has an idea where to look for. Maybe alongside this we could introduce debug support for the comparator to make debugging this easier?

[lucas-koehler]

Hi @sgraband, unfortunately I don't know in detail where to look at but I can give you some initial pointers.

Some pointer where I'd try to see how it's parsed is in parser.ts#serializeFunction. See

vscode-theia-comparator/src/parser.ts

Line 100 in 389faf3

toHash += memberDoc.type.replace('Thenable', 'PromiseLike');

to where Thenable and PromiseLike are replaced. I assume in the end the hash will be different for the new signature for some reason.
I'd also check around here in the comparator: https://github.com/eclipse-theia/vscode-theia-comparator/blob/master/src/comparator.ts#L234
This compares the hashs of two version entries. Might be a good point to debug to see the difference.

Also, I'm all for adding some debug configs to make investigating issues like this easier :)

[sgraband]

So i was able to debug this a bit further. The issue seems to be that this line:

vscode-theia-comparator/src/parser.ts

Line 146 in 389faf3

const interfaceType = this.checker.getTypeAtLocation(symbol.declarations[0]) as ts.UnionOrIntersectionType;

evaluates to the correct type when parsing from the VSCode file, but evaluates to an any type when parsing the Theia file with the same typing as the VSCode file. As i highly doubt. that this method is not deterministic the issue is probably within the symbol.declarations[0].

What i find strange is that it works for the VSCode file as there seems to be no difference between parsing VSCode API and the Theia API. Or is there something that i am missing?

[sgraband]

Together with @lucas-koehler we were able to find the issue of this. The Thenable type is directly defined in the vscode-files but not in the theia-files. Therefore, the parser cannot get the type and instead returns any. So this is not directly a problem in the comparator, but rather in the API file itself.

IMHO we should not use types, that aren't defined in typescript or the file itself, in the API files.

I would propose to change all Thenables in the API file to PromiseLike as we should not mix between the two of them. @planger WDYT?

We also noticed, that the existing theia Thenables (which all are return types) matched with the vscode methods. This is due to the fact, that methods with any return type will always match. I will open a follow up bug for this (#25).

[planger]

Thanks for investigating this issue!
Yes, I think switching all usages of Thenable to PromiseLike sounds like a good step towards consistency within the API. But best to also raise this issue in the Theia main repository and ask for feedback if anyone has objections.
Once you opened this issue in Theia, please link it here and close this one.
Thanks!

[sgraband]

Opened eclipse-theia/theia#11214 for this. Will close this!