Fixes based on PR review

darrelmiller · darrelmiller · commit cca306e35521 · 2018-06-12T14:42:14.000-04:00
diff --git a/src/Microsoft.OpenApi.Readers/Exceptions/OpenApiReaderException.cs b/src/Microsoft.OpenApi.Readers/Exceptions/OpenApiReaderException.cs
@@ -31,8 +31,9 @@ public OpenApiReaderException(string message) : base(message) { }
         /// <param name="node">Parsing node where error occured</param>
         public OpenApiReaderException(string message, YamlNode node) : base(message)
         {
-            Pointer = $"#line={node.Start.Line}";  // This only includes line because using a char range causes tests to break due to CR/LF & LF differences
-                                                   // See https://tools.ietf.org/html/rfc5147 for syntax
+            // This only includes line because using a char range causes tests to break due to CR/LF & LF differences
+            // See https://tools.ietf.org/html/rfc5147 for syntax
+            Pointer = $"#line={node.Start.Line}";  
         }
 
         /// <summary>
diff --git a/src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs b/src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs
@@ -49,13 +49,19 @@ 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));
+            catch (SemanticErrorException ex)
+            {
+                diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));
                 return new OpenApiDocument();
             }
             catch (SyntaxErrorException ex)
             {
-                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));
+                diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));
+                return new OpenApiDocument();
+            }
+            catch (YamlException ex)
+            {
+                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Line}", ex.Message));
                 return new OpenApiDocument();
             }
 
@@ -128,7 +134,17 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi
             }
             catch (SyntaxErrorException ex)
             {
-                diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));
+                diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));
+                return default(T);
+            }
+            catch (SemanticErrorException ex)
+            {
+                diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));
+                return default(T);
+            }
+            catch (YamlException ex)
+            {
+                diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));
                 return default(T);
             }
 
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/ParseNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/ParseNode.cs
@@ -8,6 +8,7 @@
 using Microsoft.OpenApi.Exceptions;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Models;
+using Microsoft.OpenApi.Readers.Exceptions;
 using SharpYaml.Serialization;
 
 namespace Microsoft.OpenApi.Readers.ParseNodes
