From 63e13282d21ba3f20920913e865677cb30c231d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20Sch=C3=A4fer?= Date: Tue, 20 Oct 2026 15:55:51 +0200 Subject: [PATCH] Check executed F# cells as FSI runs them, each in a fragment of its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The F# kernel checks a cell for diagnostics, hover and completions in one virtual script that holds every executed cell followed by the cell being edited. FSI compiles each execution as a fragment of its own and opens it for the ones after, so a later execution may define again a type or module an earlier one defined, as a cell does each time it is run again. In one file that is a duplicate definition: once a cell defining a type or module has run, checking it again reports FS0037 on each of its definitions, and the editor shows those as errors. Each executed source is now wrapped in a module of its own, which is then opened, the way FSI does it. Script directives (#r, #load, #I, #nowarn, ...) are lifted out of the module, since they apply to the whole script, and a fragment's top-level opens are repeated after it, since in FSI they stay open for later cells. The cell being edited stays at the top level, so its positions are unchanged. The wrapper modules are kept out of what a user sees: completions leave them out, and hover text and diagnostic messages drop their names, so a type reads as Notebook.Shape rather than Notebook.VersoExecutedCell1.Shape. Signed-off-by: Jonathan Schäfer Co-Authored-By: Claude Opus 5.5 --- src/Verso.FSharp/Helpers/DiagnosticMapper.cs | 2 +- src/Verso.FSharp/Kernel/FSharpKernel.cs | 6 +- .../Kernel/FSharpProjectContext.cs | 78 +++++++++- .../Kernel/ExecutedCellsTests.cs | 145 ++++++++++++++++++ 4 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs diff --git a/src/Verso.FSharp/Helpers/DiagnosticMapper.cs b/src/Verso.FSharp/Helpers/DiagnosticMapper.cs index 0c1f3112..d4c49fe9 100644 --- a/src/Verso.FSharp/Helpers/DiagnosticMapper.cs +++ b/src/Verso.FSharp/Helpers/DiagnosticMapper.cs @@ -61,7 +61,7 @@ internal static class DiagnosticMapper // FCS columns are 0-based — no change needed return new VersoDiagnostic( Severity: severity.Value, - Message: diag.Message, + Message: Kernel.FSharpProjectContext.HideExecutedCellModules(diag.Message), StartLine: cellStartLine, StartColumn: diag.StartColumn, EndLine: cellEndLine, diff --git a/src/Verso.FSharp/Kernel/FSharpKernel.cs b/src/Verso.FSharp/Kernel/FSharpKernel.cs index 4ba2520e..68aaeb6e 100644 --- a/src/Verso.FSharp/Kernel/FSharpKernel.cs +++ b/src/Verso.FSharp/Kernel/FSharpKernel.cs @@ -468,6 +468,10 @@ public async Task> GetCompletionsAsync(string code, in var completions = new List(); foreach (var item in declInfo.Items) { + // The modules the executed cells are checked in are not the user's names. + if (item.NameInList.StartsWith(FSharpProjectContext.ExecutedCellModulePrefix, StringComparison.Ordinal)) + continue; + completions.Add(new Completion( DisplayText: item.NameInList, InsertText: item.NameInCode, @@ -578,7 +582,7 @@ public async Task> GetDiagnosticsAsync(string code) adjustedLine, colAtEnd, lineText, fsharpNames, FSharpTokenTag.Identifier, FSharpOption.None); - var content = FormatToolTip(toolTip); + var content = FSharpProjectContext.HideExecutedCellModules(FormatToolTip(toolTip)); if (string.IsNullOrWhiteSpace(content)) return null; // Calculate cell-local range for the identifier diff --git a/src/Verso.FSharp/Kernel/FSharpProjectContext.cs b/src/Verso.FSharp/Kernel/FSharpProjectContext.cs index 67462850..217e21b8 100644 --- a/src/Verso.FSharp/Kernel/FSharpProjectContext.cs +++ b/src/Verso.FSharp/Kernel/FSharpProjectContext.cs @@ -59,6 +59,14 @@ public void AddReference(string assemblyPath) /// A tuple of (SourceText, PrefixLineCount, FSharpProjectOptions) where PrefixLineCount /// is the number of lines before the current cell code begins. /// + /// + /// FSI compiles each execution as a fragment of its own and opens it for the ones after, so + /// a later execution may define again a type or module an earlier one defined, as a cell does + /// each time it is run again. In one file that is a duplicate definition, so each executed + /// source is wrapped in a module of its own, which is then opened, the way FSI does it. Script + /// directives are lifted out of the module, since they apply to the whole script, and the + /// fragment's top-level opens are repeated after it, since in FSI they stay open. + /// public (string SourceText, int PrefixLineCount, FSharpProjectOptions Options) BuildDocument(string currentCellCode) { var prefixBuilder = new StringBuilder(); @@ -69,10 +77,10 @@ public void AddReference(string assemblyPath) prefixBuilder.AppendLine($"open {ns}"); } - // Add previously executed sources - foreach (var source in _executedSources) + // Add previously executed sources, one fragment each + for (var i = 0; i < _executedSources.Count; i++) { - prefixBuilder.AppendLine(source); + AppendFragment(prefixBuilder, $"{ExecutedCellModulePrefix}{i + 1}", _executedSources[i]); } var prefix = prefixBuilder.ToString(); @@ -90,6 +98,70 @@ public void AddReference(string assemblyPath) return (combinedSource, prefixLineCount, options); } + /// + /// The name every executed cell's module starts with, so completions can leave them out. + /// + public const string ExecutedCellModulePrefix = "VersoExecutedCell"; + + private static readonly System.Text.RegularExpressions.Regex ExecutedCellModuleName = + new($@"\.{ExecutedCellModulePrefix}\d+\b|\b{ExecutedCellModulePrefix}\d+\.", System.Text.RegularExpressions.RegexOptions.CultureInvariant); + + /// + /// Removes the executed cells' module names from compiler text (a tooltip, a diagnostic), so + /// Notebook.VersoExecutedCell1.Shape reads as Notebook.Shape, as it would with + /// the cells checked in one file. + /// + public static string HideExecutedCellModules(string text) => + text.Contains(ExecutedCellModulePrefix, StringComparison.Ordinal) + ? ExecutedCellModuleName.Replace(text, "") + : text; + + private static readonly string[] ScriptDirectives = + { "#r", "#load", "#I", "#nowarn", "#warnon", "#time", "#help", "#quit", "#i" }; + + private static void AppendFragment(StringBuilder builder, string moduleName, string source) + { + var lines = source.Replace("\r\n", "\n").Split('\n'); + var body = new StringBuilder(); + var opens = new List(); + + foreach (var line in lines) + { + if (IsScriptDirective(line)) + { + builder.AppendLine(line); + continue; + } + + if (line.StartsWith("open ", StringComparison.Ordinal)) + opens.Add(line); + + body.Append(" ").AppendLine(line); + } + + builder.Append("module ").Append(moduleName).AppendLine(" ="); + builder.Append(body); + // A fragment of nothing but directives, or a cell whose last line is a comment, still + // needs a body. + builder.AppendLine(" do ()"); + builder.Append("open ").AppendLine(moduleName); + foreach (var open in opens) + builder.AppendLine(open); + } + + private static bool IsScriptDirective(string line) + { + foreach (var directive in ScriptDirectives) + { + if (line.StartsWith(directive, StringComparison.Ordinal) + && (line.Length == directive.Length || char.IsWhiteSpace(line[directive.Length]))) + { + return true; + } + } + return false; + } + /// /// Clears all accumulated state (for kernel restart). /// diff --git a/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs b/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs new file mode 100644 index 00000000..ab6fab11 --- /dev/null +++ b/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs @@ -0,0 +1,145 @@ +using Verso.Abstractions; +using Verso.FSharp.Kernel; +using Verso.Testing.Stubs; + +namespace Verso.FSharp.Tests.Kernel; + +/// +/// FSI compiles each execution as a fragment of its own, so a cell may define again what an +/// earlier execution defined, the cell that was itself just run among them. The document the +/// kernel checks a cell in has to allow the same, or a cell that has run reports its own +/// definitions as duplicates. +/// +[TestClass] +public class ExecutedCellsTests +{ + private FSharpKernel _kernel = null!; + private StubExecutionContext _context = null!; + + [TestInitialize] + public async Task Setup() + { + _kernel = new FSharpKernel(); + await _kernel.InitializeAsync(); + _context = new StubExecutionContext(); + } + + [TestCleanup] + public async Task Cleanup() + { + await _kernel.DisposeAsync(); + } + + private static string Describe(IEnumerable diagnostics) => + string.Join("; ", diagnostics.Select(d => $"{d.StartLine}:{d.StartColumn} {d.Code} {d.Message}")); + + [TestMethod] + public async Task RunCell_CheckedAgain_HasNoDuplicateDefinitions() + { + var code = "module Geometry =\n let area r = r * r\ntype Shape = Circle of float\nlet unit = 1.0"; + await _kernel.ExecuteAsync(code, _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync(code); + + Assert.AreEqual(0, diagnostics.Count(d => d.Severity == DiagnosticSeverity.Error), Describe(diagnostics)); + } + + [TestMethod] + public async Task RunCell_RedefinedByAnotherCell_TheLaterDefinitionWins() + { + await _kernel.ExecuteAsync("let value = 1", _context); + await _kernel.ExecuteAsync("let value = \"text\"", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n : string = value"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task OpenInARunCell_StaysOpenForLaterCells() + { + await _kernel.ExecuteAsync("open System.Text", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let sb = StringBuilder()"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task ModuleFromARunCell_CanBeOpenedLater() + { + await _kernel.ExecuteAsync("module Geometry =\n let area r = r * r", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("open Geometry\nlet a = area 2.0\nlet b = Geometry.area 3.0"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task OpenOfAModuleInTheSameRunCell_StaysOpenForLaterCells() + { + await _kernel.ExecuteAsync("module Geometry =\n let area r = r * r\nopen Geometry", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let a = area 2.0"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task RunCellWithAReference_StillResolvesLater() + { + var dll = typeof(System.Text.Json.JsonSerializer).Assembly.Location; + await _kernel.ExecuteAsync($"#r @\"{dll}\"\nlet options = System.Text.Json.JsonSerializerOptions()", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let indented = options.WriteIndented"); + + Assert.AreEqual(0, diagnostics.Count(d => d.Severity == DiagnosticSeverity.Error), Describe(diagnostics)); + } + + [TestMethod] + public async Task RunCellWithTripleQuotedString_StillChecks() + { + await _kernel.ExecuteAsync("let text = \"\"\"\nline one\nline two\"\"\"", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = text.Length"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task Completions_DoNotOfferTheExecutedCellModules() + { + await _kernel.ExecuteAsync("let value = 1", _context); + + var completions = await _kernel.GetCompletionsAsync("let x = valu", 12); + + Assert.IsTrue(completions.Any(c => c.DisplayText == "value"), "the earlier cell's binding"); + Assert.IsFalse(completions.Any(c => c.DisplayText.Contains("Verso", StringComparison.OrdinalIgnoreCase) + && c.Kind == "Module"), + string.Join(", ", completions.Where(c => c.Kind == "Module").Select(c => c.DisplayText))); + } + [TestMethod] + public async Task HoverAndDiagnostics_DoNotNameTheExecutedCellModules() + { + await _kernel.ExecuteAsync("module Geometry =\n let area r = r * r\ntype Shape = Circle of float", _context); + + var code = "let a = Geometry.area 2.0\nlet s : Shape = 1"; + var hover = await _kernel.GetHoverInfoAsync(code, code.IndexOf("Geometry", StringComparison.Ordinal) + 1); + var diagnostics = await _kernel.GetDiagnosticsAsync(code); + + Assert.IsNotNull(hover); + StringAssert.DoesNotMatch(hover.Content, new System.Text.RegularExpressions.Regex(FSharpProjectContext.ExecutedCellModulePrefix)); + Assert.IsTrue(diagnostics.Count > 0, "the int assigned to a Shape"); + foreach (var d in diagnostics) + StringAssert.DoesNotMatch(d.Message, new System.Text.RegularExpressions.Regex(FSharpProjectContext.ExecutedCellModulePrefix)); + } + + [TestMethod] + public void HideExecutedCellModules_KeepsTheRestOfTheName() + { + Assert.AreEqual("module Geometry from Notebook", + FSharpProjectContext.HideExecutedCellModules("module Geometry from Notebook.VersoExecutedCell1")); + Assert.AreEqual("expected 'Geometry.Shape'", + FSharpProjectContext.HideExecutedCellModules("expected 'VersoExecutedCell12.Geometry.Shape'")); + } +}