Fixes to syntax related reading errors

darrelmiller · darrelmiller · commit ea0dee1cdd3f · 2018-06-11T21:18:02.000-04:00
diff --git a/src/Microsoft.OpenApi.Readers/Exceptions/OpenApiReaderException.cs b/src/Microsoft.OpenApi.Readers/Exceptions/OpenApiReaderException.cs
@@ -2,14 +2,16 @@
 // Licensed under the MIT license. 
 
 using System;
+using Microsoft.OpenApi.Exceptions;
+using SharpYaml.Serialization;
 
 namespace Microsoft.OpenApi.Readers.Exceptions
 {
     /// <summary>
     /// Defines an exception indicating OpenAPI Reader encountered an issue while reading.
     /// </summary>
     [Serializable]
-    public class OpenApiReaderException : Exception
+    public class OpenApiReaderException : OpenApiException
     {
         /// <summary>
         /// Initializes the <see cref="OpenApiReaderException"/> class.
@@ -22,6 +24,16 @@ public OpenApiReaderException() { }
         /// <param name="message">Plain text error message for this exception.</param>
         public OpenApiReaderException(string message) : base(message) { }
 
+        /// <summary>
+        /// Initializes the <see cref="OpenApiReaderException"/> class with a message and line, column location of error.
+        /// </summary>
+        /// <param name="message">Plain text error message for this exception.</param>
+        /// <param name="node">Parsing node where error occured</param>
+        public OpenApiReaderException(string message, YamlNode node) : base(message)
+        {
+            Pointer = $"#char={node.Start.Index},{node.End.Index}";
+        }
+
         /// <summary>
         /// Initializes the <see cref="OpenApiReaderException"/> class with a custom message and inner exception.
         /// </summary>
diff --git a/src/Microsoft.OpenApi.Readers/OpenApiReaderError.cs b/src/Microsoft.OpenApi.Readers/OpenApiReaderError.cs
diff --git a/src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs b/src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs
@@ -49,9 +49,13 @@ public OpenApiDocument Read(Stream input, out OpenApiDiagnostic diagnostic)
             {
                 yamlDocument = LoadYamlDocument(input);
             }
+            catch (SemanticErrorException ex) {
+                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));
+                return new OpenApiDocument();
+            }
             catch (SyntaxErrorException ex)
             {
-                diagnostic.Errors.Add(new OpenApiReaderError(ex));
+                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));
                 return new OpenApiDocument();
             }
 
@@ -124,7 +128,7 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi
             }
             catch (SyntaxErrorException ex)
             {
-                diagnostic.Errors.Add(new OpenApiReaderError(ex));
+                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));
                 return default(T);
             }
 
@@ -158,7 +162,6 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi
             return (T)element;
         }
 
-
         /// <summary>
         /// Helper method to turn streams into YamlDocument
         /// </summary>
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/ListNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/ListNode.cs
@@ -52,7 +52,7 @@ public override List<T> CreateSimpleList<T>(Func<ValueNode, T> map)
                     $"Expected list at line {_nodeList.Start.Line} while parsing {typeof(T).Name}");
             }
 
-            return _nodeList.Select(n => map(new ValueNode(Context, Diagnostic, (YamlScalarNode)n))).ToList();
+            return _nodeList.Select(n => map(new ValueNode(Context, Diagnostic, n))).ToList();
         }
 
         public IEnumerator<ParseNode> GetEnumerator()
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/MapNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/MapNode.cs
@@ -9,6 +9,7 @@
 using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Readers.Exceptions;
 using SharpYaml.Schemas;
 using SharpYaml.Serialization;
 
@@ -27,16 +28,16 @@ public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, string yaml
         {
         }
 
