Markup diagnostics are now reported by the compiler

Fixes YarnSpinnerTool/IssuesDiscussion#118

Author: McJones

CHANGELOG.md

@@ -12,13 +12,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   - The old tagging approach now exists as `RandomLineTagGenerator`
   - A new, more human readable, tagger `DescriptiveLineTagGenerator`
 - Multiple line or shadow IDs on a single line now generate a diagnostic
+- `ParseStringAndIncludeMarkupDiagnostics` call on the `LineParser`
+  - this allows access to the diagnostics that are generated by parsing the markup
 
 ### Changed
 
 - Fixed an issue where `.yarnproject` files couldn't be loaded if the C# project was trimmed.
 - Compiling a `CompilationJob` in type-check-only mode now includes the generated string table in its results.
 - When tagging lines can now define what `ILineTagGenerator` you want to use
   - defaults to using `RandomLineTagGenerator` if you don't set one
+- Compiling a `CompilationJob` now includes diagnostics generated by markup parsing.
 
 ### Removed
 

YarnSpinner.Compiler/Compiler.cs

@@ -235,6 +235,25 @@ public static CompilationResult Compile(CompilationJob compilationJob)
                 }
             }
 
+            // we run through each line and parse it looking for any markup
+            // this way we can send over diagnostics at the same time
+            // later might want to make this a different compilation job type
+            // or maybe be a different flow entirely
+            var lineParser = new Yarn.Markup.LineParser();
+            foreach (var line in stringTableManager.StringTable)
+            {
+                if (line.Value.text == null)
+                {
+                    continue;
+                }
+                var result = lineParser.ParseStringAndIncludeMarkupDiagnostics(line.Value.text, System.Globalization.CultureInfo.InvariantCulture.TwoLetterISOLanguageName);
+                foreach (var diag in result.diagnostics)
+                {
+                    diagnostics.Add(DiagnosticDescriptor.MarkupFailedToParse.Create(line.Value.fileName, diag.Message));
+                }
+            }
+
+
             if (compilationJob.CompilationType == CompilationJob.Type.StringsOnly)
             {
                 // Stop at this point

YarnSpinner.Diagnostics/Definitions/YS0063-MarkupFailedToParse.md

@@ -0,0 +1,17 @@
+---
+name: MarkupFailedToParse
+code: YS0063
+tags: ["shadow-lines", "line-content", "markup"]
+
+description: Dialogue has malformed or invalid markup.
+messageTemplate: "Dialogue has malformed or invalid markup. {0}"
+messageValues: 
+    - Markup parse message
+
+summary: |
+    Markup has it's own specific syntax. The line of dialogue has malformed, invalid, unbalanced, or incomplete markup.
+
+defaultSeverity: warning
+minimumSeverity: none
+---
+

YarnSpinner.Tests/ErrorHandlingTests.cs

@@ -1300,5 +1300,19 @@ public void TestDiagnosticsCanHaveOverriddenSeverities(Diagnostic.DiagnosticSeve
 
             diag.Severity.Should().Be(severity);
         }
+
+
+        [Fact]
+        public void TestCompilerGeneratesMarkupDiagnostics()
+        {
+            // this is just checking a single diag comes over
+            // ensuring the right diags are generated is over in the markuptests
+            var source = CreateTestNode("A line with [a]invalid markup");
+            var job = CompilationJob.CreateFromString("<input>", source);
+            job.CompilationType = CompilationJob.Type.StringsOnly;
+
+            var result = Compiler.Compile(job);
+            var diag = result.Diagnostics.Should().ContainSingle(d => d.Code == DiagnosticDescriptor.MarkupFailedToParse.Code);
+        }
     }
 }

YarnSpinner.Tests/MarkupTests.cs

@@ -1579,6 +1579,38 @@ public void TestMarkerProcessorsCanProcessCharacterNames()
                     kv => kv.Key == "name" && kv.Value.StringValue == "Mae", "the marker's properties should be unmodified"
                 );
         }
