From f28cf454c0cc9bc254a272d92accf20968c3975d Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 8 Jul 2022 09:50:44 +0200 Subject: [PATCH 1/4] fix problem tab accidentally being cleared due to syntax error diagnostics publishing not accounting for existing compiler errors for current file --- server/src/server.ts | 33 +++++++++++++++++++++++++++------ server/src/utils.ts | 10 +++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 411c4a8f8..87797421e 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -20,6 +20,7 @@ import { assert } from "console"; import { fileURLToPath } from "url"; import { ChildProcess } from "child_process"; import { WorkspaceEdit } from "vscode-languageserver"; +import { filesDiagnostics } from "./utils"; interface extensionConfiguration { askToStartBuild: boolean; @@ -41,6 +42,8 @@ let projectsFiles: Map< { openFiles: Set; filesWithDiagnostics: Set; + filesDiagnostics: filesDiagnostics; + bsbWatcherByEditor: null | ChildProcess; // This keeps track of whether we've prompted the user to start a build @@ -79,8 +82,20 @@ let openCompiledFileRequest = new v.RequestType< void >("rescript-vscode.open_compiled"); +let getDiagnosticsForFile = (fileUri: string): p.Diagnostic[] => { + let diagnostics: p.Diagnostic[] | null = null; + + projectsFiles.forEach((projectFile, _projectRootPath) => { + if (diagnostics == null && projectFile.filesDiagnostics[fileUri] != null) { + diagnostics = projectFile.filesDiagnostics[fileUri].slice(); + } + }); + + return diagnostics ?? []; +}; let sendUpdatedDiagnostics = () => { - projectsFiles.forEach(({ filesWithDiagnostics }, projectRootPath) => { + projectsFiles.forEach((projectFile, projectRootPath) => { + let { filesWithDiagnostics } = projectFile; let content = fs.readFileSync( path.join(projectRootPath, c.compilerLogPartialPath), { encoding: "utf-8" } @@ -91,6 +106,7 @@ let sendUpdatedDiagnostics = () => { codeActions, } = utils.parseCompilerLogOutput(content); + projectFile.filesDiagnostics = filesAndErrors; codeActionsFromDiagnostics = codeActions; // diff @@ -188,6 +204,7 @@ let openedFile = (fileUri: string, fileContent: string) => { projectRootState = { openFiles: new Set(), filesWithDiagnostics: new Set(), + filesDiagnostics: {}, bsbWatcherByEditor: null, hasPromptedToStartBuild: /(\/|\\)node_modules(\/|\\)/.test( projectRootPath @@ -608,17 +625,21 @@ let updateDiagnosticSyntax = (fileUri: string, fileContent: string) => { let tmpname = utils.createFileInTempDir(extension); fs.writeFileSync(tmpname, fileContent, { encoding: "utf-8" }); - let items: p.Diagnostic[] | [] = utils.runAnalysisAfterSanityCheck(filePath, [ - "diagnosticSyntax", - tmpname, - ]); + // We need to account for any existing diagnostics from the compiler for this + // file. If we don't we might accidentally clear the current file's compiler + // diagnostics if there's no syntax diagostics to send. This is because + // publishing an empty diagnostics array is equivalent to saying "clear all + // errors". + let compilerDiagnosticsForFile = getDiagnosticsForFile(fileUri); + let syntaxDiagnosticsForFile: p.Diagnostic[] = + utils.runAnalysisAfterSanityCheck(filePath, ["diagnosticSyntax", tmpname]); let notification: p.NotificationMessage = { jsonrpc: c.jsonrpcVersion, method: "textDocument/publishDiagnostics", params: { uri: fileUri, - diagnostics: items, + diagnostics: [...syntaxDiagnosticsForFile, ...compilerDiagnosticsForFile], }, }; diff --git a/server/src/utils.ts b/server/src/utils.ts index c67fb5fd7..01ed4c8af 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -474,7 +474,7 @@ let parseFileAndRange = (fileAndRange: string) => { }; // main parsing logic -type filesDiagnostics = { +export type filesDiagnostics = { [key: string]: p.Diagnostic[]; }; type parsedCompilerLogResult = { @@ -589,7 +589,7 @@ export let parseCompilerLogOutput = ( code: undefined, severity: t.DiagnosticSeverity.Error, tag: undefined, - content: [lines[i], lines[i+1]], + content: [lines[i], lines[i + 1]], }); i++; } else if (/^ +([0-9]+| +|\.) (│|┆)/.test(line)) { @@ -603,9 +603,9 @@ export let parseCompilerLogOutput = ( // 10 ┆ } else if (line.startsWith(" ")) { // part of the actual diagnostics message - parsedDiagnostics[parsedDiagnostics.length - 1].content.push( - line.slice(2) - ); + parsedDiagnostics[parsedDiagnostics.length - 1].content.push( + line.slice(2) + ); } else if (line.trim() != "") { // We'll assume that everything else is also part of the diagnostics too. // Most of these should have been indented 2 spaces; sadly, some of them From 06eddfea7490b3d7772ca6c26da42db4e089c52e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 8 Jul 2022 09:53:43 +0200 Subject: [PATCH 2/4] name --- server/src/server.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 87797421e..855a7d84a 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -82,7 +82,9 @@ let openCompiledFileRequest = new v.RequestType< void >("rescript-vscode.open_compiled"); -let getDiagnosticsForFile = (fileUri: string): p.Diagnostic[] => { +let getCurrentCompilerDiagnosticsForFile = ( + fileUri: string +): p.Diagnostic[] => { let diagnostics: p.Diagnostic[] | null = null; projectsFiles.forEach((projectFile, _projectRootPath) => { @@ -630,7 +632,8 @@ let updateDiagnosticSyntax = (fileUri: string, fileContent: string) => { // diagnostics if there's no syntax diagostics to send. This is because // publishing an empty diagnostics array is equivalent to saying "clear all // errors". - let compilerDiagnosticsForFile = getDiagnosticsForFile(fileUri); + let compilerDiagnosticsForFile = + getCurrentCompilerDiagnosticsForFile(fileUri); let syntaxDiagnosticsForFile: p.Diagnostic[] = utils.runAnalysisAfterSanityCheck(filePath, ["diagnosticSyntax", tmpname]); From 4fceee873a9ac821c68b154d10ef7873b75e01dc Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 8 Jul 2022 09:54:29 +0200 Subject: [PATCH 3/4] remove irrelevant format change to clean up diff --- server/src/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/utils.ts b/server/src/utils.ts index 01ed4c8af..f8a4bb89a 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -603,9 +603,9 @@ export let parseCompilerLogOutput = ( // 10 ┆ } else if (line.startsWith(" ")) { // part of the actual diagnostics message - parsedDiagnostics[parsedDiagnostics.length - 1].content.push( - line.slice(2) - ); + parsedDiagnostics[parsedDiagnostics.length - 1].content.push( + line.slice(2) + ); } else if (line.trim() != "") { // We'll assume that everything else is also part of the diagnostics too. // Most of these should have been indented 2 spaces; sadly, some of them From eb760255d182f32e2f3b813f839dfb9ff8fe1871 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 8 Jul 2022 09:55:13 +0200 Subject: [PATCH 4/4] remove irrelevant format change to clean up diff --- server/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/utils.ts b/server/src/utils.ts index f8a4bb89a..0e814e470 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -589,7 +589,7 @@ export let parseCompilerLogOutput = ( code: undefined, severity: t.DiagnosticSeverity.Error, tag: undefined, - content: [lines[i], lines[i + 1]], + content: [lines[i], lines[i+1]], }); i++; } else if (/^ +([0-9]+| +|\.) (│|┆)/.test(line)) {