-        public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlMappingNode node) : base(
+        public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode node) : base(
             context,
             diagnostic)
         {
-            if (node == null)
+            if (!(node is YamlMappingNode mapNode))
             {
-                throw new OpenApiException("Expected map");
+                throw new OpenApiReaderException("Expected map.", node);
             }
 
-            this._node = node;
+            this._node = mapNode;
 
             _nodes = this._node.Children
                 .Select(kvp => new PropertyNode(Context, Diagnostic, kvp.Key.GetScalarValue(), kvp.Value))
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/ParseNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/ParseNode.cs
@@ -26,8 +26,7 @@ protected ParseNode(ParsingContext parsingContext, OpenApiDiagnostic diagnostic)
 
         public MapNode CheckMapNode(string nodeName)
         {
-            var mapNode = this as MapNode;
-            if (mapNode == null)
+            if (!(this is MapNode mapNode))
             {
                 throw new OpenApiException($"{nodeName} must be a map/object");
             }
@@ -37,15 +36,13 @@ public MapNode CheckMapNode(string nodeName)
 
         public static ParseNode Create(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode node)
         {
-            var listNode = node as YamlSequenceNode;
 
-            if (listNode != null)
+            if (node is YamlSequenceNode listNode)
             {
                 return new ListNode(context, diagnostic, listNode);
             }
 
-            var mapNode = node as YamlMappingNode;
-            if (mapNode != null)
+            if (node is YamlMappingNode mapNode)
             {
                 return new MapNode(context, diagnostic, mapNode);
             }
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/PropertyNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/PropertyNode.cs
@@ -7,6 +7,7 @@
 using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Readers.Exceptions;
 using SharpYaml.Serialization;
 
 namespace Microsoft.OpenApi.Readers.ParseNodes
@@ -40,10 +41,14 @@ public void ParseField<T>(
                     Context.StartObject(Name);
                     fixedFieldMap(parentInstance, Value);
                 }
+                catch (OpenApiReaderException ex)
+                {
+                    Diagnostic.Errors.Add(new OpenApiError(ex));
+                }
                 catch (OpenApiException ex)
                 {
                     ex.Pointer = Context.GetLocation();
-                    Diagnostic.Errors.Add(new OpenApiReaderError(ex));
+                    Diagnostic.Errors.Add(new OpenApiError(ex));
                 }
                 finally
                 {
@@ -63,7 +68,7 @@ public void ParseField<T>(
                     catch (OpenApiException ex)
                     {
                         ex.Pointer = Context.GetLocation();
-                        Diagnostic.Errors.Add(new OpenApiReaderError(ex));
+                        Diagnostic.Errors.Add(new OpenApiError(ex));
                     }
                     finally
                     {
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/ValueNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/ValueNode.cs
@@ -4,6 +4,7 @@
 using System;
 using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Exceptions;
+using Microsoft.OpenApi.Readers.Exceptions;
 using SharpYaml.Serialization;
 
 namespace Microsoft.OpenApi.Readers.ParseNodes
@@ -12,10 +13,14 @@ internal class ValueNode : ParseNode
     {
         private readonly YamlScalarNode _node;
 
-        public ValueNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlScalarNode scalarNode) : base(
+        public ValueNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode node) : base(
             context,
             diagnostic)
         {
+            if (!(node is YamlScalarNode scalarNode))
+            {
+                throw new OpenApiReaderException("Expected a value.", node);
+            }
             _node = scalarNode;
         }
 
diff --git a/src/Microsoft.OpenApi/Models/OpenApiError.cs b/src/Microsoft.OpenApi/Models/OpenApiError.cs
@@ -41,7 +41,7 @@ public OpenApiError(string pointer, string message)
         /// </summary>
         public override string ToString()
         {
-            return Message + (!string.IsNullOrEmpty(Pointer) ? " at " + Pointer : "");
+            return Message + (!string.IsNullOrEmpty(Pointer) ? " [" + Pointer + "]" : "" );
         }
     }
 }
diff --git a/test/Microsoft.OpenApi.Readers.Tests/NodeTests.cs b/test/Microsoft.OpenApi.Readers.Tests/NodeTests.cs
@@ -0,0 +1,37 @@
+﻿using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using FluentAssertions;
+using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Readers;
+using Microsoft.OpenApi.Readers.Exceptions;
+using Xunit;
+
+namespace Microsoft.OpenApi.Tests
+{
+    public class NodeTests
+    {
+
+        [Fact]
+        public void BrokenSimpleList()
+        {
+            var input = @"swagger: 2.0
+info:
+  title: hey
+  version: 1.0.0
+schemes: [ { ""hello"" }]
+paths: { }";
+
+            var reader = new OpenApiStringReader();
+            reader.Read(input, out var diagnostic);
+
+            diagnostic.Errors.ShouldBeEquivalentTo(new List<OpenApiError>() {
+                new OpenApiError(new OpenApiReaderException("Expected a value. Line: 4 Column: 11") {
+                    Pointer = "#char=64,64"
+                })
+            });
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -49,9 +49,13 @@ public OpenApiDocument Read(Stream input, out OpenApiDiagnostic diagnostic)`
`49`	`49`	`{`
`50`	`50`	`yamlDocument = LoadYamlDocument(input);`
`51`	`51`	`}`
	`52`	`+ catch (SemanticErrorException ex) {`
	`53`	`+ diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));`
	`54`	`+ return new OpenApiDocument();`
	`55`	`+ }`
`52`	`56`	`catch (SyntaxErrorException ex)`
`53`	`57`	`{`
`54`		`- diagnostic.Errors.Add(new OpenApiReaderError(ex));`
	`58`	`+ diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));`
`55`	`59`	`return new OpenApiDocument();`
`56`	`60`	`}`
`57`	`61`
`@@ -124,7 +128,7 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi`
`124`	`128`	`}`
`125`	`129`	`catch (SyntaxErrorException ex)`
`126`	`130`	`{`
`127`		`- diagnostic.Errors.Add(new OpenApiReaderError(ex));`
	`131`	`+ diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));`
`128`	`132`	`return default(T);`
`129`	`133`	`}`
`130`	`134`
`@@ -158,7 +162,6 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi`
`158`	`162`	`return (T)element;`
`159`	`163`	`}`
`160`	`164`
`161`		`-`
`162`	`165`	`/// <summary>`
`163`	`166`	`/// Helper method to turn streams into YamlDocument`
`164`	`167`	`/// </summary>`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ public override List<T> CreateSimpleList<T>(Func<ValueNode, T> map)`
`52`	`52`	`$"Expected list at line {_nodeList.Start.Line} while parsing {typeof(T).Name}");`
`53`	`53`	`}`
`54`	`54`
`55`		`- return _nodeList.Select(n => map(new ValueNode(Context, Diagnostic, (YamlScalarNode)n))).ToList();`
	`55`	`+ return _nodeList.Select(n => map(new ValueNode(Context, Diagnostic, n))).ToList();`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`public IEnumerator<ParseNode> GetEnumerator()`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@`