@@ -28,7 +29,7 @@ public MapNode CheckMapNode(string nodeName)
         {
             if (!(this is MapNode mapNode))
             {
-                throw new OpenApiException($"{nodeName} must be a map/object");
+                throw new OpenApiReaderException($"{nodeName} must be a map/object");
             }
 
             return mapNode;
@@ -52,50 +53,50 @@ public static ParseNode Create(ParsingContext context, OpenApiDiagnostic diagnos
 
         public virtual List<T> CreateList<T>(Func<MapNode, T> map)
         {
-            throw new OpenApiException("Cannot create list from this type of node.");
+            throw new OpenApiReaderException("Cannot create list from this type of node.");
         }
 
         public virtual Dictionary<string, T> CreateMap<T>(Func<MapNode, T> map)
         {
-            throw new OpenApiException("Cannot create map from this type of node.");
+            throw new OpenApiReaderException("Cannot create map from this type of node.");
         }
 
         public virtual Dictionary<string, T> CreateMapWithReference<T>(
             ReferenceType referenceType,
             Func<MapNode, T> map)
             where T : class, IOpenApiReferenceable
         {
-            throw new OpenApiException("Cannot create map from this reference.");
+            throw new OpenApiReaderException("Cannot create map from this reference.");
         }
 
         public virtual List<T> CreateSimpleList<T>(Func<ValueNode, T> map)
         {
-            throw new OpenApiException("Cannot create simple list from this type of node.");
+            throw new OpenApiReaderException("Cannot create simple list from this type of node.");
         }
 
         public virtual Dictionary<string, T> CreateSimpleMap<T>(Func<ValueNode, T> map)
         {
-            throw new OpenApiException("Cannot create simple map from this type of node.");
+            throw new OpenApiReaderException("Cannot create simple map from this type of node.");
         }
 
         public virtual IOpenApiAny CreateAny()
         {
-            throw new OpenApiException("Cannot create an Any object this type of node.");
+            throw new OpenApiReaderException("Cannot create an Any object this type of node.");
         }
 
         public virtual string GetRaw()
         {
-            throw new OpenApiException("Cannot get raw value from this type of node.");
+            throw new OpenApiReaderException("Cannot get raw value from this type of node.");
         }
 
         public virtual string GetScalarValue()
         {
-            throw new OpenApiException("Cannot create a scalar value from this type of node.");
+            throw new OpenApiReaderException("Cannot create a scalar value from this type of node.");
         }
 
         public virtual List<IOpenApiAny> CreateListOfAny()
         {
-            throw new OpenApiException("Cannot create a list from this type of node.");
+            throw new OpenApiReaderException("Cannot create a list from this type of node.");
         }
 
     }
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/PropertyNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/PropertyNode.cs
@@ -65,6 +65,10 @@ public void ParseField<T>(
                         Context.StartObject(Name);
                         map(parentInstance, Name, Value);
                     }
+                    catch (OpenApiReaderException ex)
+                    {
+                        Diagnostic.Errors.Add(new OpenApiError(ex));
+                    }
                     catch (OpenApiException ex)
                     {
                         ex.Pointer = Context.GetLocation();
diff --git a/src/Microsoft.OpenApi.Readers/ParseNodes/ValueNode.cs b/src/Microsoft.OpenApi.Readers/ParseNodes/ValueNode.cs
@@ -26,14 +26,7 @@ public ValueNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode
 
         public override string GetScalarValue()
         {
-            var scalarNode = _node;
-
-            if (scalarNode == null)
-            {
-                throw new OpenApiException($"Expected scalar at line {_node.Start.Line}");
-            }
-
-            return scalarNode.Value;
+            return _node.Value;
        }
 
         /// <summary>
diff --git a/test/Microsoft.OpenApi.Readers.Tests/ParseNodeTests.cs b/test/Microsoft.OpenApi.Readers.Tests/ParseNodeTests.cs
@@ -1,8 +1,7 @@
-﻿using System;
+﻿// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. 
+
 using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 using FluentAssertions;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Readers;
@@ -11,9 +10,8 @@
 
 namespace Microsoft.OpenApi.Tests
 {
-    public class NodeTests
+    public class ParseNodeTests
     {
-
         [Fact]
         public void BrokenSimpleList()
         {

Original file line number	Diff line number	Diff line change
`@@ -31,8 +31,9 @@ public OpenApiReaderException(string message) : base(message) { }`
`31`	`31`	`/// <param name="node">Parsing node where error occured</param>`
`32`	`32`	`public OpenApiReaderException(string message, YamlNode node) : base(message)`
`33`	`33`	`{`
`34`		`- Pointer = $"#line={node.Start.Line}"; // This only includes line because using a char range causes tests to break due to CR/LF & LF differences`
`35`		`- // See https://tools.ietf.org/html/rfc5147 for syntax`
	`34`	`+ // This only includes line because using a char range causes tests to break due to CR/LF & LF differences`
	`35`	`+ // See https://tools.ietf.org/html/rfc5147 for syntax`
	`36`	`+ Pointer = $"#line={node.Start.Line}";`
`36`	`37`	`}`
`37`	`38`
`38`	`39`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -49,13 +49,19 @@ 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));`
	`52`	`+ catch (SemanticErrorException ex)`
	`53`	`+ {`
	`54`	`+ diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));`
`54`	`55`	`return new OpenApiDocument();`
`55`	`56`	`}`
`56`	`57`	`catch (SyntaxErrorException ex)`
`57`	`58`	`{`
`58`		`- diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));`
	`59`	`+ diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));`
	`60`	`+ return new OpenApiDocument();`
	`61`	`+ }`
	`62`	`+ catch (YamlException ex)`
	`63`	`+ {`
	`64`	`+ diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Line}", ex.Message));`
`59`	`65`	`return new OpenApiDocument();`
`60`	`66`	`}`
`61`	`67`
`@@ -128,7 +134,17 @@ public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDi`
`128`	`134`	`}`
`129`	`135`	`catch (SyntaxErrorException ex)`
`130`	`136`	`{`
`131`		`- diagnostic.Errors.Add(new OpenApiError($"#char={ex.Start.Index},{ex.End.Index}", ex.Message));`
	`137`	`+ diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));`
	`138`	`+ return default(T);`
	`139`	`+ }`
	`140`	`+ catch (SemanticErrorException ex)`
	`141`	`+ {`
	`142`	`+ diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));`
	`143`	`+ return default(T);`
	`144`	`+ }`
	`145`	`+ catch (YamlException ex)`
	`146`	`+ {`
	`147`	`+ diagnostic.Errors.Add(new OpenApiError($"#line={ex.Start.Line}", ex.Message));`