+
+        [Fact]
+        public void TestUnderscoreCanBeIdentifiers()
+        {
+            var lineParser = new LineParser();
+
+            // checking self-closing tags can have underscores in their name
+            var markup = lineParser.ParseString("Narrator: Self-closing tag [under_tag /]with an underscore.", "en-AU");
+            markup.Text.Should().Be("Narrator: Self-closing tag with an underscore.");
+
+            markup.Attributes.Should().HaveCount(2);
+            markup.Attributes.Should().Contain(m => m.Name == "under_tag");
+            markup.Attributes.Should().Contain(m => m.Name == "character")
+                .Which.Properties.Should().Contain(kv => kv.Key == "name" && kv.Value.StringValue == "Narrator");
+
+            // checking regular markup can have underscores in their name
+            markup = lineParser.ParseString("Narrator: This is a [under_tag]regular markup[/under_tag] with underscores", "en-AU");
+            markup.Text.Should().Be("Narrator: This is a regular markup with underscores");
+
+            markup.Attributes.Should().HaveCount(2);
+            markup.Attributes.Should().Contain(m => m.Name == "under_tag");
+            markup.Attributes.Should().Contain(m => m.Name == "character")
+                .Which.Properties.Should().Contain(kv => kv.Key == "name" && kv.Value.StringValue == "Narrator");
+
+            // checking markup can have properties with underscores
+            markup = lineParser.ParseString("Line with a regular [under_tag under_property=\"hello\"]underscored tag with an underscored property also[/under_tag] in it.", "en-AU");
+            markup.Text.Should().Be("Line with a regular underscored tag with an underscored property also in it.");
+            
+            markup.Attributes.Should().HaveCount(1);
+            markup.Attributes.Should().Contain(m => m.Name == "under_tag")
+                .Which.Properties.Should().Contain(kv => kv.Key == "under_property" && kv.Value.StringValue == "hello");
+        }
     }
 
     public class BBCodeChevronReplacer : IAttributeMarkerProcessor

YarnSpinner/YarnSpinner.Markup/LineParser.cs

@@ -298,6 +298,12 @@ internal bool ComparePattern(LexerTokenTypes[] pattern)
 
         internal List<LexerToken> LexMarkup(string input)
         {
+            HashSet<char> allowedIdentifierPunctuation = new()
+            {
+                '_',
+                '|'
+            };
+
             List<LexerToken> tokens = new List<LexerToken>();
             if (string.IsNullOrEmpty(input))
             {
@@ -433,12 +439,14 @@ internal List<LexerToken> LexMarkup(string input)
                                 {
                                     var start = currentPosition;
 
-                                    // keep reading characters until the NEXT character is not a letter or digit
+                                    // keep reading characters until the NEXT character is not a letter or digit or allowed punctuation symbol
                                     // when that happens we will stop at that point, emit an id token
-                                    while (char.IsLetterOrDigit((char)this.stringReader.Peek()))
+                                    char peek = (char)this.stringReader.Peek();
+                                    while (char.IsLetterOrDigit(peek) || allowedIdentifierPunctuation.Contains(peek))
                                     {
                                         _ = this.stringReader.Read();
                                         currentPosition += 1;
+                                        peek = (char)this.stringReader.Peek();
                                     }
 
                                     last = new LexerToken(LexerTokenTypes.Identifier)
@@ -1467,6 +1475,13 @@ public MarkupParseResult ParseString(string input, string localeCode, bool addIm
         {
             return ParseString(input, localeCode, squish: true, sort: true, addImplicitCharacterAttribute);
         }
+        
+        /// <returns>A markup parse result and a collection of diagnostics encountered while parsing the markup.</returns>
+        /// <inheritdoc cref="ParseString(string, string, bool, bool, bool)" />
+        public (MarkupParseResult markup, List<MarkupDiagnostic> diagnostics) ParseStringAndIncludeMarkupDiagnostics(string input, string localeCode, bool addImplicitCharacterAttribute = true)
+        {
+            return ParseStringWithDiagnostics(input, localeCode, squish: true, sort: true, addImplicitCharacterAttribute);
+        }
 
         /// <summary>
         /// Parses a string of text and produces a markup parse result.