Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Do not include compiler-generated names in expression names #3027

Merged
merged 1 commit into from
Aug 24, 2015
Merged

Do not include compiler-generated names in expression names #3027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/Expressions/ExpressionHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public static string GetExpressionText([NotNull] LambdaExpression expression)
if (part.NodeType == ExpressionType.Call)
{
var methodExpression = (MethodCallExpression)part;

if (!IsSingleArgumentIndexer(methodExpression))
{
// Unsupported.
break;
}

Expand All @@ -58,7 +58,18 @@ public static string GetExpressionText([NotNull] LambdaExpression expression)
else if (part.NodeType == ExpressionType.MemberAccess)
{
var memberExpressionPart = (MemberExpression)part;
nameParts.Push("." + memberExpressionPart.Member.Name);
var name = memberExpressionPart.Member.Name;

// If identifier contains "__", it is "reserved for use by the implementation" and likely compiler-
// or Razor-generated e.g. the name of a field in a delegate's generated class.
if (name.Contains("__"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC __ is only reserved at the start, might want to double check me. I double-checked me, and I was wrong. https://msdn.microsoft.com/en-us/library/aa664670(v=vs.71).aspx

Another way to verify this would probably to look at memberExpressionPart.Member and it's declaring class, which has [CompilerGenerated].

What we have here is probably good enough, but just in case we need to be more certain in the future.

{
// Exit loop. Should have the entire name because previous MemberAccess has same name as the
// leftmost expression node (a variable).
break;
}

nameParts.Push("." + name);
part = memberExpressionPart.Expression;
}
else if (part.NodeType == ExpressionType.Parameter)
Expand All @@ -67,15 +78,19 @@ public static string GetExpressionText([NotNull] LambdaExpression expression)
// string onto the stack and stop evaluating. The extra empty string makes sure that
// we don't accidentally cut off too much of m => m.Model.
nameParts.Push(string.Empty);
part = null;

// Exit loop. Have the entire name because the parameter cannot be used as an indexer; always the
// leftmost expression node.
break;
}
else
{
// Unsupported.
break;
}
}

// If it starts with "model", then strip that away
// If parts start with "model", then strip that part away.
if (nameParts.Count > 0 && string.Equals(nameParts.Peek(), ".model", StringComparison.OrdinalIgnoreCase))
{
nameParts.Pop();
Expand Down
124 changes: 113 additions & 11 deletions test/Microsoft.AspNet.Mvc.ViewFeatures.Test/Rendering/HtmlHelperNameExtensionsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// 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 System.Collections.Generic;
using System.Linq.Expressions;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.ModelBinding.Metadata;
using Microsoft.AspNet.Mvc.Rendering;
using Microsoft.AspNet.Testing;
using Moq;
using Xunit;

Expand All @@ -14,6 +18,26 @@ namespace Microsoft.AspNet.Mvc.Core
/// </summary>
public class HtmlHelperNameExtensionsTest
{
private static List<string> _staticCollection = new List<string>();
private static int _staticIndex = 6;

private List<string> _collection = new List<string>();
private int _index = 7;
private List<string[]> _nestedCollection = new List<string[]>();
private string _string = string.Empty;

private static List<string> StaticCollection { get; }

private static int StaticIndex { get; } = 8;

private List<string> Collection { get; }

private int Index { get; } = 9;

private List<string[]> NestedCollection { get; }

private string StringProperty { get; }

[Fact]
public void IdAndNameHelpers_ReturnEmptyForModel()
{
Expand Down Expand Up @@ -140,7 +164,7 @@ public void IdAndNameHelpers_DoNotConsultMetadataOrMetadataProvider()
// Arrange
var provider = new Mock<IModelMetadataProvider>(MockBehavior.Strict);
var metadata = new Mock<ModelMetadata>(
MockBehavior.Loose,
MockBehavior.Loose,
ModelMetadataIdentity.ForType(typeof(DefaultTemplatesUtilities.ObjectTemplateModel)));
provider
.Setup(m => m.GetMetadataForType(typeof(DefaultTemplatesUtilities.ObjectTemplateModel)))
Expand Down Expand Up @@ -223,28 +247,106 @@ public void IdForAndNameFor_ReturnEmpty_IfExpressionUnsupported()
Assert.Empty(nameResult);
}

[Fact]
public void IdForAndNameFor_ReturnVariableName()
// expression, expected name, expected id
public static TheoryData StaticExpressionNamesData
{
get
{
// Give expressions access to non-static fields and properties.
var test = new HtmlHelperNameExtensionsTest();
return test.ExpressionNamesData;
}
}

// expression, expected name, expected id
private TheoryData ExpressionNamesData
{
get
{
var collection = new List<string>();
var nestedCollection = new List<string[]>();
var index1 = 5;
var index2 = 23;
var unknownKey = "this is a dummy parameter value";

return new TheoryData<Expression<Func<List<OuterClass>, string>>, string, string>
{
{ m => unknownKey, "unknownKey", "unknownKey" },
{ m => collection[index1], "collection[5]", "collection_5_" },
{ m => nestedCollection[index1][23], "nestedCollection[5][23]", "nestedCollection_5__23_" },
{ m => nestedCollection[index1][index2], "nestedCollection[5][23]", "nestedCollection_5__23_" },
{ m => nestedCollection[_index][Index], "nestedCollection[7][9]", "nestedCollection_7__9_" },
{ m => nestedCollection[Index][StaticIndex], "nestedCollection[9][8]", "nestedCollection_9__8_" },
{ m => _string, "_string", "zstring" },
{ m => _collection[_index], "_collection[7]", "zcollection_7_" },
{ m => _nestedCollection[_index][23], "_nestedCollection[7][23]", "znestedCollection_7__23_" },
{ m => _nestedCollection[_index][index2], "_nestedCollection[7][23]", "znestedCollection_7__23_" },
{ m => _nestedCollection[Index][_staticIndex], "_nestedCollection[9][6]", "znestedCollection_9__6_" },
{ m => _nestedCollection[StaticIndex][_index], "_nestedCollection[8][7]", "znestedCollection_8__7_" },
{ m => StringProperty, "StringProperty", "StringProperty" },
{ m => Collection[Index], "Collection[9]", "Collection_9_" },
{ m => NestedCollection[Index][23], "NestedCollection[9][23]", "NestedCollection_9__23_" },
{ m => NestedCollection[Index][index2], "NestedCollection[9][23]", "NestedCollection_9__23_" },
{ m => NestedCollection[_index][Index], "NestedCollection[7][9]", "NestedCollection_7__9_" },
{ m => NestedCollection[Index][StaticIndex], "NestedCollection[9][8]", "NestedCollection_9__8_" },
{ m => _staticCollection[_staticIndex], "_staticCollection[6]", "zstaticCollection_6_" },
{ m => _staticCollection[Index], "_staticCollection[9]", "zstaticCollection_9_" },
{ m => _staticCollection[_index], "_staticCollection[7]", "zstaticCollection_7_" },
{ m => StaticCollection[StaticIndex], "StaticCollection[8]", "StaticCollection_8_" },
{ m => StaticCollection[_staticIndex], "StaticCollection[6]", "StaticCollection_6_" },
{ m => StaticCollection[index1], "StaticCollection[5]", "StaticCollection_5_" },
{ m => m[index1].Inner.Name, "[5].Inner.Name", "z5__Inner_Name" },
{ m => m[_staticIndex].Inner.Name, "[6].Inner.Name", "z6__Inner_Name" },
{ m => m[_index].Inner.Name, "[7].Inner.Name", "z7__Inner_Name" },
{ m => m[StaticIndex].Inner.Name, "[8].Inner.Name", "z8__Inner_Name" },
{ m => m[Index].Inner.Name, "[9].Inner.Name", "z9__Inner_Name" },
};
}
}

[Theory]
[MemberData(nameof(StaticExpressionNamesData))]
public void IdForAndNameFor_ReturnExpectedValues_WithVariablesInExpression(
Expression<Func<List<OuterClass>, string>> expression,
string expectedName,
string expectedId)
{
// Arrange
var unknownKey = "this is a dummy parameter value";
var helper = DefaultTemplatesUtilities.GetHtmlHelper();
var model = new List<OuterClass>();
var helper = DefaultTemplatesUtilities.GetHtmlHelper(model);

// Act
var idResult = helper.IdFor(model => unknownKey);
var nameResult = helper.NameFor(model => unknownKey);
var idResult = helper.IdFor(expression);
var nameResult = helper.NameFor(expression);

// Assert
Assert.Equal("unknownKey", idResult);
Assert.Equal("unknownKey", nameResult);
Assert.Equal(expectedId, idResult);
Assert.Equal(expectedName, nameResult);
}

private sealed class InnerClass
[Fact]
public void IdForAndNameFor_Throws_WhenParameterUsedAsIndexer()
{
// Arrange
var collection = new List<string>();
var index = 24;
var helper = DefaultTemplatesUtilities.GetHtmlHelper(index);
var message = "The expression compiler was unable to evaluate the indexer expression 'm' because it " +
"references the model parameter 'm' which is unavailable.";

// Act & Assert
ExceptionAssert.Throws<InvalidOperationException>(() => helper.IdFor(m => collection[m]), message);
ExceptionAssert.Throws<InvalidOperationException>(() => helper.NameFor(m => collection[m]), message);
}

public sealed class InnerClass
{
public int Id { get; set; }

public string Name { get; set; }
}

private sealed class OuterClass
public sealed class OuterClass
{
public InnerClass Inner { get; set; }
}
Expand Down