Missing node warning and quickfixes

CHANGELOG.md

@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - To enable cancelling a compilation, supply a `CancellationToken` to your `CompilationJob` object. You can request that the compilation be cancelled by cancelling the token. For more information, see [Task Cancellation](https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/task-cancellation).
 - Added `NodeDebugInfo.Range`, which contains the range in which a node appears in the source code.
 - Added `NodeDebugInfo.IsImplicit`, which indicates whether the node was created by the compiler (and does not appear in the source code).
+- Language Server: Warn on `jump` statements if destination node that does not exist. Offer to create a stub node, or rename to an existing node title.
 
 ### Changed
 

YarnSpinner.LanguageServer.Tests/CodeActionTests.cs

@@ -0,0 +1,129 @@
+using FluentAssertions;
+using Microsoft.Extensions.DependencyInjection;
+using Newtonsoft.Json.Linq;
+using OmniSharp.Extensions.LanguageServer.Protocol;
+using OmniSharp.Extensions.LanguageServer.Protocol.Document;
+using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;
+using System.IO;
+using System.Linq;
+using System.Threading.Tasks;
+using Xunit;
+using Xunit.Abstractions;
+using Yarn.Compiler;
+using YarnLanguageServer.Diagnostics;
+using YarnLanguageServer.Handlers;
+
+namespace YarnLanguageServer.Tests;
+
+#pragma warning disable VSTHRD200 // async methods should end in 'Async'
+
+public class CodeActionTests : LanguageServerTestsBase
+{
+    public CodeActionTests(ITestOutputHelper outputHelper) : base(outputHelper)
+    {
+    }
+
+    [Fact]
+    public async Task Server_FixesJumpDestinationTypo()
+    {
+        var filePath = Path.Combine(TestUtility.PathToTestWorkspace, "Project1", "Test.yarn");
+
+        Task<NodesChangedParams> getInitialNodesChanged = GetNodesChangedNotificationAsync(n => n.Uri.ToString().Contains(filePath));
+
+        // Set up the server
+        var (client, server) = await Initialize(ConfigureClient, ConfigureServer);
+        NodesChangedParams? nodeInfo = await getInitialNodesChanged;
+        var workspace = server.Workspace.GetService<Workspace>();
+        var diagnostics = workspace!.GetDiagnostics().SelectMany(d => d.Value);
+
+        var jumpToWarning = diagnostics.FirstOrDefault(d => d.Code.HasValue && d.Code.Value.String == nameof(YarnDiagnosticCode.YRNMsngJumpDest));
+
+        jumpToWarning.Should().NotBeNull("Expecting a warning for the missing jump destination");
+
+        var codeActionHandler = new CodeActionHandler(workspace);
+        var CodeActionParams = new CodeActionParams
+        {
+            Context = new CodeActionContext { Diagnostics = Container.From(jumpToWarning!)}, 
+            TextDocument = new TextDocumentIdentifier(DocumentUri.FromFileSystemPath(filePath)) 
+        };
+
+        var commandOrCodeActions = await codeActionHandler.Handle(CodeActionParams, default);
+
+        var typoFix = commandOrCodeActions.FirstOrDefault(c => c.CodeAction?.Title?.Contains("Rename to 'JumpToTest'") ?? false);
+        typoFix.Should().NotBeNull("Expecting a code action to fix the jump destination typo");
+
+        var typoFixEdit = typoFix!.CodeAction!.Edit;
+        typoFixEdit.Should().NotBeNull("Expecting the typo fix action to have a workspace edit");
+
+        // Remember how many nodes we had before making the change
+        var nodeCount = nodeInfo.Nodes.Count;
+
+        // Expect to receive a 'nodes changed' notification
+        Task<NodesChangedParams> nodesChangedAfterRemovingNode = GetNodesChangedNotificationAsync(n => n.Uri.ToString().Contains(filePath));
+
+        ChangeTextInDocuments(client, typoFixEdit!);
+
+        nodeInfo = await nodesChangedAfterRemovingNode;
+
+        var jumpToNode = nodeInfo.Nodes.Find(n => n.Title == "JumpToTest");
+        nodeInfo.Nodes.Should().HaveCount(nodeCount, "because didn't change any nodes");
+        jumpToNode!.Jumps.Where(j=>j.DestinationTitle == "JumpToTest").Should().HaveCount(1, "because we fixed the typo and are jumping to the existing JumpToTest node");
+    }
+
+    [Fact]
+    public async Task Server_CreatesNewNodeBasedOnJumpTarget()
+    {
+        var filePath = Path.Combine(TestUtility.PathToTestWorkspace, "Project1", "Test.yarn");
+
+        Task<NodesChangedParams> getInitialNodesChanged = GetNodesChangedNotificationAsync(n => n.Uri.ToString().Contains(filePath));
+
+        // Set up the server
+        var (client, server) = await Initialize(ConfigureClient, ConfigureServer);
+        NodesChangedParams? nodeInfo = await getInitialNodesChanged;
+        var workspace = server.Workspace.GetService<Workspace>();
+        var diagnostics = workspace!.GetDiagnostics().SelectMany(d => d.Value);
+
+        var jumpToWarning = diagnostics.FirstOrDefault(d => d.Code.HasValue && d.Code.Value.String == nameof(YarnDiagnosticCode.YRNMsngJumpDest));
+
+        jumpToWarning.Should().NotBeNull("Expecting a warning for the missing jump destination");
+
+
+        var codeActionHandler = new CodeActionHandler(workspace);
+        var CodeActionParams = new CodeActionParams
+        {
+            Context = new CodeActionContext { Diagnostics = Container.From(jumpToWarning!) },
+            TextDocument = new TextDocumentIdentifier(DocumentUri.FromFileSystemPath(filePath))
+        };
+        var commandOrCodeActions = await codeActionHandler.Handle(CodeActionParams, default);
+
+        var generateNodeFix = commandOrCodeActions.FirstOrDefault(c => 
+        c.CodeAction?.Title?.Contains("Generate node 'Jump2Test'") ?? false);
+
+        generateNodeFix.Should().NotBeNull("Expecting a code action to generate a new node");
+
+        var generateNodeFixEdit = generateNodeFix!.CodeAction!.Edit;
+        generateNodeFixEdit.Should().NotBeNull("Expecting the typo fix action to have a workspace edit");
+
+        // Remember how many nodes we had before making the change
+        var nodeCount = nodeInfo.Nodes.Count;
+
+        // Remember how many jumps we had to JumpToTest we had before making the change
+        var jumpCount = nodeInfo.Nodes.Find(n => n.Title == "JumpToTest")?.Jumps.Count ?? 0;
+
+        // Expect to receive a 'nodes changed' notification
+        Task<NodesChangedParams> nodesChangedAfterRemovingNode = GetNodesChangedNotificationAsync(n => n.Uri.ToString().Contains(filePath));
+
+        ChangeTextInDocuments(client, generateNodeFixEdit!);
+
+        nodeInfo = await nodesChangedAfterRemovingNode;
+
+        var jumpToNode = nodeInfo.Nodes.Find(n => n.Title == "JumpToTest");
+        var jump2Node = nodeInfo.Nodes.Find(n => n.Title == "Jump2Test");
+        jump2Node.Should().NotBeNull("because we have created a new node");
+        nodeInfo.Nodes.Should().HaveCount(nodeCount + 1, "because we have added a single new node");
+        jumpToNode!.Jumps.Where(j=>j.DestinationTitle == "JumpToTest").Should().HaveCount(0, "because we are jumping to the new generated node, not the exisiting JumpToTest node");
+        jumpToNode!.Jumps.Where(j=>j.DestinationTitle == "Jump2Test").Should().HaveCount(1, "because we are jumping to the new generated node, not the exisiting JumpToTest node");
+    }
+
+}

