Diagnostic list flickers annoyingly when opening a document

[erictraut]

In VSCode, if you display all diagnostics for your project (i.e. set the "python.analysis.openFilesOnly" to "false"), you can view the list in the "Problems" panel. If you open a Python file that contains a reported diagnostic the analysis diagnostics for that file briefly disappear and then reappear. This is not only aesthetically unpleasing, it's also disorienting if you just clicked on one of these items to open the file and view an error in the editor.

The problem appears to be that the PLS is reporting diagnostics in two phases: parsing and analysis. When a file is closed, the server sees version -1 of the file, but when it's opened, the server sees version 1. This causes the server to re-parse and re-analyze the file from scratch. (As an aside, this seems wasteful given that the file hasn't changed. Maybe it could be skipped altogether?) When the parse is complete, the server reports the incomplete diagnostics, which don't contain the analysis drags. Then shortly later it reports the full diagnostics. In the meantime, VSCode has updated the diagnostic list in the "Problems" panel.

I don't fully understand the original reason behind updating the parse and analysis errors independently, so I'm not sure of the correct fix. My attempt at a fix is to comment out the following three lines from EditorFile.UpdateParseDiagnostics:

// if (diags != null) {
//     PublishDiagnostics(diags);
// }

This skips the reporting of the partial diagnostics and waits for completion of the analysis. With this change, the annoying flicker, and I haven't noticed any bad side effects.

[MikhailArkhipov]

The parse errors are produced fast and the separate publishing allows users to see syntax issues like missing braces faster as they type. Analysis can take literally tens of minutes to complete. Live syntax check was a very popular request since the only alternative is to run PyLint on save (which is what happens with Jedi completion engine).

And yes, the issue with open/not open files is known, the code is somewhat legacy since it came from VS which is project based into the world where there is no project and just bunch of folders.

[MikhailArkhipov]

So, bottom line is that first reporting syntax errors and then combo of syntax + analysis is inevitable as this is how VS Code works. It has no API for incremental error list population like big VS has and the entire snapshot of diagnostics has to be sent each time. Delaying reporting until analysis completes deprives user from fast appearance of squiggles over syntax errors.

What we can do in the new code base is throttle reporting of syntax errors a bit so if analysis completes within, say, 200-300 ms which is realistic in the new analysis engine, then only combo will be reported thus reducing flicker.

[erictraut]

The problem here is not with VS Code but in the way you're reporting the errors. It's fine to report the errors in two passes — syntax and then analysis. That's how most compilers and language tools work. The problem is that when you run the pass a second time (due to a change in the file), you're discarding all of the previous errors and reporting only the syntax errors for the new version of the file.

What you're doing now is this:

1. Report syntax errors [version 1]
2. Report syntax errors [version 1] + analysis errors [version 1]
3. Report syntax errors [version 2]
   4.Report syntax [version 2] + analysis errors [version 2]

What you should be doing is retaining the analysis errors from the previous pass until they are replaced by new analysis errors. Like this:

1. Report syntax errors [version 1]
2. Report syntax errors [version 1] + analysis errors [version 1]
3. Report syntax errors [version 2] + analysis errors [version 1]
   4.Report syntax [version 2] + analysis errors [version 2]

After step 3, there's a small chance the reported analysis errors will be out of date for a short period (until you get to step 4), but that's not a big deal. It's preferable to the current behavior. And most of the time the analysis errors won't change much (if at all) between steps 3 and 4, so the user won't notice any difference.

[MikhailArkhipov]

We could try that. However, openFilesOnly = true is the only option now. We no longer open and analyze all files in the workspace and the setting has no effect. Therefore difference most probably will be very small since new analysis engine adds very few messages at the moment, like unresolved imports.

[erictraut]

Presumably, the analysis engine will add more diagnostics in the future. Even if you currently support only a few analysis errors at the moment, this change is still important because the annoying and disorienting error behavior will occur after an open file with analysis errors are edited.

[MikhailArkhipov]

So the method below produces odd presentation, may not be desirable. Say, I type import pand. it gets squiggled as unresolved. Then I continue typing but squiggle may remain for a several seconds until analysis finishes. I'd rather take error list update, it is easy to hide it. Persistent stale squiggle is annoying and there is no way to get rid of it.

[erictraut]

I'm surprised to hear that analysis of a single file can take several seconds. It should be at the front of the priority queue in this case. I'd expect it to take tens of milliseconds, or hundreds at most.

Even if it takes a couple of seconds, I still think it's preferable to retain the existing errors until they are replaced. The case you described is an edge case, and it's better to optimize the experience for the more common case.

[MikhailArkhipov]

When dependencies are loaded, yes. In the case above fixing import brings huge subsystem into the view. Its is analysis and analysis of all its dependencies may take 10-20 seconds.