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

Use in-memory program to generate strict diagnostics for plugin #62

Closed
wants to merge 6 commits into from

Conversation

hnra
Copy link
Contributor

@hnra hnra commented Dec 15, 2023

The PR changes the strategy used to generate the strict diagnostics in the plugin by keeping an in-memory version of the TypeScript program instead of mutating the existing program used by the language service.

The in-memory version is a SemanticDiagnosticsBuilderProgram which the same type used in the TypeScript API example "Writing an incremental program watcher". This program type is partially internal since it requires that SourceFiles produced by the compiler host are versioned (which is an internal property). Luckily the language service produces versioned source files of the files currently being edited. I have only found one case where this does not happen which is when importing from reference projects, in particular it seems the language service does not keep track of the declaration files of referenced projects. In this case we fallback to the standard compiler host and generate a file version using a hashing algorithm which is also used as a fallback in the TypeScript repo.

I have tried to document assumptions made. This PR does not contain any tests and I have not looked at the non-plugin code to see if there is any possibility for code sharing. I have mainly implemented this to resolve #61 for a project where I introduced this plugin (and would be very sad to lose it).

Fixes: #61

@KostkaBrukowa
Copy link
Collaborator

I've released beta version with this code https://www.npmjs.com/package/typescript-strict-plugin?activeTab=readme and I will test on my repo if this works. Thanks for contribution

@KostkaBrukowa
Copy link
Collaborator

Unfortunately with this change I have this view in my VSCode. Tested with [email protected] and [email protected]
image

@hnra
Copy link
Contributor Author

hnra commented Dec 29, 2023

Do you have any minimal repro steps? I tried the version you released the follow way:

npm init -y
npm install [email protected] [email protected] @types/node

tsconfig.json:

{
    "compilerOptions": {
        "strict": false,
        "plugins": [
            {
                "name": "typescript-strict-plugin"
            }
        ]
    }
}

index.ts:

import { resolve } from "path";

let foo: number | undefined = 42;
foo = undefined;

resolve(".");

Then:

  1. Open VSCode (1.85.0) using code .
  2. Select TypeScript Version > Use Workspace Version 4.8.4

Result: No errors as expected. If I remove | undefined I get a strictness error. If I add // @ts-strict-ignore the error is ignored.

I have not tried the released version yet on our 100k+ LOC repo due to Christmas holidays so can't comment on that yet.

@KostkaBrukowa
Copy link
Collaborator

I've tried on my 100k+ LOC project, so I don't have repro steps, but I will try to find how to reproduce it

@KostkaBrukowa
Copy link
Collaborator

Ok, i think I found it. I think that now the plugin reports errors from all of the files in the project.
From you example you can add index2.ts with simillar file contents (but different lines)

import { resolve } from "path";
// 
//
let foo2: number | undefined = 42;
foo2 = undefined;

resolve(".");

and then, in index.ts file there would be an error

@hnra
Copy link
Contributor Author

hnra commented Dec 29, 2023

So I added the a file index2.ts with the same contents and changed index.ts such that it produces an error (removing | undefined), but I do not get the same behavior (see screenshot). Is the issue reproducible with the vanilla version of latest vscode?

However, the issue you are having should occur if getSourceFile returns undefined since that will result in getSemanticDiagnostics returning all project errors. I should add some robustness and logging around these parts, e.g. fallback to the language service if getSourceFile returns undefined. But if it's returning undefined for index2.ts then I'm guessing something has gone wrong with either createSemanticDiagnosticsBuilderProgram or the creation of the compiler host.

Screenshot from 2023-12-29 13-04-41

@KostkaBrukowa
Copy link
Collaborator

It's my VSCode view. You can
image
image
My package.json

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "dependencies": {
    "typescript": "5.3.3",
    "typescript-strict-plugin": "2.2.2-beta.1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

@hnra
Copy link
Contributor Author

hnra commented Dec 29, 2023

Our project setup looks the same and we have the same version of VSCode. Maybe due to OS (Linux vs macOS) or how vscode is setup? FWIW I cannot reproduce the issue in neovim (using typescript-tools) either.

@KostkaBrukowa
Copy link
Collaborator

I'm using macOS and I can reproduce it in neovim using typescript-tools as well :(
Very weird. I will try to debug what is happening inside the plugin itself

@KostkaBrukowa
Copy link
Collaborator

Hmmm... In my case I'm always getting SourceFile as undefined

Info 85   [13:27:37.275] [typescript-strict-plugin]: Test SourceFile: undefined, Path: /Users/.../Documents/Work/test/index.ts, languageVersionOrOptions: undefined
Info 86   [13:27:37.275] [typescript-strict-plugin]: Could not obtain SourceFile from language service and 'languageVerisionOrOptions' is not set for path '/Users/.../Documents/Work/test/index.ts'
Info 87   [13:27:37.275] [typescript-strict-plugin]: Could not obtain SourceFile for file with path '/Users/.../Documents/Work/test/index.ts'

@heyimalex
Copy link
Contributor

I'm seeing the same behavior and logs for for the beta version on on mac. For anyone else who maybe wants to contribute:

A plugin is really a PluginModuleFactory. The interface works through decorating a LanguageService through a create function which gets passed a PluginCreateInfo.

Just thinking about this idea of maintaining two parallel language services and switching between diagnostics in the proxy. The LanguageService interface seems like it only has getters, so we can't just proxy calls on the language service to another instance, intercept compiler options calls to add strict or something. Looking at createLanguageService maybe we could do that through LanguageServiceHost though? Seems like that's how extensions interact with the language service. So something like:

function create(info) {
  // Proxy LanguageServiceHost to override compilation settings.
  const strictHost = {
    ...info.languageServiceHost,
    getCompilationSettings: () => ({
      ...info.languageServiceHost.getCompilcationSettings(),
      strict: true,
    }),
  };
  const strictLanguageServer = ts.createLanguageServer(strictHost);

  proxy.getSemanticDiagnostics = function (filePath) {
    const strictFile = new PluginStrictFileChecker(info).isFileStrict(filePath);

    if (strictFile) {
      return strictLanguageServer.getSemanticDiagnostics(filePath);
    } else {
      return info.languageService.getSemanticDiagnostics(filePath);
    }
  };
  // Maybe also forward cleanupSemanticCache and dispose calls.
}

Just spitballing. Will play around with it more if I've got time.

@heyimalex
Copy link
Contributor

heyimalex commented Jan 24, 2024

Tested my approach in a branch here. Needs some more work, but looking promising! Tested in my work repo and it's working well.

Screenshot 2024-01-23 at 8 18 49 PM

@hnra
Copy link
Contributor Author

hnra commented Jan 30, 2024

Closing this since #67 fixes #61.

@hnra hnra closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin has stopped working with Visual Studio Code
3 participants