Add and modify tests to validate TagHelperDescriptorResolver doesn't throw.

- Modified existing tests that expected the resolvers to throw to no longer throw.
- Added new test to validate that unexpected errors that are thrown are also handled.

#210

src/Microsoft.AspNet.Razor/Parser/TagHelpers/AddOrRemoveTagHelperSpanVisitor.cs

@@ -19,6 +19,12 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers
 
         private List<TagHelperDirectiveDescriptor> _directiveDescriptors;
 
+        // Internal for testing use
+        internal AddOrRemoveTagHelperSpanVisitor(ITagHelperDescriptorResolver descriptorResolver)
+            : this(descriptorResolver, new ParserErrorSink())
+        {
+        }
+
         public AddOrRemoveTagHelperSpanVisitor([NotNull] ITagHelperDescriptorResolver descriptorResolver,
                                                [NotNull] ParserErrorSink errorSink)
         {

src/Microsoft.AspNet.Razor/TagHelpers/TagHelperDescriptorResolutionContext.cs

@@ -11,6 +11,12 @@ namespace Microsoft.AspNet.Razor.TagHelpers
     /// </summary>
     public class TagHelperDescriptorResolutionContext
     {
+        // Internal for testing purposes
+        internal TagHelperDescriptorResolutionContext(IEnumerable<TagHelperDirectiveDescriptor> directiveDescriptors)
+            : this(directiveDescriptors, new ParserErrorSink())
+        {
+        }
+
         /// <summary>
         /// Instantiates a new instance of <see cref="TagHelperDescriptorResolutionContext"/>.
         /// </summary>

src/Microsoft.AspNet.Razor/TagHelpers/TagHelperDirectiveDescriptor.cs

@@ -10,6 +10,13 @@ namespace Microsoft.AspNet.Razor.TagHelpers
     /// </summary>
     public class TagHelperDirectiveDescriptor
     {
+        // Internal for testing purposes.
+        internal TagHelperDirectiveDescriptor(string lookupText,
+                                              TagHelperDirectiveType directiveType)
+            : this(lookupText, SourceLocation.Zero, directiveType)
+        {
+        }
+
         /// <summary>
         /// Instantiates a new instance of <see cref="TagHelperDirectiveDescriptor"/>.
         /// </summary>

test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperDescriptorResolverTest.cs

@@ -5,7 +5,9 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
+using Microsoft.AspNet.Razor.Parser;
 using Microsoft.AspNet.Razor.TagHelpers;
+using Microsoft.AspNet.Razor.Text;
 using Xunit;
 
 namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
@@ -49,7 +51,8 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             // Arrange
             var tagHelperDescriptorResolver = new AssemblyCheckingTagHelperDescriptorResolver();
             var context = new TagHelperDescriptorResolutionContext(
-                new[] { new TagHelperDirectiveDescriptor(lookupText, TagHelperDirectiveType.AddTagHelper) });
+                new[] { new TagHelperDirectiveDescriptor(lookupText, TagHelperDirectiveType.AddTagHelper) },
+                new ParserErrorSink());
 
             // Act
             tagHelperDescriptorResolver.Resolve(context);
@@ -200,7 +203,9 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             var tagHelperDescriptorResolver =
                 new TestTagHelperDescriptorResolver(
                     new LookupBasedTagHelperTypeResolver(descriptorAssemblyLookups));
-            var resolutionContext = new TagHelperDescriptorResolutionContext(directiveDescriptors);
+            var resolutionContext = new TagHelperDescriptorResolutionContext(
+                directiveDescriptors, 
+                new ParserErrorSink());
 
             // Act
             var descriptors = tagHelperDescriptorResolver.Resolve(resolutionContext);
@@ -305,7 +310,9 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             var tagHelperDescriptorResolver =
                 new TestTagHelperDescriptorResolver(
                     new LookupBasedTagHelperTypeResolver(descriptorAssemblyLookups));
-            var resolutionContext = new TagHelperDescriptorResolutionContext(directiveDescriptors);
+            var resolutionContext = new TagHelperDescriptorResolutionContext(
+                directiveDescriptors,
+                new ParserErrorSink());
 
             // Act
             var descriptors = tagHelperDescriptorResolver.Resolve(resolutionContext);
@@ -437,26 +444,56 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
         [Theory]
         [InlineData("")]
         [InlineData(null)]
-        public void DescriptorResolver_ResolveThrowsIfNullOrEmptyLookupText(string lookupText)
+        public void DescriptorResolver_CreatesErrorIfNullOrEmptyLookupText_DoesNotThrow(string lookupText)
         {
             // Arrange
+            var errorSink = new ParserErrorSink();
             var tagHelperDescriptorResolver =
                 new TestTagHelperDescriptorResolver(
                     new TestTagHelperTypeResolver(InvalidTestableTagHelpers));
+            var documentLocation = new SourceLocation(1, 2, 3);
+            var directiveType = TagHelperDirectiveType.AddTagHelper;
+            var expectedErrorMessage =
+                Resources.FormatTagHelperDescriptorResolver_InvalidTagHelperLookupText(lookupText);
+            var resolutionContext = new TagHelperDescriptorResolutionContext(
+                new [] { new TagHelperDirectiveDescriptor(lookupText, documentLocation, directiveType)},
+                errorSink);
 
-            var expectedMessage =
+            // Act
-                Resources.FormatTagHelperDescriptorResolver_InvalidTagHelperLookupText(lookupText) +
+            tagHelperDescriptorResolver.Resolve(resolutionContext);
-                Environment.NewLine +
-                "Parameter name: lookupText";
 
-            // Act & Assert
+            // Assert
-            var ex = Assert.Throws<ArgumentException>(nameof(lookupText),
+            var error = Assert.Single(errorSink.Errors);
-            () =>
+            Assert.Equal(1, error.Length);
-            {
+            Assert.Equal(documentLocation, error.Location);
-                tagHelperDescriptorResolver.Resolve(lookupText);
+            Assert.Equal(expectedErrorMessage, error.Message);
-            });
+        }
 
-            Assert.Equal(expectedMessage, ex.Message);
+        [Fact]
+        public void DescriptorResolver_UnderstandsUnexpectedExceptions_DoesNotThrow()
+        {
+            // Arrange
+            var expectedErrorMessage = "Encountered an unexpected error when attempting to resolve tag helper " +
+                                       "directive '@addtaghelper' with value 'A custom lookup text'. Error: A " +
+                                       "custom exception";
+            var documentLocation = new SourceLocation(1, 2, 3);
+            var directiveType = TagHelperDirectiveType.AddTagHelper;
+            var errorSink = new ParserErrorSink();
+            var expectedError = new Exception("A custom exception");
+            var tagHelperDescriptorResolver = new ThrowingTagHelperDescriptorResolver(expectedError);
+            var resolutionContext = new TagHelperDescriptorResolutionContext(
+                new[] { new TagHelperDirectiveDescriptor("A custom lookup text", documentLocation, directiveType) },
+                errorSink);
+
+
+            // Act
+            tagHelperDescriptorResolver.Resolve(resolutionContext);
+
+            // Assert
+            var error = Assert.Single(errorSink.Errors);
+            Assert.Equal(1, error.Length);
+            Assert.Equal(documentLocation, error.Location);
+            Assert.Equal(expectedErrorMessage, error.Message);
         }
 
         private class TestTagHelperDescriptorResolver : TagHelperDescriptorResolver
@@ -472,7 +509,8 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
                     new TagHelperDescriptorResolutionContext(
                         lookupTexts.Select(
                             lookupText =>
-                                new TagHelperDirectiveDescriptor(lookupText, TagHelperDirectiveType.AddTagHelper))));
+                                new TagHelperDirectiveDescriptor(lookupText, TagHelperDirectiveType.AddTagHelper)),
+                    new ParserErrorSink()));
             }
         }
 
