From 2cc49900a66d993bf11f4cd2f96c6ce71612f343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Wed, 30 Apr 2025 02:27:05 +0100 Subject: [PATCH 1/6] Don't cut off last character of last record field mentioned in missing fields error --- server/src/codeActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 7d1c10100..f1942f516 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -458,7 +458,7 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ if (line.startsWith("Some required record fields are missing:")) { let theLine = line; if (theLine.endsWith(".")) { - theLine = theLine.slice(0, theLine.length - 2); + theLine = theLine.slice(0, theLine.length - 1); } let recordFieldNames = theLine From 58b2d2dcf3defe6dfb57bcba5aab181436b7891c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Wed, 30 Apr 2025 02:31:28 +0100 Subject: [PATCH 2/6] Typo --- server/src/codeActions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index f1942f516..96ff4aa97 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -151,7 +151,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ diagnosticMessage.forEach((line, index, array) => { // Because of how actions work, there can only be one per diagnostic. So, // halt whenever a code action has been found. - let codeActionEtractors = [ + let codeActionExtractors = [ simpleTypeMismatches, didYouMeanAction, addUndefinedRecordFieldsV10, @@ -162,7 +162,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ wrapInSome, ]; - for (let extractCodeAction of codeActionEtractors) { + for (let extractCodeAction of codeActionExtractors) { let didFindAction = false; try { From 2265806f63b6e87c97a13782eedead476abf58bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Wed, 30 Apr 2025 15:16:09 +0100 Subject: [PATCH 3/6] Use file content cache for detecting empty record --- package-lock.json | 14 ++++++++++++++ package.json | 3 +++ server/src/codeActions.ts | 24 +++++++++++++++++++++++- server/src/incrementalCompilation.ts | 7 ++++++- server/src/server.ts | 2 +- server/src/utils.ts | 4 +++- 6 files changed, 50 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index e45304ee5..e150d3870 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,9 @@ "version": "1.62.0", "hasInstallScript": true, "license": "MIT", + "dependencies": { + "vscode-languageserver-textdocument": "^1.0.12" + }, "devDependencies": { "@types/node": "^14.14.41", "@types/vscode": "1.68.0", @@ -478,6 +481,12 @@ "node": ">=4.2.0" } }, + "node_modules/vscode-languageserver-textdocument": { + "version": "1.0.12", + "resolved": "https://registry.npmjs.org/vscode-languageserver-textdocument/-/vscode-languageserver-textdocument-1.0.12.tgz", + "integrity": "sha512-cxWNPesCnQCcMPeenjKKsOCKQZ/L6Tv19DTRIGuLWe32lyzWhihGVJ/rcckZXJxfdKCFvRLS3fpBIsV/ZGX4zA==", + "license": "MIT" + }, "node_modules/yallist": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", @@ -714,6 +723,11 @@ "integrity": "sha512-WOkT3XYvrpXx4vMMqlD+8R8R37fZkjyLGlxavMc4iB8lrl8L0DeTcHbYgw/v0N/z9wAFsgBhcsF0ruoySS22mA==", "dev": true }, + "vscode-languageserver-textdocument": { + "version": "1.0.12", + "resolved": "https://registry.npmjs.org/vscode-languageserver-textdocument/-/vscode-languageserver-textdocument-1.0.12.tgz", + "integrity": "sha512-cxWNPesCnQCcMPeenjKKsOCKQZ/L6Tv19DTRIGuLWe32lyzWhihGVJ/rcckZXJxfdKCFvRLS3fpBIsV/ZGX4zA==" + }, "yallist": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", diff --git a/package.json b/package.json index 5f9ae3e91..2e3ff8aef 100644 --- a/package.json +++ b/package.json @@ -259,5 +259,8 @@ "esbuild": "^0.20.1", "semver": "^7.3.7", "typescript": "^4.7.3" + }, + "dependencies": { + "vscode-languageserver-textdocument": "^1.0.12" } } diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 96ff4aa97..3880b95dc 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -2,6 +2,7 @@ // actions available in the extension, but they are derived via the analysis // OCaml binary. import * as p from "vscode-languageserver-protocol"; +import { TextDocument } from "vscode-languageserver-textdocument"; import * as utils from "./utils"; import { fileURLToPath } from "url"; @@ -16,6 +17,7 @@ interface findCodeActionsConfig { diagnosticMessage: string[]; file: string; range: p.Range; + fileContentCache: Map; addFoundActionsHere: filesCodeActions; } @@ -146,6 +148,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ diagnosticMessage, file, range, + fileContentCache, addFoundActionsHere: codeActions, }: findCodeActionsConfig) => { diagnosticMessage.forEach((line, index, array) => { @@ -174,6 +177,7 @@ export let findCodeActionsInDiagnosticsMessage = ({ index, line, range, + fileContentCache, }); } catch (e) { console.error(e); @@ -194,6 +198,7 @@ interface codeActionExtractorConfig { range: p.Range; diagnostic: p.Diagnostic; codeActions: filesCodeActions; + fileContentCache: Map; } type codeActionExtractor = (config: codeActionExtractorConfig) => boolean; @@ -319,12 +324,14 @@ let handleUndefinedRecordFieldsAction = ({ file, range, diagnostic, + fileContentCache }: { recordFieldNames: string[]; codeActions: filesCodeActions; file: string; range: p.Range; diagnostic: p.Diagnostic; + fileContentCache: Map; }) => { if (recordFieldNames != null) { codeActions[file] = codeActions[file] || []; @@ -379,8 +386,19 @@ let handleUndefinedRecordFieldsAction = ({ // Let's put the end brace back where it was (we still have it to the direct right of us). newText += `${paddingContentEndBrace}`; } else { + let insertLeadingComma = true + const fileContent = fileContentCache.get(file.replace("file://", "")) + if (fileContent) { + const textDocument = TextDocument.create(file, "text", 0, fileContent) + if (textDocument.getText(range) == "{}") { + insertLeadingComma = false + } + } + // A single line record definition body is a bit easier - we'll just add the new fields on the same line. - newText += ", "; + if (insertLeadingComma) { + newText += ", "; + } newText += recordFieldNames .map((fieldName) => `${fieldName}: failwith("TODO")`) .join(", "); @@ -421,6 +439,7 @@ let addUndefinedRecordFieldsV10: codeActionExtractor = ({ index, line, range, + fileContentCache, }) => { if (line.startsWith("Some record fields are undefined:")) { let recordFieldNames = line @@ -440,6 +459,7 @@ let addUndefinedRecordFieldsV10: codeActionExtractor = ({ diagnostic, file, range, + fileContentCache, }); } @@ -454,6 +474,7 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ index, line, range, + fileContentCache }) => { if (line.startsWith("Some required record fields are missing:")) { let theLine = line; @@ -486,6 +507,7 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ diagnostic, file, range, + fileContentCache, }); } diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index 0ca5e28d5..17ff3a37b 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -644,8 +644,13 @@ async function compileContents( } // Reset compilation status as this compilation finished entry.compilation = null; + + const fileContentCache = new Map(); + fileContentCache.set(entry.file.incrementalFilePath, fileContent) + const { result, codeActions } = utils.parseCompilerLogOutput( - `${stderr}\n#Done()` + `${stderr}\n#Done()`, + fileContentCache ); const actions = Object.values(codeActions)[0] ?? []; diff --git a/server/src/server.ts b/server/src/server.ts index d7800c048..891b1b1a5 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -102,7 +102,7 @@ let sendUpdatedDiagnostics = () => { result: filesAndErrors, codeActions, linesWithParseErrors, - } = utils.parseCompilerLogOutput(content); + } = utils.parseCompilerLogOutput(content, stupidFileContentCache); if (linesWithParseErrors.length > 0) { let params: p.ShowMessageParams = { diff --git a/server/src/utils.ts b/server/src/utils.ts index 63d52bebe..1278dd069 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -509,7 +509,8 @@ type parsedCompilerLogResult = { linesWithParseErrors: string[]; }; export let parseCompilerLogOutput = ( - content: string + content: string, + fileContentCache: Map ): parsedCompilerLogResult => { type parsedDiagnostic = { code: number | undefined; @@ -680,6 +681,7 @@ export let parseCompilerLogOutput = ( diagnosticMessage, file, range, + fileContentCache, }); result[file].push(diagnostic); From 8ec2efb527948f57f08ac32647d2480f25c7f543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Wed, 30 Apr 2025 15:16:18 +0100 Subject: [PATCH 4/6] Revert "Use file content cache for detecting empty record" This reverts commit 2265806f63b6e87c97a13782eedead476abf58bf. --- package-lock.json | 14 -------------- package.json | 3 --- server/src/codeActions.ts | 24 +----------------------- server/src/incrementalCompilation.ts | 7 +------ server/src/server.ts | 2 +- server/src/utils.ts | 4 +--- 6 files changed, 4 insertions(+), 50 deletions(-) diff --git a/package-lock.json b/package-lock.json index e150d3870..e45304ee5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,9 +9,6 @@ "version": "1.62.0", "hasInstallScript": true, "license": "MIT", - "dependencies": { - "vscode-languageserver-textdocument": "^1.0.12" - }, "devDependencies": { "@types/node": "^14.14.41", "@types/vscode": "1.68.0", @@ -481,12 +478,6 @@ "node": ">=4.2.0" } }, - "node_modules/vscode-languageserver-textdocument": { - "version": "1.0.12", - "resolved": "https://registry.npmjs.org/vscode-languageserver-textdocument/-/vscode-languageserver-textdocument-1.0.12.tgz", - "integrity": "sha512-cxWNPesCnQCcMPeenjKKsOCKQZ/L6Tv19DTRIGuLWe32lyzWhihGVJ/rcckZXJxfdKCFvRLS3fpBIsV/ZGX4zA==", - "license": "MIT" - }, "node_modules/yallist": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", @@ -723,11 +714,6 @@ "integrity": "sha512-WOkT3XYvrpXx4vMMqlD+8R8R37fZkjyLGlxavMc4iB8lrl8L0DeTcHbYgw/v0N/z9wAFsgBhcsF0ruoySS22mA==", "dev": true }, - "vscode-languageserver-textdocument": { - "version": "1.0.12", - "resolved": "https://registry.npmjs.org/vscode-languageserver-textdocument/-/vscode-languageserver-textdocument-1.0.12.tgz", - "integrity": "sha512-cxWNPesCnQCcMPeenjKKsOCKQZ/L6Tv19DTRIGuLWe32lyzWhihGVJ/rcckZXJxfdKCFvRLS3fpBIsV/ZGX4zA==" - }, "yallist": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", diff --git a/package.json b/package.json index 2e3ff8aef..5f9ae3e91 100644 --- a/package.json +++ b/package.json @@ -259,8 +259,5 @@ "esbuild": "^0.20.1", "semver": "^7.3.7", "typescript": "^4.7.3" - }, - "dependencies": { - "vscode-languageserver-textdocument": "^1.0.12" } } diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 3880b95dc..96ff4aa97 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -2,7 +2,6 @@ // actions available in the extension, but they are derived via the analysis // OCaml binary. import * as p from "vscode-languageserver-protocol"; -import { TextDocument } from "vscode-languageserver-textdocument"; import * as utils from "./utils"; import { fileURLToPath } from "url"; @@ -17,7 +16,6 @@ interface findCodeActionsConfig { diagnosticMessage: string[]; file: string; range: p.Range; - fileContentCache: Map; addFoundActionsHere: filesCodeActions; } @@ -148,7 +146,6 @@ export let findCodeActionsInDiagnosticsMessage = ({ diagnosticMessage, file, range, - fileContentCache, addFoundActionsHere: codeActions, }: findCodeActionsConfig) => { diagnosticMessage.forEach((line, index, array) => { @@ -177,7 +174,6 @@ export let findCodeActionsInDiagnosticsMessage = ({ index, line, range, - fileContentCache, }); } catch (e) { console.error(e); @@ -198,7 +194,6 @@ interface codeActionExtractorConfig { range: p.Range; diagnostic: p.Diagnostic; codeActions: filesCodeActions; - fileContentCache: Map; } type codeActionExtractor = (config: codeActionExtractorConfig) => boolean; @@ -324,14 +319,12 @@ let handleUndefinedRecordFieldsAction = ({ file, range, diagnostic, - fileContentCache }: { recordFieldNames: string[]; codeActions: filesCodeActions; file: string; range: p.Range; diagnostic: p.Diagnostic; - fileContentCache: Map; }) => { if (recordFieldNames != null) { codeActions[file] = codeActions[file] || []; @@ -386,19 +379,8 @@ let handleUndefinedRecordFieldsAction = ({ // Let's put the end brace back where it was (we still have it to the direct right of us). newText += `${paddingContentEndBrace}`; } else { - let insertLeadingComma = true - const fileContent = fileContentCache.get(file.replace("file://", "")) - if (fileContent) { - const textDocument = TextDocument.create(file, "text", 0, fileContent) - if (textDocument.getText(range) == "{}") { - insertLeadingComma = false - } - } - // A single line record definition body is a bit easier - we'll just add the new fields on the same line. - if (insertLeadingComma) { - newText += ", "; - } + newText += ", "; newText += recordFieldNames .map((fieldName) => `${fieldName}: failwith("TODO")`) .join(", "); @@ -439,7 +421,6 @@ let addUndefinedRecordFieldsV10: codeActionExtractor = ({ index, line, range, - fileContentCache, }) => { if (line.startsWith("Some record fields are undefined:")) { let recordFieldNames = line @@ -459,7 +440,6 @@ let addUndefinedRecordFieldsV10: codeActionExtractor = ({ diagnostic, file, range, - fileContentCache, }); } @@ -474,7 +454,6 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ index, line, range, - fileContentCache }) => { if (line.startsWith("Some required record fields are missing:")) { let theLine = line; @@ -507,7 +486,6 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ diagnostic, file, range, - fileContentCache, }); } diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts index 17ff3a37b..0ca5e28d5 100644 --- a/server/src/incrementalCompilation.ts +++ b/server/src/incrementalCompilation.ts @@ -644,13 +644,8 @@ async function compileContents( } // Reset compilation status as this compilation finished entry.compilation = null; - - const fileContentCache = new Map(); - fileContentCache.set(entry.file.incrementalFilePath, fileContent) - const { result, codeActions } = utils.parseCompilerLogOutput( - `${stderr}\n#Done()`, - fileContentCache + `${stderr}\n#Done()` ); const actions = Object.values(codeActions)[0] ?? []; diff --git a/server/src/server.ts b/server/src/server.ts index 891b1b1a5..d7800c048 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -102,7 +102,7 @@ let sendUpdatedDiagnostics = () => { result: filesAndErrors, codeActions, linesWithParseErrors, - } = utils.parseCompilerLogOutput(content, stupidFileContentCache); + } = utils.parseCompilerLogOutput(content); if (linesWithParseErrors.length > 0) { let params: p.ShowMessageParams = { diff --git a/server/src/utils.ts b/server/src/utils.ts index 1278dd069..63d52bebe 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -509,8 +509,7 @@ type parsedCompilerLogResult = { linesWithParseErrors: string[]; }; export let parseCompilerLogOutput = ( - content: string, - fileContentCache: Map + content: string ): parsedCompilerLogResult => { type parsedDiagnostic = { code: number | undefined; @@ -681,7 +680,6 @@ export let parseCompilerLogOutput = ( diagnosticMessage, file, range, - fileContentCache, }); result[file].push(diagnostic); From 314fe3b33a4056d0d1efe39c640120c0ae1e0f71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Wed, 30 Apr 2025 15:35:21 +0100 Subject: [PATCH 5/6] Use range character difference to decide whether to insert initial trailing comma --- server/src/codeActions.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 96ff4aa97..ebf3526f9 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -380,7 +380,17 @@ let handleUndefinedRecordFieldsAction = ({ newText += `${paddingContentEndBrace}`; } else { // A single line record definition body is a bit easier - we'll just add the new fields on the same line. - newText += ", "; + + // For an empty record (`range.end.character - range.start.character == 2`), + // we don't want to add an initial trailing comma as that would be invalid syntax. + // + // We assume that records that already contain some characters between + // their braces have at least one field and therefore we need to insert + // an initial trailing comma. + if (range.end.character - range.start.character > 2) { + newText += ", "; + } + newText += recordFieldNames .map((fieldName) => `${fieldName}: failwith("TODO")`) .join(", "); From df9e9f5549a91b7128c17af0ab3c81e4d34d4f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9di-R=C3=A9mi=20Hashim?= Date: Mon, 5 May 2025 18:10:23 +0100 Subject: [PATCH 6/6] Use %todo instead of failwith on v11+ --- server/src/codeActions.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index ebf3526f9..d790f3616 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -319,12 +319,14 @@ let handleUndefinedRecordFieldsAction = ({ file, range, diagnostic, + todoValue }: { recordFieldNames: string[]; codeActions: filesCodeActions; file: string; range: p.Range; diagnostic: p.Diagnostic; + todoValue: string }) => { if (recordFieldNames != null) { codeActions[file] = codeActions[file] || []; @@ -373,7 +375,7 @@ let handleUndefinedRecordFieldsAction = ({ newText += paddingContentRecordField; } - newText += `${fieldName}: failwith("TODO"),\n`; + newText += `${fieldName}: ${todoValue},\n`; }); // Let's put the end brace back where it was (we still have it to the direct right of us). @@ -392,7 +394,7 @@ let handleUndefinedRecordFieldsAction = ({ } newText += recordFieldNames - .map((fieldName) => `${fieldName}: failwith("TODO")`) + .map((fieldName) => `${fieldName}: ${todoValue}`) .join(", "); } @@ -450,6 +452,7 @@ let addUndefinedRecordFieldsV10: codeActionExtractor = ({ diagnostic, file, range, + todoValue: `failwith("TODO")` }); } @@ -496,6 +499,7 @@ let addUndefinedRecordFieldsV11: codeActionExtractor = ({ diagnostic, file, range, + todoValue: `%todo` }); }