From 7baf8ba615ee09c046ec223745612382d396bbfe Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 23 Feb 2016 22:26:18 -0600 Subject: [PATCH] Changed argument parser to parse everything as strings and refactored the IndexerNode.GetValue to special case places where we can check validity (and multidimensional arrays since they don't work like everything else). Removed LiteralParser since it isn't used by anything any more. --- src/Markup/Perspex.Markup/Data/IndexerNode.cs | 201 ++++++++++++------ .../Data/Parsers/ArgumentListParser.cs | 25 +-- .../Data/Parsers/LiteralParser.cs | 47 ---- .../Perspex.Markup/Perspex.Markup.csproj | 1 - .../Data/ExpressionNodeBuilderTests.cs | 27 ++- 5 files changed, 158 insertions(+), 143 deletions(-) delete mode 100644 src/Markup/Perspex.Markup/Data/Parsers/LiteralParser.cs 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)