From d5ed8d1109e0f7f0d8a112fd2bb58fc0b7704588 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 2 Mar 2024 11:53:06 +0100 Subject: [PATCH 01/14] basic PoC of doing a sort of incremental typechecking --- analysis/src/Cmt.ml | 22 ++-- client/src/extension.ts | 3 +- package-lock.json | 4 +- package.json | 7 +- server/package-lock.json | 4 +- server/package.json | 2 +- server/src/server.ts | 241 ++++++++++++++++++++++++++++++++++++++- 7 files changed, 266 insertions(+), 17 deletions(-) diff --git a/analysis/src/Cmt.ml b/analysis/src/Cmt.ml index 7380c9fec..cf3f7ff94 100644 --- a/analysis/src/Cmt.ml +++ b/analysis/src/Cmt.ml @@ -16,13 +16,21 @@ let fullFromUri ~uri = let moduleName = BuildSystem.namespacedName package.namespace (FindFiles.getName path) in - match Hashtbl.find_opt package.pathsForModule moduleName with - | Some paths -> - let cmt = getCmtPath ~uri paths in - fullForCmt ~moduleName ~package ~uri cmt - | None -> - prerr_endline ("can't find module " ^ moduleName); - None) + let incrementalCmtPath = + package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName ^ ".cmt" + in + match fullForCmt ~moduleName ~package ~uri incrementalCmtPath with + | Some cmtInfo -> + if Debug.verbose () then Printf.printf "[cmt] Found incremental cmt\n"; + Some cmtInfo + | None -> ( + match Hashtbl.find_opt package.pathsForModule moduleName with + | Some paths -> + let cmt = getCmtPath ~uri paths in + fullForCmt ~moduleName ~package ~uri cmt + | None -> + prerr_endline ("can't find module " ^ moduleName); + None)) let fullsFromModule ~package ~moduleName = if Hashtbl.mem package.pathsForModule moduleName then diff --git a/client/src/extension.ts b/client/src/extension.ts index 5e34dae37..24ba8c9d8 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -294,7 +294,8 @@ export function activate(context: ExtensionContext) { if ( affectsConfiguration("rescript.settings.inlayHints") || affectsConfiguration("rescript.settings.codeLens") || - affectsConfiguration("rescript.settings.signatureHelp") + affectsConfiguration("rescript.settings.signatureHelp") || + affectsConfiguration("rescript.settings.incrementalTypechecking") ) { commands.executeCommand("rescript-vscode.restart_language_server"); } else { diff --git a/package-lock.json b/package-lock.json index fa782c666..c907f904b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rescript-vscode", - "version": "1.42.0", + "version": "1.44.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "rescript-vscode", - "version": "1.42.0", + "version": "1.44.0", "hasInstallScript": true, "license": "MIT", "devDependencies": { diff --git a/package.json b/package.json index ef9bc033f..ad93c832b 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "ReScript language support (official)", "author": "ReScript Team", "license": "MIT", - "version": "1.42.0", + "version": "1.44.0", "repository": { "type": "git", "url": "https://github.com/rescript-lang/rescript-vscode" @@ -176,6 +176,11 @@ "default": true, "description": "Enable signature help for function calls." }, + "rescript.settings.incrementalTypechecking": { + "type": "boolean", + "default": true, + "description": "Enable incremental type checking." + }, "rescript.settings.binaryPath": { "type": [ "string", diff --git a/server/package-lock.json b/server/package-lock.json index d177d5509..4e56a7f5c 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -1,12 +1,12 @@ { "name": "@rescript/language-server", - "version": "1.42.0", + "version": "1.44.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@rescript/language-server", - "version": "1.42.0", + "version": "1.44.0", "license": "MIT", "dependencies": { "chokidar": "^3.5.1", diff --git a/server/package.json b/server/package.json index 6052d373b..7d4aad10c 100644 --- a/server/package.json +++ b/server/package.json @@ -1,7 +1,7 @@ { "name": "@rescript/language-server", "description": "LSP server for ReScript", - "version": "1.42.0", + "version": "1.44.0", "author": "ReScript Team", "license": "MIT", "bin": { diff --git a/server/src/server.ts b/server/src/server.ts index 9cd7e68a8..55e626b5f 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -21,11 +21,12 @@ import * as codeActions from "./codeActions"; import * as c from "./constants"; import * as chokidar from "chokidar"; import { assert } from "console"; -import { fileURLToPath } from "url"; -import { ChildProcess } from "child_process"; +import { fileURLToPath, pathToFileURL } from "url"; +import * as cp from "node:child_process"; import { WorkspaceEdit } from "vscode-languageserver"; import { filesDiagnostics } from "./utils"; import { onErrorReported } from "./errorReporter"; +import readline from "readline"; interface extensionConfiguration { allowBuiltInFormatter: boolean; @@ -40,6 +41,7 @@ interface extensionConfiguration { signatureHelp: { enabled: boolean; }; + incrementalTypechecking: boolean; } // This holds client capabilities specific to our extension, and not necessarily @@ -66,6 +68,7 @@ let extensionConfiguration: extensionConfiguration = { signatureHelp: { enabled: true, }, + incrementalTypechecking: true, }; // Below here is some state that's not important exactly how long it lives. let hasPromptedAboutBuiltInFormatter = false; @@ -85,7 +88,7 @@ let projectsFiles: Map< filesWithDiagnostics: Set; filesDiagnostics: filesDiagnostics; - bsbWatcherByEditor: null | ChildProcess; + bsbWatcherByEditor: null | cp.ChildProcess; // This keeps track of whether we've prompted the user to start a build // automatically, if there's no build currently running for the project. We @@ -103,6 +106,8 @@ let codeActionsFromDiagnostics: codeActions.filesCodeActions = {}; // will be properly defined later depending on the mode (stdio/node-rpc) let send: (msg: p.Message) => void = (_) => {}; +let buildNinjaCache: Map]> = new Map(); + let findRescriptBinary = (projectRootPath: p.DocumentUri | null) => extensionConfiguration.binaryPath == null ? lookup.findFilePathFromProjectRoot( @@ -247,6 +252,9 @@ let deleteProjectDiagnostics = (projectRootPath: string) => { }); projectsFiles.delete(projectRootPath); + if (extensionConfiguration.incrementalTypechecking) { + removeIncrementalFileFolder(projectRootPath); + } } }; let sendCompilationFinishedMessage = () => { @@ -290,8 +298,14 @@ let openedFile = (fileUri: string, fileContent: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { + if (extensionConfiguration.incrementalTypechecking) { + cleanUpIncrementalFiles(filePath, projectRootPath); + } let projectRootState = projectsFiles.get(projectRootPath); if (projectRootState == null) { + if (extensionConfiguration.incrementalTypechecking) { + recreateIncrementalFileFolder(projectRootPath); + } projectRootState = { openFiles: new Set(), filesWithDiagnostics: new Set(), @@ -365,6 +379,44 @@ let openedFile = (fileUri: string, fileContent: string) => { // call the listener which calls it } }; +function removeIncrementalFileFolder( + projectRootPath: string, + onAfterRemove?: () => void +) { + fs.rm( + path.resolve(projectRootPath, "lib/bs/___incremental"), + { force: true, recursive: true }, + (_) => { + onAfterRemove?.(); + } + ); +} +function recreateIncrementalFileFolder(projectRootPath: string) { + removeIncrementalFileFolder(projectRootPath, () => { + fs.mkdir(path.resolve(projectRootPath, "lib/bs/___incremental"), (_) => {}); + }); +} +/** + * TODO CMT stuff + * - Make sure cmt is deleted/ignore if original contents is newer. How? + * - Clear incremental folder when starting LSP + * - Optimize recompiles + * - Races when recompiling? + */ +function cleanUpIncrementalFiles(filePath: string, projectRootPath: string) { + [ + path.basename(filePath, ".res") + ".ast", + path.basename(filePath, ".res") + ".cmt", + path.basename(filePath, ".res") + ".cmi", + path.basename(filePath, ".res") + ".cmj", + path.basename(filePath), + ].forEach((file) => { + fs.unlink( + path.resolve(projectRootPath, "lib/bs/___incremental", file), + (_) => {} + ); + }); +} let closedFile = (fileUri: string) => { let filePath = fileURLToPath(fileUri); @@ -372,6 +424,9 @@ let closedFile = (fileUri: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { + if (extensionConfiguration.incrementalTypechecking) { + cleanUpIncrementalFiles(filePath, projectRootPath); + } let root = projectsFiles.get(projectRootPath); if (root != null) { root.openFiles.delete(filePath); @@ -389,10 +444,190 @@ let closedFile = (fileUri: string) => { } } }; +function getBscArgs(projectRootPath: string): Promise> { + let buildNinjaPath = path.resolve(projectRootPath, "lib/bs/build.ninja"); + let cacheEntry = buildNinjaCache.get(buildNinjaPath); + let stat: fs.Stats | null = null; + if (cacheEntry != null) { + stat = fs.statSync(buildNinjaPath); + if (cacheEntry[0] >= stat.mtimeMs) { + return Promise.resolve(cacheEntry[1]); + } + } + return new Promise((resolve, _reject) => { + function resolveResult(result: Array) { + if (stat != null) { + buildNinjaCache.set(buildNinjaPath, [stat.mtimeMs, result]); + } + resolve(result); + } + const fileStream = fs.createReadStream( + path.resolve(projectRootPath, "lib/bs/build.ninja") + ); + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }); + let captureNextLine = false; + let done = false; + let stopped = false; + let captured: Array = []; + rl.on("line", (line) => { + if (stopped) { + return; + } + if (captureNextLine) { + captured.push(line); + captureNextLine = false; + } + if (done) { + fileStream.destroy(); + rl.close(); + resolveResult(captured); + stopped = true; + return; + } + if (line.startsWith("rule astj")) { + captureNextLine = true; + } + if (line.startsWith("rule mij")) { + captureNextLine = true; + done = true; + } + }); + rl.on("close", () => { + resolveResult(captured); + }); + }); +} +function argsFromCommandString(cmdString: string): Array> { + let s = cmdString + .trim() + .split("command = ")[1] + .split(" ") + .map((v) => v.trim()) + .filter((v) => v !== ""); + let args: Array> = []; + + for (let i = 0; i <= s.length - 1; i++) { + let item = s[i]; + let nextItem = s[i + 1] ?? ""; + if (item.startsWith("-") && nextItem.startsWith("-")) { + // Single entry arg + args.push([item]); + } else if (item.startsWith("-") && s[i + 1]?.startsWith("'")) { + let nextIndex = i + 1; + // Quoted arg, take until ending ' + let arg = [s[nextIndex]]; + for (let x = nextIndex + 1; x <= s.length - 1; x++) { + let nextItem = s[x]; + arg.push(nextItem); + if (nextItem.endsWith("'")) { + i = x + 1; + break; + } + } + args.push([item, arg.join(" ")]); + } else if (item.startsWith("-")) { + args.push([item, nextItem]); + } + } + return args; +} +function removeAnsiCodes(s: string): string { + const ansiEscape = /\x1B[@-_][0-?]*[ -/]*[@-~]/g; + return s.replace(ansiEscape, ""); +} +async function compileContents(filePath: string, fileContent: string) { + const projectRootPath = utils.findProjectRootOfFile(filePath); + if (projectRootPath == null) { + console.log("Did not find root project."); + return; + } + let bscExe = findBscExeBinary(projectRootPath); + if (bscExe == null) { + console.log("Did not find bsc."); + return; + } + let fileName = path.basename(filePath); + let incrementalFilePath = path.resolve( + projectRootPath, + "lib/bs/___incremental", + fileName + ); + + fs.writeFileSync(incrementalFilePath, fileContent); + + try { + let [astBuildCommand, fullBuildCommand] = await getBscArgs(projectRootPath); + + let astArgs = argsFromCommandString(astBuildCommand); + let buildArgs = argsFromCommandString(fullBuildCommand); + + let callArgs: Array = []; + + buildArgs.forEach(([key, value]: Array) => { + if (key === "-I") { + callArgs.push("-I", path.resolve(projectRootPath, "lib/bs", value)); + } else if (key === "-bs-v") { + callArgs.push("-bs-v", Date.now().toString()); + } else if (key === "-bs-package-output") { + return; + } else if (value == null || value === "") { + callArgs.push(key); + } else { + callArgs.push(key, value); + } + }); + + astArgs.forEach(([key, value]: Array) => { + if (key.startsWith("-bs-jsx")) { + callArgs.push(key, value); + } else if (key.startsWith("-ppx")) { + callArgs.push(key, value); + } + }); + + callArgs.push("-color", "never"); + + callArgs = callArgs.filter((v) => v != null && v !== ""); + callArgs.push(incrementalFilePath); + + cp.execFile( + bscExe, + callArgs, + { cwd: projectRootPath }, + (_error, _stdout, stderr) => { + // DOCUMENT HERE + if (stderr !== "") { + let { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); + let res = Object.values(result)[0].map((d) => ({ + ...d, + message: removeAnsiCodes(d.message), + })); + let notification: p.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "textDocument/publishDiagnostics", + params: { + uri: pathToFileURL(filePath), + diagnostics: res, + }, + }; + send(notification); + } + } + ); + } catch (e) { + console.error(e); + } +} let updateOpenedFile = (fileUri: string, fileContent: string) => { let filePath = fileURLToPath(fileUri); assert(stupidFileContentCache.has(filePath)); stupidFileContentCache.set(filePath, fileContent); + if (extensionConfiguration.incrementalTypechecking) { + compileContents(filePath, fileContent); + } }; let getOpenedFileContent = (fileUri: string) => { let filePath = fileURLToPath(fileUri); From b76a4d6b1ca5c5821f5aa5f4df99da90634cc914 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 2 Mar 2024 19:27:15 +0100 Subject: [PATCH 02/14] move incremental compilation stuff to its own file --- server/src/config.ts | 45 ++++ server/src/incrementalCompilation.ts | 343 +++++++++++++++++++++++++ server/src/server.ts | 366 +++++---------------------- server/src/utils.ts | 31 +++ 4 files changed, 479 insertions(+), 306 deletions(-) create mode 100644 server/src/config.ts create mode 100644 server/src/incrementalCompilation.ts diff --git a/server/src/config.ts b/server/src/config.ts new file mode 100644 index 000000000..a56ea65e2 --- /dev/null +++ b/server/src/config.ts @@ -0,0 +1,45 @@ +import { Message } from "vscode-languageserver-protocol"; + +export type send = (msg: Message) => void; + +export interface extensionConfiguration { + allowBuiltInFormatter: boolean; + askToStartBuild: boolean; + inlayHints: { + enable: boolean; + maxLength: number | null; + }; + codeLens: boolean; + binaryPath: string | null; + platformPath: string | null; + signatureHelp: { + enabled: boolean; + }; + incrementalTypechecking: { + enabled: boolean; + }; +} + +// All values here are temporary, and will be overridden as the server is +// initialized, and the current config is received from the client. +let config: { extensionConfiguration: extensionConfiguration } = { + extensionConfiguration: { + allowBuiltInFormatter: false, + askToStartBuild: true, + inlayHints: { + enable: false, + maxLength: 25, + }, + codeLens: false, + binaryPath: null, + platformPath: null, + signatureHelp: { + enabled: true, + }, + incrementalTypechecking: { + enabled: true, + }, + }, +}; + +export default config; diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts new file mode 100644 index 000000000..5a5792816 --- /dev/null +++ b/server/src/incrementalCompilation.ts @@ -0,0 +1,343 @@ +import * as path from "path"; +import fs from "fs"; +import * as utils from "./utils"; +import { pathToFileURL } from "url"; +import readline from "readline"; +import { performance } from "perf_hooks"; +import * as p from "vscode-languageserver-protocol"; +import * as cp from "node:child_process"; +import { send } from "./config"; +import * as c from "./constants"; + +/* + * TODO CMT stuff + * - Compile resi + * - Wait a certain threshold for compilation before using the old cmt + */ + +let debug = true; + +let buildNinjaCache: Map]> = new Map(); +let savedIncrementalFiles: Set = new Set(); +let compileContentsCache: Map = + new Map(); +let compileContentsListeners: Map void>> = new Map(); + +export function cleanupIncrementalFilesAfterCompilation(changedPath: string) { + const projectRootPath = utils.findProjectRootOfFile(changedPath); + if (projectRootPath != null) { + savedIncrementalFiles.forEach((filePath) => { + if (filePath.startsWith(projectRootPath)) { + cleanUpIncrementalFiles(filePath, projectRootPath); + savedIncrementalFiles.delete(filePath); + } + }); + } +} + +export function removeIncrementalFileFolder( + projectRootPath: string, + onAfterRemove?: () => void +) { + fs.rm( + path.resolve(projectRootPath, "lib/bs/___incremental"), + { force: true, recursive: true }, + (_) => { + onAfterRemove?.(); + } + ); +} + +export function recreateIncrementalFileFolder(projectRootPath: string) { + removeIncrementalFileFolder(projectRootPath, () => { + fs.mkdir(path.resolve(projectRootPath, "lib/bs/___incremental"), (_) => {}); + }); +} + +export function fileIsIncrementallyCompiled(filePath: string): boolean { + let projectRootPath = utils.findProjectRootOfFile(filePath); + let fileName = path.basename(filePath, ".res"); + if (projectRootPath != null) { + return fs.existsSync( + path.resolve(projectRootPath, "lib/bs/___incremental", fileName + ".cmt") + ); + } + return false; +} + +export function cleanUpIncrementalFiles( + filePath: string, + projectRootPath: string +) { + [ + path.basename(filePath, ".res") + ".ast", + path.basename(filePath, ".res") + ".cmt", + path.basename(filePath, ".res") + ".cmi", + path.basename(filePath, ".res") + ".cmj", + path.basename(filePath), + ].forEach((file) => { + fs.unlink( + path.resolve(projectRootPath, "lib/bs/___incremental", file), + (_) => {} + ); + }); +} +function getBscArgs(projectRootPath: string): Promise> { + let buildNinjaPath = path.resolve(projectRootPath, "lib/bs/build.ninja"); + let cacheEntry = buildNinjaCache.get(buildNinjaPath); + let stat: fs.Stats | null = null; + if (cacheEntry != null) { + stat = fs.statSync(buildNinjaPath); + if (cacheEntry[0] >= stat.mtimeMs) { + return Promise.resolve(cacheEntry[1]); + } + } + return new Promise((resolve, _reject) => { + function resolveResult(result: Array) { + if (stat != null) { + buildNinjaCache.set(buildNinjaPath, [stat.mtimeMs, result]); + } + resolve(result); + } + const fileStream = fs.createReadStream( + path.resolve(projectRootPath, "lib/bs/build.ninja") + ); + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }); + let captureNextLine = false; + let done = false; + let stopped = false; + let captured: Array = []; + rl.on("line", (line) => { + if (stopped) { + return; + } + if (captureNextLine) { + captured.push(line); + captureNextLine = false; + } + if (done) { + fileStream.destroy(); + rl.close(); + resolveResult(captured); + stopped = true; + return; + } + if (line.startsWith("rule astj")) { + captureNextLine = true; + } + if (line.startsWith("rule mij")) { + captureNextLine = true; + done = true; + } + }); + rl.on("close", () => { + resolveResult(captured); + }); + }); +} +function argsFromCommandString(cmdString: string): Array> { + let s = cmdString + .trim() + .split("command = ")[1] + .split(" ") + .map((v) => v.trim()) + .filter((v) => v !== ""); + let args: Array> = []; + + for (let i = 0; i <= s.length - 1; i++) { + let item = s[i]; + let nextItem = s[i + 1] ?? ""; + if (item.startsWith("-") && nextItem.startsWith("-")) { + // Single entry arg + args.push([item]); + } else if (item.startsWith("-") && s[i + 1]?.startsWith("'")) { + let nextIndex = i + 1; + // Quoted arg, take until ending ' + let arg = [s[nextIndex]]; + for (let x = nextIndex + 1; x <= s.length - 1; x++) { + let nextItem = s[x]; + arg.push(nextItem); + if (nextItem.endsWith("'")) { + i = x + 1; + break; + } + } + args.push([item, arg.join(" ")]); + } else if (item.startsWith("-")) { + args.push([item, nextItem]); + } + } + return args; +} +function removeAnsiCodes(s: string): string { + const ansiEscape = /\x1B[@-_][0-?]*[ -/]*[@-~]/g; + return s.replace(ansiEscape, ""); +} +function triggerIncrementalCompilationOfFile( + filePath: string, + fileContent: string, + send: send, + onCompilationFinished?: () => void +) { + let cacheEntry = compileContentsCache.get(filePath); + if (cacheEntry != null) { + clearTimeout(cacheEntry.timeout); + compileContentsListeners.get(filePath)?.forEach((cb) => cb()); + compileContentsListeners.delete(filePath); + } + let triggerToken = performance.now(); + compileContentsCache.set(filePath, { + timeout: setTimeout(() => { + compileContents( + filePath, + fileContent, + triggerToken, + send, + onCompilationFinished + ); + }, 20), + triggerToken, + }); +} +function verifyTriggerToken(filePath: string, triggerToken: number): boolean { + return compileContentsCache.get(filePath)?.triggerToken === triggerToken; +} +async function compileContents( + filePath: string, + fileContent: string, + triggerToken: number, + send: (msg: p.Message) => void, + onCompilationFinished?: () => void +) { + let startTime = performance.now(); + let fileName = path.basename(filePath); + const projectRootPath = utils.findProjectRootOfFile(filePath); + if (projectRootPath == null) { + if (debug) console.log("Did not find root project."); + return; + } + let bscExe = utils.findBscExeBinary(projectRootPath); + if (bscExe == null) { + if (debug) console.log("Did not find bsc."); + return; + } + let incrementalFilePath = path.resolve( + projectRootPath, + "lib/bs/___incremental", + fileName + ); + + fs.writeFileSync(incrementalFilePath, fileContent); + + try { + let [astBuildCommand, fullBuildCommand] = await getBscArgs(projectRootPath); + + let astArgs = argsFromCommandString(astBuildCommand); + let buildArgs = argsFromCommandString(fullBuildCommand); + + let callArgs: Array = [ + "-I", + path.resolve(projectRootPath, "lib/bs/___incremental"), + ]; + + buildArgs.forEach(([key, value]: Array) => { + if (key === "-I") { + callArgs.push("-I", path.resolve(projectRootPath, "lib/bs", value)); + } else if (key === "-bs-v") { + callArgs.push("-bs-v", Date.now().toString()); + } else if (key === "-bs-package-output") { + return; + } else if (value == null || value === "") { + callArgs.push(key); + } else { + callArgs.push(key, value); + } + }); + + astArgs.forEach(([key, value]: Array) => { + if (key.startsWith("-bs-jsx")) { + callArgs.push(key, value); + } else if (key.startsWith("-ppx")) { + callArgs.push(key, value); + } + }); + + callArgs.push("-color", "never"); + callArgs.push("-ignore-parse-errors"); + + callArgs = callArgs.filter((v) => v != null && v !== ""); + callArgs.push(incrementalFilePath); + + let process = cp.execFile( + bscExe, + callArgs, + { cwd: projectRootPath }, + (error, _stdout, stderr) => { + if (!error?.killed) { + if (debug) + console.log( + `Recompiled ${fileName} in ${ + (performance.now() - startTime) / 1000 + }s` + ); + } else { + if (debug) console.log(`Compilation of ${fileName} was killed.`); + } + onCompilationFinished?.(); + if (!error?.killed && verifyTriggerToken(filePath, triggerToken)) { + let { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); + let res = (Object.values(result)[0] ?? []) + .map((d) => ({ + ...d, + message: removeAnsiCodes(d.message), + })) + // Filter out a few unwanted parser errors since we run the parser in ignore mode + .filter( + (d) => + !d.message.startsWith("Uninterpreted extension 'rescript.") && + !d.message.includes(`/___incremental/${fileName}`) + ); + + let notification: p.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "textDocument/publishDiagnostics", + params: { + uri: pathToFileURL(filePath), + diagnostics: res, + }, + }; + send(notification); + } + } + ); + let listeners = compileContentsListeners.get(filePath) ?? []; + listeners.push(() => { + process.kill("SIGKILL"); + }); + compileContentsListeners.set(filePath, listeners); + } catch (e) { + console.error(e); + } +} + +export function handleUpdateOpenedFile( + filePath: string, + fileContent: string, + send: send, + onCompilationFinished?: () => void +) { + savedIncrementalFiles.delete(filePath); + triggerIncrementalCompilationOfFile( + filePath, + fileContent, + send, + onCompilationFinished + ); +} + +export function handleDidSaveTextDocument(filePath: string) { + savedIncrementalFiles.add(filePath); +} diff --git a/server/src/server.ts b/server/src/server.ts index 55e626b5f..2db2e25d2 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -21,28 +21,13 @@ import * as codeActions from "./codeActions"; import * as c from "./constants"; import * as chokidar from "chokidar"; import { assert } from "console"; -import { fileURLToPath, pathToFileURL } from "url"; +import { fileURLToPath } from "url"; import * as cp from "node:child_process"; import { WorkspaceEdit } from "vscode-languageserver"; import { filesDiagnostics } from "./utils"; import { onErrorReported } from "./errorReporter"; -import readline from "readline"; - -interface extensionConfiguration { - allowBuiltInFormatter: boolean; - askToStartBuild: boolean; - inlayHints: { - enable: boolean; - maxLength: number | null; - }; - codeLens: boolean; - binaryPath: string | null; - platformPath: string | null; - signatureHelp: { - enabled: boolean; - }; - incrementalTypechecking: boolean; -} +import * as ic from "./incrementalCompilation"; +import config, { extensionConfiguration } from "./config"; // This holds client capabilities specific to our extension, and not necessarily // related to the LS protocol. It's for enabling/disabling features that might @@ -53,23 +38,6 @@ export interface extensionClientCapabilities { } let extensionClientCapabilities: extensionClientCapabilities = {}; -// All values here are temporary, and will be overridden as the server is -// initialized, and the current config is received from the client. -let extensionConfiguration: extensionConfiguration = { - allowBuiltInFormatter: false, - askToStartBuild: true, - inlayHints: { - enable: false, - maxLength: 25, - }, - codeLens: false, - binaryPath: null, - platformPath: null, - signatureHelp: { - enabled: true, - }, - incrementalTypechecking: true, -}; // Below here is some state that's not important exactly how long it lives. let hasPromptedAboutBuiltInFormatter = false; let pullConfigurationPeriodically: NodeJS.Timeout | null = null; @@ -106,45 +74,16 @@ let codeActionsFromDiagnostics: codeActions.filesCodeActions = {}; // will be properly defined later depending on the mode (stdio/node-rpc) let send: (msg: p.Message) => void = (_) => {}; -let buildNinjaCache: Map]> = new Map(); - let findRescriptBinary = (projectRootPath: p.DocumentUri | null) => - extensionConfiguration.binaryPath == null + config.extensionConfiguration.binaryPath == null ? lookup.findFilePathFromProjectRoot( projectRootPath, path.join(c.nodeModulesBinDir, c.rescriptBinName) ) - : utils.findBinary(extensionConfiguration.binaryPath, c.rescriptBinName); - -let findPlatformPath = (projectRootPath: p.DocumentUri | null) => { - if (extensionConfiguration.platformPath != null) { - return extensionConfiguration.platformPath; - } - - let rescriptDir = lookup.findFilePathFromProjectRoot( - projectRootPath, - path.join("node_modules", "rescript") - ); - if (rescriptDir == null) { - return null; - } - - let platformPath = path.join(rescriptDir, c.platformDir); - - // Workaround for darwinarm64 which has no folder yet in ReScript <= 9.1.4 - if ( - process.platform == "darwin" && - process.arch == "arm64" && - !fs.existsSync(platformPath) - ) { - platformPath = path.join(rescriptDir, process.platform); - } - - return platformPath; -}; - -let findBscExeBinary = (projectRootPath: p.DocumentUri | null) => - utils.findBinary(findPlatformPath(projectRootPath), c.bscExeName); + : utils.findBinary( + config.extensionConfiguration.binaryPath, + c.rescriptBinName + ); let createInterfaceRequest = new v.RequestType< p.TextDocumentIdentifier, @@ -252,8 +191,8 @@ let deleteProjectDiagnostics = (projectRootPath: string) => { }); projectsFiles.delete(projectRootPath); - if (extensionConfiguration.incrementalTypechecking) { - removeIncrementalFileFolder(projectRootPath); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.removeIncrementalFileFolder(projectRootPath); } } }; @@ -273,12 +212,18 @@ let compilerLogsWatcher = chokidar }, }) .on("all", (_e, changedPath) => { + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + // We clean up all incremental file assets when the compiler has rebuilt, + // because at that point we want to use the main, compiled type assets + // instead of the incremental type assets. + ic.cleanupIncrementalFilesAfterCompilation(changedPath); + } sendUpdatedDiagnostics(); sendCompilationFinishedMessage(); - if (extensionConfiguration.inlayHints?.enable === true) { + if (config.extensionConfiguration.inlayHints?.enable === true) { sendInlayHintsRefresh(); } - if (extensionConfiguration.codeLens === true) { + if (config.extensionConfiguration.codeLens === true) { sendCodeLensRefresh(); } }); @@ -298,13 +243,13 @@ let openedFile = (fileUri: string, fileContent: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { - if (extensionConfiguration.incrementalTypechecking) { - cleanUpIncrementalFiles(filePath, projectRootPath); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.cleanUpIncrementalFiles(filePath, projectRootPath); } let projectRootState = projectsFiles.get(projectRootPath); if (projectRootState == null) { - if (extensionConfiguration.incrementalTypechecking) { - recreateIncrementalFileFolder(projectRootPath); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.recreateIncrementalFileFolder(projectRootPath); } projectRootState = { openFiles: new Set(), @@ -329,7 +274,7 @@ let openedFile = (fileUri: string, fileContent: string) => { let bsbLockPath = path.join(projectRootPath, c.bsbLock); if ( projectRootState.hasPromptedToStartBuild === false && - extensionConfiguration.askToStartBuild === true && + config.extensionConfiguration.askToStartBuild === true && !fs.existsSync(bsbLockPath) ) { // TODO: sometime stale .bsb.lock dangling. bsb -w knows .bsb.lock is @@ -363,12 +308,12 @@ let openedFile = (fileUri: string, fileContent: string) => { params: { type: p.MessageType.Error, message: - extensionConfiguration.binaryPath == null + config.extensionConfiguration.binaryPath == null ? `Can't find ReScript binary in ${path.join( projectRootPath, c.nodeModulesBinDir )} or parent directories. Did you install it? It's required to use "rescript" > 9.1` - : `Can't find ReScript binary in the directory ${extensionConfiguration.binaryPath}`, + : `Can't find ReScript binary in the directory ${config.extensionConfiguration.binaryPath}`, }, }; send(request); @@ -379,44 +324,7 @@ let openedFile = (fileUri: string, fileContent: string) => { // call the listener which calls it } }; -function removeIncrementalFileFolder( - projectRootPath: string, - onAfterRemove?: () => void -) { - fs.rm( - path.resolve(projectRootPath, "lib/bs/___incremental"), - { force: true, recursive: true }, - (_) => { - onAfterRemove?.(); - } - ); -} -function recreateIncrementalFileFolder(projectRootPath: string) { - removeIncrementalFileFolder(projectRootPath, () => { - fs.mkdir(path.resolve(projectRootPath, "lib/bs/___incremental"), (_) => {}); - }); -} -/** - * TODO CMT stuff - * - Make sure cmt is deleted/ignore if original contents is newer. How? - * - Clear incremental folder when starting LSP - * - Optimize recompiles - * - Races when recompiling? - */ -function cleanUpIncrementalFiles(filePath: string, projectRootPath: string) { - [ - path.basename(filePath, ".res") + ".ast", - path.basename(filePath, ".res") + ".cmt", - path.basename(filePath, ".res") + ".cmi", - path.basename(filePath, ".res") + ".cmj", - path.basename(filePath), - ].forEach((file) => { - fs.unlink( - path.resolve(projectRootPath, "lib/bs/___incremental", file), - (_) => {} - ); - }); -} + let closedFile = (fileUri: string) => { let filePath = fileURLToPath(fileUri); @@ -424,8 +332,8 @@ let closedFile = (fileUri: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { - if (extensionConfiguration.incrementalTypechecking) { - cleanUpIncrementalFiles(filePath, projectRootPath); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.cleanUpIncrementalFiles(filePath, projectRootPath); } let root = projectsFiles.get(projectRootPath); if (root != null) { @@ -444,189 +352,20 @@ let closedFile = (fileUri: string) => { } } }; -function getBscArgs(projectRootPath: string): Promise> { - let buildNinjaPath = path.resolve(projectRootPath, "lib/bs/build.ninja"); - let cacheEntry = buildNinjaCache.get(buildNinjaPath); - let stat: fs.Stats | null = null; - if (cacheEntry != null) { - stat = fs.statSync(buildNinjaPath); - if (cacheEntry[0] >= stat.mtimeMs) { - return Promise.resolve(cacheEntry[1]); - } - } - return new Promise((resolve, _reject) => { - function resolveResult(result: Array) { - if (stat != null) { - buildNinjaCache.set(buildNinjaPath, [stat.mtimeMs, result]); - } - resolve(result); - } - const fileStream = fs.createReadStream( - path.resolve(projectRootPath, "lib/bs/build.ninja") - ); - const rl = readline.createInterface({ - input: fileStream, - crlfDelay: Infinity, - }); - let captureNextLine = false; - let done = false; - let stopped = false; - let captured: Array = []; - rl.on("line", (line) => { - if (stopped) { - return; - } - if (captureNextLine) { - captured.push(line); - captureNextLine = false; - } - if (done) { - fileStream.destroy(); - rl.close(); - resolveResult(captured); - stopped = true; - return; - } - if (line.startsWith("rule astj")) { - captureNextLine = true; - } - if (line.startsWith("rule mij")) { - captureNextLine = true; - done = true; - } - }); - rl.on("close", () => { - resolveResult(captured); - }); - }); -} -function argsFromCommandString(cmdString: string): Array> { - let s = cmdString - .trim() - .split("command = ")[1] - .split(" ") - .map((v) => v.trim()) - .filter((v) => v !== ""); - let args: Array> = []; - - for (let i = 0; i <= s.length - 1; i++) { - let item = s[i]; - let nextItem = s[i + 1] ?? ""; - if (item.startsWith("-") && nextItem.startsWith("-")) { - // Single entry arg - args.push([item]); - } else if (item.startsWith("-") && s[i + 1]?.startsWith("'")) { - let nextIndex = i + 1; - // Quoted arg, take until ending ' - let arg = [s[nextIndex]]; - for (let x = nextIndex + 1; x <= s.length - 1; x++) { - let nextItem = s[x]; - arg.push(nextItem); - if (nextItem.endsWith("'")) { - i = x + 1; - break; - } - } - args.push([item, arg.join(" ")]); - } else if (item.startsWith("-")) { - args.push([item, nextItem]); - } - } - return args; -} -function removeAnsiCodes(s: string): string { - const ansiEscape = /\x1B[@-_][0-?]*[ -/]*[@-~]/g; - return s.replace(ansiEscape, ""); -} -async function compileContents(filePath: string, fileContent: string) { - const projectRootPath = utils.findProjectRootOfFile(filePath); - if (projectRootPath == null) { - console.log("Did not find root project."); - return; - } - let bscExe = findBscExeBinary(projectRootPath); - if (bscExe == null) { - console.log("Did not find bsc."); - return; - } - let fileName = path.basename(filePath); - let incrementalFilePath = path.resolve( - projectRootPath, - "lib/bs/___incremental", - fileName - ); - - fs.writeFileSync(incrementalFilePath, fileContent); - - try { - let [astBuildCommand, fullBuildCommand] = await getBscArgs(projectRootPath); - - let astArgs = argsFromCommandString(astBuildCommand); - let buildArgs = argsFromCommandString(fullBuildCommand); - - let callArgs: Array = []; - - buildArgs.forEach(([key, value]: Array) => { - if (key === "-I") { - callArgs.push("-I", path.resolve(projectRootPath, "lib/bs", value)); - } else if (key === "-bs-v") { - callArgs.push("-bs-v", Date.now().toString()); - } else if (key === "-bs-package-output") { - return; - } else if (value == null || value === "") { - callArgs.push(key); - } else { - callArgs.push(key, value); - } - }); - - astArgs.forEach(([key, value]: Array) => { - if (key.startsWith("-bs-jsx")) { - callArgs.push(key, value); - } else if (key.startsWith("-ppx")) { - callArgs.push(key, value); - } - }); - callArgs.push("-color", "never"); - - callArgs = callArgs.filter((v) => v != null && v !== ""); - callArgs.push(incrementalFilePath); - - cp.execFile( - bscExe, - callArgs, - { cwd: projectRootPath }, - (_error, _stdout, stderr) => { - // DOCUMENT HERE - if (stderr !== "") { - let { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); - let res = Object.values(result)[0].map((d) => ({ - ...d, - message: removeAnsiCodes(d.message), - })); - let notification: p.NotificationMessage = { - jsonrpc: c.jsonrpcVersion, - method: "textDocument/publishDiagnostics", - params: { - uri: pathToFileURL(filePath), - diagnostics: res, - }, - }; - send(notification); - } - } - ); - } catch (e) { - console.error(e); - } -} let updateOpenedFile = (fileUri: string, fileContent: string) => { let filePath = fileURLToPath(fileUri); assert(stupidFileContentCache.has(filePath)); stupidFileContentCache.set(filePath, fileContent); - if (extensionConfiguration.incrementalTypechecking) { - compileContents(filePath, fileContent); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.handleUpdateOpenedFile(filePath, fileContent, send, () => { + if (config.extensionConfiguration.codeLens) { + sendCodeLensRefresh(); + } + if (config.extensionConfiguration.inlayHints) { + sendInlayHintsRefresh(); + } + }); } }; let getOpenedFileContent = (fileUri: string) => { @@ -687,7 +426,7 @@ function inlayHint(msg: p.RequestMessage) { filePath, params.range.start.line, params.range.end.line, - extensionConfiguration.inlayHints.maxLength, + config.extensionConfiguration.inlayHints.maxLength, ], msg ); @@ -1008,13 +747,13 @@ function format(msg: p.RequestMessage): Array { let code = getOpenedFileContent(params.textDocument.uri); let projectRootPath = utils.findProjectRootOfFile(filePath); - let bscExeBinaryPath = findBscExeBinary(projectRootPath); + let bscExeBinaryPath = utils.findBscExeBinary(projectRootPath); let formattedResult = utils.formatCode( bscExeBinaryPath, filePath, code, - extensionConfiguration.allowBuiltInFormatter + config.extensionConfiguration.allowBuiltInFormatter ); if (formattedResult.kind === "success") { let max = code.length; @@ -1064,6 +803,12 @@ function format(msg: p.RequestMessage): Array { let updateDiagnosticSyntax = (fileUri: string, fileContent: string) => { let filePath = fileURLToPath(fileUri); + if ( + config.extensionConfiguration.incrementalTypechecking.enabled && + ic.fileIsIncrementallyCompiled(filePath) + ) { + return; + } let extension = path.extname(filePath); let tmpname = utils.createFileInTempDir(extension); fs.writeFileSync(tmpname, fileContent, { encoding: "utf-8" }); @@ -1275,6 +1020,14 @@ function onMessage(msg: p.Message) { } else { process.exit(1); } + } else if (msg.method === p.DidSaveTextDocumentNotification.method) { + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + // We track all incremental file assets when a file is actually saved, + // so we can clean them up. + let params = msg.params as p.DidSaveTextDocumentParams; + let filePath = fileURLToPath(params.textDocument.uri); + ic.handleDidSaveTextDocument(filePath); + } } else if (msg.method === DidOpenTextDocumentNotification.method) { let params = msg.params as p.DidOpenTextDocumentParams; openedFile(params.textDocument.uri, params.textDocument.text); @@ -1324,7 +1077,7 @@ function onMessage(msg: p.Message) { ?.extensionConfiguration as extensionConfiguration | undefined; if (initialConfiguration != null) { - extensionConfiguration = initialConfiguration; + config.extensionConfiguration = initialConfiguration; } // These are static configuration options the client can set to enable certain @@ -1379,13 +1132,14 @@ function onMessage(msg: p.Message) { // TODO: Support range for full, and add delta support full: true, }, - inlayHintProvider: extensionConfiguration.inlayHints?.enable, - codeLensProvider: extensionConfiguration.codeLens + inlayHintProvider: config.extensionConfiguration.inlayHints?.enable, + codeLensProvider: config.extensionConfiguration.codeLens ? { workDoneProgress: false, } : undefined, - signatureHelpProvider: extensionConfiguration.signatureHelp?.enabled + signatureHelpProvider: config.extensionConfiguration.signatureHelp + ?.enabled ? { triggerCharacters: ["("], retriggerCharacters: ["=", ","], @@ -1513,7 +1267,7 @@ function onMessage(msg: p.Message) { extensionConfiguration | null | undefined ]; if (configuration != null) { - extensionConfiguration = configuration; + config.extensionConfiguration = configuration; } } } else if ( diff --git a/server/src/utils.ts b/server/src/utils.ts index 662c7ef99..4d5549f20 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -13,6 +13,7 @@ import * as codeActions from "./codeActions"; import * as c from "./constants"; import * as lookup from "./lookup"; import { reportError } from "./errorReporter"; +import config from "./config"; let tempFilePrefix = "rescript_format_file_" + process.pid + "_"; let tempFileId = 0; @@ -661,3 +662,33 @@ export let rangeContainsRange = ( } return true; }; + +let findPlatformPath = (projectRootPath: p.DocumentUri | null) => { + if (config.extensionConfiguration.platformPath != null) { + return config.extensionConfiguration.platformPath; + } + + let rescriptDir = lookup.findFilePathFromProjectRoot( + projectRootPath, + path.join("node_modules", "rescript") + ); + if (rescriptDir == null) { + return null; + } + + let platformPath = path.join(rescriptDir, c.platformDir); + + // Workaround for darwinarm64 which has no folder yet in ReScript <= 9.1.4 + if ( + process.platform == "darwin" && + process.arch == "arm64" && + !fs.existsSync(platformPath) + ) { + platformPath = path.join(rescriptDir, process.platform); + } + + return platformPath; +}; + +export let findBscExeBinary = (projectRootPath: p.DocumentUri | null) => + findBinary(findPlatformPath(projectRootPath), c.bscExeName); From 2176b0a02745fda1990fe6cabd33ec4d17286401 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sat, 2 Mar 2024 19:27:23 +0100 Subject: [PATCH 03/14] reset back version --- package-lock.json | 4 ++-- package.json | 4 ++-- server/package-lock.json | 4 ++-- server/package.json | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index c907f904b..fa782c666 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rescript-vscode", - "version": "1.44.0", + "version": "1.42.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "rescript-vscode", - "version": "1.44.0", + "version": "1.42.0", "hasInstallScript": true, "license": "MIT", "devDependencies": { diff --git a/package.json b/package.json index ad93c832b..29b479c44 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "ReScript language support (official)", "author": "ReScript Team", "license": "MIT", - "version": "1.44.0", + "version": "1.42.0", "repository": { "type": "git", "url": "https://github.com/rescript-lang/rescript-vscode" @@ -176,7 +176,7 @@ "default": true, "description": "Enable signature help for function calls." }, - "rescript.settings.incrementalTypechecking": { + "rescript.settings.incrementalTypechecking.enabled": { "type": "boolean", "default": true, "description": "Enable incremental type checking." diff --git a/server/package-lock.json b/server/package-lock.json index 4e56a7f5c..d177d5509 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -1,12 +1,12 @@ { "name": "@rescript/language-server", - "version": "1.44.0", + "version": "1.42.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@rescript/language-server", - "version": "1.44.0", + "version": "1.42.0", "license": "MIT", "dependencies": { "chokidar": "^3.5.1", diff --git a/server/package.json b/server/package.json index 7d4aad10c..6052d373b 100644 --- a/server/package.json +++ b/server/package.json @@ -1,7 +1,7 @@ { "name": "@rescript/language-server", "description": "LSP server for ReScript", - "version": "1.44.0", + "version": "1.42.0", "author": "ReScript Team", "license": "MIT", "bin": { From 42a64bfa5db797bb243c9a6b1765cc4c3c0182a6 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 3 Mar 2024 12:47:22 +0100 Subject: [PATCH 04/14] more work --- .vscodeignore | 3 +- client/src/extension.ts | 3 +- package.json | 4 +- server/src/incrementalCompilation.ts | 58 ++++++++++++++++++++-------- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/.vscodeignore b/.vscodeignore index 00b201d30..a20fda66d 100644 --- a/.vscodeignore +++ b/.vscodeignore @@ -18,4 +18,5 @@ tools.opam client/node_modules server/node_modules _opam -_build \ No newline at end of file +_build +Makefile diff --git a/client/src/extension.ts b/client/src/extension.ts index 24ba8c9d8..5e34dae37 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -294,8 +294,7 @@ export function activate(context: ExtensionContext) { if ( affectsConfiguration("rescript.settings.inlayHints") || affectsConfiguration("rescript.settings.codeLens") || - affectsConfiguration("rescript.settings.signatureHelp") || - affectsConfiguration("rescript.settings.incrementalTypechecking") + affectsConfiguration("rescript.settings.signatureHelp") ) { commands.executeCommand("rescript-vscode.restart_language_server"); } else { diff --git a/package.json b/package.json index 29b479c44..c17968600 100644 --- a/package.json +++ b/package.json @@ -178,8 +178,8 @@ }, "rescript.settings.incrementalTypechecking.enabled": { "type": "boolean", - "default": true, - "description": "Enable incremental type checking." + "default": false, + "description": "(beta/experimental) Enable incremental type checking." }, "rescript.settings.binaryPath": { "type": [ diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index 5a5792816..e224a1f66 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -13,6 +13,12 @@ import * as c from "./constants"; * TODO CMT stuff * - Compile resi * - Wait a certain threshold for compilation before using the old cmt + * - Monorepos? Namespaces + * Questions: + * - We trigger no incremental compilation for other files. This might be problematic if we want to go across files. Track dependencies? What's supposed to be used when and where? + * Improvements: + * - Ask build system for complete build command for file X, instead of piecing together from build.ninja. Rewatch? + * - Have build system communicate what was actually rebuilt after compilation finishes. */ let debug = true; @@ -22,6 +28,8 @@ let savedIncrementalFiles: Set = new Set(); let compileContentsCache: Map = new Map(); let compileContentsListeners: Map void>> = new Map(); +const incrementalFolderName = "___incremental"; +const incrementalFileFolderLocation = `lib/bs/${incrementalFolderName}`; export function cleanupIncrementalFilesAfterCompilation(changedPath: string) { const projectRootPath = utils.findProjectRootOfFile(changedPath); @@ -40,7 +48,7 @@ export function removeIncrementalFileFolder( onAfterRemove?: () => void ) { fs.rm( - path.resolve(projectRootPath, "lib/bs/___incremental"), + path.resolve(projectRootPath, incrementalFileFolderLocation), { force: true, recursive: true }, (_) => { onAfterRemove?.(); @@ -50,7 +58,10 @@ export function removeIncrementalFileFolder( export function recreateIncrementalFileFolder(projectRootPath: string) { removeIncrementalFileFolder(projectRootPath, () => { - fs.mkdir(path.resolve(projectRootPath, "lib/bs/___incremental"), (_) => {}); + fs.mkdir( + path.resolve(projectRootPath, incrementalFileFolderLocation), + (_) => {} + ); }); } @@ -59,7 +70,11 @@ export function fileIsIncrementallyCompiled(filePath: string): boolean { let fileName = path.basename(filePath, ".res"); if (projectRootPath != null) { return fs.existsSync( - path.resolve(projectRootPath, "lib/bs/___incremental", fileName + ".cmt") + path.resolve( + projectRootPath, + incrementalFileFolderLocation, + fileName + ".cmt" + ) ); } return false; @@ -77,7 +92,7 @@ export function cleanUpIncrementalFiles( path.basename(filePath), ].forEach((file) => { fs.unlink( - path.resolve(projectRootPath, "lib/bs/___incremental", file), + path.resolve(projectRootPath, incrementalFileFolderLocation, file), (_) => {} ); }); @@ -149,19 +164,24 @@ function argsFromCommandString(cmdString: string): Array> { for (let i = 0; i <= s.length - 1; i++) { let item = s[i]; - let nextItem = s[i + 1] ?? ""; + let nextIndex = i + 1; + let nextItem = s[nextIndex] ?? ""; if (item.startsWith("-") && nextItem.startsWith("-")) { // Single entry arg args.push([item]); - } else if (item.startsWith("-") && s[i + 1]?.startsWith("'")) { - let nextIndex = i + 1; + } else if (item.startsWith("-") && nextItem.startsWith("'")) { // Quoted arg, take until ending ' - let arg = [s[nextIndex]]; + let arg = [nextItem.slice(1)]; for (let x = nextIndex + 1; x <= s.length - 1; x++) { - let nextItem = s[x]; - arg.push(nextItem); - if (nextItem.endsWith("'")) { - i = x + 1; + let subItem = s[x]; + let break_ = false; + if (subItem.endsWith("'")) { + subItem = subItem.slice(0, subItem.length - 1); + i = x; + break_ = true; + } + arg.push(subItem); + if (break_) { break; } } @@ -224,11 +244,15 @@ async function compileContents( if (debug) console.log("Did not find bsc."); return; } - let incrementalFilePath = path.resolve( + let incrementalFolderPath = path.resolve( projectRootPath, - "lib/bs/___incremental", - fileName + incrementalFileFolderLocation ); + let incrementalFilePath = path.resolve(incrementalFolderPath, fileName); + + if (!fs.existsSync(incrementalFolderPath)) { + fs.mkdirSync(incrementalFolderPath); + } fs.writeFileSync(incrementalFilePath, fileContent); @@ -240,7 +264,7 @@ async function compileContents( let callArgs: Array = [ "-I", - path.resolve(projectRootPath, "lib/bs/___incremental"), + path.resolve(projectRootPath, incrementalFileFolderLocation), ]; buildArgs.forEach(([key, value]: Array) => { @@ -298,7 +322,7 @@ async function compileContents( .filter( (d) => !d.message.startsWith("Uninterpreted extension 'rescript.") && - !d.message.includes(`/___incremental/${fileName}`) + !d.message.includes(`/${incrementalFolderName}/${fileName}`) ); let notification: p.NotificationMessage = { From 7b0deba4c90010732dce72562a86cd2ab051f14d Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Mar 2024 09:38:31 +0100 Subject: [PATCH 05/14] refactor --- server/src/incrementalCompilation.ts | 454 +++++++++++++++++---------- server/src/server.ts | 11 +- 2 files changed, 286 insertions(+), 179 deletions(-) diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index e224a1f66..cb18402f4 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -14,25 +14,71 @@ import * as c from "./constants"; * - Compile resi * - Wait a certain threshold for compilation before using the old cmt * - Monorepos? Namespaces + * - Fix races by using fs.promises + * - Split some of the cache per project root instead so less recalc is needed + * - Throttle error messages? * Questions: * - We trigger no incremental compilation for other files. This might be problematic if we want to go across files. Track dependencies? What's supposed to be used when and where? * Improvements: - * - Ask build system for complete build command for file X, instead of piecing together from build.ninja. Rewatch? + * - Ask build system for compconste build command for file X, instead of piecing together from build.ninja. Rewatch? * - Have build system communicate what was actually rebuilt after compilation finishes. */ -let debug = true; +const debug = true; -let buildNinjaCache: Map]> = new Map(); -let savedIncrementalFiles: Set = new Set(); -let compileContentsCache: Map = - new Map(); -let compileContentsListeners: Map void>> = new Map(); -const incrementalFolderName = "___incremental"; -const incrementalFileFolderLocation = `lib/bs/${incrementalFolderName}`; +const INCREMENTAL_FOLDER_NAME = "___incremental"; +const INCREMENTAL_FILE_FOLDER_LOCATION = `lib/bs/${INCREMENTAL_FOLDER_NAME}`; -export function cleanupIncrementalFilesAfterCompilation(changedPath: string) { - const projectRootPath = utils.findProjectRootOfFile(changedPath); +type IncrementallyCompiledFileInfo = { + file: { + /** Path to the source file. */ + sourceFilePath: string; + /** Name of the source file. */ + sourceFileName: string; + /** Module name of the source file. */ + moduleName: string; + /** Path to where the incremental file is saved. */ + incrementalFilePath: string; + }; + /** Cache for build.ninja assets. */ + buildNinja: { + /** When build.ninja was last modified. Used as a cache key. */ + fileMtime: number; + /** The raw, extracted needed info from build.ninja. Needs processing. */ + rawExtracted: Array; + } | null; + /** Info of the currently active incremental compilation. `null` if no incremental compilation is active. */ + compilation: { + /** The timeout of the currently active compilation for this incremental file. */ + timeout: NodeJS.Timeout; + /** The trigger token for the currently active compilation. */ + triggerToken: number; + } | null; + /** Listeners for when compilation of this file is killed. List always cleared after each invocation. */ + compilationKilledListeners: Array<() => void>; + /** Project specific information. */ + project: { + /** The root path of the project. */ + rootPath: string; + /** Computed location of bsc. */ + bscBinaryLocation: string; + /** The arguments needed for bsc, derived from the project configuration/build.ninja. */ + callArgs: Promise>; + /** The location of the incremental folder for this project. */ + incrementalFolderPath: string; + }; +}; + +const incrementallyCompiledFileInfo: Map< + string, + IncrementallyCompiledFileInfo +> = new Map(); +const savedIncrementalFiles: Set = new Set(); + +export function cleanupIncrementalFilesAfterCompilation( + compilerLogPath: string +) { + const projectRootPath = utils.findProjectRootOfFile(compilerLogPath); if (projectRootPath != null) { savedIncrementalFiles.forEach((filePath) => { if (filePath.startsWith(projectRootPath)) { @@ -48,7 +94,7 @@ export function removeIncrementalFileFolder( onAfterRemove?: () => void ) { fs.rm( - path.resolve(projectRootPath, incrementalFileFolderLocation), + path.resolve(projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION), { force: true, recursive: true }, (_) => { onAfterRemove?.(); @@ -59,64 +105,71 @@ export function removeIncrementalFileFolder( export function recreateIncrementalFileFolder(projectRootPath: string) { removeIncrementalFileFolder(projectRootPath, () => { fs.mkdir( - path.resolve(projectRootPath, incrementalFileFolderLocation), + path.resolve(projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION), (_) => {} ); }); } export function fileIsIncrementallyCompiled(filePath: string): boolean { - let projectRootPath = utils.findProjectRootOfFile(filePath); - let fileName = path.basename(filePath, ".res"); - if (projectRootPath != null) { - return fs.existsSync( - path.resolve( - projectRootPath, - incrementalFileFolderLocation, - fileName + ".cmt" - ) - ); + const entry = incrementallyCompiledFileInfo.get(filePath); + + if (entry == null) { + return false; } - return false; + + const pathToCheck = path.resolve( + entry.project.incrementalFolderPath, + entry.file.moduleName + ".cmt" + ); + + return fs.existsSync(pathToCheck); } export function cleanUpIncrementalFiles( filePath: string, projectRootPath: string ) { + const fileNameNoExt = path.basename(filePath, ".res"); [ - path.basename(filePath, ".res") + ".ast", - path.basename(filePath, ".res") + ".cmt", - path.basename(filePath, ".res") + ".cmi", - path.basename(filePath, ".res") + ".cmj", - path.basename(filePath), + fileNameNoExt + ".ast", + fileNameNoExt + ".cmt", + fileNameNoExt + ".cmi", + fileNameNoExt + ".cmj", + fileNameNoExt + ".res", ].forEach((file) => { fs.unlink( - path.resolve(projectRootPath, incrementalFileFolderLocation, file), + path.resolve(projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION, file), (_) => {} ); }); } -function getBscArgs(projectRootPath: string): Promise> { - let buildNinjaPath = path.resolve(projectRootPath, "lib/bs/build.ninja"); - let cacheEntry = buildNinjaCache.get(buildNinjaPath); +function getBscArgs( + entry: IncrementallyCompiledFileInfo +): Promise> { + const buildNinjaPath = path.resolve( + entry.project.rootPath, + "lib/bs/build.ninja" + ); + const cacheEntry = entry.buildNinja; let stat: fs.Stats | null = null; if (cacheEntry != null) { stat = fs.statSync(buildNinjaPath); - if (cacheEntry[0] >= stat.mtimeMs) { - return Promise.resolve(cacheEntry[1]); + if (cacheEntry.fileMtime >= stat.mtimeMs) { + return Promise.resolve(cacheEntry.rawExtracted); } } return new Promise((resolve, _reject) => { function resolveResult(result: Array) { if (stat != null) { - buildNinjaCache.set(buildNinjaPath, [stat.mtimeMs, result]); + entry.buildNinja = { + fileMtime: stat.mtimeMs, + rawExtracted: result, + }; } resolve(result); } - const fileStream = fs.createReadStream( - path.resolve(projectRootPath, "lib/bs/build.ninja") - ); + const fileStream = fs.createReadStream(buildNinjaPath); const rl = readline.createInterface({ input: fileStream, crlfDelay: Infinity, @@ -124,7 +177,7 @@ function getBscArgs(projectRootPath: string): Promise> { let captureNextLine = false; let done = false; let stopped = false; - let captured: Array = []; + const captured: Array = []; rl.on("line", (line) => { if (stopped) { return; @@ -154,24 +207,24 @@ function getBscArgs(projectRootPath: string): Promise> { }); } function argsFromCommandString(cmdString: string): Array> { - let s = cmdString + const s = cmdString .trim() .split("command = ")[1] .split(" ") .map((v) => v.trim()) .filter((v) => v !== ""); - let args: Array> = []; + const args: Array> = []; for (let i = 0; i <= s.length - 1; i++) { - let item = s[i]; - let nextIndex = i + 1; - let nextItem = s[nextIndex] ?? ""; + const item = s[i]; + const nextIndex = i + 1; + const nextItem = s[nextIndex] ?? ""; if (item.startsWith("-") && nextItem.startsWith("-")) { // Single entry arg args.push([item]); } else if (item.startsWith("-") && nextItem.startsWith("'")) { // Quoted arg, take until ending ' - let arg = [nextItem.slice(1)]; + const arg = [nextItem.slice(1)]; for (let x = nextIndex + 1; x <= s.length - 1; x++) { let subItem = s[x]; let break_ = false; @@ -202,149 +255,194 @@ function triggerIncrementalCompilationOfFile( send: send, onCompilationFinished?: () => void ) { - let cacheEntry = compileContentsCache.get(filePath); - if (cacheEntry != null) { - clearTimeout(cacheEntry.timeout); - compileContentsListeners.get(filePath)?.forEach((cb) => cb()); - compileContentsListeners.delete(filePath); + let incrementalFileCacheEntry = incrementallyCompiledFileInfo.get(filePath); + if (incrementalFileCacheEntry == null) { + // New file + const projectRootPath = utils.findProjectRootOfFile(filePath); + if (projectRootPath == null) return; + const namespaceName = utils.getNamespaceNameFromConfigFile(projectRootPath); + if (namespaceName.kind === "error") return; + const bscBinaryLocation = utils.findBscExeBinary(projectRootPath); + if (bscBinaryLocation == null) return; + const filenameNoExt = path.basename(filePath, ".res"); + const moduleName = + namespaceName.result === "" + ? filenameNoExt + : `${namespaceName.result}-${filenameNoExt}`; + + const incrementalFolderPath = path.join( + projectRootPath, + INCREMENTAL_FILE_FOLDER_LOCATION + ); + + incrementalFileCacheEntry = { + file: { + moduleName, + sourceFileName: moduleName + ".res", + sourceFilePath: filePath, + incrementalFilePath: path.join( + incrementalFolderPath, + moduleName + ".res" + ), + }, + project: { + rootPath: projectRootPath, + callArgs: Promise.resolve([]), + bscBinaryLocation, + incrementalFolderPath, + }, + buildNinja: null, + compilation: null, + compilationKilledListeners: [], + }; + + incrementalFileCacheEntry.project.callArgs = figureOutBscArgs( + incrementalFileCacheEntry + ); + incrementallyCompiledFileInfo.set(filePath, incrementalFileCacheEntry); + } + + if (incrementalFileCacheEntry == null) return; + const entry = incrementalFileCacheEntry; + if (entry.compilation != null) { + clearTimeout(entry.compilation.timeout); + entry.compilationKilledListeners.forEach((cb) => cb()); + entry.compilationKilledListeners = []; + } + const triggerToken = performance.now(); + const timeout = setTimeout(() => { + compileContents(entry, fileContent, send, onCompilationFinished); + }, 20); + + if (entry.compilation != null) { + entry.compilation.timeout = timeout; + entry.compilation.triggerToken = triggerToken; + } else { + entry.compilation = { + timeout, + triggerToken, + }; } - let triggerToken = performance.now(); - compileContentsCache.set(filePath, { - timeout: setTimeout(() => { - compileContents( - filePath, - fileContent, - triggerToken, - send, - onCompilationFinished - ); - }, 20), - triggerToken, - }); } function verifyTriggerToken(filePath: string, triggerToken: number): boolean { - return compileContentsCache.get(filePath)?.triggerToken === triggerToken; + return ( + incrementallyCompiledFileInfo.get(filePath)?.compilation?.triggerToken === + triggerToken + ); +} +async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) { + const [astBuildCommand, fullBuildCommand] = await getBscArgs(entry); + + const astArgs = argsFromCommandString(astBuildCommand); + const buildArgs = argsFromCommandString(fullBuildCommand); + + let callArgs: Array = [ + "-I", + path.resolve(entry.project.rootPath, INCREMENTAL_FILE_FOLDER_LOCATION), + ]; + + buildArgs.forEach(([key, value]: Array) => { + if (key === "-I") { + callArgs.push( + "-I", + path.resolve(entry.project.rootPath, "lib/bs", value) + ); + } else if (key === "-bs-v") { + callArgs.push("-bs-v", Date.now().toString()); + } else if (key === "-bs-package-output") { + return; + } else if (value == null || value === "") { + callArgs.push(key); + } else { + callArgs.push(key, value); + } + }); + + astArgs.forEach(([key, value]: Array) => { + if (key.startsWith("-bs-jsx")) { + callArgs.push(key, value); + } else if (key.startsWith("-ppx")) { + callArgs.push(key, value); + } + }); + + callArgs.push("-color", "never"); + callArgs.push("-ignore-parse-errors"); + + callArgs = callArgs.filter((v) => v != null && v !== ""); + callArgs.push(entry.file.incrementalFilePath); + return callArgs; } async function compileContents( - filePath: string, + entry: IncrementallyCompiledFileInfo, fileContent: string, - triggerToken: number, send: (msg: p.Message) => void, onCompilationFinished?: () => void ) { - let startTime = performance.now(); - let fileName = path.basename(filePath); - const projectRootPath = utils.findProjectRootOfFile(filePath); - if (projectRootPath == null) { - if (debug) console.log("Did not find root project."); - return; - } - let bscExe = utils.findBscExeBinary(projectRootPath); - if (bscExe == null) { - if (debug) console.log("Did not find bsc."); - return; - } - let incrementalFolderPath = path.resolve( - projectRootPath, - incrementalFileFolderLocation - ); - let incrementalFilePath = path.resolve(incrementalFolderPath, fileName); - - if (!fs.existsSync(incrementalFolderPath)) { - fs.mkdirSync(incrementalFolderPath); + const triggerToken = entry.compilation?.triggerToken; + const startTime = performance.now(); + // TODO: Move somewhere else? + if (!fs.existsSync(entry.project.incrementalFolderPath)) { + fs.mkdirSync(entry.project.incrementalFolderPath); } - fs.writeFileSync(incrementalFilePath, fileContent); - - try { - let [astBuildCommand, fullBuildCommand] = await getBscArgs(projectRootPath); - - let astArgs = argsFromCommandString(astBuildCommand); - let buildArgs = argsFromCommandString(fullBuildCommand); - - let callArgs: Array = [ - "-I", - path.resolve(projectRootPath, incrementalFileFolderLocation), - ]; + fs.writeFileSync(entry.file.incrementalFilePath, fileContent); - buildArgs.forEach(([key, value]: Array) => { - if (key === "-I") { - callArgs.push("-I", path.resolve(projectRootPath, "lib/bs", value)); - } else if (key === "-bs-v") { - callArgs.push("-bs-v", Date.now().toString()); - } else if (key === "-bs-package-output") { - return; - } else if (value == null || value === "") { - callArgs.push(key); + const process = cp.execFile( + entry.project.bscBinaryLocation, + await entry.project.callArgs, + { cwd: entry.project.rootPath }, + (error, _stdout, stderr) => { + if (!error?.killed) { + if (debug) + console.log( + `Recompiled ${entry.file.sourceFileName} in ${ + (performance.now() - startTime) / 1000 + }s` + ); } else { - callArgs.push(key, value); + if (debug) + console.log( + `Compilation of ${entry.file.sourceFileName} was killed.` + ); } - }); + if ( + !error?.killed && + triggerToken != null && + verifyTriggerToken(entry.file.sourceFilePath, triggerToken) + ) { + const { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); + const res = (Object.values(result)[0] ?? []) + .map((d) => ({ + ...d, + message: removeAnsiCodes(d.message), + })) + // Filter out a few unwanted parser errors since we run the parser in ignore mode + .filter( + (d) => + !d.message.startsWith("Uninterpreted extension 'rescript.") && + !d.message.includes( + `/${INCREMENTAL_FOLDER_NAME}/${entry.file.sourceFileName}` + ) + ); - astArgs.forEach(([key, value]: Array) => { - if (key.startsWith("-bs-jsx")) { - callArgs.push(key, value); - } else if (key.startsWith("-ppx")) { - callArgs.push(key, value); + const notification: p.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "textDocument/publishDiagnostics", + params: { + uri: pathToFileURL(entry.file.sourceFilePath), + diagnostics: res, + }, + }; + send(notification); } - }); - - callArgs.push("-color", "never"); - callArgs.push("-ignore-parse-errors"); - - callArgs = callArgs.filter((v) => v != null && v !== ""); - callArgs.push(incrementalFilePath); - - let process = cp.execFile( - bscExe, - callArgs, - { cwd: projectRootPath }, - (error, _stdout, stderr) => { - if (!error?.killed) { - if (debug) - console.log( - `Recompiled ${fileName} in ${ - (performance.now() - startTime) / 1000 - }s` - ); - } else { - if (debug) console.log(`Compilation of ${fileName} was killed.`); - } - onCompilationFinished?.(); - if (!error?.killed && verifyTriggerToken(filePath, triggerToken)) { - let { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); - let res = (Object.values(result)[0] ?? []) - .map((d) => ({ - ...d, - message: removeAnsiCodes(d.message), - })) - // Filter out a few unwanted parser errors since we run the parser in ignore mode - .filter( - (d) => - !d.message.startsWith("Uninterpreted extension 'rescript.") && - !d.message.includes(`/${incrementalFolderName}/${fileName}`) - ); - - let notification: p.NotificationMessage = { - jsonrpc: c.jsonrpcVersion, - method: "textDocument/publishDiagnostics", - params: { - uri: pathToFileURL(filePath), - diagnostics: res, - }, - }; - send(notification); - } - } - ); - let listeners = compileContentsListeners.get(filePath) ?? []; - listeners.push(() => { - process.kill("SIGKILL"); - }); - compileContentsListeners.set(filePath, listeners); - } catch (e) { - console.error(e); - } + onCompilationFinished?.(); + entry.compilation = null; + } + ); + entry.compilationKilledListeners.push(() => { + process.kill("SIGKILL"); + }); } export function handleUpdateOpenedFile( @@ -353,6 +451,7 @@ export function handleUpdateOpenedFile( send: send, onCompilationFinished?: () => void ) { + // Clear save status, since it's now updated. savedIncrementalFiles.delete(filePath); triggerIncrementalCompilationOfFile( filePath, @@ -365,3 +464,12 @@ export function handleUpdateOpenedFile( export function handleDidSaveTextDocument(filePath: string) { savedIncrementalFiles.add(filePath); } + +export function handleClosedFile(filePath: string) { + const entry = incrementallyCompiledFileInfo.get(filePath); + if (entry == null) return; + cleanUpIncrementalFiles(filePath, entry.project.rootPath); + incrementallyCompiledFileInfo.delete(filePath); + savedIncrementalFiles.delete(filePath); +} +export function handleOpenedFile(_filePath: string, _projectRootPath: string) {} diff --git a/server/src/server.ts b/server/src/server.ts index 2db2e25d2..c12cba7a1 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -243,9 +243,7 @@ let openedFile = (fileUri: string, fileContent: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { - if (config.extensionConfiguration.incrementalTypechecking.enabled) { - ic.cleanUpIncrementalFiles(filePath, projectRootPath); - } + ic.handleOpenedFile(filePath, projectRootPath); let projectRootState = projectsFiles.get(projectRootPath); if (projectRootState == null) { if (config.extensionConfiguration.incrementalTypechecking.enabled) { @@ -328,13 +326,14 @@ let openedFile = (fileUri: string, fileContent: string) => { let closedFile = (fileUri: string) => { let filePath = fileURLToPath(fileUri); + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + ic.handleClosedFile(filePath); + } + stupidFileContentCache.delete(filePath); let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { - if (config.extensionConfiguration.incrementalTypechecking.enabled) { - ic.cleanUpIncrementalFiles(filePath, projectRootPath); - } let root = projectsFiles.get(projectRootPath); if (root != null) { root.openFiles.delete(filePath); From b0a9db691d44e753c623e98ec48b7cdd1e645e94 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Mar 2024 18:57:52 +0100 Subject: [PATCH 06/14] fixes and adjustments for namespaced packages --- server/src/incrementalCompilation.ts | 72 ++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index cb18402f4..a914b8edb 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -37,6 +37,8 @@ type IncrementallyCompiledFileInfo = { sourceFileName: string; /** Module name of the source file. */ moduleName: string; + /** Namespaced module name of the source file. */ + moduleNameNamespaced: string; /** Path to where the incremental file is saved. */ incrementalFilePath: string; }; @@ -66,6 +68,8 @@ type IncrementallyCompiledFileInfo = { callArgs: Promise>; /** The location of the incremental folder for this project. */ incrementalFolderPath: string; + /** The ReScript version. */ + rescriptVersion: string; }; }; @@ -120,7 +124,7 @@ export function fileIsIncrementallyCompiled(filePath: string): boolean { const pathToCheck = path.resolve( entry.project.incrementalFolderPath, - entry.file.moduleName + ".cmt" + entry.file.moduleNameNamespaced + ".cmt" ); return fs.existsSync(pathToCheck); @@ -130,12 +134,18 @@ export function cleanUpIncrementalFiles( filePath: string, projectRootPath: string ) { + const namespace = utils.getNamespaceNameFromConfigFile(projectRootPath); const fileNameNoExt = path.basename(filePath, ".res"); + const moduleNameNamespaced = + namespace.kind === "success" && namespace.result !== "" + ? `${fileNameNoExt}-${namespace.result}` + : fileNameNoExt; + [ - fileNameNoExt + ".ast", - fileNameNoExt + ".cmt", - fileNameNoExt + ".cmi", - fileNameNoExt + ".cmj", + moduleNameNamespaced + ".ast", + moduleNameNamespaced + ".cmt", + moduleNameNamespaced + ".cmi", + moduleNameNamespaced + ".cmj", fileNameNoExt + ".res", ].forEach((file) => { fs.unlink( @@ -259,25 +269,50 @@ function triggerIncrementalCompilationOfFile( if (incrementalFileCacheEntry == null) { // New file const projectRootPath = utils.findProjectRootOfFile(filePath); - if (projectRootPath == null) return; + if (projectRootPath == null) { + if (debug) console.log("Did not find project root path for " + filePath); + return; + } const namespaceName = utils.getNamespaceNameFromConfigFile(projectRootPath); - if (namespaceName.kind === "error") return; + if (namespaceName.kind === "error") { + if (debug) + console.log("Getting namespace config errored for " + filePath); + return; + } const bscBinaryLocation = utils.findBscExeBinary(projectRootPath); - if (bscBinaryLocation == null) return; - const filenameNoExt = path.basename(filePath, ".res"); - const moduleName = - namespaceName.result === "" - ? filenameNoExt - : `${namespaceName.result}-${filenameNoExt}`; + if (bscBinaryLocation == null) { + if (debug) + console.log("Could not find bsc binary location for " + filePath); + return; + } + const moduleName = path.basename(filePath, ".res"); + const moduleNameNamespaced = + namespaceName.result !== "" + ? `${moduleName}-${namespaceName.result}` + : moduleName; const incrementalFolderPath = path.join( projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION ); + let rescriptVersion = ""; + try { + rescriptVersion = cp + .execFileSync(bscBinaryLocation, ["-version"]) + .toString() + .trim(); + } catch (e) { + console.error(e); + } + if (rescriptVersion.startsWith("ReScript ")) { + rescriptVersion = rescriptVersion.replace("ReScript ", ""); + } + incrementalFileCacheEntry = { file: { moduleName, + moduleNameNamespaced, sourceFileName: moduleName + ".res", sourceFilePath: filePath, incrementalFilePath: path.join( @@ -290,6 +325,7 @@ function triggerIncrementalCompilationOfFile( callArgs: Promise.resolve([]), bscBinaryLocation, incrementalFolderPath, + rescriptVersion, }, buildNinja: null, compilation: null, @@ -367,7 +403,13 @@ async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) { }); callArgs.push("-color", "never"); - callArgs.push("-ignore-parse-errors"); + if ( + !entry.project.rescriptVersion.startsWith("9.") && + !entry.project.rescriptVersion.startsWith("10.") + ) { + // Only available in v11+ + callArgs.push("-ignore-parse-errors"); + } callArgs = callArgs.filter((v) => v != null && v !== ""); callArgs.push(entry.file.incrementalFilePath); @@ -426,6 +468,8 @@ async function compileContents( ) ); + // if (res.length === 0) console.log(stderr, _stdout); TODO: Log that there was an error executing..? Maybe write to a log file. + const notification: p.NotificationMessage = { jsonrpc: c.jsonrpcVersion, method: "textDocument/publishDiagnostics", From e78d3a257861f15ec4ca497dc5c39bc11a5816bc Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Mar 2024 18:59:21 +0100 Subject: [PATCH 07/14] adjust watching --- .vscode/launch.json | 4 ++-- package.json | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 641bd40e8..8c9ceb4bc 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -11,7 +11,7 @@ "--extensionDevelopmentPath=${workspaceRoot}" ], "outFiles": [ - "${workspaceRoot}/client/out/*.js" + "${workspaceRoot}/client/out/**/*.js" ], "preLaunchTask": { "type": "npm", @@ -25,7 +25,7 @@ "port": 6009, "restart": true, "outFiles": [ - "${workspaceRoot}/server/out/*.js" + "${workspaceRoot}/server/out/**/*.js" ] }, { diff --git a/package.json b/package.json index c17968600..c20536ece 100644 --- a/package.json +++ b/package.json @@ -235,9 +235,9 @@ }, "scripts": { "clean": "rm -rf client/out server/out", - "vscode:prepublish": "npm run clean && npm run compile && npm run bundle", - "compile": "tsc", - "watch": "tsc -w", + "vscode:prepublish": "npm run clean && npm run bundle", + "compile": "tsc -b", + "watch": "tsc -b -w", "postinstall": "cd server && npm i && cd ../client && npm i && cd ../tools && npm i && cd ../tools/tests && npm i && cd ../../analysis/tests && npm i && cd ../reanalyze/examples/deadcode && npm i && cd ../termination && npm i", "bundle-server": "esbuild server/src/cli.ts --bundle --sourcemap --outfile=server/out/cli.js --format=cjs --platform=node --loader:.node=file --minify", "bundle-client": "esbuild client/src/extension.ts --bundle --external:vscode --sourcemap --outfile=client/out/extension.js --format=cjs --platform=node --loader:.node=file --minify", From f43f9a27bf40f8daef9e809657e3a83ca18cefde Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Mar 2024 20:17:21 +0100 Subject: [PATCH 08/14] make incremental compilation of resi work --- analysis/src/Cmt.ml | 6 +++- server/src/incrementalCompilation.ts | 41 ++++++++++++---------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/analysis/src/Cmt.ml b/analysis/src/Cmt.ml index cf3f7ff94..228d64b59 100644 --- a/analysis/src/Cmt.ml +++ b/analysis/src/Cmt.ml @@ -17,7 +17,11 @@ let fullFromUri ~uri = BuildSystem.namespacedName package.namespace (FindFiles.getName path) in let incrementalCmtPath = - package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName ^ ".cmt" + package.rootPath ^ "/lib/bs/___incremental" ^ "/" ^ moduleName + ^ + match Files.classifySourceFile path with + | Resi -> ".cmti" + | _ -> ".cmt" in match fullForCmt ~moduleName ~package ~uri incrementalCmtPath with | Some cmtInfo -> diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index a914b8edb..e24e26db9 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -9,21 +9,6 @@ import * as cp from "node:child_process"; import { send } from "./config"; import * as c from "./constants"; -/* - * TODO CMT stuff - * - Compile resi - * - Wait a certain threshold for compilation before using the old cmt - * - Monorepos? Namespaces - * - Fix races by using fs.promises - * - Split some of the cache per project root instead so less recalc is needed - * - Throttle error messages? - * Questions: - * - We trigger no incremental compilation for other files. This might be problematic if we want to go across files. Track dependencies? What's supposed to be used when and where? - * Improvements: - * - Ask build system for compconste build command for file X, instead of piecing together from build.ninja. Rewatch? - * - Have build system communicate what was actually rebuilt after compilation finishes. - */ - const debug = true; const INCREMENTAL_FOLDER_NAME = "___incremental"; @@ -31,6 +16,8 @@ const INCREMENTAL_FILE_FOLDER_LOCATION = `lib/bs/${INCREMENTAL_FOLDER_NAME}`; type IncrementallyCompiledFileInfo = { file: { + /** File type. */ + extension: ".res" | ".resi"; /** Path to the source file. */ sourceFilePath: string; /** Name of the source file. */ @@ -134,19 +121,28 @@ export function cleanUpIncrementalFiles( filePath: string, projectRootPath: string ) { + const ext = filePath.endsWith(".resi") ? ".resi" : ".res"; const namespace = utils.getNamespaceNameFromConfigFile(projectRootPath); - const fileNameNoExt = path.basename(filePath, ".res"); + const fileNameNoExt = path.basename(filePath, ext); const moduleNameNamespaced = namespace.kind === "success" && namespace.result !== "" ? `${fileNameNoExt}-${namespace.result}` : fileNameNoExt; + fs.unlink( + path.resolve( + projectRootPath, + INCREMENTAL_FILE_FOLDER_LOCATION, + path.basename(filePath) + ), + (_) => {} + ); + [ moduleNameNamespaced + ".ast", moduleNameNamespaced + ".cmt", moduleNameNamespaced + ".cmi", moduleNameNamespaced + ".cmj", - fileNameNoExt + ".res", ].forEach((file) => { fs.unlink( path.resolve(projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION, file), @@ -285,7 +281,8 @@ function triggerIncrementalCompilationOfFile( console.log("Could not find bsc binary location for " + filePath); return; } - const moduleName = path.basename(filePath, ".res"); + const ext = filePath.endsWith(".resi") ? ".resi" : ".res"; + const moduleName = path.basename(filePath, ext); const moduleNameNamespaced = namespaceName.result !== "" ? `${moduleName}-${namespaceName.result}` @@ -311,14 +308,12 @@ function triggerIncrementalCompilationOfFile( incrementalFileCacheEntry = { file: { + extension: ext, moduleName, moduleNameNamespaced, - sourceFileName: moduleName + ".res", + sourceFileName: moduleName + ext, sourceFilePath: filePath, - incrementalFilePath: path.join( - incrementalFolderPath, - moduleName + ".res" - ), + incrementalFilePath: path.join(incrementalFolderPath, moduleName + ext), }, project: { rootPath: projectRootPath, From 3e2ba1b17294949bc87da945e033853daaad09b2 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 4 Mar 2024 20:32:55 +0100 Subject: [PATCH 09/14] report error when incremental typechecking fails --- server/src/incrementalCompilation.ts | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index e24e26db9..d34bf8d27 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -65,6 +65,7 @@ const incrementallyCompiledFileInfo: Map< IncrementallyCompiledFileInfo > = new Map(); const savedIncrementalFiles: Set = new Set(); +const hasReportedFeatureFailedError: Set = new Set(); export function cleanupIncrementalFilesAfterCompilation( compilerLogPath: string @@ -418,7 +419,6 @@ async function compileContents( ) { const triggerToken = entry.compilation?.triggerToken; const startTime = performance.now(); - // TODO: Move somewhere else? if (!fs.existsSync(entry.project.incrementalFolderPath)) { fs.mkdirSync(entry.project.incrementalFolderPath); } @@ -463,7 +463,28 @@ async function compileContents( ) ); - // if (res.length === 0) console.log(stderr, _stdout); TODO: Log that there was an error executing..? Maybe write to a log file. + if ( + res.length === 0 && + stderr !== "" && + !hasReportedFeatureFailedError.has(entry.project.rootPath) + ) { + hasReportedFeatureFailedError.add(entry.project.rootPath); + const logfile = path.resolve( + entry.project.incrementalFolderPath, + "error.log" + ); + fs.writeFileSync(logfile, stderr); + let params: p.ShowMessageParams = { + type: p.MessageType.Warning, + message: `[Incremental typechecking] Something might have gone wrong with incremental type checking. Check out the [error log](file://${logfile}) and report this issue please.`, + }; + let message: p.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "window/showMessage", + params: params, + }; + send(message); + } const notification: p.NotificationMessage = { jsonrpc: c.jsonrpcVersion, From a0ba60b2ccb635a3f5d72ff391bcd0f21e6ff81b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 5 Mar 2024 08:04:32 +0100 Subject: [PATCH 10/14] config tweaks --- package.json | 5 +++++ server/src/config.ts | 4 +++- server/src/incrementalCompilation.ts | 19 ++++++++++--------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index c20536ece..f6b8c6e4d 100644 --- a/package.json +++ b/package.json @@ -181,6 +181,11 @@ "default": false, "description": "(beta/experimental) Enable incremental type checking." }, + "rescript.settings.incrementalTypechecking.acrossFiles": { + "type": "boolean", + "default": false, + "description": "(beta/experimental) Enable incremental type checking across files, so that unsaved file A gets access to unsaved file B." + }, "rescript.settings.binaryPath": { "type": [ "string", diff --git a/server/src/config.ts b/server/src/config.ts index a56ea65e2..97eea31f5 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -17,6 +17,7 @@ export interface extensionConfiguration { }; incrementalTypechecking: { enabled: boolean; + acrossFiles: boolean; }; } @@ -37,7 +38,8 @@ let config: { extensionConfiguration: extensionConfiguration } = { enabled: true, }, incrementalTypechecking: { - enabled: true, + enabled: false, + acrossFiles: true, }, }, }; diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index d34bf8d27..1e2411f75 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -6,7 +6,7 @@ import readline from "readline"; import { performance } from "perf_hooks"; import * as p from "vscode-languageserver-protocol"; import * as cp from "node:child_process"; -import { send } from "./config"; +import config, { send } from "./config"; import * as c from "./constants"; const debug = true; @@ -368,10 +368,14 @@ async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) { const astArgs = argsFromCommandString(astBuildCommand); const buildArgs = argsFromCommandString(fullBuildCommand); - let callArgs: Array = [ - "-I", - path.resolve(entry.project.rootPath, INCREMENTAL_FILE_FOLDER_LOCATION), - ]; + let callArgs: Array = []; + + if (config.extensionConfiguration.incrementalTypechecking.acrossFiles) { + callArgs.push( + "-I", + path.resolve(entry.project.rootPath, INCREMENTAL_FILE_FOLDER_LOCATION) + ); + } buildArgs.forEach(([key, value]: Array) => { if (key === "-I") { @@ -399,10 +403,7 @@ async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) { }); callArgs.push("-color", "never"); - if ( - !entry.project.rescriptVersion.startsWith("9.") && - !entry.project.rescriptVersion.startsWith("10.") - ) { + if (parseInt(entry.project.rescriptVersion.split(".")[0] ?? "10") >= 11) { // Only available in v11+ callArgs.push("-ignore-parse-errors"); } From c442f7ebac094ef98ead3d972a8e907ec7e05d27 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 5 Mar 2024 08:17:29 +0100 Subject: [PATCH 11/14] more guard rails --- server/src/incrementalCompilation.ts | 190 +++++++++++++++------------ 1 file changed, 109 insertions(+), 81 deletions(-) diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index 1e2411f75..fbe630ce0 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -52,7 +52,7 @@ type IncrementallyCompiledFileInfo = { /** Computed location of bsc. */ bscBinaryLocation: string; /** The arguments needed for bsc, derived from the project configuration/build.ninja. */ - callArgs: Promise>; + callArgs: Promise | null>; /** The location of the incremental folder for this project. */ incrementalFolderPath: string; /** The ReScript version. */ @@ -153,7 +153,7 @@ export function cleanUpIncrementalFiles( } function getBscArgs( entry: IncrementallyCompiledFileInfo -): Promise> { +): Promise | null> { const buildNinjaPath = path.resolve( entry.project.rootPath, "lib/bs/build.ninja" @@ -161,9 +161,15 @@ function getBscArgs( const cacheEntry = entry.buildNinja; let stat: fs.Stats | null = null; if (cacheEntry != null) { - stat = fs.statSync(buildNinjaPath); - if (cacheEntry.fileMtime >= stat.mtimeMs) { - return Promise.resolve(cacheEntry.rawExtracted); + try { + stat = fs.statSync(buildNinjaPath); + } catch {} + if (stat != null) { + if (cacheEntry.fileMtime >= stat.mtimeMs) { + return Promise.resolve(cacheEntry.rawExtracted); + } + } else { + return Promise.resolve(null); } } return new Promise((resolve, _reject) => { @@ -363,7 +369,9 @@ function verifyTriggerToken(filePath: string, triggerToken: number): boolean { ); } async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) { - const [astBuildCommand, fullBuildCommand] = await getBscArgs(entry); + const res = await getBscArgs(entry); + if (res == null) return null; + const [astBuildCommand, fullBuildCommand] = res; const astArgs = argsFromCommandString(astBuildCommand); const buildArgs = argsFromCommandString(fullBuildCommand); @@ -419,91 +427,111 @@ async function compileContents( onCompilationFinished?: () => void ) { const triggerToken = entry.compilation?.triggerToken; + const callArgs = await entry.project.callArgs; + if (callArgs == null) { + if (debug) { + console.log( + "Could not figure out call args. Maybe build.ninja does not exist yet?" + ); + } + return; + } + const startTime = performance.now(); if (!fs.existsSync(entry.project.incrementalFolderPath)) { - fs.mkdirSync(entry.project.incrementalFolderPath); + try { + fs.mkdirSync(entry.project.incrementalFolderPath); + } catch {} } - fs.writeFileSync(entry.file.incrementalFilePath, fileContent); - - const process = cp.execFile( - entry.project.bscBinaryLocation, - await entry.project.callArgs, - { cwd: entry.project.rootPath }, - (error, _stdout, stderr) => { - if (!error?.killed) { - if (debug) - console.log( - `Recompiled ${entry.file.sourceFileName} in ${ - (performance.now() - startTime) / 1000 - }s` - ); - } else { - if (debug) - console.log( - `Compilation of ${entry.file.sourceFileName} was killed.` - ); - } - if ( - !error?.killed && - triggerToken != null && - verifyTriggerToken(entry.file.sourceFilePath, triggerToken) - ) { - const { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); - const res = (Object.values(result)[0] ?? []) - .map((d) => ({ - ...d, - message: removeAnsiCodes(d.message), - })) - // Filter out a few unwanted parser errors since we run the parser in ignore mode - .filter( - (d) => - !d.message.startsWith("Uninterpreted extension 'rescript.") && - !d.message.includes( - `/${INCREMENTAL_FOLDER_NAME}/${entry.file.sourceFileName}` - ) - ); - + try { + fs.writeFileSync(entry.file.incrementalFilePath, fileContent); + + const process = cp.execFile( + entry.project.bscBinaryLocation, + callArgs, + { cwd: entry.project.rootPath }, + (error, _stdout, stderr) => { + if (!error?.killed) { + if (debug) + console.log( + `Recompiled ${entry.file.sourceFileName} in ${ + (performance.now() - startTime) / 1000 + }s` + ); + } else { + if (debug) + console.log( + `Compilation of ${entry.file.sourceFileName} was killed.` + ); + } if ( - res.length === 0 && - stderr !== "" && - !hasReportedFeatureFailedError.has(entry.project.rootPath) + !error?.killed && + triggerToken != null && + verifyTriggerToken(entry.file.sourceFilePath, triggerToken) ) { - hasReportedFeatureFailedError.add(entry.project.rootPath); - const logfile = path.resolve( - entry.project.incrementalFolderPath, - "error.log" - ); - fs.writeFileSync(logfile, stderr); - let params: p.ShowMessageParams = { - type: p.MessageType.Warning, - message: `[Incremental typechecking] Something might have gone wrong with incremental type checking. Check out the [error log](file://${logfile}) and report this issue please.`, - }; - let message: p.NotificationMessage = { + const { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); + const res = (Object.values(result)[0] ?? []) + .map((d) => ({ + ...d, + message: removeAnsiCodes(d.message), + })) + // Filter out a few unwanted parser errors since we run the parser in ignore mode + .filter( + (d) => + !d.message.startsWith("Uninterpreted extension 'rescript.") && + !d.message.includes( + `/${INCREMENTAL_FOLDER_NAME}/${entry.file.sourceFileName}` + ) + ); + + if ( + res.length === 0 && + stderr !== "" && + !hasReportedFeatureFailedError.has(entry.project.rootPath) + ) { + try { + hasReportedFeatureFailedError.add(entry.project.rootPath); + const logfile = path.resolve( + entry.project.incrementalFolderPath, + "error.log" + ); + fs.writeFileSync(logfile, stderr); + let params: p.ShowMessageParams = { + type: p.MessageType.Warning, + message: `[Incremental typechecking] Something might have gone wrong with incremental type checking. Check out the [error log](file://${logfile}) and report this issue please.`, + }; + let message: p.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "window/showMessage", + params: params, + }; + send(message); + } catch (e) { + console.error(e); + } + } + + const notification: p.NotificationMessage = { jsonrpc: c.jsonrpcVersion, - method: "window/showMessage", - params: params, + method: "textDocument/publishDiagnostics", + params: { + uri: pathToFileURL(entry.file.sourceFilePath), + diagnostics: res, + }, }; - send(message); + send(notification); } - - const notification: p.NotificationMessage = { - jsonrpc: c.jsonrpcVersion, - method: "textDocument/publishDiagnostics", - params: { - uri: pathToFileURL(entry.file.sourceFilePath), - diagnostics: res, - }, - }; - send(notification); + onCompilationFinished?.(); + entry.compilation = null; } - onCompilationFinished?.(); - entry.compilation = null; - } - ); - entry.compilationKilledListeners.push(() => { - process.kill("SIGKILL"); - }); + ); + entry.compilationKilledListeners.push(() => { + process.kill("SIGKILL"); + }); + } catch (e) { + console.error(e); + } } export function handleUpdateOpenedFile( From 64f8633b2998764abaf52698458cd9a1f56eee79 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 5 Mar 2024 08:53:29 +0100 Subject: [PATCH 12/14] debug logging switch --- package.json | 5 +++++ server/src/config.ts | 2 ++ server/src/incrementalCompilation.ts | 28 +++++++++++++++++++--------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index f6b8c6e4d..8931307dd 100644 --- a/package.json +++ b/package.json @@ -186,6 +186,11 @@ "default": false, "description": "(beta/experimental) Enable incremental type checking across files, so that unsaved file A gets access to unsaved file B." }, + "rescript.settings.incrementalTypechecking.debugLogging": { + "type": "boolean", + "default": false, + "description": "(debug) Enable debug logging (ends up in the extension output)." + }, "rescript.settings.binaryPath": { "type": [ "string", diff --git a/server/src/config.ts b/server/src/config.ts index 97eea31f5..33db6743e 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -18,6 +18,7 @@ export interface extensionConfiguration { incrementalTypechecking: { enabled: boolean; acrossFiles: boolean; + debugLogging: boolean; }; } @@ -40,6 +41,7 @@ let config: { extensionConfiguration: extensionConfiguration } = { incrementalTypechecking: { enabled: false, acrossFiles: true, + debugLogging: true, }, }, }; diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index fbe630ce0..42c6c9ab5 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -9,7 +9,9 @@ import * as cp from "node:child_process"; import config, { send } from "./config"; import * as c from "./constants"; -const debug = true; +function debug() { + return config.extensionConfiguration.incrementalTypechecking.debugLogging; +} const INCREMENTAL_FOLDER_NAME = "___incremental"; const INCREMENTAL_FILE_FOLDER_LOCATION = `lib/bs/${INCREMENTAL_FOLDER_NAME}`; @@ -112,7 +114,9 @@ export function fileIsIncrementallyCompiled(filePath: string): boolean { const pathToCheck = path.resolve( entry.project.incrementalFolderPath, - entry.file.moduleNameNamespaced + ".cmt" + entry.file.moduleNameNamespaced + entry.file.extension === ".res" + ? ".cmt" + : ".cmti" ); return fs.existsSync(pathToCheck); @@ -142,6 +146,7 @@ export function cleanUpIncrementalFiles( [ moduleNameNamespaced + ".ast", moduleNameNamespaced + ".cmt", + moduleNameNamespaced + ".cmti", moduleNameNamespaced + ".cmi", moduleNameNamespaced + ".cmj", ].forEach((file) => { @@ -163,7 +168,11 @@ function getBscArgs( if (cacheEntry != null) { try { stat = fs.statSync(buildNinjaPath); - } catch {} + } catch { + if (debug()) { + console.log("Did not find build.ninja, cannot proceed."); + } + } if (stat != null) { if (cacheEntry.fileMtime >= stat.mtimeMs) { return Promise.resolve(cacheEntry.rawExtracted); @@ -273,18 +282,19 @@ function triggerIncrementalCompilationOfFile( // New file const projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath == null) { - if (debug) console.log("Did not find project root path for " + filePath); + if (debug()) + console.log("Did not find project root path for " + filePath); return; } const namespaceName = utils.getNamespaceNameFromConfigFile(projectRootPath); if (namespaceName.kind === "error") { - if (debug) + if (debug()) console.log("Getting namespace config errored for " + filePath); return; } const bscBinaryLocation = utils.findBscExeBinary(projectRootPath); if (bscBinaryLocation == null) { - if (debug) + if (debug()) console.log("Could not find bsc binary location for " + filePath); return; } @@ -429,7 +439,7 @@ async function compileContents( const triggerToken = entry.compilation?.triggerToken; const callArgs = await entry.project.callArgs; if (callArgs == null) { - if (debug) { + if (debug()) { console.log( "Could not figure out call args. Maybe build.ninja does not exist yet?" ); @@ -453,14 +463,14 @@ async function compileContents( { cwd: entry.project.rootPath }, (error, _stdout, stderr) => { if (!error?.killed) { - if (debug) + if (debug()) console.log( `Recompiled ${entry.file.sourceFileName} in ${ (performance.now() - startTime) / 1000 }s` ); } else { - if (debug) + if (debug()) console.log( `Compilation of ${entry.file.sourceFileName} was killed.` ); From 34793b6dc8c89c34fd6141928ead337b147c19de Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 5 Mar 2024 21:26:02 +0100 Subject: [PATCH 13/14] robustness --- server/src/incrementalCompilation.ts | 139 ++++++++++++++++++--------- server/src/server.ts | 25 +---- 2 files changed, 95 insertions(+), 69 deletions(-) diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index 42c6c9ab5..baf450a15 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -8,6 +8,7 @@ import * as p from "vscode-languageserver-protocol"; import * as cp from "node:child_process"; import config, { send } from "./config"; import * as c from "./constants"; +import * as chokidar from "chokidar"; function debug() { return config.extensionConfiguration.incrementalTypechecking.debugLogging; @@ -30,6 +31,8 @@ type IncrementallyCompiledFileInfo = { moduleNameNamespaced: string; /** Path to where the incremental file is saved. */ incrementalFilePath: string; + /** Location of the original type file. */ + originalTypeFileLocation: string; }; /** Cache for build.ninja assets. */ buildNinja: { @@ -46,7 +49,7 @@ type IncrementallyCompiledFileInfo = { triggerToken: number; } | null; /** Listeners for when compilation of this file is killed. List always cleared after each invocation. */ - compilationKilledListeners: Array<() => void>; + killCompilationListeners: Array<() => void>; /** Project specific information. */ project: { /** The root path of the project. */ @@ -66,22 +69,41 @@ const incrementallyCompiledFileInfo: Map< string, IncrementallyCompiledFileInfo > = new Map(); -const savedIncrementalFiles: Set = new Set(); const hasReportedFeatureFailedError: Set = new Set(); - -export function cleanupIncrementalFilesAfterCompilation( - compilerLogPath: string -) { - const projectRootPath = utils.findProjectRootOfFile(compilerLogPath); - if (projectRootPath != null) { - savedIncrementalFiles.forEach((filePath) => { - if (filePath.startsWith(projectRootPath)) { - cleanUpIncrementalFiles(filePath, projectRootPath); - savedIncrementalFiles.delete(filePath); +const originalTypeFileToFilePath: Map = new Map(); + +let incrementalFilesWatcher = chokidar + .watch([], { + awaitWriteFinish: { + stabilityThreshold: 1, + }, + }) + .on("all", (e, changedPath) => { + if (e !== "change" && e !== "unlink") return; + const filePath = originalTypeFileToFilePath.get(changedPath); + if (filePath != null) { + const entry = incrementallyCompiledFileInfo.get(filePath); + if (entry != null) { + if (debug()) { + console.log( + "[watcher] Cleaning up incremental files for " + filePath + ); + } + if (entry.compilation != null) { + if (debug()) { + console.log("[watcher] Was compiling, killing"); + } + clearTimeout(entry.compilation.timeout); + entry.killCompilationListeners.forEach((cb) => cb()); + entry.compilation = null; + } + cleanUpIncrementalFiles( + entry.file.sourceFilePath, + entry.project.rootPath + ); } - }); - } -} + } + }); export function removeIncrementalFileFolder( projectRootPath: string, @@ -97,6 +119,9 @@ export function removeIncrementalFileFolder( } export function recreateIncrementalFileFolder(projectRootPath: string) { + if (debug()) { + console.log("Recreating incremental file folder"); + } removeIncrementalFileFolder(projectRootPath, () => { fs.mkdir( path.resolve(projectRootPath, INCREMENTAL_FILE_FOLDER_LOCATION), @@ -105,23 +130,6 @@ export function recreateIncrementalFileFolder(projectRootPath: string) { }); } -export function fileIsIncrementallyCompiled(filePath: string): boolean { - const entry = incrementallyCompiledFileInfo.get(filePath); - - if (entry == null) { - return false; - } - - const pathToCheck = path.resolve( - entry.project.incrementalFolderPath, - entry.file.moduleNameNamespaced + entry.file.extension === ".res" - ? ".cmt" - : ".cmti" - ); - - return fs.existsSync(pathToCheck); -} - export function cleanUpIncrementalFiles( filePath: string, projectRootPath: string @@ -134,6 +142,10 @@ export function cleanUpIncrementalFiles( ? `${fileNameNoExt}-${namespace.result}` : fileNameNoExt; + if (debug()) { + console.log("Cleaning up incremental file assets for: " + fileNameNoExt); + } + fs.unlink( path.resolve( projectRootPath, @@ -323,8 +335,20 @@ function triggerIncrementalCompilationOfFile( rescriptVersion = rescriptVersion.replace("ReScript ", ""); } + let originalTypeFileLocation = path.resolve( + projectRootPath, + "lib/bs", + path.relative(projectRootPath, filePath) + ); + + const parsed = path.parse(originalTypeFileLocation); + parsed.ext = ext === ".res" ? ".cmt" : ".cmti"; + parsed.base = ""; + originalTypeFileLocation = path.format(parsed); + incrementalFileCacheEntry = { file: { + originalTypeFileLocation, extension: ext, moduleName, moduleNameNamespaced, @@ -341,12 +365,20 @@ function triggerIncrementalCompilationOfFile( }, buildNinja: null, compilation: null, - compilationKilledListeners: [], + killCompilationListeners: [], }; incrementalFileCacheEntry.project.callArgs = figureOutBscArgs( incrementalFileCacheEntry ); + // Set up watcher for relevant cmt/cmti + incrementalFilesWatcher.add([ + incrementalFileCacheEntry.file.originalTypeFileLocation, + ]); + originalTypeFileToFilePath.set( + incrementalFileCacheEntry.file.originalTypeFileLocation, + incrementalFileCacheEntry.file.sourceFilePath + ); incrementallyCompiledFileInfo.set(filePath, incrementalFileCacheEntry); } @@ -354,8 +386,8 @@ function triggerIncrementalCompilationOfFile( const entry = incrementalFileCacheEntry; if (entry.compilation != null) { clearTimeout(entry.compilation.timeout); - entry.compilationKilledListeners.forEach((cb) => cb()); - entry.compilationKilledListeners = []; + entry.killCompilationListeners.forEach((cb) => cb()); + entry.killCompilationListeners = []; } const triggerToken = performance.now(); const timeout = setTimeout(() => { @@ -475,11 +507,17 @@ async function compileContents( `Compilation of ${entry.file.sourceFileName} was killed.` ); } + let hasIgnoredErrorMessages = false; if ( !error?.killed && triggerToken != null && verifyTriggerToken(entry.file.sourceFilePath, triggerToken) ) { + if (debug()) { + console.log("Resetting compilation status."); + } + // Reset compilation status as this compilation finished + entry.compilation = null; const { result } = utils.parseCompilerLogOutput(`${stderr}\n#Done()`); const res = (Object.values(result)[0] ?? []) .map((d) => ({ @@ -487,17 +525,23 @@ async function compileContents( message: removeAnsiCodes(d.message), })) // Filter out a few unwanted parser errors since we run the parser in ignore mode - .filter( - (d) => + .filter((d) => { + if ( !d.message.startsWith("Uninterpreted extension 'rescript.") && !d.message.includes( `/${INCREMENTAL_FOLDER_NAME}/${entry.file.sourceFileName}` ) - ); + ) { + hasIgnoredErrorMessages = true; + return true; + } + return false; + }); if ( res.length === 0 && stderr !== "" && + !hasIgnoredErrorMessages && !hasReportedFeatureFailedError.has(entry.project.rootPath) ) { try { @@ -533,10 +577,9 @@ async function compileContents( send(notification); } onCompilationFinished?.(); - entry.compilation = null; } ); - entry.compilationKilledListeners.push(() => { + entry.killCompilationListeners.push(() => { process.kill("SIGKILL"); }); } catch (e) { @@ -550,8 +593,9 @@ export function handleUpdateOpenedFile( send: send, onCompilationFinished?: () => void ) { - // Clear save status, since it's now updated. - savedIncrementalFiles.delete(filePath); + if (debug()) { + console.log("Updated: " + filePath); + } triggerIncrementalCompilationOfFile( filePath, fileContent, @@ -560,15 +604,14 @@ export function handleUpdateOpenedFile( ); } -export function handleDidSaveTextDocument(filePath: string) { - savedIncrementalFiles.add(filePath); -} - export function handleClosedFile(filePath: string) { + if (debug()) { + console.log("Closed: " + filePath); + } const entry = incrementallyCompiledFileInfo.get(filePath); if (entry == null) return; cleanUpIncrementalFiles(filePath, entry.project.rootPath); incrementallyCompiledFileInfo.delete(filePath); - savedIncrementalFiles.delete(filePath); + originalTypeFileToFilePath.delete(entry.file.originalTypeFileLocation); + incrementalFilesWatcher.unwatch([entry.file.originalTypeFileLocation]); } -export function handleOpenedFile(_filePath: string, _projectRootPath: string) {} diff --git a/server/src/server.ts b/server/src/server.ts index c12cba7a1..d25976550 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -211,13 +211,7 @@ let compilerLogsWatcher = chokidar stabilityThreshold: 1, }, }) - .on("all", (_e, changedPath) => { - if (config.extensionConfiguration.incrementalTypechecking.enabled) { - // We clean up all incremental file assets when the compiler has rebuilt, - // because at that point we want to use the main, compiled type assets - // instead of the incremental type assets. - ic.cleanupIncrementalFilesAfterCompilation(changedPath); - } + .on("all", (_e, _changedPath) => { sendUpdatedDiagnostics(); sendCompilationFinishedMessage(); if (config.extensionConfiguration.inlayHints?.enable === true) { @@ -243,7 +237,6 @@ let openedFile = (fileUri: string, fileContent: string) => { let projectRootPath = utils.findProjectRootOfFile(filePath); if (projectRootPath != null) { - ic.handleOpenedFile(filePath, projectRootPath); let projectRootState = projectsFiles.get(projectRootPath); if (projectRootState == null) { if (config.extensionConfiguration.incrementalTypechecking.enabled) { @@ -801,13 +794,11 @@ function format(msg: p.RequestMessage): Array { } let updateDiagnosticSyntax = (fileUri: string, fileContent: string) => { - let filePath = fileURLToPath(fileUri); - if ( - config.extensionConfiguration.incrementalTypechecking.enabled && - ic.fileIsIncrementallyCompiled(filePath) - ) { + if (config.extensionConfiguration.incrementalTypechecking.enabled) { + // The incremental typechecking already sends syntax diagnostics. return; } + let filePath = fileURLToPath(fileUri); let extension = path.extname(filePath); let tmpname = utils.createFileInTempDir(extension); fs.writeFileSync(tmpname, fileContent, { encoding: "utf-8" }); @@ -1019,14 +1010,6 @@ function onMessage(msg: p.Message) { } else { process.exit(1); } - } else if (msg.method === p.DidSaveTextDocumentNotification.method) { - if (config.extensionConfiguration.incrementalTypechecking.enabled) { - // We track all incremental file assets when a file is actually saved, - // so we can clean them up. - let params = msg.params as p.DidSaveTextDocumentParams; - let filePath = fileURLToPath(params.textDocument.uri); - ic.handleDidSaveTextDocument(filePath); - } } else if (msg.method === DidOpenTextDocumentNotification.method) { let params = msg.params as p.DidOpenTextDocumentParams; openedFile(params.textDocument.uri, params.textDocument.text); From b2200b56da53b347f4fb23bc206aa1a86f9c1383 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 5 Mar 2024 21:40:09 +0100 Subject: [PATCH 14/14] changelog --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32a86574..ab829dc8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,14 +12,15 @@ ## master -## 1.42.0 +#### :rocket: New Feature -#### :bug: Bug Fix +- Experimental support for type checking without saving the file :tada:. https://github.com/rescript-lang/rescript-vscode/pull/939 -- Fix issue with unlabelled arg code swallowing completions. https://github.com/rescript-lang/rescript-vscode/pull/937 +## 1.42.0 #### :bug: Bug Fix +- Fix issue with unlabelled arg code swallowing completions. https://github.com/rescript-lang/rescript-vscode/pull/937 - Fix issue where completion inside of switch expression would not work in some cases. https://github.com/rescript-lang/rescript-vscode/pull/936 - Fix bug that made empty prop expressions in JSX not complete if in the middle of a JSX element. https://github.com/rescript-lang/rescript-vscode/pull/935