latest plugin (0.1.1.17) is not consistent in marking vulnerability

[arajkumar]

Problem

While editing the file, vulnerability marking disappears!

IJ-0.1.1-17-bug.mov

[arajkumar]

The LSP client[1] which we use implements diagnostics[2] on top of IJ's ExternalAnnotator[3]. On every file content change, LSP client[1] sends onDidChangeTextDocument event and our LSP server[4] implementations handles it and generates diagnostics asynchronously.

Due to the async nature, Whenever a diagnostic arrives from the Language Server, Language Client invokes[5] DaemonCodeAnalyzer::restart to retrigger the ExternalAnnotator|LSPAnnotator to get the diagnostic annotation get populated on the editor. However IJ also invokes ExternalAnnotator|LSPAnnotator on every content change, but this behaviour could cause potential parsing overhead. The excessive re parsing has been recently optimised in IJ[6] and it broke our LSP client's assumption and this caused missing of vulnerability marking.

The immediate workaround would be changing your IJ Settings/Preferences | Editor | Code Editing | Error Highlighting | Autoreparse delay[7] to some higher values. e.g. 1000.

Actual solution would be to increasing the debounce value of our LSP server parsing to higher value[8]. e.g. 2000.

[1] https://github.com/ballerina-platform/lsp4intellij
[2] https://github.com/ballerina-platform/lsp4intellij/blob/28d5a1d0aaeeeacf9025a45b520e4fb718d82c04/src/main/java/org/wso2/lsp4intellij/contributors/annotator/LSPAnnotator.java#L41
[4] https://plugins.jetbrains.com/docs/intellij/syntax-highlighting-and-error-highlighting.html#external-tool
[5] https://github.com/fabric8-analytics/fabric8-analytics-lsp-server/blob/4ea91fd08dc05bc956d10b20d9e51a3dc2ecdcc6/src/server.ts#L329
[6] JetBrains/intellij-community@75fa8ab
[7] https://intellij-support.jetbrains.com/hc/en-us/community/posts/206818675-Change-inspection-timeout-delay
[8] https://github.com/fabric8-analytics/fabric8-analytics-lsp-server/blob/4ea91fd08dc05bc956d10b20d9e51a3dc2ecdcc6/src/server.ts#L330

[jeffmaury]

I understand that the LSP annotator may be called twice but what causes the diagnostics to be cleared ? Don't we have a race condition between the 2 executions ?

[arajkumar]

Here is a flow,

1. On each file content change, LSPAnnotator is invoked to fetch diagnostics and onDidChangeTextDocument is fired from DocumentEventManager.
2. LSPAnnotator doesn't have diagnostics yet, returns empty diagnostics.
3. onDidChangeTextDocument creates diagnostics and calls back to EditorEventManager and it retrigger LSPAnnotator using DaemonCodeAnalyzer::restart, due to recent optimisations in IJ, it never re parses file, so no call to LSPAnnotator again.

@jeffmaury This is what I understood from lsp4intellij.

[TomerFi]

This is still relevant with current versions.

[TomerFi]

I wonder if this would be the same if we switch to use proper custom language plugins instead of the lsp wrapper.

[TomerFi]

I bumped into this issue...

But from a couple of tests I made, it look like this was fixed here with #107 .
So I'm not sure if the aforementioned issue is actually related.