`9`	`9`	`using Microsoft.OpenApi.Exceptions;`
`10`	`10`	`using Microsoft.OpenApi.Interfaces;`
`11`	`11`	`using Microsoft.OpenApi.Models;`
	`12`	`+using Microsoft.OpenApi.Readers.Exceptions;`
`12`	`13`	`using SharpYaml.Schemas;`
`13`	`14`	`using SharpYaml.Serialization;`
`14`	`15`
`@@ -27,16 +28,16 @@ public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, string yaml`
`27`	`28`	`{`
`28`	`29`	`}`
`29`	`30`
`30`		`- public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlMappingNode node) : base(`
	`31`	`+ public MapNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode node) : base(`
`31`	`32`	`context,`
`32`	`33`	`diagnostic)`
`33`	`34`	`{`
`34`		`- if (node == null)`
	`35`	`+ if (!(node is YamlMappingNode mapNode))`
`35`	`36`	`{`
`36`		`- throw new OpenApiException("Expected map");`
	`37`	`+ throw new OpenApiReaderException("Expected map.", node);`
`37`	`38`	`}`
`38`	`39`
`39`		`- this._node = node;`
	`40`	`+ this._node = mapNode;`
`40`	`41`
`41`	`42`	`_nodes = this._node.Children`
`42`	`43`	`.Select(kvp => new PropertyNode(Context, Diagnostic, kvp.Key.GetScalarValue(), kvp.Value))`
Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,7 @@ protected ParseNode(ParsingContext parsingContext, OpenApiDiagnostic diagnostic)`
`26`	`26`
`27`	`27`	`public MapNode CheckMapNode(string nodeName)`
`28`	`28`	`{`
`29`		`- var mapNode = this as MapNode;`
`30`		`- if (mapNode == null)`
	`29`	`+ if (!(this is MapNode mapNode))`
`31`	`30`	`{`
`32`	`31`	`throw new OpenApiException($"{nodeName} must be a map/object");`
`33`	`32`	`}`
`@@ -37,15 +36,13 @@ public MapNode CheckMapNode(string nodeName)`
`37`	`36`
`38`	`37`	`public static ParseNode Create(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode node)`
`39`	`38`	`{`
`40`		`- var listNode = node as YamlSequenceNode;`
`41`	`39`
`42`		`- if (listNode != null)`
	`40`	`+ if (node is YamlSequenceNode listNode)`
`43`	`41`	`{`
`44`	`42`	`return new ListNode(context, diagnostic, listNode);`
`45`	`43`	`}`
`46`	`44`
`47`		`- var mapNode = node as YamlMappingNode;`
`48`		`- if (mapNode != null)`
	`45`	`+ if (node is YamlMappingNode mapNode)`
`49`	`46`	`{`
`50`	`47`	`return new MapNode(context, diagnostic, mapNode);`
`51`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@`
`7`	`7`	`using Microsoft.OpenApi.Any;`
`8`	`8`	`using Microsoft.OpenApi.Exceptions;`
`9`	`9`	`using Microsoft.OpenApi.Models;`
	`10`	`+using Microsoft.OpenApi.Readers.Exceptions;`
`10`	`11`	`using SharpYaml.Serialization;`
`11`	`12`
`12`	`13`	`namespace Microsoft.OpenApi.Readers.ParseNodes`
`@@ -40,10 +41,14 @@ public void ParseField<T>(`
`40`	`41`	`Context.StartObject(Name);`
`41`	`42`	`fixedFieldMap(parentInstance, Value);`
`42`	`43`	`}`
	`44`	`+ catch (OpenApiReaderException ex)`
	`45`	`+ {`
	`46`	`+ Diagnostic.Errors.Add(new OpenApiError(ex));`
	`47`	`+ }`
`43`	`48`	`catch (OpenApiException ex)`
`44`	`49`	`{`
`45`	`50`	`ex.Pointer = Context.GetLocation();`
`46`		`- Diagnostic.Errors.Add(new OpenApiReaderError(ex));`
	`51`	`+ Diagnostic.Errors.Add(new OpenApiError(ex));`
`47`	`52`	`}`
`48`	`53`	`finally`
`49`	`54`	`{`
`@@ -63,7 +68,7 @@ public void ParseField<T>(`
`63`	`68`	`catch (OpenApiException ex)`
`64`	`69`	`{`
`65`	`70`	`ex.Pointer = Context.GetLocation();`
`66`		`- Diagnostic.Errors.Add(new OpenApiReaderError(ex));`
	`71`	`+ Diagnostic.Errors.Add(new OpenApiError(ex));`
`67`	`72`	`}`
`68`	`73`	`finally`
`69`	`74`	`{`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ public OpenApiError(string pointer, string message)`
`41`	`41`	`/// </summary>`
`42`	`42`	`public override string ToString()`
`43`	`43`	`{`
`44`		`- return Message + (!string.IsNullOrEmpty(Pointer) ? " at " + Pointer : "");`
	`44`	`+ return Message + (!string.IsNullOrEmpty(Pointer) ? " [" + Pointer + "]" : "" );`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`	`}`