`132`	`148`	`return default(T);`
`133`	`149`	`}`
`134`	`150`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`	`using Microsoft.OpenApi.Exceptions;`
`9`	`9`	`using Microsoft.OpenApi.Interfaces;`
`10`	`10`	`using Microsoft.OpenApi.Models;`
	`11`	`+using Microsoft.OpenApi.Readers.Exceptions;`
`11`	`12`	`using SharpYaml.Serialization;`
`12`	`13`
`13`	`14`	`namespace Microsoft.OpenApi.Readers.ParseNodes`
`@@ -28,7 +29,7 @@ public MapNode CheckMapNode(string nodeName)`
`28`	`29`	`{`
`29`	`30`	`if (!(this is MapNode mapNode))`
`30`	`31`	`{`
`31`		`- throw new OpenApiException($"{nodeName} must be a map/object");`
	`32`	`+ throw new OpenApiReaderException($"{nodeName} must be a map/object");`
`32`	`33`	`}`
`33`	`34`
`34`	`35`	`return mapNode;`
`@@ -52,50 +53,50 @@ public static ParseNode Create(ParsingContext context, OpenApiDiagnostic diagnos`
`52`	`53`
`53`	`54`	`public virtual List<T> CreateList<T>(Func<MapNode, T> map)`
`54`	`55`	`{`
`55`		`- throw new OpenApiException("Cannot create list from this type of node.");`
	`56`	`+ throw new OpenApiReaderException("Cannot create list from this type of node.");`
`56`	`57`	`}`
`57`	`58`
`58`	`59`	`public virtual Dictionary<string, T> CreateMap<T>(Func<MapNode, T> map)`
`59`	`60`	`{`
`60`		`- throw new OpenApiException("Cannot create map from this type of node.");`
	`61`	`+ throw new OpenApiReaderException("Cannot create map from this type of node.");`
`61`	`62`	`}`
`62`	`63`
`63`	`64`	`public virtual Dictionary<string, T> CreateMapWithReference<T>(`
`64`	`65`	`ReferenceType referenceType,`
`65`	`66`	`Func<MapNode, T> map)`
`66`	`67`	`where T : class, IOpenApiReferenceable`
`67`	`68`	`{`
`68`		`- throw new OpenApiException("Cannot create map from this reference.");`
	`69`	`+ throw new OpenApiReaderException("Cannot create map from this reference.");`
`69`	`70`	`}`
`70`	`71`
`71`	`72`	`public virtual List<T> CreateSimpleList<T>(Func<ValueNode, T> map)`
`72`	`73`	`{`
`73`		`- throw new OpenApiException("Cannot create simple list from this type of node.");`
	`74`	`+ throw new OpenApiReaderException("Cannot create simple list from this type of node.");`
`74`	`75`	`}`
`75`	`76`
`76`	`77`	`public virtual Dictionary<string, T> CreateSimpleMap<T>(Func<ValueNode, T> map)`
`77`	`78`	`{`
`78`		`- throw new OpenApiException("Cannot create simple map from this type of node.");`
	`79`	`+ throw new OpenApiReaderException("Cannot create simple map from this type of node.");`
`79`	`80`	`}`
`80`	`81`
`81`	`82`	`public virtual IOpenApiAny CreateAny()`
`82`	`83`	`{`
`83`		`- throw new OpenApiException("Cannot create an Any object this type of node.");`
	`84`	`+ throw new OpenApiReaderException("Cannot create an Any object this type of node.");`
`84`	`85`	`}`
`85`	`86`
`86`	`87`	`public virtual string GetRaw()`
`87`	`88`	`{`
`88`		`- throw new OpenApiException("Cannot get raw value from this type of node.");`
	`89`	`+ throw new OpenApiReaderException("Cannot get raw value from this type of node.");`
`89`	`90`	`}`
`90`	`91`
`91`	`92`	`public virtual string GetScalarValue()`
`92`	`93`	`{`
`93`		`- throw new OpenApiException("Cannot create a scalar value from this type of node.");`
	`94`	`+ throw new OpenApiReaderException("Cannot create a scalar value from this type of node.");`
`94`	`95`	`}`
`95`	`96`
`96`	`97`	`public virtual List<IOpenApiAny> CreateListOfAny()`
`97`	`98`	`{`
`98`		`- throw new OpenApiException("Cannot create a list from this type of node.");`
	`99`	`+ throw new OpenApiReaderException("Cannot create a list from this type of node.");`
`99`	`100`	`}`
`100`	`101`
`101`	`102`	`}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,10 @@ public void ParseField<T>(`
`65`	`65`	`Context.StartObject(Name);`
`66`	`66`	`map(parentInstance, Name, Value);`
`67`	`67`	`}`
	`68`	`+ catch (OpenApiReaderException ex)`
	`69`	`+ {`
	`70`	`+ Diagnostic.Errors.Add(new OpenApiError(ex));`
	`71`	`+ }`
`68`	`72`	`catch (OpenApiException ex)`
`69`	`73`	`{`
`70`	`74`	`ex.Pointer = Context.GetLocation();`
Original file line number	Diff line number	Diff line change
`@@ -26,14 +26,7 @@ public ValueNode(ParsingContext context, OpenApiDiagnostic diagnostic, YamlNode`
`26`	`26`
`27`	`27`	`public override string GetScalarValue()`
`28`	`28`	`{`
`29`		`- var scalarNode = _node;`
`30`		`-`
`31`		`- if (scalarNode == null)`
`32`		`- {`
`33`		`- throw new OpenApiException($"Expected scalar at line {_node.Start.Line}");`
`34`		`- }`
`35`		`-`
`36`		`- return scalarNode.Value;`
	`29`	`+ return _node.Value;`
`37`	`30`	`}`
`38`	`31`
`39`	`32`	`/// <summary>`