Check for properties that can't befound

If you give ModelExpressionProvider a lambda with a private property
you'll end up here. This wasn't common before, but it seems like users
are more likely to try it with pages.

Model Metadata and Model Binding don't handle private properties, so
supporting it in Model Expressions seems less than useful.

This isn't a breaking change because this case would have resulted in a
null-ref. Addresses #6400

Author: rynowak

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Diagnostics;
 using System.Globalization;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -60,7 +61,12 @@ public static ModelExplorer FromLambdaExpression<TModel, TResult>(
                         return FromModel(viewData, metadataProvider);
                     }
 
-                    containerType = memberExpression.Expression.Type;
+                    // memberExpression.Expression can be null when this is a static field or property.
+                    //
+                    // This can be the case if the expression is like (m => Person.Name) where Name is a static field
+                    // or property on the Person type.
+                    containerType = memberExpression.Expression?.Type;
+
                     legalExpression = true;
                     break;
 
@@ -86,23 +92,31 @@ public static ModelExplorer FromLambdaExpression<TModel, TResult>(
                 }
             };
 
-            ModelMetadata metadata;
-            if (propertyName == null)
-            {
-                // Ex:
-                //    m => 5 (arbitrary expression)
-                //    m => foo (arbitrary expression)
-                //    m => m.Widgets[0] (expression ending with non-property-access)
-                metadata = metadataProvider.GetMetadataForType(typeof(TResult));
-            }
-            else
+            ModelMetadata metadata = null;
+            if (containerType != null && propertyName != null)
             {
                 // Ex:
                 //    m => m.Color (simple property access)
                 //    m => m.Color.Red (nested property access)
                 //    m => m.Widgets[0].Size (expression ending with property-access)
                 metadata = metadataProvider.GetMetadataForType(containerType).Properties[propertyName];
             }
+            
+            if (metadata == null)
+            {
+                // Ex:
+                //    m => 5 (arbitrary expression)
+                //    m => foo (arbitrary expression)
+                //    m => m.Widgets[0] (expression ending with non-property-access)
+                //
+                // This can also happen for any case where we cannot retrieve a model metadata.
+                // This will happen for:
+                // - fields
+                // - statics
+                // - non-visibility (internal/private)
+                metadata = metadataProvider.GetMetadataForType(typeof(TResult));
+                Debug.Assert(metadata != null);
+            }
 
             return viewData.ModelExplorer.GetExplorerForExpression(metadata, modelAccessor);
         }

test/Microsoft.AspNetCore.Mvc.FunctionalTests/HtmlGenerationTest.cs

@@ -523,6 +523,32 @@ public async Task EditorTemplateWithSpecificModel_RendersWithCorrectMetadata()
             Assert.Equal(expected, response, ignoreLineEndingDifferences: true);
         }
 
