Check executed F# cells as FSI runs them, each in a fragment of its own

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,

src/Verso.FSharp/Kernel/FSharpKernel.cs

@@ -468,6 +468,10 @@ public async Task<IReadOnlyList<Completion>> GetCompletionsAsync(string code, in
             var completions = new List<Completion>();
             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<IReadOnlyList<Diagnostic>> GetDiagnosticsAsync(string code)
                 adjustedLine, colAtEnd, lineText, fsharpNames, FSharpTokenTag.Identifier,
                 FSharpOption<int>.None);
 
-            var content = FormatToolTip(toolTip);
+            var content = FSharpProjectContext.HideExecutedCellModules(FormatToolTip(toolTip));
             if (string.IsNullOrWhiteSpace(content)) return null;
 
             // Calculate cell-local range for the identifier

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.
     /// </returns>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
     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);
     }
 
+    /// <summary>
+    /// The name every executed cell's module starts with, so completions can leave them out.
+    /// </summary>
+    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);
+
+    /// <summary>
+    /// Removes the executed cells' module names from compiler text (a tooltip, a diagnostic), so
+    /// <c>Notebook.VersoExecutedCell1.Shape</c> reads as <c>Notebook.Shape</c>, as it would with
+    /// the cells checked in one file.
+    /// </summary>
+    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<string>();
+
+        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;
+    }
+
     /// <summary>
     /// Clears all accumulated state (for kernel restart).
     /// </summary>

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;
+
+/// <summary>
+/// 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.
+/// </summary>
+[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<Diagnostic> 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'"));
+    }
+}