YarnSpinner.LanguageServer.Tests/LanguageServerTests.cs

@@ -116,15 +116,15 @@ public async Task Server_OnChangingDocument_SendsNodesChangedNotification()
 
             nodeInfo.Uri.ToString().Should().Be("file://" + filePath, "because this is the URI of the file we opened");
 
-            nodeInfo.Nodes.Should().HaveCount(4, "because there are three nodes in the file before we make changes");
+            nodeInfo.Nodes.Should().HaveCount(5, "because there are four nodes in the file before we make changes");
 
             var nodesChanged = GetNodesChangedNotificationAsync((nodesResult) =>
                 nodesResult.Uri.AbsolutePath.Contains(filePath)
             );
             ChangeTextInDocument(client, filePath, new Position(20, 0), "title: Node3\n---\n===\n");
             nodeInfo = await nodesChanged;
 
-            nodeInfo.Nodes.Should().HaveCount(5, "because we added a new node");
+            nodeInfo.Nodes.Should().HaveCount(6, "because we added a new node");
             nodeInfo.Nodes.Should().Contain(n => n.Title == "Node3", "because the new node we added has this title");
         }
 

YarnSpinner.LanguageServer.Tests/LanguageServerTestsBase.cs

@@ -106,6 +106,31 @@ protected static void ChangeTextInDocument(ILanguageClient client, TextDocumentE
             }
         }
 
