Language-service-plugin can cause 'scriptInfo for file ... missing' assertion in tsserver

[egamma]

// CC @mjbvz

The tslint-language-service plugin runs tslint as a language service extension. This works nicely for simple tslint rules, but for rules that require the typechecker the tsserver ends up hanging with a 'scriptInfo for file ... lib/lib.d.ts' is missing assertion.

This could be an issue in tslint (the rules that use the type checker work fine on the command line) or this can be an issue in the tsserver.

Here are the steps to reproduce (update: does not repro on Linux, repos on Windows):

• enable debugging of the tsserver so that we can attach a debugger later set TSS_DEBUG=5859
• enable tracing set TSS_LOG=-level verbose -file c:\tmp\tsserver.log
• git clone https://github.com/egamma/test-ts-server-plugin.git
• cd test-ts-server-plugin
• npm install (installs tslint, ts-language-service plugin, and the TS insider version)
• code-insiders .
• switch to the workspace version of TS using the version switcher in the bottom right corner
• open hello.ts, there is a tslint warning with a quick fix and all is good, you can make edits in the file
  
• open tslint.json and uncomment the no-unused-variable rule on line 6. Since tslint 5 this rule requires the TS type checker, see the breaking changes section in the tslint release notes.
• save and restart code-insiders or run reload window
• open hello.ts
• you will still see tslint warning
• change the contents of hello.ts to trigger a syntax check ->the tsserver is now no longer responding to requests, e.g. no more hovers. You can also verify this by inspecting the trace in c:\tmp\tsserver.log.

Debugging

To debug the current server state you can attach the debugger from another VS Code version. In this setup I describe using the vscode stable version.

• open code (the stable version) on an empty workspace.
• create the following launch configuration

{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "node",
            "request": "attach",
            "name": "Attach to TS Server",
            "port": 5859,
            "sourceMaps": true,
            "protocol": "legacy",
			"outFiles": [
				"${workspaceRoot}/out/**/*.js"
			]
        }
    ]
}

• attach the debugger and you will notice that the tsserver stopped in qn assertion failure script info is missing on a debugger statement:
  

[egamma]

@mjbvz some updates.

The assertion failure is caused by a casing issue in the file paths. The file path is looked up with:
c:/Users/egamma/Projects/tries/ls-plugin/tmp2/test-ts-server-plugin/node_modules/typescript/lib/lib.d.ts
but it is stored with all lower case in the sourceInfo map.

The assertion failure does not occur when the program is run on Linux.

The interesting question is how can running tslint in the tsserver have this side effect.

[egamma]

@RyanCavanaugh
Some more insights.

Tslint is invoked from the extension as follows:

let tslint = new linter(options, oldLS.getProgram());

It gets passed in the program with the intent to reuse Program inside tslint when validating rules. What happens is that running the noUsedVariable rule has side effects on the Program. After running tslint the SourceFileObjects.path attribute has changed from all lower case to case sensitive.
See the debugging snapshots below.

Before:

After:

As a consequence the ScriptInfo look up through the FileNameToScriptInfo map files since the key of the FileNameToScriptInfo is all lower case.

The rule's implementation creates a new Program without passing in the Program it has received as an argument. When constructing the Program then it creates a fake Host. I can make the problem go away by patching getCanonicalFileName with at this line with a function that returns the file as lower case:

getCanonicalFileName: function (f) { return f.toLowerCase(); }

Questions:

• the fakt that the fake host has a side effect on the existing Program is scary, is there a way to make this more safe in the TS language API?
• what would be the proper fix for in tslint for the noUnusedVariable tslint rule 9does the implementation really need to create another program, reanalyze the sources?

The good news is that the noUnusedVariable seems to be the only rule that creates a Program with a fake Host.

[egamma]

The problem with the case sensitivity in the host implementation used by tslint has been addressed in palantir/tslint#2819. I'm closing this issue.