Improve diagnostic story for build and reconcile diagnostics

[dbaeumer]

This issue summaries the current diagnostic / problem marker story and outlines implementation changes to improve todays story.

Current Implementation

The current implementation is based on a diagnostic service which manages diagnostic objects generated by different participants (please note that for the discussion I use the API visible type names. Internally the names might be different). A diagnostic literal has the following attributes:

/**
 * Represents a diagnostic, such as a compiler error or warning. Diagnostic objects
 * are only valid in the scope of a file.
 */
export interface Diagnostic {

    /**
     * The range to which this diagnostic applies.
     */
    range: Range;

    /**
     * The human-readable message.
     */
    message: string;

    /**
     * A human-readable string describing the source of this
     * diagnostic, e.g. 'typescript' or 'super lint'.
     */
    source: string;

    /**
     * The severity, default is [error](#DiagnosticSeverity.Error).
     */
    severity: DiagnosticSeverity;

    /**
     * A code or identifier for this diagnostics. Will not be surfaced
     * to the user, but should be used for later processing, e.g. when
     * providing [code actions](#CodeActionContext).
     */
    code: string | number;
}

In addition to these properties a diagnostic object has an owner which uniquely identifies the owner/producer of a diagnostic literal. In VS Code's API the owner can be specified when the creating a diagnostic collection (see: createDiagnosticCollection(name?: string): DiagnosticCollection). If no name is provided VS Code generates a unique name.

Diagnostic objects are created for a given resource, which is identified by its Uri. In the diagnostic service the diagnostic objects are keyed along two dimensions: the 'uri' and the 'owner'. So you can for example delete all diagnostic objects for a given uri (for example when a file gets deleted) or you can delete diagnostic objects for a given owner across all resources (for example when the producer of these diagnostic object want to clear them all).

The model of a producer allows that different extensions can produce diagnostic objects for resources. ESLint is an example, which when installed validates JavaScript files and produces diagnostics for it. In additional VS Code's internal JavaScript language service produces diagnostic objects as well.

This sound pretty good already: so what is the actual problem with that story. The problem shows up when we look at the lifetime of the diagnostic objects and need to decide when they are valid and when they become invalid.

To illustrate the problem consider the following setup for a JavaScript workspace:

• a task (provided through VS Code's task system) that runs ESLint over the whole project and validates every JavaScript file and produces diagnostic objects for it. The task piggy bags on a gulp file that does the actual eslint execution. The task must be rerun to produce an updated set of diagnostic objects. Conceptually these are build problems.
• an installed ESLint extension (https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) that validates the JavaScript on type. Conceptually these are live/reconciled problems. The content on disk is still different which is especially important for language were the compiler produce output files (e.g. C#, TypeScript).

To keep things easy for now we assume that both ESLint checkers use the same configuration options (e.g. that produce the same set of diagnostics for the same JS file).

The user opens the workspace and does the following:

1. triggers the build via the task system. Errors are reported on files and the errors are navigatable in an error list.
2. selects a file and opens it in the editor. The user expects that the problems reported on the file are immediately visible.
3. the user fixes the problem. User expects that the problem at least disappears from the editor. It is to be discussed what should happen with the error in the error list.
4. the user saves the file and closes it. The error should disappear from the error list as well.

Actually VS Code behaves like this right now. However for the price of being wrong in the following scenarios:

1. Same as above
2. Same as above
3. Closes the file without fixing the problem and removes it from the working set (which we be automatic with the new tab design).

In these steps the errors disappear from the system although they still exist.

Another case tricky case is when the build system produces a different set of diagnostic objects then a language service. Given the different tool chain used for builds and language services this is not a rare scenario. So we at least need to inform the user about this.

Proposed Implementation Changes

The problem could be addressed by only allowing one provider for diagnostics of a given source. In the above example that would mean that either the build diagnostics need to come from the ESLint extension or that the task system produces the reconciled diagnostics and that that provider ensures consistency on the diagnostics. This sounds not very practical given the fact that there are already a lot of projects out there with a build story based on an open tool chain.

Therefore we propose the following implementation changes:

• diagnostic objects carry information whether they got created by a reconciler or a builder.
• if present we 'prefer' reconcile diagnostics over build diagnostics per owner/producer. However we will make it clear to the user that there are build errors as well which are currently filtered.
• when a file gets closed we remove its reconcile diagnostics (if any exist). The interesting question is what to do with existing build errors for that file. If no reconcile errors for the same owner/producer were reported (even not an empty set) then we fully restore the build errors. If reconcile errors were reported for the same owner/producer we will restore the build errors however we will make it clear that they might be outdated (for example by graying the diagnostic icon)

We also discussed whether we can auto merge build and reconcile errors. However given the asynchronicity of VS Code this would require a lot more infrastructure and API (diagnostic would need to carry a document version, we would need to request diagnostics for a specific version, ...). In addition merging is still questionable if the build tool chain is different than the reconcile tool chain.

[dbaeumer]

@alexandrudima please give it a read and comment/edit.

[dbaeumer]

Closing since this got outdated.