+        protected static void ChangeTextInDocuments(ILanguageClient client, WorkspaceEdit workspaceEdit)
+        {
+            if (workspaceEdit.Changes == null) { return; }
+
+            foreach ((var docUri, var textEdits) in workspaceEdit.Changes) 
+            {
+                foreach (var edit in textEdits)
+                {
+                    client.DidChangeTextDocument(new DidChangeTextDocumentParams
+                    {
+                        TextDocument = new OptionalVersionedTextDocumentIdentifier
+                        {
+                            Uri = docUri,
+                        },
+                        ContentChanges = new[] {
+                        new TextDocumentContentChangeEvent {
+                            Range = edit.Range,
+                            Text = edit.NewText,
+                        },
+                    }
+                    });
+                }
+            }
+        }
+
         protected void ConfigureClient(LanguageClientOptions options)
         {
             options.OnRequest("keepalive", ct => Task.FromResult(true));

YarnSpinner.LanguageServer.Tests/TestData/TestWorkspace/Project1/Test.yarn

@@ -34,4 +34,11 @@ title: SmartVariables
 <<declare $isAlive = false>>
 <<declare $canEnterDoor = $isAlive>>
 <<declare $complexTest = $foo and ($bar or not true)>>
-=== 
+=== 
+
+title: JumpToTest
+---
+// This is used to test typo fixing and creating stub nodes for jumps to node titles that don't exist
+<<jump Jump2Test>>
+
+===

YarnSpinner.LanguageServer/src/Server/Diagnostics/ErrorCodes.cs

@@ -20,6 +20,11 @@ public enum YarnDiagnosticCode
         /// </summary>
         YRNMsngCmdDef,
 
+        /// <summary>
+        /// Warning: Jump to undefined node
+        /// </summary>
+        YRNMsngJumpDest,
+
         /// <summary>
         /// Error: Command or Function that has an incorrect number of parameters
         /// </summary>

YarnSpinner.LanguageServer/src/Server/Diagnostics/Warnings.cs

@@ -16,6 +16,7 @@ public static IEnumerable<Diagnostic> GetWarnings(YarnFileData yarnFile, Configu
             results = results.Concat(UnknownCommands(yarnFile));
             results = results.Concat(UndefinedFunctions(yarnFile, configuration));
             results = results.Concat(UndeclaredVariables(yarnFile));
+            results = results.Concat(UndefinedJumpDestination(yarnFile));
 
             return results;
         }
@@ -91,5 +92,22 @@ private static IEnumerable<Diagnostic> UndeclaredVariables(YarnFileData yarnFile
                 Data = JToken.FromObject(v.Text),
             });
         }
+
+        private static IEnumerable<Diagnostic> UndefinedJumpDestination(YarnFileData yarnFile)
+        {
+            var project = yarnFile.Project;
+
+            var undefinedJumpTargets = yarnFile.NodeJumps
+                .Where(jump => !project.FindNodes(jump.DestinationTitle).Any());
+
+            return undefinedJumpTargets.Select(t => new Diagnostic
+            {
+                Message = $"Jump to unknown node '{t.DestinationTitle}'",
+                Severity = DiagnosticSeverity.Warning,
+                Range = PositionHelper.GetRange(yarnFile.LineStarts, t.DestinationToken),
+                Code = nameof(YarnDiagnosticCode.YRNMsngJumpDest),
+                Data = JToken.FromObject(t.DestinationTitle),
+            });
+        }
     }
 }

YarnSpinner.LanguageServer/src/Server/Handlers/CodeActionHandler.cs

@@ -37,6 +37,9 @@ public Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, Cance
                     case nameof(YarnDiagnosticCode.YRNMsngVarDec):
                         results.AddRange(HandleYRNMsngVarDec(diagnostic, request.TextDocument.Uri));
                         break;
+                    case nameof(YarnDiagnosticCode.YRNMsngJumpDest):
+                        results.AddRange(HandleYRNMsngJumpDest(diagnostic, request.TextDocument.Uri));
+                        break;
                 }
             }
 
@@ -157,5 +160,92 @@ private IEnumerable<CommandOrCodeAction> HandleYRNMsngCmdDef(Diagnostic diagnost
 
             return suggestions;
         }
