Adapt plugin creation/initialization so that it works with TS 2.2.2 out of the box

[egamma]

A summary of the changes can be found in this comment.

[angelozerr]

@Istar-Eldritch I would like to accept this PR, even if it breaks our tests. Are you OK?

[Istar-Eldritch]

@angelozerr The test basically ensure backwards compatibility with tsserver-plugin, it requires the plugin to export a function create. If this is no longer a requirement feel free to blow them up.

[angelozerr]

The test basically ensure backwards compatibility with tsserver-plugin

I have aligned tsserver-plugins with TypeScript 2.2.1, so now create function is not required. Thanks for the info.

Adding a comment why this code is duplicated would make me feel better ;-)

Yes sure!

[angelozerr]

Thanks @egamma!

I will adapt your work to fix tests and uses registerCodeFix

[egamma]

@angelozerr I've tested the latest from VS Code. With the change to use registeredCodeFix I don't get quick fix proposal for tslint errors inside VS Code.

I appears that Eclipse doesn't use the sequence of requests for getting quick fixes.

Here is an edited trace from VS Code:

Info 24   request: {"seq":4,"type":"request","command":"getSupportedCodeFixes","arguments":null}
Info 26   response: {"seq":0,"type":"response","command":"getSupportedCodeFixes","request_seq":4,"success":true,"body":["2304","2339","2377","2420","2503","2515","2653","2663","2686","2689","6133","6138","17009"]}
...
Info 39   request: {"seq":8,"type":"request","command":"getCodeFixes","arguments":{"file":"c:/Users/egamma/Projects/tries/ls-plugin/test-plugin/hello.ts","startLine":13,"endLine":13,"startOffset":17,"endOffset":24,"errorCodes":[2515]}}
Info 40   tslint-language-service getCodeFixes 2515

Notice that the VS Code first makes a getSupportedCodeFixes request. This request returns the error codes which have quick fixes. Only if an error code has a quick fix, VS Code sends the requrest getCodeFixes. I haven't figured out why 6999 isn't returned as a supported code fix, since you have registered.

Obviously changing the tslint error code to one returned by getSupportedCodeFixes will make the quick fix appear.

[angelozerr]

Thanks for the feedback @egamma. I will try with VSCode to discover the problem.

[egamma]

@angelozerr I don't know your development setup, but do you have a setup where you can debug the language server/plugin? If not I can describe how I do it with VS Code.

[Istar-Eldritch]

@egamma I think we should do more serious unit testing. I still don't know what is going on in the plugin internals, but I would like to "emulate" the calls from tsserver instead of doing manual Q.A.

[egamma]

@Istar-Eldritch getting more unit testing coverage is the right thing to do.

However, one issue is that the TS protocol is not fully specified, while the individual request/responses are documented in protocol.ts, the request/response sequence isn't. The best you can do for now is to consider vscode as a reference implementation and trace its sequence to the server.

[egamma]

@angelozerr here is what happens.

The registerCodeFix call from the plugin works properly and registers the code fix. However, the issue is that the VS Code client makes a request to get the 'supportedCodeFixes' only once during initialization and stores the result. Then later it only makes a server request for getFixeswhen an error code is a supported error code. Now the problem is that errorCode 6999 is registered after the VS Code client made the call to getSupportedCodeFixes, so it is registered too late and VS Code will never request getFixes for it.

[egamma]

The other option is to not use caching of supported error codes in VS Code, but since getQuickFixes is called in VS Code on each cursor change, this is on a critical path and the caching optimization makes good sense.

[angelozerr]

The other option is to not use caching of supported error codes in VS Code

I cache it too, but I call getSupportedCodeFixes only when I need it, that's to say for the first quick fix to display.

I see VSCode call getSupportedCodeFixes in the constructor https://github.com/Microsoft/vscode/blob/master/extensions/typescript/src/features/codeActionProvider.ts#L34

But why not calling getSupportedCodeFixes just only one at https://github.com/Microsoft/vscode/blob/master/extensions/typescript/src/features/codeActionProvider.ts#L86

[angelozerr]

@egamma I think we should do more serious unit testing. I still don't know what is going on in the plugin internals, but I would like to "emulate" the calls from tsserver instead of doing manual Q.A.

+1

Perhaps we should do the same thing than TypeScript which does some tsserver test (with protocol), not sure with that.

So I really think there needs to be supported API from TS to register quick fixes, the only other work around that comes to mind is to the hack to reuse an existing error code with code fixes...

Do you want I use existing error code like you have fone for the moment? Perhaps my suggestion https://github.com/angelozerr/tslint-language-service/pull/13#issuecomment-295334208 could resolve the problem? Tell me.

[egamma]

@angelozerr your suggestion from above makes sense and I'll discuss it with @mjbvz he owns the TypeScript integration. In the meantime I suggest to do the existing error code work around. It enables to do some quick fix testing.

We should also start to think about what is needed to be done before we invite early adopters to try the tslint-language plugin from VS Code. I'll file a separate issue for this #15.