+        // We want to make sure that for 'wierd' model expressions involving:
+        // - fields
+        // - statics
+        // - private
+        //
+        // These tests verify that we don't throw, and can evaluate the expression to get the model
+        // value. One quirk of behavior for these cases is that we can't return a correct model metadata
+        // instance (this is true for anything other than a public instance property). We're not overly
+        // concerned with that, and so the accuracy of the model metadata is is not verified by the test.
+        [Theory]
+        [InlineData("GetWeirdWithHtmlHelpers")]
+        [InlineData("GetWeirdWithTagHelpers")]
+        public async Task WeirdModelExpressions_CanAccessModelValues(string action)
+        {
+            // Arrange
+            var url = "http://localhost/HtmlGeneration_WeirdExpressions/" + action;
+
+            // Act
+            var response = await Client.GetStringAsync(url);
+
+            // Assert
+            Assert.Contains("Hello, Field World!", response);
+            Assert.Contains("Hello, Static World!", response);
+            Assert.Contains("Hello, Private World!", response);
+        }
+
         private static HttpRequestMessage RequestWithLocale(string url, string locale)
         {
             var request = new HttpRequestMessage(HttpMethod.Get, url);

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionMetadataProviderTest.cs

@@ -10,6 +10,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 {
     public class ExpressionMetadataProviderTest
     {
+        private string PrivateProperty { get; set; }
+
+        public static string StaticProperty { get; set; }
+
+        public string Field = "Hello";
+
         [Fact]
         public void FromLambdaExpression_GetsExpectedMetadata_ForIdentityExpression()
         {
@@ -148,6 +154,72 @@ public void FromStringExpression_SetsContainerAsExpected()
             Assert.Same(myModel, metadata.Container.Model);
         }
 
+        // A private property can't be found by the model metadata provider, so return the property's type
+        // as a best-effort mechanism.
+        [Fact]
+        public void FromLambdaExpression_ForPrivateProperty_ReturnsMetadataOfExpressionType()
+        {
+            // Arrange
+            var provider = new EmptyModelMetadataProvider();
+            var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
+
+            // Act
+            var explorer = ExpressionMetadataProvider.FromLambdaExpression(
+                m => m.PrivateProperty,
+                viewData,
+                provider);
+
+            // Assert
+            Assert.NotNull(explorer);
+            Assert.NotNull(explorer.Metadata);
+            Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
+            Assert.Equal(typeof(string), explorer.ModelType);
+        }
+
+        // A static property can't be found by the model metadata provider, so return the property's type
+        // as a best-effort mechanism.
+        [Fact]
+        public void FromLambdaExpression_ForStaticProperty_ReturnsMetadataOfExpressionType()
+        {
+            // Arrange
+            var provider = new EmptyModelMetadataProvider();
+            var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
+
+            // Act
+            var explorer = ExpressionMetadataProvider.FromLambdaExpression(
+                m => ExpressionMetadataProviderTest.StaticProperty,
+                viewData,
+                provider);
+
+            // Assert
+            Assert.NotNull(explorer);
+            Assert.NotNull(explorer.Metadata);
+            Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
+            Assert.Equal(typeof(string), explorer.ModelType);
+        }
+
+        // A field can't be found by the model metadata provider, so return the field's type
+        // as a best-effort mechanism.
+        [Fact]
+        public void FromLambdaExpression_ForField_ReturnsMetadataOfExpressionType()
+        {
+            // Arrange
+            var provider = new EmptyModelMetadataProvider();
+            var viewData = new ViewDataDictionary<ExpressionMetadataProviderTest>(provider);
+
+            // Act
+            var explorer = ExpressionMetadataProvider.FromLambdaExpression(
+                m => m.Field,
+                viewData,
+                provider);
+
+            // Assert
+            Assert.NotNull(explorer);
+            Assert.NotNull(explorer.Metadata);
+            Assert.Equal(ModelMetadataKind.Type, explorer.Metadata.MetadataKind);
+            Assert.Equal(typeof(string), explorer.ModelType);
+        }
+
         private class TestModel
         {
             public Category SelectedCategory { get; set; }

test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_WeirdExpressions.cs

@@ -0,0 +1,22 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using HtmlGenerationWebSite.Models;
+using Microsoft.AspNetCore.Mvc;
+
+namespace HtmlGenerationWebSite.Controllers
+{
+    public class HtmlGeneration_WeirdExpressionsController : Controller
+    {
+        public IActionResult GetWeirdWithHtmlHelpers()
+        {
+            return View(new WeirdModel());
+        }
+
+        public IActionResult GetWeirdWithTagHelpers()
+        {
+            return View(new WeirdModel());
+        }
+    }
+}

test/WebSites/HtmlGenerationWebSite/Models/WeirdModel.cs

@@ -0,0 +1,14 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.ComponentModel.DataAnnotations;
+
+namespace HtmlGenerationWebSite.Models
+{
+    public class WeirdModel
+    {
+        public string Field = "Hello, Field World!";
+
+        public static string StaticProperty { get; set; } = "Hello, Static World!";
+    }
+}

test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithHtmlHelpers.cshtml

@@ -0,0 +1,27 @@
+@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
+@using HtmlGenerationWebSite.Models
+@model WeirdModel
+
+@functions{
+    private string PrivateProperty { get; set; } = "Hello, Private World!";
+}
+
+<html>
+<body>
+    <div>
+        <p>Field</p>
+        @Html.LabelFor(m => m.Field)
+        @Html.TextBoxFor(m => m.Field)
+    </div>
+    <div>
+        <p>Static Property</p>
+        @Html.LabelFor(m => WeirdModel.StaticProperty)
+        @Html.TextBoxFor(m => WeirdModel.StaticProperty)
+    </div>
+    <div>
+        <p>Private Property</p>
+        @Html.LabelFor(m => PrivateProperty)
+        @Html.TextBoxFor(m => PrivateProperty)
+    </div>
+</body>
+</html>

test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_WeirdExpressions/GetWeirdWithTagHelpers.cshtml

@@ -0,0 +1,27 @@
+@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
+@using HtmlGenerationWebSite.Models
+@model WeirdModel
+
+@functions{
+    private string PrivateProperty { get; set; } = "Hello, Private World!";
+}
+
+<html>
+<body>
+    <div>
+        <p>Field</p>
+        <label asp-for="Field" />
+        <input asp-for="Field" />
+    </div>
+    <div>
+        <p>Static Property</p>
+        <label asp-for="@WeirdModel.StaticProperty" />
+        <input asp-for="@WeirdModel.StaticProperty" />
+    </div>
+    <div>
+        <p>Private Property</p>
+        <label asp-for="@PrivateProperty" />
+        <input asp-for="@PrivateProperty" />
+    </div>
+</body>
+</html>