Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
egamma opened this issue Apr 24, 2017 · 3 comments
Assignees
Labels
Bug A bug in TypeScript VS Code Tracked There is a VS Code equivalent to this issue

Comments

@egamma
Copy link
Member

egamma commented Apr 24, 2017

// 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
    image
  • 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:
    image
@egamma
Copy link
Member Author

egamma commented Apr 25, 2017

@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.

@mjbvz mjbvz added the VS Code Tracked There is a VS Code equivalent to this issue label Apr 26, 2017
@egamma
Copy link
Member Author

egamma commented Apr 26, 2017

@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:
image

After:
image

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
Copy link
Member Author

egamma commented Sep 26, 2017

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.

@egamma egamma closed this as completed Sep 26, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

5 participants