@@ -504,12 +542,33 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
         {
             public string CalledWithAssemblyName { get; set; }
 
-            protected override IEnumerable<TagHelperDescriptor> ResolveDescriptorsInAssembly(string assemblyName)
+            protected override IEnumerable<TagHelperDescriptor> ResolveDescriptorsInAssembly(
+                string assemblyName,
+                SourceLocation documentLocation,
+                ParserErrorSink errorSink)
             {
                 CalledWithAssemblyName = assemblyName;
 
                 return Enumerable.Empty<TagHelperDescriptor>();
             }
         }
+
+        private class ThrowingTagHelperDescriptorResolver : TagHelperDescriptorResolver
+        {
+            private readonly Exception _error;
+
+            public ThrowingTagHelperDescriptorResolver(Exception error)
+            {
+                _error = error;
+            }
+
+            protected override IEnumerable<TagHelperDescriptor> ResolveDescriptorsInAssembly(
+                string assemblyName, 
+                SourceLocation documentLocation, 
+                ParserErrorSink errorSink)
+            {
+                throw _error;
+            }
+        }
     }
 }

test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperTypeResolverTest.cs

@@ -3,11 +3,11 @@
 
 using System;
 using System.Collections.Generic;
-using System.Globalization;
-using System.IO;
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Microsoft.AspNet.Razor.Parser;
+using Microsoft.AspNet.Razor.Text;
 using Xunit;
 
 namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
@@ -35,27 +35,29 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             ValidTestableTagHelpers.Concat(InvalidTestableTagHelpers).ToArray();
 
         [Fact]
