diff --git a/src/Markup/Perspex.Markup/Data/IndexerNode.cs b/src/Markup/Perspex.Markup/Data/IndexerNode.cs index 0364689735..712adeebcf 100644 --- a/src/Markup/Perspex.Markup/Data/IndexerNode.cs +++ b/src/Markup/Perspex.Markup/Data/IndexerNode.cs @@ -15,21 +15,12 @@ namespace Perspex.Markup.Data { internal class IndexerNode : ExpressionNode { - private readonly int[] _intArgs; - - public IndexerNode(IList arguments) + public IndexerNode(IList arguments) { Arguments = arguments; - - var intArgs = Arguments.OfType().ToArray(); - - if (intArgs.Length == arguments.Count) - { - _intArgs = intArgs; - } } - public IList Arguments { get; } + public IList Arguments { get; } protected override void SubscribeAndUpdate(object target) { @@ -75,29 +66,41 @@ namespace Perspex.Markup.Data private void CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { - bool update = false; - - switch (e.Action) + var update = false; + if (sender is IList) + { + object indexObject; + if (!TypeUtilities.TryConvert(typeof(int), Arguments[0], CultureInfo.InvariantCulture, out indexObject)) + { + return; + } + var index = (int)indexObject; + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + update = index >= e.NewStartingIndex; + break; + case NotifyCollectionChangedAction.Remove: + update = index >= e.OldStartingIndex; + break; + case NotifyCollectionChangedAction.Replace: + update = index >= e.NewStartingIndex && + index < e.NewStartingIndex + e.NewItems.Count; + break; + case NotifyCollectionChangedAction.Move: + update = (index >= e.NewStartingIndex && + index < e.NewStartingIndex + e.NewItems.Count) || + (index >= e.OldStartingIndex && + index < e.OldStartingIndex + e.OldItems.Count); + break; + case NotifyCollectionChangedAction.Reset: + update = true; + break; + } + } + else { - case NotifyCollectionChangedAction.Add: - update = _intArgs[0] >= e.NewStartingIndex; - break; - case NotifyCollectionChangedAction.Remove: - update = _intArgs[0] >= e.OldStartingIndex; - break; - case NotifyCollectionChangedAction.Replace: - update = _intArgs[0] >= e.NewStartingIndex && - _intArgs[0] < e.NewStartingIndex + e.NewItems.Count; - break; - case NotifyCollectionChangedAction.Move: - update = (_intArgs[0] >= e.NewStartingIndex && - _intArgs[0] < e.NewStartingIndex + e.NewItems.Count) || - (_intArgs[0] >= e.OldStartingIndex && - _intArgs[0] < e.OldStartingIndex + e.OldItems.Count); - break; - case NotifyCollectionChangedAction.Reset: - update = true; - break; + update = true; } if (update) @@ -110,72 +113,130 @@ namespace Perspex.Markup.Data { var typeInfo = target.GetType().GetTypeInfo(); var list = target as IList; - - if (typeInfo.IsArray && _intArgs != null) + var dictionary = target as IDictionary; + //TODO: Implement array as special case. It doesn't have an indexer property. + var indexerProperty = GetIndexer(typeInfo); + var indexerParameters = indexerProperty?.GetIndexParameters(); + if (indexerProperty != null && indexerParameters.Length == Arguments.Count) { - var array = (Array)target; - - if (InBounds(_intArgs, array)) + var convertedObjectArray = new object[indexerParameters.Length]; + for (int i = 0; i < Arguments.Count; i++) { - return array.GetValue(_intArgs); + object temp = null; + if (!TypeUtilities.TryConvert(indexerParameters[i].ParameterType, Arguments[i], CultureInfo.InvariantCulture, out temp)) + { + return PerspexProperty.UnsetValue; + } + convertedObjectArray[i] = temp; } - } - else if (target is IList && _intArgs?.Length == 1) - { - if (_intArgs[0] < list.Count) + var intArgs = convertedObjectArray.OfType().ToArray(); + + // Try special cases where we can validate indicies + if (typeInfo.IsArray) { - return list[_intArgs[0]]; + return GetValueFromArray((Array)target, intArgs); } - } - else - { - var indexerProperty = GetIndexer(typeInfo); - var indexerParameters = indexerProperty?.GetIndexParameters(); - if (indexerProperty != null && indexerParameters.Length == Arguments.Count) + else if (Arguments.Count == 1) { - var convertedObjectArray = new object[indexerParameters.Length]; - for (int i = 0; i < Arguments.Count; i++) + if (list != null) + { + if (intArgs.Length == Arguments.Count && intArgs[0] >= 0 && intArgs[0] < list.Count) + { + return list[intArgs[0]]; + } + return PerspexProperty.UnsetValue; + } + else if (dictionary != null) { - object temp = null; - if (!TypeUtilities.TryConvert(indexerParameters[i].ParameterType, Arguments[i], CultureInfo.InvariantCulture, out temp)) + if (dictionary.Contains(convertedObjectArray[0])) { - return PerspexProperty.UnsetValue; + return dictionary[convertedObjectArray[0]]; } - convertedObjectArray[i] = temp; + return PerspexProperty.UnsetValue; } - return indexerProperty.GetValue(target, convertedObjectArray); + else + { + // Fallback to unchecked access + return indexerProperty.GetValue(target, convertedObjectArray); + } + } + else + { + // Fallback to unchecked access + return indexerProperty.GetValue(target, convertedObjectArray); } } + // Multidimensional arrays end up here because the indexer search picks up the IList indexer instead of the + // multidimensional indexer, which doesn't take the same number of arguments + else if (typeInfo.IsArray) + { + return GetValueFromArray((Array)target); + } return PerspexProperty.UnsetValue; } - private static PropertyInfo GetIndexer(TypeInfo typeInfo) + private object GetValueFromArray(Array array) { - PropertyInfo indexer; - // Check for the default indexer name first to make this faster. - // This will only be false when a class in VB has a custom indexer name. - if ((indexer = typeInfo.GetDeclaredProperty(CommonPropertyNames.IndexerName)) != null) + int[] intArgs; + if (!ConvertArgumentsToInts(out intArgs)) + return PerspexProperty.UnsetValue; + return GetValueFromArray(array, intArgs); + } + + private object GetValueFromArray(Array array, int[] indicies) + { + if (ValidBounds(indicies, array)) { - return indexer; + return array.GetValue(indicies); } - foreach (var property in typeInfo.DeclaredProperties) + return PerspexProperty.UnsetValue; + } + + private bool ConvertArgumentsToInts(out int[] intArgs) + { + intArgs = new int[Arguments.Count]; + for (int i = 0; i < Arguments.Count; ++i) { - if (property.GetIndexParameters().Any()) + object value; + if (!TypeUtilities.TryConvert(typeof(int), Arguments[i], CultureInfo.InvariantCulture, out value)) { - return property; + return false; } + intArgs[i] = (int)value; + } + return true; + } + + private static PropertyInfo GetIndexer(TypeInfo typeInfo) + { + PropertyInfo indexer; + for (;typeInfo != null; typeInfo = typeInfo.BaseType?.GetTypeInfo()) + { + // Check for the default indexer name first to make this faster. + // This will only be false when a class in VB has a custom indexer name. + if ((indexer = typeInfo.GetDeclaredProperty(CommonPropertyNames.IndexerName)) != null) + { + return indexer; + } + foreach (var property in typeInfo.DeclaredProperties) + { + if (property.GetIndexParameters().Any()) + { + return property; + } + } } return null; } - private bool InBounds(int[] args, Array array) + private bool ValidBounds(int[] indicies, Array array) { - if (args.Length == array.Rank) + if (indicies.Length == array.Rank) { - for (var i = 0; i < args.Length; ++i) + for (var i = 0; i < indicies.Length; ++i) { - if (args[i] >= array.GetLength(i)) + if (indicies[i] >= array.GetLength(i)) { return false; } diff --git a/src/Markup/Perspex.Markup/Data/Parsers/ArgumentListParser.cs b/src/Markup/Perspex.Markup/Data/Parsers/ArgumentListParser.cs index 6e6c3d21fd..21061d7075 100644 --- a/src/Markup/Perspex.Markup/Data/Parsers/ArgumentListParser.cs +++ b/src/Markup/Perspex.Markup/Data/Parsers/ArgumentListParser.cs @@ -9,35 +9,26 @@ namespace Perspex.Markup.Data.Parsers { internal static class ArgumentListParser { - public static IList Parse(Reader r, char open, char close) + public static IList Parse(Reader r, char open, char close) { if (r.Peek == open) { - var result = new List(); + var result = new List(); r.Take(); while (!r.End) { - var literal = LiteralParser.Parse(r); - - if (literal != null) + var builder = new StringBuilder(); + while (!r.End && r.Peek != ',' && r.Peek != close && !char.IsWhiteSpace(r.Peek)) { - result.Add(literal); + builder.Append(r.Take()); } - else + if (builder.Length == 0) { - var builder = new StringBuilder(); - while (!r.End && r.Peek != ',' && r.Peek != close) - { - builder.Append(r.Take()); - } - if (builder.Length == 0) - { - throw new ExpressionParseException(r.Position, "Expected indexer argument."); - } - result.Add(builder.ToString()); + throw new ExpressionParseException(r.Position, "Expected indexer argument."); } + result.Add(builder.ToString()); r.SkipWhitespace(); diff --git a/src/Markup/Perspex.Markup/Data/Parsers/LiteralParser.cs b/src/Markup/Perspex.Markup/Data/Parsers/LiteralParser.cs deleted file mode 100644 index b50e46f635..0000000000 --- a/src/Markup/Perspex.Markup/Data/Parsers/LiteralParser.cs +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (c) The Perspex Project. All rights reserved. -// Licensed under the MIT license. See licence.md file in the project root for full license information. - -using System.Globalization; -using System.Text; - -namespace Perspex.Markup.Data.Parsers -{ - internal static class LiteralParser - { - public static object Parse(Reader r) - { - if (char.IsDigit(r.Peek)) - { - var result = new StringBuilder(); - var foundDecimal = false; - while (!r.End) - { - if (char.IsDigit(r.Peek)) - { - result.Append(r.Take()); - } - else if (!foundDecimal && r.Peek == '.') - { - result.Append(r.Take()); - foundDecimal = true; - } - else - { - break; - } - } - - if (!foundDecimal) - { - return int.Parse(result.ToString(), CultureInfo.InvariantCulture); - } - else - { - return result.ToString(); // Leave as a string to support double, float, and decimal indicies - } - } - - return null; - } - } -} diff --git a/src/Markup/Perspex.Markup/Perspex.Markup.csproj b/src/Markup/Perspex.Markup/Perspex.Markup.csproj index ba6e7d9331..f5b61e2cea 100644 --- a/src/Markup/Perspex.Markup/Perspex.Markup.csproj +++ b/src/Markup/Perspex.Markup/Perspex.Markup.csproj @@ -53,7 +53,6 @@ - diff --git a/tests/Perspex.Markup.UnitTests/Data/ExpressionNodeBuilderTests.cs b/tests/Perspex.Markup.UnitTests/Data/ExpressionNodeBuilderTests.cs index 33d8a6ff6c..3c2ff39873 100644 --- a/tests/Perspex.Markup.UnitTests/Data/ExpressionNodeBuilderTests.cs +++ b/tests/Perspex.Markup.UnitTests/Data/ExpressionNodeBuilderTests.cs @@ -77,7 +77,18 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(2, result.Count); AssertIsProperty(result[0], "Foo"); - AssertIsIndexer(result[1], 15); + AssertIsIndexer(result[1], "15"); + Assert.IsType(result[1]); + } + + [Fact] + public void Should_Build_Indexed_Property_StringIndex() + { + var result = ToList(ExpressionNodeBuilder.Build("Foo[Key]")); + + Assert.Equal(2, result.Count); + AssertIsProperty(result[0], "Foo"); + AssertIsIndexer(result[1], "Key"); Assert.IsType(result[1]); } @@ -88,7 +99,7 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(2, result.Count); AssertIsProperty(result[0], "Foo"); - AssertIsIndexer(result[1], 15, 6); + AssertIsIndexer(result[1], "15", "6"); } [Fact] @@ -98,7 +109,7 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(2, result.Count); AssertIsProperty(result[0], "Foo"); - AssertIsIndexer(result[1], 5, 16); + AssertIsIndexer(result[1], "5", "16"); } [Fact] @@ -108,8 +119,8 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(3, result.Count); AssertIsProperty(result[0], "Foo"); - AssertIsIndexer(result[1], 15); - AssertIsIndexer(result[2], 16); + AssertIsIndexer(result[1], "15"); + AssertIsIndexer(result[2], "16"); } [Fact] @@ -120,7 +131,7 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(4, result.Count); AssertIsProperty(result[0], "Foo"); AssertIsProperty(result[1], "Bar"); - AssertIsIndexer(result[2], 5, 6); + AssertIsIndexer(result[2], "5", "6"); AssertIsProperty(result[3], "Baz"); } @@ -132,12 +143,12 @@ namespace Perspex.Markup.UnitTests.Data Assert.Equal(name, p.PropertyName); } - private void AssertIsIndexer(ExpressionNode node, params object[] args) + private void AssertIsIndexer(ExpressionNode node, params string[] args) { Assert.IsType(node); var e = (IndexerNode)node; - Assert.Equal(e.Arguments.ToArray(), args.ToArray()); + Assert.Equal(e.Arguments.ToArray(), args); } private List ToList(ExpressionNode node)