+
+        private IEnumerable<CommandOrCodeAction> HandleYRNMsngJumpDest(Diagnostic diagnostic, DocumentUri uri)
+        {
+            var jumpDestination = diagnostic.Data?.ToString();
+            if (string.IsNullOrEmpty(jumpDestination)) { return Enumerable.Empty<CommandOrCodeAction>(); }
+
+            var project = workspace.GetProjectsForUri(uri).FirstOrDefault();
+            if (project == null) { return Enumerable.Empty<CommandOrCodeAction>(); }
+
+            // Suggest potential renamings by fuzzy-searching for the existing
+            // input, and offering suggestions that replace the text.
+            var suggestions =
+                project.FindNodes(jumpDestination, true)
+                .Where(name => name != jumpDestination)
+                .Distinct()
+                .Take(10)
+                .Select(name =>
+                {
+                    var edits = new Dictionary<DocumentUri, IEnumerable<TextEdit>>();
+                    edits[uri] = new List<TextEdit> { new TextEdit { NewText = name, Range = diagnostic.Range } };
+
+                    var replaceAction = new CodeAction
+                    {
+                        Title = $"Rename to '{name}'",
+                        Kind = CodeActionKind.QuickFix,
+                        Edit = new WorkspaceEdit { Changes = edits },
+                    };
+                    return replaceAction;
+                })
+                .Select(s => new CommandOrCodeAction(s));
+
+            var yarnFile = project.GetFileData((System.Uri)uri);
+            if (yarnFile == null) { return suggestions; }
+
+            // Offer to create a new node with the missing node title if the jump destination doesn't exist
+            var insertDeclarationEdit = new Dictionary<DocumentUri, IEnumerable<TextEdit>>();
+
+            // Could share most of this code with AddNodeToDocumentAsync command
+            var newNodeText = new System.Text.StringBuilder()
+                .AppendLine($"title: {jumpDestination}")
+                .AppendLine("---")
+                .AppendLine()
+                .AppendLine("===");
+
+            Position insertPosition;
+
+            var lastLineIsEmpty = yarnFile.Text.EndsWith('\n');
+
+            int lastLineIndex = yarnFile.LineCount - 1;
+
+            if (lastLineIsEmpty)
+            {
+                // The final line ends with a newline. Insert the node
+                // there.
+                insertPosition = new Position(lastLineIndex, 0);
+            }
+            else
+            {
+                // The final line does not end with a newline. Insert a
+                // newline at the end of the last line, followed by the new
+                // text.
+                var endOfLastLine = yarnFile.GetLineLength(lastLineIndex);
+                newNodeText.Insert(0, System.Environment.NewLine);
+                insertPosition = new Position(lastLineIndex, endOfLastLine);
+            }
+
+            insertDeclarationEdit[uri] = new List<TextEdit> {
+                new TextEdit {
+                    NewText = newNodeText.ToString(),
+                    Range = new Range(insertPosition, insertPosition),
+                },
+            };
+
+            suggestions = suggestions.Prepend(
+                new CommandOrCodeAction(
+                    new CodeAction
+                    {
+                        Title = $"Generate node '{jumpDestination}'",
+                        Kind = CodeActionKind.QuickFix,
+                        IsPreferred = true,
+                        Edit = new WorkspaceEdit { Changes = insertDeclarationEdit },
+                    }
+                )
+            );
+
+            return suggestions;
+        }
     }
 }

YarnSpinner.LanguageServer/src/Server/Workspace/Project.cs

@@ -157,6 +157,12 @@ internal YarnFileData AddNewFile(Uri uri, string text)
             return FindDeclarations(Variables, name, fuzzySearch);
         }
 
+        internal IEnumerable<string> FindNodes(string name, bool fuzzySearch = false)
+        {
+            var nodeNames = this.yarnFiles.Values.SelectMany(file => file.NodeInfos.Select(node => node.Title)).Distinct();
+            return FindNodes(nodeNames, name, fuzzySearch);
+        }
+
         /// <summary>
         /// Finds actions of the given type that match a name.
         /// </summary>
@@ -299,5 +305,16 @@ internal DebugOutput GetDebugOutput(CancellationToken cancellationToken)
 
             return Workspace.FuzzySearchItem(declarations.Select(d => (d.Name, d)), name, ConfigurationSource?.Configuration.DidYouMeanThreshold ?? Configuration.Defaults.DidYouMeanThreshold);
         }
+
+        private IEnumerable<string> FindNodes(IEnumerable<string> nodeNames, string name, bool fuzzySearch)
+        {
+            if (fuzzySearch == false)
+            {
+                return nodeNames.Where(n => n.Equals(name));
+            }
+
+            return Workspace.FuzzySearchItem(nodeNames.Select(n => (n, n)), name, ConfigurationSource?.Configuration.DidYouMeanThreshold ?? Configuration.Defaults.DidYouMeanThreshold)
+                .Select(n => n);
+        }
     }
 }