-        public void TypeResolver_ThrowsWhenCannotResolveAssembly()
+        public void TypeResolver_RecordsErrorWhenCannotResolveAssembly()
         {
             // Arrange
             var tagHelperTypeResolver = new TagHelperTypeResolver();
-            var expectedErrorMessage = string.Format(
+            var errorSink = new ParserErrorSink();
-                CultureInfo.InvariantCulture,
+            var documentLocation = new SourceLocation(1, 2, 3);
-                "Cannot resolve TagHelper containing assembly '{0}'.",
+            var expectedErrorMessage = "Cannot resolve TagHelper containing assembly 'abcd'. Error: " +
-                "abcd");
+                "Could not load file or assembly '" +
-
+#if ASPNET50
-            // Act & Assert
+                "abcd' or one of its dependencies. The system cannot find the file specified.";
-            var ex = Assert.Throws<InvalidOperationException>(() =>
-            {
-                tagHelperTypeResolver.Resolve("abcd");
-            });
-
-            Assert.Equal(expectedErrorMessage, ex.Message);
-#if ASPNETCORE50
-            Assert.IsType<FileLoadException>(ex.InnerException);
 #else
-            Assert.IsType<FileNotFoundException>(ex.InnerException);
+                "abcd, Culture=neutral, PublicKeyToken=null' or one of its dependencies. Could not find or load a " +
+                "specific file. (Exception from HRESULT: 0x80131621)";
 #endif
+
+            // Act
+            tagHelperTypeResolver.Resolve("abcd", documentLocation, errorSink);
+
+            // Assert
+            var error = Assert.Single(errorSink.Errors);
+            Assert.Equal(1, error.Length);
+            Assert.Equal(documentLocation, error.Location);
+            Assert.Equal(expectedErrorMessage, error.Message);
         }
 
         [Fact]
@@ -65,7 +67,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             var tagHelperTypeResolver = new TestTagHelperTypeResolver(TestableTagHelpers);
 
             // Act
-            var types = tagHelperTypeResolver.Resolve("Foo");
+            var types = tagHelperTypeResolver.Resolve("Foo", SourceLocation.Zero, new ParserErrorSink());
 
             // Assert
             Assert.Equal(ValidTestableTagHelpers, types);
@@ -78,7 +80,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
             var tagHelperTypeResolver = new TestTagHelperTypeResolver(InvalidTestableTagHelpers);
 
             // Act
-            var types = tagHelperTypeResolver.Resolve("Foo");
+            var types = tagHelperTypeResolver.Resolve("Foo", SourceLocation.Zero, new ParserErrorSink());
 
             // Assert
             Assert.Empty(types);
@@ -87,22 +89,22 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
         [Theory]
         [InlineData("")]
         [InlineData(null)]
-        public void TypeResolver_ResolveThrowsIfEmptyOrNullLookupText(string name)
+        public void TypeResolver_CreatesErrorIfNullOrEmptyAssmblyName_DoesNotThrow(string name)
         {
             // Arrange
             var tagHelperTypeResolver = new TestTagHelperTypeResolver(InvalidTestableTagHelpers);
-            var expectedMessage = "Tag helper directive assembly name cannot be null or empty." +
+            var errorSink = new ParserErrorSink();
-                Environment.NewLine +
+            var documentLocation = new SourceLocation(1, 2, 3);
-                "Parameter name: name";
+            var expectedErrorMessage = "Tag helper directive assembly name cannot be null or empty.";
 
-            // Act & Assert
+            // Act
-            var ex = Assert.Throws<ArgumentException>(nameof(name),
+            tagHelperTypeResolver.Resolve(name, documentLocation, errorSink);
-            () =>
-            {
-                tagHelperTypeResolver.Resolve(name);
-            });
 
-            Assert.Equal(expectedMessage, ex.Message);
+            // Assert
+            var error = Assert.Single(errorSink.Errors);
+            Assert.Equal(1, error.Length);
+            Assert.Equal(documentLocation, error.Location);
+            Assert.Equal(expectedErrorMessage, error.Message);
         }
 
         protected class TestTagHelperTypeResolver : TagHelperTypeResolver

test/Microsoft.AspNet.Razor.Test/Parser/RazorParserTest.cs

@@ -77,7 +77,9 @@ namespace Microsoft.AspNet.Razor.Test.Parser
                                                Mock.Of<ITagHelperDescriptorResolver>());
             parser.CallBase = true;
             parser.Protected()
-                  .Setup<IEnumerable<TagHelperDescriptor>>("GetTagHelperDescriptors", ItExpr.IsAny<Block>())
+                  .Setup<IEnumerable<TagHelperDescriptor>>("GetTagHelperDescriptors", 
+                                                           ItExpr.IsAny<Block>(), 
+                                                           ItExpr.IsAny<ParserErrorSink>())
                   .Returns(Enumerable.Empty<TagHelperDescriptor>())
                   .Verifiable();
 

