Added support of collection initializer syntax.#246
Added support of collection initializer syntax.#246holdenmai wants to merge 4 commits intodynamicexpresso:masterfrom
Conversation
metoule
left a comment
There was a problem hiding this comment.
This looks really good, thank you!! The only problem I can see is that you can't use any expression that starts with an identifier in the collection initializer list:
new List<string> { string.Empty }is valid, but not accepted with your code.
| var member = FindPropertyOrField(newType, propertyOrFieldName, false); | ||
| if (member == null) | ||
| throw CreateParseException(_token.pos, ErrorMessages.UnknownPropertyOrField, propertyOrFieldName, GetTypeName(newType)); | ||
| if (_token.id != TokenId.Identifier) |
There was a problem hiding this comment.
I understand why you perform the check, but that's a bit limiting, because an identifier can be used:
new List<string> { string.Empty } You can peek at the next token to check if it's a equal sign: if it's not, you can use SetTextPos to restart parsing at the wanted location and call ParseExpressionSegment.
There was a problem hiding this comment.
I was having some trouble getting that to work when I was trying to account for the invalid case of
new MyClassAdder(){{StrProp = "SomeValue"}}
but I will try again according to your review comments
There was a problem hiding this comment.
Resolved in latest commit.
Also accounted for the ability to perform collection of the .Add while simultaneously setting a variable/parameter value even if that matches a property/field name.
var target = new Interpreter();
target.Reference(typeof(MyClassAdder));
Assert.DoesNotThrow(() => target.Eval<MyClassAdder>("new MyClassAdder(){{ 1, 2, 3, 4, 5},{StrProp = \"6\" },7}", new Parameter("StrProp", "0")));This is the equivalent of
string StrProp = "";
var mc = new MyClassAdder()
{
{1,2,3 },
{StrProp = "6" }
};
Assert.AreEqual(StrProp, "6")There was a problem hiding this comment.
I was so focused on the improvements around the checking of the additional cases, I missed the original List work.
There was a problem hiding this comment.
Thank you @holdenmai . Also remember to update/rebase your branch so that we will not have conflicts during merge.
Misc style changes from PR review
…hen calling the collection initialization syntax.
…r/collection initialization.
Implementation of collection initializer syntax support for constructor #194 Copied a little bit of conflicting code from pull 245.
You can now write
as well as implementations supporting multiple parameters in the Add method such as
new MyClassAdder(){{ 1, 2, 3, 4, 5},{\"6\" },7 }and
new Dictionary<int, string>(){{1, \"1\"}, {2, \"2\"}, {3, \"3\"}, {4, \"4\"}, {5, \"5\"}}