From ddf27f5c906b33e387df2d0c0fc471c4a06ea96f Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 28 Dec 2021 12:00:42 +0100 Subject: [PATCH 01/14] implement basic commands for running dead code analysis via reanalyze, and reporting the results to the VSCode problems pane --- client/src/commands.ts | 9 +- client/src/commands/dead_code_analysis.ts | 242 ++++++++++++++++++++++ client/src/extension.ts | 17 +- package.json | 8 + 4 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 client/src/commands/dead_code_analysis.ts diff --git a/client/src/commands.ts b/client/src/commands.ts index e580e40f8..0aa31cc98 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -1,6 +1,7 @@ import * as fs from "fs"; -import { window } from "vscode"; +import { window, DiagnosticCollection } from "vscode"; import { LanguageClient, RequestType } from "vscode-languageclient/node"; +import { runDeadCodeAnalysisWithReanalyze } from "./commands/dead_code_analysis"; interface CreateInterfaceRequestParams { uri: string; @@ -31,3 +32,9 @@ export const createInterface = (client: LanguageClient) => { uri: editor.document.uri.toString(), }); }; + +export const deadCodeAnalysisWithReanalyze = ( + diagnosticsCollection: DiagnosticCollection +) => { + runDeadCodeAnalysisWithReanalyze(diagnosticsCollection); +}; diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts new file mode 100644 index 000000000..029c927db --- /dev/null +++ b/client/src/commands/dead_code_analysis.ts @@ -0,0 +1,242 @@ +import * as fs from "fs"; +import * as cp from "child_process"; +import * as path from "path"; +import { + window, + DiagnosticCollection, + Diagnostic, + Range, + Position, + DiagnosticSeverity, + Uri, +} from "vscode"; +import { DocumentUri } from "vscode-languageclient"; + +let findProjectRootOfFile = (source: DocumentUri): null | DocumentUri => { + let dir = path.dirname(source); + if (fs.existsSync(path.join(dir, "bsconfig.json"))) { + return dir; + } else { + if (dir === source) { + // reached top + return null; + } else { + return findProjectRootOfFile(dir); + } + } +}; + +let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/; + +let extractFileInfo = ( + fileInfo: string +): { + filePath: string; + line: string; + characters: string; +} | null => { + let m; + + let filePath: string | null = null; + let line: string | null = null; + let characters: string | null = null; + + while ((m = fileInfoRegex.exec(fileInfo)) !== null) { + if (m.index === fileInfoRegex.lastIndex) { + fileInfoRegex.lastIndex++; + } + + m.forEach((match: string, groupIndex: number) => { + switch (groupIndex) { + case 1: { + filePath = match; + break; + } + case 2: { + line = match; + break; + } + case 3: { + characters = match; + break; + } + } + }); + } + + if (filePath != null && line != null && characters != null) { + return { + filePath, + line, + characters, + }; + } + + return null; +}; + +let dceTextToDiagnostics = ( + dceText: string +): { + diagnosticsMap: Map; + hasMoreDiagnostics: boolean; + totalDiagnosticsCount: number; + savedDiagnostics: number; +} => { + let diagnosticsMap: Map = new Map(); + let savedDiagnostics = 0; + let totalDiagnosticsCount = 0; + + // Each section with a single issue found is seprated by two line breaks in + // the reanalyze output. Here's an example of how a section typically looks: + // + // Warning Dead Value + // File "/Users/zth/git/rescript-intro/src/Machine.res", line 2, characters 0-205 + // +use is never used + // <-- line 2 + // @dead("+use") let use = (initialState: 'a, handleEvent: ('a, 'b) => 'a) => { + dceText.split("\n\n").forEach((chunk) => { + let [ + title, + fileInfo, + text, + + // These aren't in use yet, but can power code actions that can + // automatically add the @dead annotations reanalyze is suggesting to us. + _lineNumToReplace, + _lineContentToReplace, + + ..._rest + ] = chunk.split("\n"); + + let processedFileInfo = extractFileInfo(fileInfo); + + if (processedFileInfo != null) { + // We'll limit the amount of diagnostics to display somewhat. This is also + // in part because we don't "watch" for changes with reanalyze, so the + // user will need to re-run the command fairly often anyway as issues are + // fixed, in order to get rid of the stale problems reported. + if (savedDiagnostics > 20) { + totalDiagnosticsCount++; + return; + } + + // reanalyze prints the severity first in the title, and then the rest of + // the title after. + let [severityRaw, ...issueTitleParts] = title.split(" "); + let issueTitle = issueTitleParts.join(" "); + + let [startCharacter, endCharacter] = + processedFileInfo.characters.split("-"); + + let startPos = new Position( + // reanalyze reports lines as index 1 based, while VSCode wants them + // index 0 based. + parseInt(processedFileInfo.line, 10) - 1, + parseInt(startCharacter, 10) + ); + + // This isn't correct, and will only highlight a single line, even if the + // highlight should in fact span multiple lines. This is because reanalyze + // gives us a line start, and a character interval. But, VSCode wants a + // line for the end of the selection too. And, to figure that out, we'd + // need to read the entire file with the issues present and calculate it. + // So, postponing that for now. Maybe there's a better way. + let endPos = new Position( + parseInt(processedFileInfo.line, 10) - 1, + parseInt(endCharacter, 10) + ); + + let severity = + severityRaw === "Error" + ? DiagnosticSeverity.Error + : DiagnosticSeverity.Warning; + + let diagnostic = new Diagnostic( + new Range(startPos, endPos), + `${issueTitle}: ${text}`, + severity + ); + + savedDiagnostics++; + + if (diagnosticsMap.has(processedFileInfo.filePath)) { + diagnosticsMap.get(processedFileInfo.filePath).push(diagnostic); + } else { + diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); + } + } + }); + + return { + diagnosticsMap, + hasMoreDiagnostics: totalDiagnosticsCount > savedDiagnostics, + savedDiagnostics, + totalDiagnosticsCount, + }; +}; + +export const runDeadCodeAnalysisWithReanalyze = ( + diagnosticsCollection: DiagnosticCollection +) => { + diagnosticsCollection.clear(); + let currentDocument = window.activeTextEditor.document; + let projectRootOfFile = findProjectRootOfFile(currentDocument.uri.fsPath); + + if (!projectRootOfFile) { + window.showWarningMessage( + "Could not determine project root of current file." + ); + return; + } + + const p = cp.spawn("npx", ["reanalyze", "-dce"], { + cwd: projectRootOfFile, + }); + + if (p.stdout == null) { + window.showErrorMessage("Something went wrong."); + return; + } + + let data = ""; + + p.stdout.on("data", (d) => { + data += d; + }); + + p.stderr?.on("data", (e) => { + // Sometimes the compiler artifacts has been corrupted in some way, and + // reanalyze will spit out a "End_of_file" exception. The solution is to + // clean and rebuild the ReScript project, which we can tell the user about + // here. + if (e.includes("End_of_file")) { + window.showErrorMessage( + `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project, and then run this command again.` + ); + } else { + window.showErrorMessage( + `Something went wrong trying to run reanalyze: '${e}'` + ); + } + }); + + p.on("close", () => { + let { + diagnosticsMap, + hasMoreDiagnostics, + savedDiagnostics, + totalDiagnosticsCount, + } = dceTextToDiagnostics(data); + + diagnosticsMap.forEach((diagnostics, filePath) => { + diagnosticsCollection.set(Uri.parse(filePath), diagnostics); + }); + + if (hasMoreDiagnostics) { + window.showInformationMessage( + `Showing ${savedDiagnostics} of in total ${totalDiagnosticsCount} issues found. Re-run this command again as you fix issues.` + ); + } + }); +}; diff --git a/client/src/extension.ts b/client/src/extension.ts index 6bcc9a7de..8c7e2acb2 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -1,5 +1,5 @@ import * as path from "path"; -import { workspace, ExtensionContext, commands } from "vscode"; +import { workspace, ExtensionContext, commands, languages } from "vscode"; import { LanguageClient, @@ -98,11 +98,26 @@ export function activate(context: ExtensionContext) { clientOptions ); + // Create a custom diagnostics collection, for cases where we want to report diagnostics + // programatically from inside of the extension. + let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); + // Register custom commands commands.registerCommand("rescript-vscode.create_interface", () => { customCommands.createInterface(client); }); + commands.registerCommand("rescript-vscode.run_dead_code_analysis", () => { + customCommands.deadCodeAnalysisWithReanalyze(diagnosticsCollection); + }); + + commands.registerCommand( + "rescript-vscode.clear_dead_code_analysis_results", + () => { + diagnosticsCollection.clear(); + } + ); + // Start the client. This will also launch the server client.start(); } diff --git a/package.json b/package.json index b17f237e9..c8dadd399 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,14 @@ { "command": "rescript-vscode.create_interface", "title": "ReScript: Create an interface file for this implementation file." + }, + { + "command": "rescript-vscode.run_dead_code_analysis", + "title": "ReScript: Run dead code analysis." + }, + { + "command": "rescript-vscode.clear_dead_code_analysis_results", + "title": "ReScript: Clear dead code analysis results." } ], "snippets": [ From 5e2f0bc66d33717e208c697e4cec68469b560f70 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 20:54:26 +0100 Subject: [PATCH 02/14] change approach, and set up the dead code analysis as an explicit mode you can activate and deactivate --- client/src/commands.ts | 13 ++- client/src/commands/dead_code_analysis.ts | 129 +++++++++++----------- client/src/extension.ts | 82 ++++++++++++-- package.json | 8 +- server/src/server.ts | 9 ++ 5 files changed, 158 insertions(+), 83 deletions(-) diff --git a/client/src/commands.ts b/client/src/commands.ts index 0aa31cc98..9eb86c76c 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -1,7 +1,10 @@ import * as fs from "fs"; import { window, DiagnosticCollection } from "vscode"; import { LanguageClient, RequestType } from "vscode-languageclient/node"; -import { runDeadCodeAnalysisWithReanalyze } from "./commands/dead_code_analysis"; +import { + DiagnosticsResultCodeActionsMap, + runDeadCodeAnalysisWithReanalyze, +} from "./commands/dead_code_analysis"; interface CreateInterfaceRequestParams { uri: string; @@ -34,7 +37,11 @@ export const createInterface = (client: LanguageClient) => { }; export const deadCodeAnalysisWithReanalyze = ( - diagnosticsCollection: DiagnosticCollection + diagnosticsCollection: DiagnosticCollection, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { - runDeadCodeAnalysisWithReanalyze(diagnosticsCollection); + runDeadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); }; diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 029c927db..792e99b5d 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -1,4 +1,3 @@ -import * as fs from "fs"; import * as cp from "child_process"; import * as path from "path"; import { @@ -9,24 +8,17 @@ import { Position, DiagnosticSeverity, Uri, + CodeAction, + CodeActionKind, + WorkspaceEdit, } from "vscode"; -import { DocumentUri } from "vscode-languageclient"; - -let findProjectRootOfFile = (source: DocumentUri): null | DocumentUri => { - let dir = path.dirname(source); - if (fs.existsSync(path.join(dir, "bsconfig.json"))) { - return dir; - } else { - if (dir === source) { - // reached top - return null; - } else { - return findProjectRootOfFile(dir); - } - } -}; -let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/; +export type DiagnosticsResultCodeActionsMap = Map< + string, + { range: Range; codeAction: CodeAction }[] +>; + +let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/g; let extractFileInfo = ( fileInfo: string @@ -76,16 +68,12 @@ let extractFileInfo = ( }; let dceTextToDiagnostics = ( - dceText: string + dceText: string, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ): { diagnosticsMap: Map; - hasMoreDiagnostics: boolean; - totalDiagnosticsCount: number; - savedDiagnostics: number; } => { let diagnosticsMap: Map = new Map(); - let savedDiagnostics = 0; - let totalDiagnosticsCount = 0; // Each section with a single issue found is seprated by two line breaks in // the reanalyze output. Here's an example of how a section typically looks: @@ -103,8 +91,8 @@ let dceTextToDiagnostics = ( // These aren't in use yet, but can power code actions that can // automatically add the @dead annotations reanalyze is suggesting to us. - _lineNumToReplace, - _lineContentToReplace, + lineNumToReplace, + lineContentToReplace, ..._rest ] = chunk.split("\n"); @@ -112,15 +100,6 @@ let dceTextToDiagnostics = ( let processedFileInfo = extractFileInfo(fileInfo); if (processedFileInfo != null) { - // We'll limit the amount of diagnostics to display somewhat. This is also - // in part because we don't "watch" for changes with reanalyze, so the - // user will need to re-run the command fairly often anyway as issues are - // fixed, in order to get rid of the stale problems reported. - if (savedDiagnostics > 20) { - totalDiagnosticsCount++; - return; - } - // reanalyze prints the severity first in the title, and then the rest of // the title after. let [severityRaw, ...issueTitleParts] = title.split(" "); @@ -147,51 +126,81 @@ let dceTextToDiagnostics = ( parseInt(endCharacter, 10) ); + let issueLocationRange = new Range(startPos, endPos); + let severity = severityRaw === "Error" ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning; let diagnostic = new Diagnostic( - new Range(startPos, endPos), + issueLocationRange, `${issueTitle}: ${text}`, severity ); - savedDiagnostics++; - if (diagnosticsMap.has(processedFileInfo.filePath)) { diagnosticsMap.get(processedFileInfo.filePath).push(diagnostic); } else { diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); } + + // Let's see if there's a valid code action that can be produced from this diagnostic. + if (lineNumToReplace != null && lineContentToReplace != null) { + let actualLineToReplaceStr = lineNumToReplace.split("<-- line ").pop(); + + if (actualLineToReplaceStr != null) { + let codeAction = new CodeAction(`Annotate with @dead`); + codeAction.kind = CodeActionKind.RefactorRewrite; + + let codeActionEdit = new WorkspaceEdit(); + + codeActionEdit.replace( + Uri.parse(processedFileInfo.filePath), + // Make sure the full line is replaced + new Range( + new Position(issueLocationRange.start.line, 0), + new Position(issueLocationRange.start.line, 999999) + ), + lineContentToReplace.trim() + ); + + codeAction.edit = codeActionEdit; + + if (diagnosticsResultCodeActions.has(processedFileInfo.filePath)) { + diagnosticsResultCodeActions + .get(processedFileInfo.filePath) + .push({ range: issueLocationRange, codeAction }); + } else { + diagnosticsResultCodeActions.set(processedFileInfo.filePath, [ + { range: issueLocationRange, codeAction }, + ]); + } + } + } } }); return { diagnosticsMap, - hasMoreDiagnostics: totalDiagnosticsCount > savedDiagnostics, - savedDiagnostics, - totalDiagnosticsCount, }; }; export const runDeadCodeAnalysisWithReanalyze = ( - diagnosticsCollection: DiagnosticCollection + diagnosticsCollection: DiagnosticCollection, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { + // Clear old diagnostics and code actions state. diagnosticsCollection.clear(); - let currentDocument = window.activeTextEditor.document; - let projectRootOfFile = findProjectRootOfFile(currentDocument.uri.fsPath); + diagnosticsResultCodeActions.clear(); - if (!projectRootOfFile) { - window.showWarningMessage( - "Could not determine project root of current file." - ); - return; - } + let currentDocument = window.activeTextEditor.document; - const p = cp.spawn("npx", ["reanalyze", "-dce"], { - cwd: projectRootOfFile, + let p = cp.spawn("npx", ["reanalyze", "-dce"], { + // Pointing reanalyze to the dir of the current file path is fine, + // because reanalyze will walk upwards looking for a bsconfig.json in + // order to find the correct project root. + cwd: path.dirname(currentDocument.uri.fsPath), }); if (p.stdout == null) { @@ -212,7 +221,7 @@ export const runDeadCodeAnalysisWithReanalyze = ( // here. if (e.includes("End_of_file")) { window.showErrorMessage( - `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project, and then run this command again.` + `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project.` ); } else { window.showErrorMessage( @@ -222,21 +231,13 @@ export const runDeadCodeAnalysisWithReanalyze = ( }); p.on("close", () => { - let { - diagnosticsMap, - hasMoreDiagnostics, - savedDiagnostics, - totalDiagnosticsCount, - } = dceTextToDiagnostics(data); + let { diagnosticsMap } = dceTextToDiagnostics( + data, + diagnosticsResultCodeActions + ); diagnosticsMap.forEach((diagnostics, filePath) => { diagnosticsCollection.set(Uri.parse(filePath), diagnostics); }); - - if (hasMoreDiagnostics) { - window.showInformationMessage( - `Showing ${savedDiagnostics} of in total ${totalDiagnosticsCount} issues found. Re-run this command again as you fix issues.` - ); - } }); }; diff --git a/client/src/extension.ts b/client/src/extension.ts index 8c7e2acb2..2f5cf3dfa 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -1,5 +1,12 @@ import * as path from "path"; -import { workspace, ExtensionContext, commands, languages } from "vscode"; +import { + workspace, + ExtensionContext, + commands, + languages, + window, + StatusBarAlignment, +} from "vscode"; import { LanguageClient, @@ -9,6 +16,7 @@ import { } from "vscode-languageclient/node"; import * as customCommands from "./commands"; +import { DiagnosticsResultCodeActionsMap } from "./commands/dead_code_analysis"; let client: LanguageClient; @@ -102,24 +110,78 @@ export function activate(context: ExtensionContext) { // programatically from inside of the extension. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); + // This map will hold code actions produced by the dead code analysis. + let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); + + let inDeadCodeAnalysisMode = { current: false }; + + let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( + StatusBarAlignment.Right + ); + + // This code actions provider yields the code actions potentially extracted + // from the dead code analysis. + languages.registerCodeActionsProvider("rescript", { + async provideCodeActions(document, rangeOrSelection) { + let availableActions = + diagnosticsResultCodeActions.get(document.uri.fsPath) ?? []; + + return availableActions + .filter( + ({ range }) => + range.contains(rangeOrSelection) || range.isEqual(rangeOrSelection) + ) + .map(({ codeAction }) => codeAction); + }, + }); + // Register custom commands commands.registerCommand("rescript-vscode.create_interface", () => { customCommands.createInterface(client); }); - commands.registerCommand("rescript-vscode.run_dead_code_analysis", () => { - customCommands.deadCodeAnalysisWithReanalyze(diagnosticsCollection); + // Starts the dead code analysis mode. + commands.registerCommand("rescript-vscode.start_dead_code_analysis", () => { + inDeadCodeAnalysisMode.current = true; + deadCodeAnalysisRunningStatusBarItem.command = + "rescript-vscode.stop_dead_code_analysis"; + deadCodeAnalysisRunningStatusBarItem.show(); + deadCodeAnalysisRunningStatusBarItem.text = + "$(debug-stop) Stop Dead Code Analysis mode"; + customCommands.deadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); }); - commands.registerCommand( - "rescript-vscode.clear_dead_code_analysis_results", - () => { - diagnosticsCollection.clear(); - } - ); + commands.registerCommand("rescript-vscode.stop_dead_code_analysis", () => { + inDeadCodeAnalysisMode.current = false; + diagnosticsCollection.clear(); + diagnosticsResultCodeActions.clear(); + deadCodeAnalysisRunningStatusBarItem.hide(); + }); + + // This sets up a listener that, if we're in dead code analysis mode, triggers + // dead code analysis as the LS server reports that ReScript compilation has + // finished. This is needed because dead code analysis must wait until + // compilation has finished, and the most reliable source for that is the LS + // server, that already keeps track of when the compiler finishes in order to + // other provide fresh diagnostics. + client.onReady().then(() => { + context.subscriptions.push( + client.onNotification("rescript/compilationFinished", () => { + if (inDeadCodeAnalysisMode.current === true) { + customCommands.deadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); + } + }) + ); + }); // Start the client. This will also launch the server - client.start(); + context.subscriptions.push(client.start()); } export function deactivate(): Thenable | undefined { diff --git a/package.json b/package.json index c8dadd399..a99c2448f 100644 --- a/package.json +++ b/package.json @@ -40,12 +40,8 @@ "title": "ReScript: Create an interface file for this implementation file." }, { - "command": "rescript-vscode.run_dead_code_analysis", - "title": "ReScript: Run dead code analysis." - }, - { - "command": "rescript-vscode.clear_dead_code_analysis_results", - "title": "ReScript: Clear dead code analysis results." + "command": "rescript-vscode.start_dead_code_analysis", + "title": "ReScript: Start dead code analysis." } ], "snippets": [ diff --git a/server/src/server.ts b/server/src/server.ts index cbb461e0e..8db3abc04 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -115,6 +115,14 @@ let deleteProjectDiagnostics = (projectRootPath: string) => { projectsFiles.delete(projectRootPath); } }; +let sendCompilationFinishedMessage = () => { + let notification: m.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "rescript/compilationFinished", + }; + + send(notification); +}; let compilerLogsWatcher = chokidar .watch([], { @@ -124,6 +132,7 @@ let compilerLogsWatcher = chokidar }) .on("all", (_e, changedPath) => { sendUpdatedDiagnostics(); + sendCompilationFinishedMessage(); }); let stopWatchingCompilerLog = () => { // TODO: cleanup of compilerLogs? From a9a0fa9394ba8240e7d19cfb7b73761ee2c73a52 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 20:55:30 +0100 Subject: [PATCH 03/14] clarify --- client/src/extension.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/src/extension.ts b/client/src/extension.ts index 2f5cf3dfa..6e399f225 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -106,15 +106,16 @@ export function activate(context: ExtensionContext) { clientOptions ); - // Create a custom diagnostics collection, for cases where we want to report diagnostics - // programatically from inside of the extension. + // Create a custom diagnostics collection, for cases where we want to report + // diagnostics programatically from inside of the extension. The reason this + // is separate from the diagnostics provided by the LS server itself, is that + // this should be possible to clear independently of the other diagnostics + // coming from the ReScript compiler itself. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); // This map will hold code actions produced by the dead code analysis. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); - let inDeadCodeAnalysisMode = { current: false }; - let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( StatusBarAlignment.Right ); From 1c204fd1b306623c07cbe5e96321ce6f61049732 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 21:05:53 +0100 Subject: [PATCH 04/14] cleanip --- client/src/commands/dead_code_analysis.ts | 26 ++++++++++++----------- client/src/extension.ts | 9 ++++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 792e99b5d..ce3a644e0 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -76,7 +76,12 @@ let dceTextToDiagnostics = ( let diagnosticsMap: Map = new Map(); // Each section with a single issue found is seprated by two line breaks in - // the reanalyze output. Here's an example of how a section typically looks: + // the reanalyze output. The section contains information about the issue + // itself, what line/char and in what file it was found, as well as a + // suggestion for what you can replace the line containing the issue with to + // suppress the issue reported. + // + // Here's an example of how a section typically looks: // // Warning Dead Value // File "/Users/zth/git/rescript-intro/src/Machine.res", line 2, characters 0-205 @@ -89,12 +94,10 @@ let dceTextToDiagnostics = ( fileInfo, text, - // These aren't in use yet, but can power code actions that can - // automatically add the @dead annotations reanalyze is suggesting to us. + // These, if they exist, will power code actions for inserting the "fixed" + // line that reanalyze might suggest. lineNumToReplace, lineContentToReplace, - - ..._rest ] = chunk.split("\n"); let processedFileInfo = extractFileInfo(fileInfo); @@ -102,6 +105,7 @@ let dceTextToDiagnostics = ( if (processedFileInfo != null) { // reanalyze prints the severity first in the title, and then the rest of // the title after. + window.showInformationMessage(title); let [severityRaw, ...issueTitleParts] = title.split(" "); let issueTitle = issueTitleParts.join(" "); @@ -115,12 +119,6 @@ let dceTextToDiagnostics = ( parseInt(startCharacter, 10) ); - // This isn't correct, and will only highlight a single line, even if the - // highlight should in fact span multiple lines. This is because reanalyze - // gives us a line start, and a character interval. But, VSCode wants a - // line for the end of the selection too. And, to figure that out, we'd - // need to read the entire file with the issues present and calculate it. - // So, postponing that for now. Maybe there's a better way. let endPos = new Position( parseInt(processedFileInfo.line, 10) - 1, parseInt(endCharacter, 10) @@ -145,7 +143,11 @@ let dceTextToDiagnostics = ( diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); } - // Let's see if there's a valid code action that can be produced from this diagnostic. + // If reanalyze suggests a fix, we'll set that up as a refactor code + // action in VSCode. This way, it'll be easy to suppress the issue + // reported if wanted. We also save the range of the issue, so we can + // leverage that to make looking up the code actions for each cursor + // position very cheap. if (lineNumToReplace != null && lineContentToReplace != null) { let actualLineToReplaceStr = lineNumToReplace.split("<-- line ").pop(); diff --git a/client/src/extension.ts b/client/src/extension.ts index 6e399f225..e2f3ce7a1 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -108,12 +108,13 @@ export function activate(context: ExtensionContext) { // Create a custom diagnostics collection, for cases where we want to report // diagnostics programatically from inside of the extension. The reason this - // is separate from the diagnostics provided by the LS server itself, is that + // is separate from the diagnostics provided by the LS server itself is that // this should be possible to clear independently of the other diagnostics - // coming from the ReScript compiler itself. + // coming from the ReScript compiler. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); - // This map will hold code actions produced by the dead code analysis. + // This map will hold code actions produced by the dead code analysis, in a + // format that's cheap to look up. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); let inDeadCodeAnalysisMode = { current: false }; let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( @@ -121,7 +122,7 @@ export function activate(context: ExtensionContext) { ); // This code actions provider yields the code actions potentially extracted - // from the dead code analysis. + // from the dead code analysis to the editor. languages.registerCodeActionsProvider("rescript", { async provideCodeActions(document, rangeOrSelection) { let availableActions = From 0dabb4ddc2a32e8fac66233c6e7f00e32b05b5de Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 21:17:19 +0100 Subject: [PATCH 05/14] keep state of what directory the dead code analysis was activated from --- client/src/commands.ts | 2 ++ client/src/commands/dead_code_analysis.ts | 8 +++--- client/src/extension.ts | 32 ++++++++++++++++++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/client/src/commands.ts b/client/src/commands.ts index 9eb86c76c..4c00b9e38 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -37,10 +37,12 @@ export const createInterface = (client: LanguageClient) => { }; export const deadCodeAnalysisWithReanalyze = ( + targetDir: string | null, diagnosticsCollection: DiagnosticCollection, diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { runDeadCodeAnalysisWithReanalyze( + targetDir, diagnosticsCollection, diagnosticsResultCodeActions ); diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index ce3a644e0..e56498be6 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -105,7 +105,6 @@ let dceTextToDiagnostics = ( if (processedFileInfo != null) { // reanalyze prints the severity first in the title, and then the rest of // the title after. - window.showInformationMessage(title); let [severityRaw, ...issueTitleParts] = title.split(" "); let issueTitle = issueTitleParts.join(" "); @@ -189,6 +188,7 @@ let dceTextToDiagnostics = ( }; export const runDeadCodeAnalysisWithReanalyze = ( + targetDir: string | null, diagnosticsCollection: DiagnosticCollection, diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { @@ -197,12 +197,10 @@ export const runDeadCodeAnalysisWithReanalyze = ( diagnosticsResultCodeActions.clear(); let currentDocument = window.activeTextEditor.document; + let cwd = targetDir ?? path.dirname(currentDocument.uri.fsPath); let p = cp.spawn("npx", ["reanalyze", "-dce"], { - // Pointing reanalyze to the dir of the current file path is fine, - // because reanalyze will walk upwards looking for a bsconfig.json in - // order to find the correct project root. - cwd: path.dirname(currentDocument.uri.fsPath), + cwd, }); if (p.stdout == null) { diff --git a/client/src/extension.ts b/client/src/extension.ts index e2f3ce7a1..36a0e5296 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -116,11 +116,15 @@ export function activate(context: ExtensionContext) { // This map will hold code actions produced by the dead code analysis, in a // format that's cheap to look up. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); - let inDeadCodeAnalysisMode = { current: false }; let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( StatusBarAlignment.Right ); + let inDeadCodeAnalysisState: { + active: boolean; + activatedFromDirectory: string | null; + } = { active: false, activatedFromDirectory: null }; + // This code actions provider yields the code actions potentially extracted // from the dead code analysis to the editor. languages.registerCodeActionsProvider("rescript", { @@ -144,22 +148,41 @@ export function activate(context: ExtensionContext) { // Starts the dead code analysis mode. commands.registerCommand("rescript-vscode.start_dead_code_analysis", () => { - inDeadCodeAnalysisMode.current = true; + // Save the directory this first ran from, and re-use that when continuously + // running the analysis. This is so that the target of the analysis does not + // change on subsequent runs, if there are multiple ReScript projects open + // in the editor. + let currentDocument = window.activeTextEditor.document; + + inDeadCodeAnalysisState.active = true; + + // Pointing reanalyze to the dir of the current file path is fine, because + // reanalyze will walk upwards looking for a bsconfig.json in order to find + // the correct project root. + inDeadCodeAnalysisState.activatedFromDirectory = path.dirname( + currentDocument.uri.fsPath + ); + deadCodeAnalysisRunningStatusBarItem.command = "rescript-vscode.stop_dead_code_analysis"; deadCodeAnalysisRunningStatusBarItem.show(); deadCodeAnalysisRunningStatusBarItem.text = "$(debug-stop) Stop Dead Code Analysis mode"; + customCommands.deadCodeAnalysisWithReanalyze( + inDeadCodeAnalysisState.activatedFromDirectory, diagnosticsCollection, diagnosticsResultCodeActions ); }); commands.registerCommand("rescript-vscode.stop_dead_code_analysis", () => { - inDeadCodeAnalysisMode.current = false; + inDeadCodeAnalysisState.active = false; + inDeadCodeAnalysisState.activatedFromDirectory = null; + diagnosticsCollection.clear(); diagnosticsResultCodeActions.clear(); + deadCodeAnalysisRunningStatusBarItem.hide(); }); @@ -172,8 +195,9 @@ export function activate(context: ExtensionContext) { client.onReady().then(() => { context.subscriptions.push( client.onNotification("rescript/compilationFinished", () => { - if (inDeadCodeAnalysisMode.current === true) { + if (inDeadCodeAnalysisState.active === true) { customCommands.deadCodeAnalysisWithReanalyze( + inDeadCodeAnalysisState.activatedFromDirectory, diagnosticsCollection, diagnosticsResultCodeActions ); From 797086949c78a3f94f0bc32d589faf594eb1c13b Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 28 Dec 2021 12:00:42 +0100 Subject: [PATCH 06/14] implement basic commands for running dead code analysis via reanalyze, and reporting the results to the VSCode problems pane --- client/src/commands.ts | 9 +- client/src/commands/dead_code_analysis.ts | 242 ++++++++++++++++++++++ client/src/extension.ts | 17 +- package.json | 8 + 4 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 client/src/commands/dead_code_analysis.ts diff --git a/client/src/commands.ts b/client/src/commands.ts index e580e40f8..0aa31cc98 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -1,6 +1,7 @@ import * as fs from "fs"; -import { window } from "vscode"; +import { window, DiagnosticCollection } from "vscode"; import { LanguageClient, RequestType } from "vscode-languageclient/node"; +import { runDeadCodeAnalysisWithReanalyze } from "./commands/dead_code_analysis"; interface CreateInterfaceRequestParams { uri: string; @@ -31,3 +32,9 @@ export const createInterface = (client: LanguageClient) => { uri: editor.document.uri.toString(), }); }; + +export const deadCodeAnalysisWithReanalyze = ( + diagnosticsCollection: DiagnosticCollection +) => { + runDeadCodeAnalysisWithReanalyze(diagnosticsCollection); +}; diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts new file mode 100644 index 000000000..029c927db --- /dev/null +++ b/client/src/commands/dead_code_analysis.ts @@ -0,0 +1,242 @@ +import * as fs from "fs"; +import * as cp from "child_process"; +import * as path from "path"; +import { + window, + DiagnosticCollection, + Diagnostic, + Range, + Position, + DiagnosticSeverity, + Uri, +} from "vscode"; +import { DocumentUri } from "vscode-languageclient"; + +let findProjectRootOfFile = (source: DocumentUri): null | DocumentUri => { + let dir = path.dirname(source); + if (fs.existsSync(path.join(dir, "bsconfig.json"))) { + return dir; + } else { + if (dir === source) { + // reached top + return null; + } else { + return findProjectRootOfFile(dir); + } + } +}; + +let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/; + +let extractFileInfo = ( + fileInfo: string +): { + filePath: string; + line: string; + characters: string; +} | null => { + let m; + + let filePath: string | null = null; + let line: string | null = null; + let characters: string | null = null; + + while ((m = fileInfoRegex.exec(fileInfo)) !== null) { + if (m.index === fileInfoRegex.lastIndex) { + fileInfoRegex.lastIndex++; + } + + m.forEach((match: string, groupIndex: number) => { + switch (groupIndex) { + case 1: { + filePath = match; + break; + } + case 2: { + line = match; + break; + } + case 3: { + characters = match; + break; + } + } + }); + } + + if (filePath != null && line != null && characters != null) { + return { + filePath, + line, + characters, + }; + } + + return null; +}; + +let dceTextToDiagnostics = ( + dceText: string +): { + diagnosticsMap: Map; + hasMoreDiagnostics: boolean; + totalDiagnosticsCount: number; + savedDiagnostics: number; +} => { + let diagnosticsMap: Map = new Map(); + let savedDiagnostics = 0; + let totalDiagnosticsCount = 0; + + // Each section with a single issue found is seprated by two line breaks in + // the reanalyze output. Here's an example of how a section typically looks: + // + // Warning Dead Value + // File "/Users/zth/git/rescript-intro/src/Machine.res", line 2, characters 0-205 + // +use is never used + // <-- line 2 + // @dead("+use") let use = (initialState: 'a, handleEvent: ('a, 'b) => 'a) => { + dceText.split("\n\n").forEach((chunk) => { + let [ + title, + fileInfo, + text, + + // These aren't in use yet, but can power code actions that can + // automatically add the @dead annotations reanalyze is suggesting to us. + _lineNumToReplace, + _lineContentToReplace, + + ..._rest + ] = chunk.split("\n"); + + let processedFileInfo = extractFileInfo(fileInfo); + + if (processedFileInfo != null) { + // We'll limit the amount of diagnostics to display somewhat. This is also + // in part because we don't "watch" for changes with reanalyze, so the + // user will need to re-run the command fairly often anyway as issues are + // fixed, in order to get rid of the stale problems reported. + if (savedDiagnostics > 20) { + totalDiagnosticsCount++; + return; + } + + // reanalyze prints the severity first in the title, and then the rest of + // the title after. + let [severityRaw, ...issueTitleParts] = title.split(" "); + let issueTitle = issueTitleParts.join(" "); + + let [startCharacter, endCharacter] = + processedFileInfo.characters.split("-"); + + let startPos = new Position( + // reanalyze reports lines as index 1 based, while VSCode wants them + // index 0 based. + parseInt(processedFileInfo.line, 10) - 1, + parseInt(startCharacter, 10) + ); + + // This isn't correct, and will only highlight a single line, even if the + // highlight should in fact span multiple lines. This is because reanalyze + // gives us a line start, and a character interval. But, VSCode wants a + // line for the end of the selection too. And, to figure that out, we'd + // need to read the entire file with the issues present and calculate it. + // So, postponing that for now. Maybe there's a better way. + let endPos = new Position( + parseInt(processedFileInfo.line, 10) - 1, + parseInt(endCharacter, 10) + ); + + let severity = + severityRaw === "Error" + ? DiagnosticSeverity.Error + : DiagnosticSeverity.Warning; + + let diagnostic = new Diagnostic( + new Range(startPos, endPos), + `${issueTitle}: ${text}`, + severity + ); + + savedDiagnostics++; + + if (diagnosticsMap.has(processedFileInfo.filePath)) { + diagnosticsMap.get(processedFileInfo.filePath).push(diagnostic); + } else { + diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); + } + } + }); + + return { + diagnosticsMap, + hasMoreDiagnostics: totalDiagnosticsCount > savedDiagnostics, + savedDiagnostics, + totalDiagnosticsCount, + }; +}; + +export const runDeadCodeAnalysisWithReanalyze = ( + diagnosticsCollection: DiagnosticCollection +) => { + diagnosticsCollection.clear(); + let currentDocument = window.activeTextEditor.document; + let projectRootOfFile = findProjectRootOfFile(currentDocument.uri.fsPath); + + if (!projectRootOfFile) { + window.showWarningMessage( + "Could not determine project root of current file." + ); + return; + } + + const p = cp.spawn("npx", ["reanalyze", "-dce"], { + cwd: projectRootOfFile, + }); + + if (p.stdout == null) { + window.showErrorMessage("Something went wrong."); + return; + } + + let data = ""; + + p.stdout.on("data", (d) => { + data += d; + }); + + p.stderr?.on("data", (e) => { + // Sometimes the compiler artifacts has been corrupted in some way, and + // reanalyze will spit out a "End_of_file" exception. The solution is to + // clean and rebuild the ReScript project, which we can tell the user about + // here. + if (e.includes("End_of_file")) { + window.showErrorMessage( + `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project, and then run this command again.` + ); + } else { + window.showErrorMessage( + `Something went wrong trying to run reanalyze: '${e}'` + ); + } + }); + + p.on("close", () => { + let { + diagnosticsMap, + hasMoreDiagnostics, + savedDiagnostics, + totalDiagnosticsCount, + } = dceTextToDiagnostics(data); + + diagnosticsMap.forEach((diagnostics, filePath) => { + diagnosticsCollection.set(Uri.parse(filePath), diagnostics); + }); + + if (hasMoreDiagnostics) { + window.showInformationMessage( + `Showing ${savedDiagnostics} of in total ${totalDiagnosticsCount} issues found. Re-run this command again as you fix issues.` + ); + } + }); +}; diff --git a/client/src/extension.ts b/client/src/extension.ts index 6bcc9a7de..8c7e2acb2 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -1,5 +1,5 @@ import * as path from "path"; -import { workspace, ExtensionContext, commands } from "vscode"; +import { workspace, ExtensionContext, commands, languages } from "vscode"; import { LanguageClient, @@ -98,11 +98,26 @@ export function activate(context: ExtensionContext) { clientOptions ); + // Create a custom diagnostics collection, for cases where we want to report diagnostics + // programatically from inside of the extension. + let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); + // Register custom commands commands.registerCommand("rescript-vscode.create_interface", () => { customCommands.createInterface(client); }); + commands.registerCommand("rescript-vscode.run_dead_code_analysis", () => { + customCommands.deadCodeAnalysisWithReanalyze(diagnosticsCollection); + }); + + commands.registerCommand( + "rescript-vscode.clear_dead_code_analysis_results", + () => { + diagnosticsCollection.clear(); + } + ); + // Start the client. This will also launch the server client.start(); } diff --git a/package.json b/package.json index 4a3c2ef3c..3360112b7 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,14 @@ { "command": "rescript-vscode.create_interface", "title": "ReScript: Create an interface file for this implementation file." + }, + { + "command": "rescript-vscode.run_dead_code_analysis", + "title": "ReScript: Run dead code analysis." + }, + { + "command": "rescript-vscode.clear_dead_code_analysis_results", + "title": "ReScript: Clear dead code analysis results." } ], "snippets": [ From ca52019610929c7c65c8086357534633227ee69c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 20:54:26 +0100 Subject: [PATCH 07/14] change approach, and set up the dead code analysis as an explicit mode you can activate and deactivate --- client/src/commands.ts | 13 ++- client/src/commands/dead_code_analysis.ts | 129 +++++++++++----------- client/src/extension.ts | 82 ++++++++++++-- package.json | 8 +- server/src/server.ts | 9 ++ 5 files changed, 158 insertions(+), 83 deletions(-) diff --git a/client/src/commands.ts b/client/src/commands.ts index 0aa31cc98..9eb86c76c 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -1,7 +1,10 @@ import * as fs from "fs"; import { window, DiagnosticCollection } from "vscode"; import { LanguageClient, RequestType } from "vscode-languageclient/node"; -import { runDeadCodeAnalysisWithReanalyze } from "./commands/dead_code_analysis"; +import { + DiagnosticsResultCodeActionsMap, + runDeadCodeAnalysisWithReanalyze, +} from "./commands/dead_code_analysis"; interface CreateInterfaceRequestParams { uri: string; @@ -34,7 +37,11 @@ export const createInterface = (client: LanguageClient) => { }; export const deadCodeAnalysisWithReanalyze = ( - diagnosticsCollection: DiagnosticCollection + diagnosticsCollection: DiagnosticCollection, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { - runDeadCodeAnalysisWithReanalyze(diagnosticsCollection); + runDeadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); }; diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 029c927db..792e99b5d 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -1,4 +1,3 @@ -import * as fs from "fs"; import * as cp from "child_process"; import * as path from "path"; import { @@ -9,24 +8,17 @@ import { Position, DiagnosticSeverity, Uri, + CodeAction, + CodeActionKind, + WorkspaceEdit, } from "vscode"; -import { DocumentUri } from "vscode-languageclient"; - -let findProjectRootOfFile = (source: DocumentUri): null | DocumentUri => { - let dir = path.dirname(source); - if (fs.existsSync(path.join(dir, "bsconfig.json"))) { - return dir; - } else { - if (dir === source) { - // reached top - return null; - } else { - return findProjectRootOfFile(dir); - } - } -}; -let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/; +export type DiagnosticsResultCodeActionsMap = Map< + string, + { range: Range; codeAction: CodeAction }[] +>; + +let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/g; let extractFileInfo = ( fileInfo: string @@ -76,16 +68,12 @@ let extractFileInfo = ( }; let dceTextToDiagnostics = ( - dceText: string + dceText: string, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ): { diagnosticsMap: Map; - hasMoreDiagnostics: boolean; - totalDiagnosticsCount: number; - savedDiagnostics: number; } => { let diagnosticsMap: Map = new Map(); - let savedDiagnostics = 0; - let totalDiagnosticsCount = 0; // Each section with a single issue found is seprated by two line breaks in // the reanalyze output. Here's an example of how a section typically looks: @@ -103,8 +91,8 @@ let dceTextToDiagnostics = ( // These aren't in use yet, but can power code actions that can // automatically add the @dead annotations reanalyze is suggesting to us. - _lineNumToReplace, - _lineContentToReplace, + lineNumToReplace, + lineContentToReplace, ..._rest ] = chunk.split("\n"); @@ -112,15 +100,6 @@ let dceTextToDiagnostics = ( let processedFileInfo = extractFileInfo(fileInfo); if (processedFileInfo != null) { - // We'll limit the amount of diagnostics to display somewhat. This is also - // in part because we don't "watch" for changes with reanalyze, so the - // user will need to re-run the command fairly often anyway as issues are - // fixed, in order to get rid of the stale problems reported. - if (savedDiagnostics > 20) { - totalDiagnosticsCount++; - return; - } - // reanalyze prints the severity first in the title, and then the rest of // the title after. let [severityRaw, ...issueTitleParts] = title.split(" "); @@ -147,51 +126,81 @@ let dceTextToDiagnostics = ( parseInt(endCharacter, 10) ); + let issueLocationRange = new Range(startPos, endPos); + let severity = severityRaw === "Error" ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning; let diagnostic = new Diagnostic( - new Range(startPos, endPos), + issueLocationRange, `${issueTitle}: ${text}`, severity ); - savedDiagnostics++; - if (diagnosticsMap.has(processedFileInfo.filePath)) { diagnosticsMap.get(processedFileInfo.filePath).push(diagnostic); } else { diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); } + + // Let's see if there's a valid code action that can be produced from this diagnostic. + if (lineNumToReplace != null && lineContentToReplace != null) { + let actualLineToReplaceStr = lineNumToReplace.split("<-- line ").pop(); + + if (actualLineToReplaceStr != null) { + let codeAction = new CodeAction(`Annotate with @dead`); + codeAction.kind = CodeActionKind.RefactorRewrite; + + let codeActionEdit = new WorkspaceEdit(); + + codeActionEdit.replace( + Uri.parse(processedFileInfo.filePath), + // Make sure the full line is replaced + new Range( + new Position(issueLocationRange.start.line, 0), + new Position(issueLocationRange.start.line, 999999) + ), + lineContentToReplace.trim() + ); + + codeAction.edit = codeActionEdit; + + if (diagnosticsResultCodeActions.has(processedFileInfo.filePath)) { + diagnosticsResultCodeActions + .get(processedFileInfo.filePath) + .push({ range: issueLocationRange, codeAction }); + } else { + diagnosticsResultCodeActions.set(processedFileInfo.filePath, [ + { range: issueLocationRange, codeAction }, + ]); + } + } + } } }); return { diagnosticsMap, - hasMoreDiagnostics: totalDiagnosticsCount > savedDiagnostics, - savedDiagnostics, - totalDiagnosticsCount, }; }; export const runDeadCodeAnalysisWithReanalyze = ( - diagnosticsCollection: DiagnosticCollection + diagnosticsCollection: DiagnosticCollection, + diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { + // Clear old diagnostics and code actions state. diagnosticsCollection.clear(); - let currentDocument = window.activeTextEditor.document; - let projectRootOfFile = findProjectRootOfFile(currentDocument.uri.fsPath); + diagnosticsResultCodeActions.clear(); - if (!projectRootOfFile) { - window.showWarningMessage( - "Could not determine project root of current file." - ); - return; - } + let currentDocument = window.activeTextEditor.document; - const p = cp.spawn("npx", ["reanalyze", "-dce"], { - cwd: projectRootOfFile, + let p = cp.spawn("npx", ["reanalyze", "-dce"], { + // Pointing reanalyze to the dir of the current file path is fine, + // because reanalyze will walk upwards looking for a bsconfig.json in + // order to find the correct project root. + cwd: path.dirname(currentDocument.uri.fsPath), }); if (p.stdout == null) { @@ -212,7 +221,7 @@ export const runDeadCodeAnalysisWithReanalyze = ( // here. if (e.includes("End_of_file")) { window.showErrorMessage( - `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project, and then run this command again.` + `Something went wrong trying to run reanalyze. Please try cleaning and rebuilding your ReScript project.` ); } else { window.showErrorMessage( @@ -222,21 +231,13 @@ export const runDeadCodeAnalysisWithReanalyze = ( }); p.on("close", () => { - let { - diagnosticsMap, - hasMoreDiagnostics, - savedDiagnostics, - totalDiagnosticsCount, - } = dceTextToDiagnostics(data); + let { diagnosticsMap } = dceTextToDiagnostics( + data, + diagnosticsResultCodeActions + ); diagnosticsMap.forEach((diagnostics, filePath) => { diagnosticsCollection.set(Uri.parse(filePath), diagnostics); }); - - if (hasMoreDiagnostics) { - window.showInformationMessage( - `Showing ${savedDiagnostics} of in total ${totalDiagnosticsCount} issues found. Re-run this command again as you fix issues.` - ); - } }); }; diff --git a/client/src/extension.ts b/client/src/extension.ts index 8c7e2acb2..2f5cf3dfa 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -1,5 +1,12 @@ import * as path from "path"; -import { workspace, ExtensionContext, commands, languages } from "vscode"; +import { + workspace, + ExtensionContext, + commands, + languages, + window, + StatusBarAlignment, +} from "vscode"; import { LanguageClient, @@ -9,6 +16,7 @@ import { } from "vscode-languageclient/node"; import * as customCommands from "./commands"; +import { DiagnosticsResultCodeActionsMap } from "./commands/dead_code_analysis"; let client: LanguageClient; @@ -102,24 +110,78 @@ export function activate(context: ExtensionContext) { // programatically from inside of the extension. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); + // This map will hold code actions produced by the dead code analysis. + let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); + + let inDeadCodeAnalysisMode = { current: false }; + + let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( + StatusBarAlignment.Right + ); + + // This code actions provider yields the code actions potentially extracted + // from the dead code analysis. + languages.registerCodeActionsProvider("rescript", { + async provideCodeActions(document, rangeOrSelection) { + let availableActions = + diagnosticsResultCodeActions.get(document.uri.fsPath) ?? []; + + return availableActions + .filter( + ({ range }) => + range.contains(rangeOrSelection) || range.isEqual(rangeOrSelection) + ) + .map(({ codeAction }) => codeAction); + }, + }); + // Register custom commands commands.registerCommand("rescript-vscode.create_interface", () => { customCommands.createInterface(client); }); - commands.registerCommand("rescript-vscode.run_dead_code_analysis", () => { - customCommands.deadCodeAnalysisWithReanalyze(diagnosticsCollection); + // Starts the dead code analysis mode. + commands.registerCommand("rescript-vscode.start_dead_code_analysis", () => { + inDeadCodeAnalysisMode.current = true; + deadCodeAnalysisRunningStatusBarItem.command = + "rescript-vscode.stop_dead_code_analysis"; + deadCodeAnalysisRunningStatusBarItem.show(); + deadCodeAnalysisRunningStatusBarItem.text = + "$(debug-stop) Stop Dead Code Analysis mode"; + customCommands.deadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); }); - commands.registerCommand( - "rescript-vscode.clear_dead_code_analysis_results", - () => { - diagnosticsCollection.clear(); - } - ); + commands.registerCommand("rescript-vscode.stop_dead_code_analysis", () => { + inDeadCodeAnalysisMode.current = false; + diagnosticsCollection.clear(); + diagnosticsResultCodeActions.clear(); + deadCodeAnalysisRunningStatusBarItem.hide(); + }); + + // This sets up a listener that, if we're in dead code analysis mode, triggers + // dead code analysis as the LS server reports that ReScript compilation has + // finished. This is needed because dead code analysis must wait until + // compilation has finished, and the most reliable source for that is the LS + // server, that already keeps track of when the compiler finishes in order to + // other provide fresh diagnostics. + client.onReady().then(() => { + context.subscriptions.push( + client.onNotification("rescript/compilationFinished", () => { + if (inDeadCodeAnalysisMode.current === true) { + customCommands.deadCodeAnalysisWithReanalyze( + diagnosticsCollection, + diagnosticsResultCodeActions + ); + } + }) + ); + }); // Start the client. This will also launch the server - client.start(); + context.subscriptions.push(client.start()); } export function deactivate(): Thenable | undefined { diff --git a/package.json b/package.json index 3360112b7..b0b7cee9f 100644 --- a/package.json +++ b/package.json @@ -40,12 +40,8 @@ "title": "ReScript: Create an interface file for this implementation file." }, { - "command": "rescript-vscode.run_dead_code_analysis", - "title": "ReScript: Run dead code analysis." - }, - { - "command": "rescript-vscode.clear_dead_code_analysis_results", - "title": "ReScript: Clear dead code analysis results." + "command": "rescript-vscode.start_dead_code_analysis", + "title": "ReScript: Start dead code analysis." } ], "snippets": [ diff --git a/server/src/server.ts b/server/src/server.ts index b407b640e..4f3159f1d 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -115,6 +115,14 @@ let deleteProjectDiagnostics = (projectRootPath: string) => { projectsFiles.delete(projectRootPath); } }; +let sendCompilationFinishedMessage = () => { + let notification: m.NotificationMessage = { + jsonrpc: c.jsonrpcVersion, + method: "rescript/compilationFinished", + }; + + send(notification); +}; let compilerLogsWatcher = chokidar .watch([], { @@ -124,6 +132,7 @@ let compilerLogsWatcher = chokidar }) .on("all", (_e, changedPath) => { sendUpdatedDiagnostics(); + sendCompilationFinishedMessage(); }); let stopWatchingCompilerLog = () => { // TODO: cleanup of compilerLogs? From 5b96014eef8f3ce3687e32b07fab6ff9ea901324 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 20:55:30 +0100 Subject: [PATCH 08/14] clarify --- client/src/extension.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/src/extension.ts b/client/src/extension.ts index 2f5cf3dfa..6e399f225 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -106,15 +106,16 @@ export function activate(context: ExtensionContext) { clientOptions ); - // Create a custom diagnostics collection, for cases where we want to report diagnostics - // programatically from inside of the extension. + // Create a custom diagnostics collection, for cases where we want to report + // diagnostics programatically from inside of the extension. The reason this + // is separate from the diagnostics provided by the LS server itself, is that + // this should be possible to clear independently of the other diagnostics + // coming from the ReScript compiler itself. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); // This map will hold code actions produced by the dead code analysis. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); - let inDeadCodeAnalysisMode = { current: false }; - let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( StatusBarAlignment.Right ); From 7171ce784e95855e6012e5e1135f3048eeccd327 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 21:05:53 +0100 Subject: [PATCH 09/14] cleanip --- client/src/commands/dead_code_analysis.ts | 26 ++++++++++++----------- client/src/extension.ts | 9 ++++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 792e99b5d..ce3a644e0 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -76,7 +76,12 @@ let dceTextToDiagnostics = ( let diagnosticsMap: Map = new Map(); // Each section with a single issue found is seprated by two line breaks in - // the reanalyze output. Here's an example of how a section typically looks: + // the reanalyze output. The section contains information about the issue + // itself, what line/char and in what file it was found, as well as a + // suggestion for what you can replace the line containing the issue with to + // suppress the issue reported. + // + // Here's an example of how a section typically looks: // // Warning Dead Value // File "/Users/zth/git/rescript-intro/src/Machine.res", line 2, characters 0-205 @@ -89,12 +94,10 @@ let dceTextToDiagnostics = ( fileInfo, text, - // These aren't in use yet, but can power code actions that can - // automatically add the @dead annotations reanalyze is suggesting to us. + // These, if they exist, will power code actions for inserting the "fixed" + // line that reanalyze might suggest. lineNumToReplace, lineContentToReplace, - - ..._rest ] = chunk.split("\n"); let processedFileInfo = extractFileInfo(fileInfo); @@ -102,6 +105,7 @@ let dceTextToDiagnostics = ( if (processedFileInfo != null) { // reanalyze prints the severity first in the title, and then the rest of // the title after. + window.showInformationMessage(title); let [severityRaw, ...issueTitleParts] = title.split(" "); let issueTitle = issueTitleParts.join(" "); @@ -115,12 +119,6 @@ let dceTextToDiagnostics = ( parseInt(startCharacter, 10) ); - // This isn't correct, and will only highlight a single line, even if the - // highlight should in fact span multiple lines. This is because reanalyze - // gives us a line start, and a character interval. But, VSCode wants a - // line for the end of the selection too. And, to figure that out, we'd - // need to read the entire file with the issues present and calculate it. - // So, postponing that for now. Maybe there's a better way. let endPos = new Position( parseInt(processedFileInfo.line, 10) - 1, parseInt(endCharacter, 10) @@ -145,7 +143,11 @@ let dceTextToDiagnostics = ( diagnosticsMap.set(processedFileInfo.filePath, [diagnostic]); } - // Let's see if there's a valid code action that can be produced from this diagnostic. + // If reanalyze suggests a fix, we'll set that up as a refactor code + // action in VSCode. This way, it'll be easy to suppress the issue + // reported if wanted. We also save the range of the issue, so we can + // leverage that to make looking up the code actions for each cursor + // position very cheap. if (lineNumToReplace != null && lineContentToReplace != null) { let actualLineToReplaceStr = lineNumToReplace.split("<-- line ").pop(); diff --git a/client/src/extension.ts b/client/src/extension.ts index 6e399f225..e2f3ce7a1 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -108,12 +108,13 @@ export function activate(context: ExtensionContext) { // Create a custom diagnostics collection, for cases where we want to report // diagnostics programatically from inside of the extension. The reason this - // is separate from the diagnostics provided by the LS server itself, is that + // is separate from the diagnostics provided by the LS server itself is that // this should be possible to clear independently of the other diagnostics - // coming from the ReScript compiler itself. + // coming from the ReScript compiler. let diagnosticsCollection = languages.createDiagnosticCollection("rescript"); - // This map will hold code actions produced by the dead code analysis. + // This map will hold code actions produced by the dead code analysis, in a + // format that's cheap to look up. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); let inDeadCodeAnalysisMode = { current: false }; let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( @@ -121,7 +122,7 @@ export function activate(context: ExtensionContext) { ); // This code actions provider yields the code actions potentially extracted - // from the dead code analysis. + // from the dead code analysis to the editor. languages.registerCodeActionsProvider("rescript", { async provideCodeActions(document, rangeOrSelection) { let availableActions = From af7bc5fb57818acc7d13a1efee8660597fc61c2f Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 30 Dec 2021 21:17:19 +0100 Subject: [PATCH 10/14] keep state of what directory the dead code analysis was activated from --- client/src/commands.ts | 2 ++ client/src/commands/dead_code_analysis.ts | 8 +++--- client/src/extension.ts | 32 ++++++++++++++++++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/client/src/commands.ts b/client/src/commands.ts index 9eb86c76c..4c00b9e38 100644 --- a/client/src/commands.ts +++ b/client/src/commands.ts @@ -37,10 +37,12 @@ export const createInterface = (client: LanguageClient) => { }; export const deadCodeAnalysisWithReanalyze = ( + targetDir: string | null, diagnosticsCollection: DiagnosticCollection, diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { runDeadCodeAnalysisWithReanalyze( + targetDir, diagnosticsCollection, diagnosticsResultCodeActions ); diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index ce3a644e0..e56498be6 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -105,7 +105,6 @@ let dceTextToDiagnostics = ( if (processedFileInfo != null) { // reanalyze prints the severity first in the title, and then the rest of // the title after. - window.showInformationMessage(title); let [severityRaw, ...issueTitleParts] = title.split(" "); let issueTitle = issueTitleParts.join(" "); @@ -189,6 +188,7 @@ let dceTextToDiagnostics = ( }; export const runDeadCodeAnalysisWithReanalyze = ( + targetDir: string | null, diagnosticsCollection: DiagnosticCollection, diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { @@ -197,12 +197,10 @@ export const runDeadCodeAnalysisWithReanalyze = ( diagnosticsResultCodeActions.clear(); let currentDocument = window.activeTextEditor.document; + let cwd = targetDir ?? path.dirname(currentDocument.uri.fsPath); let p = cp.spawn("npx", ["reanalyze", "-dce"], { - // Pointing reanalyze to the dir of the current file path is fine, - // because reanalyze will walk upwards looking for a bsconfig.json in - // order to find the correct project root. - cwd: path.dirname(currentDocument.uri.fsPath), + cwd, }); if (p.stdout == null) { diff --git a/client/src/extension.ts b/client/src/extension.ts index e2f3ce7a1..36a0e5296 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -116,11 +116,15 @@ export function activate(context: ExtensionContext) { // This map will hold code actions produced by the dead code analysis, in a // format that's cheap to look up. let diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap = new Map(); - let inDeadCodeAnalysisMode = { current: false }; let deadCodeAnalysisRunningStatusBarItem = window.createStatusBarItem( StatusBarAlignment.Right ); + let inDeadCodeAnalysisState: { + active: boolean; + activatedFromDirectory: string | null; + } = { active: false, activatedFromDirectory: null }; + // This code actions provider yields the code actions potentially extracted // from the dead code analysis to the editor. languages.registerCodeActionsProvider("rescript", { @@ -144,22 +148,41 @@ export function activate(context: ExtensionContext) { // Starts the dead code analysis mode. commands.registerCommand("rescript-vscode.start_dead_code_analysis", () => { - inDeadCodeAnalysisMode.current = true; + // Save the directory this first ran from, and re-use that when continuously + // running the analysis. This is so that the target of the analysis does not + // change on subsequent runs, if there are multiple ReScript projects open + // in the editor. + let currentDocument = window.activeTextEditor.document; + + inDeadCodeAnalysisState.active = true; + + // Pointing reanalyze to the dir of the current file path is fine, because + // reanalyze will walk upwards looking for a bsconfig.json in order to find + // the correct project root. + inDeadCodeAnalysisState.activatedFromDirectory = path.dirname( + currentDocument.uri.fsPath + ); + deadCodeAnalysisRunningStatusBarItem.command = "rescript-vscode.stop_dead_code_analysis"; deadCodeAnalysisRunningStatusBarItem.show(); deadCodeAnalysisRunningStatusBarItem.text = "$(debug-stop) Stop Dead Code Analysis mode"; + customCommands.deadCodeAnalysisWithReanalyze( + inDeadCodeAnalysisState.activatedFromDirectory, diagnosticsCollection, diagnosticsResultCodeActions ); }); commands.registerCommand("rescript-vscode.stop_dead_code_analysis", () => { - inDeadCodeAnalysisMode.current = false; + inDeadCodeAnalysisState.active = false; + inDeadCodeAnalysisState.activatedFromDirectory = null; + diagnosticsCollection.clear(); diagnosticsResultCodeActions.clear(); + deadCodeAnalysisRunningStatusBarItem.hide(); }); @@ -172,8 +195,9 @@ export function activate(context: ExtensionContext) { client.onReady().then(() => { context.subscriptions.push( client.onNotification("rescript/compilationFinished", () => { - if (inDeadCodeAnalysisMode.current === true) { + if (inDeadCodeAnalysisState.active === true) { customCommands.deadCodeAnalysisWithReanalyze( + inDeadCodeAnalysisState.activatedFromDirectory, diagnosticsCollection, diagnosticsResultCodeActions ); From aacb83d2523296f2dd9566fe426ae3e391401853 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 2 Jan 2022 18:26:37 +0100 Subject: [PATCH 11/14] support picking up full dead module diagnostics. mark dead code diagnostics as unused code. simplify/inline a few things --- client/src/commands/dead_code_analysis.ts | 39 ++++++++++++----------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index e56498be6..1b7fd89a3 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -11,6 +11,7 @@ import { CodeAction, CodeActionKind, WorkspaceEdit, + DiagnosticTag, } from "vscode"; export type DiagnosticsResultCodeActionsMap = Map< @@ -103,39 +104,41 @@ let dceTextToDiagnostics = ( let processedFileInfo = extractFileInfo(fileInfo); if (processedFileInfo != null) { - // reanalyze prints the severity first in the title, and then the rest of - // the title after. - let [severityRaw, ...issueTitleParts] = title.split(" "); - let issueTitle = issueTitleParts.join(" "); - let [startCharacter, endCharacter] = processedFileInfo.characters.split("-"); let startPos = new Position( // reanalyze reports lines as index 1 based, while VSCode wants them - // index 0 based. - parseInt(processedFileInfo.line, 10) - 1, - parseInt(startCharacter, 10) + // index 0 based. reanalyze reports diagnostics for an entire file on + // line 0 (and chars 0-0). So, we need to ensure that we don't give + // VSCode a negative line index, or it'll be sad. + Math.max(0, parseInt(processedFileInfo.line, 10) - 1), + Math.max(0, parseInt(startCharacter, 10)) ); let endPos = new Position( - parseInt(processedFileInfo.line, 10) - 1, - parseInt(endCharacter, 10) + Math.max(0, parseInt(processedFileInfo.line, 10) - 1), + Math.max(0, parseInt(endCharacter, 10)) ); - let issueLocationRange = new Range(startPos, endPos); + // Detect if this is a dead module diagnostic. If so, highlight the + // entire file. + if (title === "Warning Dead Module") { + startPos = new Position(0, 0); + endPos = new Position(99999, 0); + } - let severity = - severityRaw === "Error" - ? DiagnosticSeverity.Error - : DiagnosticSeverity.Warning; + let issueLocationRange = new Range(startPos, endPos); let diagnostic = new Diagnostic( issueLocationRange, - `${issueTitle}: ${text}`, - severity + text.trim(), + DiagnosticSeverity.Warning ); + // This will render the part of the code as unused + diagnostic.tags = [DiagnosticTag.Unnecessary]; + if (diagnosticsMap.has(processedFileInfo.filePath)) { diagnosticsMap.get(processedFileInfo.filePath).push(diagnostic); } else { @@ -151,7 +154,7 @@ let dceTextToDiagnostics = ( let actualLineToReplaceStr = lineNumToReplace.split("<-- line ").pop(); if (actualLineToReplaceStr != null) { - let codeAction = new CodeAction(`Annotate with @dead`); + let codeAction = new CodeAction(`Suppress dead code warning`); codeAction.kind = CodeActionKind.RefactorRewrite; let codeActionEdit = new WorkspaceEdit(); From 6de5e3da1e11547383f6858aecefdf0babf020fd Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 3 Jan 2022 11:55:26 +0100 Subject: [PATCH 12/14] fix line parsing issue. improve dead module full file highlight heuristic --- client/src/commands/dead_code_analysis.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 1b7fd89a3..99aea0d3c 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -19,7 +19,7 @@ export type DiagnosticsResultCodeActionsMap = Map< { range: Range; codeAction: CodeAction }[] >; -let fileInfoRegex = /File "(.+)", line (\d)+, characters ([\d-]+)/g; +let fileInfoRegex = /File "(.+)", line (\d+), characters ([\d-]+)/g; let extractFileInfo = ( fileInfo: string @@ -107,23 +107,26 @@ let dceTextToDiagnostics = ( let [startCharacter, endCharacter] = processedFileInfo.characters.split("-"); + let parsedLine = parseInt(processedFileInfo.line, 10); + let startPos = new Position( // reanalyze reports lines as index 1 based, while VSCode wants them // index 0 based. reanalyze reports diagnostics for an entire file on // line 0 (and chars 0-0). So, we need to ensure that we don't give // VSCode a negative line index, or it'll be sad. - Math.max(0, parseInt(processedFileInfo.line, 10) - 1), + Math.max(0, parsedLine - 1), Math.max(0, parseInt(startCharacter, 10)) ); let endPos = new Position( - Math.max(0, parseInt(processedFileInfo.line, 10) - 1), + Math.max(0, parsedLine - 1), Math.max(0, parseInt(endCharacter, 10)) ); - // Detect if this is a dead module diagnostic. If so, highlight the - // entire file. - if (title === "Warning Dead Module") { + // Detect if this diagnostic is for the entire file. If so, reanalyze will + // say that the issue is on line 0 and chars 0-0. This code below ensures + // that the full file is highlighted, if that's the case. + if (parsedLine === 0 && processedFileInfo.characters === "0-0") { startPos = new Position(0, 0); endPos = new Position(99999, 0); } @@ -166,7 +169,7 @@ let dceTextToDiagnostics = ( new Position(issueLocationRange.start.line, 0), new Position(issueLocationRange.start.line, 999999) ), - lineContentToReplace.trim() + lineContentToReplace ); codeAction.edit = codeActionEdit; From d7c8d8ad1df2216660d932614389c5f80746cc47 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 3 Jan 2022 20:52:03 +0100 Subject: [PATCH 13/14] minor touch ups --- client/src/commands/dead_code_analysis.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index 99aea0d3c..d57321740 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -91,7 +91,7 @@ let dceTextToDiagnostics = ( // @dead("+use") let use = (initialState: 'a, handleEvent: ('a, 'b) => 'a) => { dceText.split("\n\n").forEach((chunk) => { let [ - title, + _title, fileInfo, text, @@ -162,6 +162,8 @@ let dceTextToDiagnostics = ( let codeActionEdit = new WorkspaceEdit(); + // In the future, it would be cool to have an additional code action + // here for automatically removing whatever the thing that's dead is. codeActionEdit.replace( Uri.parse(processedFileInfo.filePath), // Make sure the full line is replaced @@ -169,7 +171,9 @@ let dceTextToDiagnostics = ( new Position(issueLocationRange.start.line, 0), new Position(issueLocationRange.start.line, 999999) ), - lineContentToReplace + // reanalyze seems to add two extra spaces at the start of the line + // content to replace. + lineContentToReplace.slice(2) ); codeAction.edit = codeActionEdit; @@ -198,10 +202,6 @@ export const runDeadCodeAnalysisWithReanalyze = ( diagnosticsCollection: DiagnosticCollection, diagnosticsResultCodeActions: DiagnosticsResultCodeActionsMap ) => { - // Clear old diagnostics and code actions state. - diagnosticsCollection.clear(); - diagnosticsResultCodeActions.clear(); - let currentDocument = window.activeTextEditor.document; let cwd = targetDir ?? path.dirname(currentDocument.uri.fsPath); @@ -237,11 +237,16 @@ export const runDeadCodeAnalysisWithReanalyze = ( }); p.on("close", () => { + diagnosticsResultCodeActions.clear(); let { diagnosticsMap } = dceTextToDiagnostics( data, diagnosticsResultCodeActions ); + // Clear old diagnostics and code actions state before setting up the new + // state. + diagnosticsCollection.clear(); + diagnosticsMap.forEach((diagnostics, filePath) => { diagnosticsCollection.set(Uri.parse(filePath), diagnostics); }); From 8b596af92475c81f11abc01cd30e94b85ad26765 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Tue, 4 Jan 2022 09:32:21 +0100 Subject: [PATCH 14/14] smoothen diagnostics update experience --- client/src/commands/dead_code_analysis.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/client/src/commands/dead_code_analysis.ts b/client/src/commands/dead_code_analysis.ts index d57321740..4171aed47 100644 --- a/client/src/commands/dead_code_analysis.ts +++ b/client/src/commands/dead_code_analysis.ts @@ -243,9 +243,14 @@ export const runDeadCodeAnalysisWithReanalyze = ( diagnosticsResultCodeActions ); - // Clear old diagnostics and code actions state before setting up the new - // state. - diagnosticsCollection.clear(); + // This smoothens the experience of the diagnostics updating a bit by + // clearing only the visible diagnostics that has been fixed after the + // updated diagnostics has been applied. + diagnosticsCollection.forEach((uri, _) => { + if (!diagnosticsMap.has(uri.fsPath)) { + diagnosticsCollection.delete(uri); + } + }); diagnosticsMap.forEach((diagnostics, filePath) => { diagnosticsCollection.set(Uri.parse(filePath), diagnostics);