test/Microsoft.AspNet.Razor.Test/TagHelpers/AddOrRemoveTagHelperSpanVisitorTest.cs

@@ -28,7 +28,9 @@ namespace Microsoft.AspNet.Razor.TagHelpers
             var resolver = new Mock<ITagHelperDescriptorResolver>();
             resolver.Setup(mock => mock.Resolve(It.IsAny<TagHelperDescriptorResolutionContext>()))
                     .Returns(Enumerable.Empty<TagHelperDescriptor>());
-            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver.Object);
+            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(
+                resolver.Object, 
+                new ParserErrorSink());
             var document = new MarkupBlock(
                 Factory.Code("\"one\"").AsAddTagHelper("one"),
                 Factory.Code("\"two\"").AsRemoveTagHelper("two"),
@@ -47,7 +49,7 @@ namespace Microsoft.AspNet.Razor.TagHelpers
         {
             // Arrange
             var resolver = new TestTagHelperDescriptorResolver();
-            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver);
+            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver, new ParserErrorSink());
             var document = new MarkupBlock(
                 Factory.Code("\"one\"").AsAddTagHelper("one"),
                 Factory.Code("\"two\"").AsRemoveTagHelper("two"),
@@ -85,13 +87,13 @@ namespace Microsoft.AspNet.Razor.TagHelpers
             };
             var addOrRemoveTagHelperSpanVisitor = new CustomAddOrRemoveTagHelperSpanVisitor(
                 resolver,
-                (descriptors) =>
+                (descriptors, errorSink) =>
                 {
                     Assert.Equal(expectedInitialDirectiveDescriptors,
                                  descriptors,
                                  TagHelperDirectiveDescriptorComparer.Default);
 
-                    return new TagHelperDescriptorResolutionContext(expectedEndDirectiveDescriptors);
+                    return new TagHelperDescriptorResolutionContext(expectedEndDirectiveDescriptors, errorSink);
                 });
             var document = new MarkupBlock(
                 Factory.Code("\"one\"").AsAddTagHelper("one"),
@@ -113,7 +115,7 @@ namespace Microsoft.AspNet.Razor.TagHelpers
         {
             // Arrange
             var resolver = new TestTagHelperDescriptorResolver();
-            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver);
+            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver, new ParserErrorSink());
             var document = new MarkupBlock(
                 new DirectiveBlock(
                     Factory.CodeTransition(),
@@ -137,7 +139,7 @@ namespace Microsoft.AspNet.Razor.TagHelpers
         {
             // Arrange
             var resolver = new TestTagHelperDescriptorResolver();
-            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver);
+            var addOrRemoveTagHelperSpanVisitor = new AddOrRemoveTagHelperSpanVisitor(resolver, new ParserErrorSink());
             var document = new MarkupBlock(
                 new DirectiveBlock(
                     Factory.CodeTransition(),
@@ -162,7 +164,8 @@ namespace Microsoft.AspNet.Razor.TagHelpers
             // Arrange
             var addOrRemoveTagHelperSpanVisitor =
                 new AddOrRemoveTagHelperSpanVisitor(
-                    new TestTagHelperDescriptorResolver());
+                    new TestTagHelperDescriptorResolver(),
+                    new ParserErrorSink());
             var document = new MarkupBlock(Factory.Markup("Hello World"));
 
             // Act & Assert
@@ -216,20 +219,25 @@ namespace Microsoft.AspNet.Razor.TagHelpers
 
         private class CustomAddOrRemoveTagHelperSpanVisitor : AddOrRemoveTagHelperSpanVisitor
         {
-            private Func<IEnumerable<TagHelperDirectiveDescriptor>, TagHelperDescriptorResolutionContext> _replacer;
+            private Func<IEnumerable<TagHelperDirectiveDescriptor>, 
+                         ParserErrorSink,
+                         TagHelperDescriptorResolutionContext> _replacer;
 
             public CustomAddOrRemoveTagHelperSpanVisitor(
                 ITagHelperDescriptorResolver descriptorResolver,
-                Func<IEnumerable<TagHelperDirectiveDescriptor>, TagHelperDescriptorResolutionContext> replacer)
+                Func<IEnumerable<TagHelperDirectiveDescriptor>, 
-                : base(descriptorResolver)
+                     ParserErrorSink, 
+                     TagHelperDescriptorResolutionContext> replacer)
+                : base(descriptorResolver, new ParserErrorSink())
             {
                 _replacer = replacer;
             }
 
             protected override TagHelperDescriptorResolutionContext GetTagHelperDescriptorResolutionContext(
-                IEnumerable<TagHelperDirectiveDescriptor> descriptors)
+                IEnumerable<TagHelperDirectiveDescriptor> descriptors,
+                ParserErrorSink errorSink)
             {
-                return _replacer(descriptors);
+                return _replacer(descriptors, errorSink);
             }
         }
     }