diff --git a/src/Verso.FSharp/Kernel/FSharpCheckerManager.cs b/src/Verso.FSharp/Kernel/FSharpCheckerManager.cs index 95dbb87..c0cf9b8 100644 --- a/src/Verso.FSharp/Kernel/FSharpCheckerManager.cs +++ b/src/Verso.FSharp/Kernel/FSharpCheckerManager.cs @@ -67,6 +67,32 @@ public void Initialize() return null; } + /// + /// Lists the files FSI compiles for the given #loaded scripts, in the order it + /// compiles them: each script after the scripts it loads itself. + /// + /// + /// The closure is the one the checker works out for a script holding just those + /// #load lines, so a nested #load resolves against the file it is in, as it + /// does in FSI. The script itself is the last file of the closure and is left out. + /// + public async Task> GetLoadClosureAsync( + string scriptFileName, IEnumerable loadedPaths, CancellationToken ct = default) + { + if (_checker is null) + throw new InvalidOperationException("Checker has not been initialized."); + + var script = string.Join("\n", loadedPaths.Select(p => $"#load @\"{p.Replace("\"", "\"\"")}\"")); + var asyncOp = _checker.GetProjectOptionsFromScript( + scriptFileName, SourceText.ofString(script), + caret: null, previewEnabled: null, loadedTimeStamp: null, otherFlags: null, + useFsiAuxLib: null, useSdkRefs: null, assumeDotNetFramework: null, sdkDirOverride: null, + optionsStamp: null, userOpName: null); + + var (options, _) = await StartAsTask(asyncOp, ct).ConfigureAwait(false); + return options.SourceFiles.Take(options.SourceFiles.Length - 1).ToArray(); + } + /// /// Triggers a background parse-and-check to pre-warm the checker cache. /// Errors are silently swallowed. diff --git a/src/Verso.FSharp/Kernel/FSharpKernel.cs b/src/Verso.FSharp/Kernel/FSharpKernel.cs index 68aaeb6..b1b2058 100644 --- a/src/Verso.FSharp/Kernel/FSharpKernel.cs +++ b/src/Verso.FSharp/Kernel/FSharpKernel.cs @@ -259,17 +259,6 @@ public async Task> ExecuteAsync(string code, IExecutio _projectContext?.AddReference(path); } - // Add #load file contents to IntelliSense context - foreach (var loadedPath in _scriptDirectiveProcessor.LoadedFilePaths) - { - try - { - var loadedSource = File.ReadAllText(loadedPath); - _projectContext?.AppendExecutedCode(loadedSource); - } - catch { /* best effort — FSI will report its own error */ } - } - // --- Snapshot store for Variables.Set detection --- _variableBridge!.SnapshotStore(context.Variables); @@ -403,6 +392,19 @@ public async Task> ExecuteAsync(string code, IExecutio _projectContext?.AppendExecutedCode(code); if (_projectContext is not null && _checkerManager is not null) { + // A loaded script is checked as a file of its own, as FSI compiles it, so later + // cells see it under the module name FSI gives it. + if (_scriptDirectiveProcessor.LoadedFilePaths.Count > 0) + { + try + { + var loadedFiles = await _checkerManager.GetLoadClosureAsync( + VirtualFileName, _scriptDirectiveProcessor.LoadedFilePaths).ConfigureAwait(false); + _projectContext.AddLoadedFiles(loadedFiles); + } + catch { /* best effort, the cell itself has run */ } + } + var (sourceText, _, options) = _projectContext.BuildDocument(""); _checkerManager.TriggerBackgroundCheck(VirtualFileName, sourceText, options); } diff --git a/src/Verso.FSharp/Kernel/FSharpProjectContext.cs b/src/Verso.FSharp/Kernel/FSharpProjectContext.cs index 217e21b..6ae505f 100644 --- a/src/Verso.FSharp/Kernel/FSharpProjectContext.cs +++ b/src/Verso.FSharp/Kernel/FSharpProjectContext.cs @@ -1,5 +1,6 @@ using System.Text; using FSharp.Compiler.CodeAnalysis; +using FSharp.Compiler.Tokenization; using Microsoft.FSharp.Collections; using Microsoft.FSharp.Core; @@ -19,9 +20,10 @@ internal sealed class FSharpProjectContext "fsi.exe", "--noninteractive", "--nologo", "--gui-" }; - private readonly List _executedSources = new(); + private readonly List _executedFragments = new(); private readonly IReadOnlyList _defaultOpens; private readonly List _additionalReferences = new(); + private readonly List _loadedFiles = new(); private readonly string[] _sessionArgs; /// @@ -40,7 +42,26 @@ public FSharpProjectContext(IReadOnlyList defaultOpens, string[] session /// public void AppendExecutedCode(string code) { - _executedSources.Add(code); + var fragment = new StringBuilder(); + AppendFragment(fragment, $"{ExecutedCellModulePrefix}{_executedFragments.Count + 1}", code); + _executedFragments.Add(fragment.ToString()); + } + + /// + /// Records the files an executed #load compiled, in the order FSI compiled them. + /// + /// + /// FSI compiles a loaded script as a file of its own, named for its module or namespace + /// header, or without one for its file name (lib.fsx is Lib), and does not + /// open it. The checker does the same when the files are part of the project ahead of the + /// notebook, so they are kept as files rather than copied into the document. A file loaded + /// again moves to the end, since its latest load is the one later cells see. + /// + public void AddLoadedFiles(IEnumerable filePaths) + { + var files = filePaths.ToList(); + _loadedFiles.RemoveAll(f => files.Contains(f, StringComparer.OrdinalIgnoreCase)); + _loadedFiles.AddRange(files); } /// @@ -78,9 +99,9 @@ public void AddReference(string assemblyPath) } // Add previously executed sources, one fragment each - for (var i = 0; i < _executedSources.Count; i++) + foreach (var fragment in _executedFragments) { - AppendFragment(prefixBuilder, $"{ExecutedCellModulePrefix}{i + 1}", _executedSources[i]); + prefixBuilder.Append(fragment); } var prefix = prefixBuilder.ToString(); @@ -119,21 +140,33 @@ public static string HideExecutedCellModules(string text) => private static readonly string[] ScriptDirectives = { "#r", "#load", "#I", "#nowarn", "#warnon", "#time", "#help", "#quit", "#i" }; + // FSI defines INTERACTIVE, so the branches it compiled are the ones the tokenizer keeps. + private static readonly FSharpList InteractiveDefines = ListModule.OfArray(new[] { "INTERACTIVE" }); + 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(); + var tokenizer = new FSharpSourceTokenizer( + InteractiveDefines, FSharpOption.None, FSharpOption.None, FSharpOption.None); + var state = FSharpTokenizerLexState.Initial; foreach (var line in lines) { - if (IsScriptDirective(line)) + // A line that starts inside a multi-line string or comment, or in a branch #if + // leaves out, is not code, whatever it starts with: it is neither a directive to + // lift nor an open to repeat. + var startsInCode = StartsInCode(state); + state = ScanLine(tokenizer, line, state); + + if (startsInCode && IsScriptDirective(line)) { builder.AppendLine(line); continue; } - if (line.StartsWith("open ", StringComparison.Ordinal)) + if (startsInCode && line.StartsWith("open ", StringComparison.Ordinal)) opens.Add(line); body.Append(" ").AppendLine(line); @@ -149,6 +182,26 @@ private static void AppendFragment(StringBuilder builder, string moduleName, str builder.AppendLine(open); } + private static bool StartsInCode(FSharpTokenizerLexState state) + { + var color = FSharpLineTokenizer.ColorStateOfLexState(state); + return color == FSharpTokenizerColorState.InitialState + || color == FSharpTokenizerColorState.Token + || color == FSharpTokenizerColorState.EndLineThenToken; + } + + private static FSharpTokenizerLexState ScanLine(FSharpSourceTokenizer tokenizer, string line, FSharpTokenizerLexState state) + { + var lineTokenizer = tokenizer.CreateLineTokenizer(line); + while (true) + { + var (token, next) = lineTokenizer.ScanToken(state); + state = next; + if (token is null) + return state; + } + } + private static bool IsScriptDirective(string line) { foreach (var directive in ScriptDirectives) @@ -167,8 +220,9 @@ private static bool IsScriptDirective(string line) /// public void Reset() { - _executedSources.Clear(); + _executedFragments.Clear(); _additionalReferences.Clear(); + _loadedFiles.Clear(); } private FSharpProjectOptions BuildProjectOptions() @@ -191,7 +245,7 @@ private FSharpProjectOptions BuildProjectOptions() return new FSharpProjectOptions( projectFileName: VirtualFileName, projectId: FSharpOption.None, - sourceFiles: new[] { VirtualFileName }, + sourceFiles: _loadedFiles.Append(VirtualFileName).ToArray(), otherOptions: otherOptions.ToArray(), referencedProjects: Array.Empty(), isIncompleteTypeCheckEnvironment: false, diff --git a/src/Verso.FSharp/NuGet/ScriptDirectiveProcessor.cs b/src/Verso.FSharp/NuGet/ScriptDirectiveProcessor.cs index ffca5c2..ba7e22e 100644 --- a/src/Verso.FSharp/NuGet/ScriptDirectiveProcessor.cs +++ b/src/Verso.FSharp/NuGet/ScriptDirectiveProcessor.cs @@ -15,7 +15,7 @@ internal sealed class ScriptDirectiveProcessor RegexOptions.Compiled | RegexOptions.Multiline); private static readonly Regex LoadRegex = new( - @"^#load\s+""([^""]+)""", + @"^#load\s+@?""([^""]+)""", RegexOptions.Compiled | RegexOptions.Multiline); private static readonly Regex IncludePathRegex = new( @@ -36,7 +36,8 @@ internal sealed class ScriptDirectiveProcessor public IReadOnlyList ResolvedAssemblyPaths => _resolvedAssemblyPaths; /// - /// File paths loaded via #load "script.fsx" directives, for IntelliSense context. + /// File paths the code last processed loads via #load "script.fsx" directives, so the + /// kernel can record them for IntelliSense once that code has run. /// public IReadOnlyList LoadedFilePaths => _loadedFilePaths; @@ -53,6 +54,7 @@ internal sealed class ScriptDirectiveProcessor public string ProcessDirectives(string code, INotebookMetadata? metadata) { var processed = code; + _loadedFilePaths.Clear(); // Process #r "assembly.dll" (non-NuGet) processed = AssemblyRefRegex.Replace(processed, match => diff --git a/src/Verso.Testing/Stubs/StubExecutionContext.cs b/src/Verso.Testing/Stubs/StubExecutionContext.cs index 74a3ecb..cd54118 100644 --- a/src/Verso.Testing/Stubs/StubExecutionContext.cs +++ b/src/Verso.Testing/Stubs/StubExecutionContext.cs @@ -14,7 +14,7 @@ public sealed class StubExecutionContext : IExecutionContext public IThemeContext Theme { get; } = new StubThemeContext(); public LayoutCapabilities LayoutCapabilities { get; set; } = LayoutCapabilities.None; public IExtensionHostContext ExtensionHost { get; set; } = new StubExtensionHostContext(() => Array.Empty()); - public INotebookMetadata NotebookMetadata { get; } = new NotebookMetadataContext(new NotebookModel()); + public INotebookMetadata NotebookMetadata { get; set; } = new NotebookMetadataContext(new NotebookModel()); public INotebookOperations Notebook { get; set; } = new StubNotebookOperations(); /// diff --git a/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs b/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs index ab6fab1..cd149bc 100644 --- a/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs +++ b/tests/Verso.FSharp.Tests/Kernel/ExecutedCellsTests.cs @@ -106,6 +106,62 @@ public async Task RunCellWithTripleQuotedString_StillChecks() Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); } + [TestMethod] + public async Task OpenLineInsideATripleQuotedString_IsNotOpenedForLaterCells() + { + await _kernel.ExecuteAsync("let text = \"\"\"\nopen System.Text\n\"\"\"", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let sb = StringBuilder()"); + + Assert.IsTrue(diagnostics.Any(d => d.Code == "FS0039"), Describe(diagnostics)); + } + + [TestMethod] + public async Task OpenLineInsideAVerbatimString_IsNotOpenedForLaterCells() + { + await _kernel.ExecuteAsync("let text = @\"\nopen System.Text\n\"", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let sb = StringBuilder()"); + + Assert.IsTrue(diagnostics.Any(d => d.Code == "FS0039"), Describe(diagnostics)); + } + + [TestMethod] + public async Task OpenLineInsideABlockComment_IsNotOpenedForLaterCells() + { + await _kernel.ExecuteAsync("(*\nopen System.Text\n*)\nlet value = 1", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let sb = StringBuilder()"); + + Assert.IsTrue(diagnostics.Any(d => d.Code == "FS0039"), Describe(diagnostics)); + } + + [TestMethod] + public async Task OpenInABranchFsiLeavesOut_IsNotOpenedForLaterCells() + { + await _kernel.ExecuteAsync("#if INTERACTIVE\nlet mode = \"fsi\"\n#else\nopen System.Text\n#endif", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let m : string = mode\nlet sb = StringBuilder()"); + + Assert.AreEqual(1, diagnostics.Count, Describe(diagnostics)); + Assert.AreEqual("FS0039", diagnostics[0].Code, Describe(diagnostics)); + Assert.AreEqual(1, diagnostics[0].StartLine, Describe(diagnostics)); + } + + [TestMethod] + public void DirectiveLineInsideAMultiLineString_StaysInTheString() + { + var context = new FSharpProjectContext(Array.Empty(), Array.Empty()); + context.AppendExecutedCode("let text = \"\"\"\n#r \"nothing.dll\"\nopen Nowhere\n\"\"\""); + + var (sourceText, _, _) = context.BuildDocument(""); + var lines = sourceText.Replace("\r\n", "\n").Split('\n'); + + CollectionAssert.Contains(lines, " #r \"nothing.dll\"", sourceText); + CollectionAssert.DoesNotContain(lines, "#r \"nothing.dll\"", sourceText); + CollectionAssert.DoesNotContain(lines, "open Nowhere", sourceText); + } + [TestMethod] public async Task Completions_DoNotOfferTheExecutedCellModules() { @@ -118,6 +174,7 @@ public async Task Completions_DoNotOfferTheExecutedCellModules() && c.Kind == "Module"), string.Join(", ", completions.Where(c => c.Kind == "Module").Select(c => c.DisplayText))); } + [TestMethod] public async Task HoverAndDiagnostics_DoNotNameTheExecutedCellModules() { diff --git a/tests/Verso.FSharp.Tests/Kernel/LoadedScriptsTests.cs b/tests/Verso.FSharp.Tests/Kernel/LoadedScriptsTests.cs new file mode 100644 index 0000000..914bc37 --- /dev/null +++ b/tests/Verso.FSharp.Tests/Kernel/LoadedScriptsTests.cs @@ -0,0 +1,167 @@ +using Verso.Abstractions; +using Verso.Contexts; +using Verso.FSharp.Kernel; +using Verso.Testing.Stubs; + +namespace Verso.FSharp.Tests.Kernel; + +/// +/// FSI compiles a #loaded script as a file of its own and does not open it: later cells +/// reach its names through its module or namespace header, or, without one, through a module +/// named for the file. The document the kernel checks a cell in has to know them the same way. +/// +[TestClass] +public class LoadedScriptsTests +{ + private FSharpKernel _kernel = null!; + private StubExecutionContext _context = null!; + private string _scriptDir = null!; + + [TestInitialize] + public async Task Setup() + { + _kernel = new FSharpKernel(); + await _kernel.InitializeAsync(); + _context = new StubExecutionContext(); + _scriptDir = Path.Combine(Path.GetTempPath(), $"verso-load-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_scriptDir); + } + + [TestCleanup] + public async Task Cleanup() + { + await _kernel.DisposeAsync(); + Directory.Delete(_scriptDir, recursive: true); + } + + private static string Describe(IEnumerable diagnostics) => + string.Join("; ", diagnostics.Select(d => $"{d.StartLine}:{d.StartColumn} {d.Code} {d.Message}")); + + private string WriteScript(string fileName, string source) + { + var path = Path.Combine(_scriptDir, fileName); + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + File.WriteAllText(path, source); + return path; + } + + private async Task LoadAsync(string path) + { + var outputs = await _kernel.ExecuteAsync($"#load @\"{path}\"", _context); + Assert.IsFalse(outputs.Any(o => o.IsError), string.Join("; ", outputs.Select(o => o.Content))); + } + + [TestMethod] + public async Task LoadedScriptWithAModuleHeader_IsKnownByItsModuleName() + { + await LoadAsync(WriteScript("withheader.fsx", "module Lib\n\nlet triple x = x * 3\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = Lib.triple 2"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadedScriptWithoutAHeader_IsKnownByItsFileName() + { + await LoadAsync(WriteScript("noheader.fsx", "let quad x = x * 4\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = Noheader.quad 2"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadedScript_IsNotOpenedForLaterCells() + { + await LoadAsync(WriteScript("noheader.fsx", "let quad x = x * 4\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = quad 2"); + + Assert.IsTrue(diagnostics.Any(d => d.Code == "FS0039"), Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadedScriptWithAnAutoOpenModule_IsOpenedForLaterCells() + { + await LoadAsync(WriteScript("auto.fsx", "[]\nmodule Auto\n\nlet seven x = x + 7\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = seven 2"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadedScriptWithANamespaceHeader_IsKnownByItsNamespace() + { + await LoadAsync(WriteScript("shapes.fsx", + "namespace Shapes\n\nmodule Area =\n let square x = x * x\n\ntype Square = { Side: int }\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let a = Shapes.Area.square 3\nlet s : Shapes.Square = { Side = 1 }"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task ScriptLoadedByALoadedScript_IsKnownToo() + { + WriteScript("inner.fsx", "let tenfold x = x * 10\n"); + await LoadAsync(WriteScript("outer.fsx", "#load \"inner.fsx\"\n\nlet plusOne x = Inner.tenfold x + 1\n")); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let a = Outer.plusOne 1\nlet b = Inner.tenfold 1"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task RelativeLoad_ResolvesAgainstTheNotebookDirectory() + { + WriteScript("scripts/scale.fsx", "let double x = x * 2\n"); + _context.NotebookMetadata = new NotebookMetadataContext(new NotebookModel(), Path.Combine(_scriptDir, "notebook.verso")); + + var outputs = await _kernel.ExecuteAsync("#load \"scripts/scale.fsx\"", _context); + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = Scale.double 2"); + + Assert.IsFalse(outputs.Any(o => o.IsError), string.Join("; ", outputs.Select(o => o.Content))); + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadThatFailedToCompile_IsNotKnownLater() + { + var path = WriteScript("broken.fsx", "let value : int = \"text\"\n"); + var outputs = await _kernel.ExecuteAsync($"#load @\"{path}\"", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n = Broken.value"); + + Assert.IsTrue(outputs.Any(o => o.IsError), "the load FSI rejects"); + Assert.IsTrue(diagnostics.Any(d => d.Code == "FS0039"), Describe(diagnostics)); + } + + [TestMethod] + public async Task LoadedScript_IsOfferedInCompletionsAndHover() + { + await LoadAsync(WriteScript("withheader.fsx", "module Lib\n\nlet triple x = x * 3\n")); + + var completions = await _kernel.GetCompletionsAsync("let n = Lib.tri", 15); + var code = "let n = Lib.triple 2"; + var hover = await _kernel.GetHoverInfoAsync(code, code.IndexOf("triple", StringComparison.Ordinal) + 1); + + Assert.IsTrue(completions.Any(c => c.DisplayText == "triple"), + string.Join(", ", completions.Select(c => c.DisplayText).Take(10))); + Assert.IsNotNull(hover); + StringAssert.Contains(hover.Content, "triple"); + } + + [TestMethod] + public async Task CellRunAfterALoad_StillChecksAgainstIt() + { + await _kernel.ExecuteAsync("let before = 1", _context); + await LoadAsync(WriteScript("withheader.fsx", "module Lib\n\nlet triple x = x * 3\n")); + await _kernel.ExecuteAsync("let after = Lib.triple before", _context); + + var diagnostics = await _kernel.GetDiagnosticsAsync("let n : int = after + before"); + + Assert.AreEqual(0, diagnostics.Count, Describe(diagnostics)); + } +} diff --git a/tests/Verso.FSharp.Tests/Kernel/NuGetExecutionTests.cs b/tests/Verso.FSharp.Tests/Kernel/NuGetExecutionTests.cs index b0b7774..10a7e54 100644 --- a/tests/Verso.FSharp.Tests/Kernel/NuGetExecutionTests.cs +++ b/tests/Verso.FSharp.Tests/Kernel/NuGetExecutionTests.cs @@ -179,23 +179,26 @@ public async Task NuGetDirective_TypeProviderResolvesTypes() [TestMethod] public async Task LoadDirective_AddsFileContentsToIntelliSense() { - // Create a temp .fsx file with a function definition - var tempFile = Path.Combine(Path.GetTempPath(), $"test_{Guid.NewGuid():N}.fsx"); + // Create a temp .fsx file with a function definition. FSI names the loaded script's + // module for the file and does not open it, so its names are reached as Helpers.x. + var tempDir = Path.Combine(Path.GetTempPath(), $"test_{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDir); + var tempFile = Path.Combine(tempDir, "helpers.fsx"); File.WriteAllText(tempFile, "let loadedHelperFn x = x + 100"); try { - // Execute #load — this should add file contents to IntelliSense context + // Execute #load, which should add the file to the IntelliSense context await _kernel.ExecuteAsync($"#load \"{tempFile}\"", _context); // Verify IntelliSense includes the loaded function definition - var completions = await _kernel.GetCompletionsAsync("loadedH", "loadedH".Length); + var completions = await _kernel.GetCompletionsAsync("Helpers.loadedH", "Helpers.loadedH".Length); Assert.IsTrue( completions.Any(c => c.DisplayText == "loadedHelperFn"), $"Expected 'loadedHelperFn' in completions after #load, got: {string.Join(", ", completions.Select(c => c.DisplayText).Take(10))}"); } finally { - File.Delete(tempFile); + Directory.Delete(tempDir, recursive: true); } } }