I just pushed a change to the compiler to swap around the check to check if the type is a string instead of testing if it’s not a number. This fixes number expressions, but likely breaks string expressions. I don’t know how to get the evaluated type of the whole expression.
At least all current tests now pass… Harbs > On Oct 3, 2018, at 11:10 AM, Harbs <[email protected]> wrote: > > OK. > > I added some test cases for XMLList indexed access and assignment. One of > those tests passed before your change and is now failing. > > I think this is a summary of the cases we need to deal with (and we probably > need more test cases…) > > Given: > var xml:XML = <foo><baz/></foo> > var baz:XMLList = foo.baz; > > baz = foo["baz”]; > Should become: > baz = foo.child("baz”); > > baz = foo[foo.length()-1]; > Should remain unchanged: > baz = foo[foo.length()-1]; > > foo["baz"] = <baz/>; > Now becomes: > foo.child("baz") = new XML("<baz/“); > But that’s wrong. It should be: > foo.setChild("baz",new XML("<baz/")); > > foo[0] = <baz/>; > Should remain unchanged: > foo[0] = new XML("<baz/>"); > > I don’t understand the logic in the compiler well enough to really know how > to go about fixing these. Numbers in brackets need to be handled differently > that strings and left hand of assignment should be different than right-hand. > > I hope this info is helpful… > > Harbs > >> On Oct 2, 2018, at 9:25 PM, Alex Harui <[email protected] >> <mailto:[email protected]>> wrote: >> >> The recommended practice is to add a test case and make sure it fails >> appropriately. Then set @ignore on it before committing so it doesn't break >> the build. Whoever works on the fix can remove the @ignore locally when >> working on a fix >> >> I thought there were already test cases for handling number/int >> appropriately, but maybe not. >> >> It may be that there is additional code and tests needed to handle setting a >> value via bracket access as well. Feel free to add test cases for that and >> make the necessary changes in compiler. >> >> -Alex >> >> On 10/2/18, 11:16 AM, "Harbs" <[email protected] >> <mailto:[email protected]>> wrote: >> >> I guess I can add a test case, but that’ll make the build fail. >> >> I’m not sure what to propose for a fix though. >> >> Any indexed (i.e. Number, int or uint type) access should always remain >> bracketed access. .child() seems wrong for assignment in any case. If >> anything it should be .setChild(), but I’m not sure how well that works for >> XMLList. It only does anything for single-item XMLList instances (which >> behave like XML instances). >> >> Thanks, >> Harbs >> >>> On Oct 2, 2018, at 7:22 PM, Alex Harui <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Add a test case to the test suite, propose the fix or ask for help on it. >>> >>> -Alex >>> >>> On 10/2/18, 1:03 AM, "Harbs" <[email protected] >>> <mailto:[email protected]> <mailto:[email protected] >>> <mailto:[email protected]>>> wrote: >>> >>> I don’t know the reason for this change, but this broke bracket access in >>> XMLList. >>> >>> myList[myList.length()] = xml should be output unchanged. >>> >>> It’s now output as: >>> >>> myList.child(myList.length()) = xml >>> >>> That’s wrong… >>> >>> Harbs >>> >>>> On Sep 28, 2018, at 6:34 AM, [email protected] <mailto:[email protected]> >>>> wrote: >>>> >>>> This is an automated email from the ASF dual-hosted git repository. >>>> >>>> aharui pushed a commit to branch develop >>>> in repository >>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.git&data=02%7C01%7Caharui%40adobe.com%7Cd1f87f436fbd44ebc97508d628933155%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636741009996972974&sdata=59bTRbYx0R8V%2FOtR3BTuTFW3u1oxfMOca8yJNUaZy3w%3D&reserved=0 >>>> >>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.git&data=02%7C01%7Caharui%40adobe.com%7Cd1f87f436fbd44ebc97508d628933155%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636741009996972974&sdata=59bTRbYx0R8V%2FOtR3BTuTFW3u1oxfMOca8yJNUaZy3w%3D&reserved=0> >>>> >>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.git&data=02%7C01%7Caharui%40adobe.com%7Cd1f87f436fbd44ebc97508d628933155%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636741009996972974&sdata=59bTRbYx0R8V%2FOtR3BTuTFW3u1oxfMOca8yJNUaZy3w%3D&reserved=0 >>>> >>>> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.git&data=02%7C01%7Caharui%40adobe.com%7Cd1f87f436fbd44ebc97508d628933155%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636741009996972974&sdata=59bTRbYx0R8V%2FOtR3BTuTFW3u1oxfMOca8yJNUaZy3w%3D&reserved=0>> >>>> >>>> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776 >>>> Author: Alex Harui <[email protected] <mailto:[email protected]>> >>>> AuthorDate: Tue Sep 25 12:19:02 2018 -0700 >>>> >>>> fix bracket access of XML/XMLList >>>> --- >>>> .../codegen/js/jx/DynamicAccessEmitter.java | 34 >>>> +++++++++++++++++++++- >>>> .../codegen/js/royale/TestRoyaleGlobalClasses.java | 30 ++++++++++++++++++- >>>> 2 files changed, 62 insertions(+), 2 deletions(-) >>>> >>>> diff --git >>>> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/DynamicAccessEmitter.java >>>> >>>> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/DynamicAccessEmitter.java >>>> index ae977f9..0d94fac 100644 >>>> --- >>>> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/DynamicAccessEmitter.java >>>> +++ >>>> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/DynamicAccessEmitter.java >>>> @@ -21,8 +21,12 @@ package >>>> org.apache.royale.compiler.internal.codegen.js.jx; >>>> >>>> import org.apache.royale.compiler.codegen.ISubEmitter; >>>> import org.apache.royale.compiler.codegen.js.IJSEmitter; >>>> +import org.apache.royale.compiler.definitions.ITypeDefinition; >>>> import org.apache.royale.compiler.internal.codegen.as.ASEmitterTokens; >>>> import org.apache.royale.compiler.internal.codegen.js.JSSubEmitter; >>>> +import >>>> org.apache.royale.compiler.internal.codegen.js.royale.JSRoyaleEmitter; >>>> +import >>>> org.apache.royale.compiler.internal.tree.as.MemberAccessExpressionNode; >>>> +import org.apache.royale.compiler.internal.tree.as.NumericLiteralNode; >>>> import org.apache.royale.compiler.tree.ASTNodeID; >>>> import org.apache.royale.compiler.tree.as.IDynamicAccessNode; >>>> import org.apache.royale.compiler.tree.as.IExpressionNode; >>>> @@ -43,11 +47,39 @@ public class DynamicAccessEmitter extends JSSubEmitter >>>> implements >>>> if (leftOperandNode.getNodeID() == ASTNodeID.Op_AtID) >>>> return; >>>> >>>> + IExpressionNode rightOperandNode = node.getRightOperandNode(); >>>> + IJSEmitter ijs = getEmitter(); >>>> + JSRoyaleEmitter fjs = (ijs instanceof JSRoyaleEmitter) ? >>>> + >>>> (JSRoyaleEmitter)ijs : null; >>>> + if (fjs != null) >>>> + { >>>> + boolean isXML = false; >>>> + if (leftOperandNode instanceof MemberAccessExpressionNode) >>>> + isXML = >>>> fjs.isLeftNodeXMLish((MemberAccessExpressionNode)leftOperandNode); >>>> + else if (leftOperandNode instanceof IExpressionNode) >>>> + isXML = fjs.isXML((IExpressionNode)leftOperandNode); >>>> + if (isXML) >>>> + { >>>> + ITypeDefinition type = >>>> rightOperandNode.resolveType(getProject()); >>>> + if (!type.isInstanceOf("int", getProject()) && >>>> !type.isInstanceOf("uint", getProject()) && !type.isInstanceOf("Number", >>>> getProject()) ) >>>> + { >>>> + String field = >>>> fjs.stringifyNode(rightOperandNode); >>>> + if (field.startsWith("\"@")) >>>> + { >>>> + field = field.replace("@", ""); >>>> + write(".attribute(" + field + >>>> ")"); >>>> + } >>>> + else >>>> + write(".child(" + field + ")"); >>>> + return; >>>> + } >>>> + } >>>> + } >>>> + >>>> startMapping(node, leftOperandNode); >>>> write(ASEmitterTokens.SQUARE_OPEN); >>>> endMapping(node); >>>> >>>> - IExpressionNode rightOperandNode = node.getRightOperandNode(); >>>> getWalker().walk(rightOperandNode); >>>> >>>> startMapping(node, rightOperandNode); >>>> diff --git >>>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java >>>> >>>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java >>>> index 85b3a15..b2c8a17 100644 >>>> --- >>>> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java >>>> +++ >>>> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java >>>> @@ -482,7 +482,17 @@ public class TestRoyaleGlobalClasses extends >>>> TestGoogGlobalClasses >>>> asBlockWalker.visitVariable(node); >>>> assertOut("var /** @type {XMLList} */ b = a.child('child')"); >>>> } >>>> - >>>> + >>>> + @Test >>>> + public void testXMLSingleDotBracket() >>>> + { >>>> + IVariableNode node = getVariable("var a:XML = new XML(\"<top >>>> attr1='cat'><child attr2='dog'><grandchild >>>> attr3='fish'>text</grandchild></child></top>\");var b:XMLList = >>>> a[\"child\"];"); >>>> + IASNode parentNode = node.getParent(); >>>> + node = (IVariableNode) parentNode.getChild(1); >>>> + asBlockWalker.visitVariable(node); >>>> + assertOut("var /** @type {XMLList} */ b = a.child(\"child\")"); >>>> + } >>>> + >>>> @Test >>>> public void testXMLSingleDotChain() >>>> { >>>> @@ -636,6 +646,16 @@ public class TestRoyaleGlobalClasses extends >>>> TestGoogGlobalClasses >>>> } >>>> >>>> @Test >>>> + public void testXMLAttributeBracket2() >>>> + { >>>> + IVariableNode node = getVariable("var a:XML = new XML(\"<top >>>> attr1='cat'><child attr2='dog'><grandchild >>>> attr3='fish'>text</grandchild></child></top>\");var b:XMLList = >>>> a[\"@attr1\"];"); >>>> + IASNode parentNode = node.getParent(); >>>> + node = (IVariableNode) parentNode.getChild(1); >>>> + asBlockWalker.visitVariable(node); >>>> + assertOut("var /** @type {XMLList} */ b = >>>> a.attribute(\"attr1\")"); >>>> + } >>>> + >>>> + @Test >>>> public void testXMLAttributeToString() >>>> { >>>> IVariableNode node = getVariable("var a:XML = new XML(\"<top >>>> attr1='cat'><child attr2='dog'><grandchild >>>> attr3='fish'>text</grandchild></child></top>\");var b:String = a.@attr1;"); >>>> @@ -726,6 +746,14 @@ public class TestRoyaleGlobalClasses extends >>>> TestGoogGlobalClasses >>>> } >>>> >>>> @Test >>>> + public void testXMLListSetAttributeIndex() >>>> + { >>>> + IBinaryOperatorNode node = getBinaryNode("var n:int = 1;var >>>> a:XMLList;a[n].@bar = 'foo'"); >>>> + asBlockWalker.visitBinaryOperator(node); >>>> + assertOut("a[n].setAttribute('bar', 'foo')"); >>>> + } >>>> + >>>> + @Test >>>> public void testXMLListSetAttributeComplex() >>>> { >>>> IBinaryOperatorNode node = getBinaryNode("var >>>> a:XMLList;a.(@id==3).@height = '100px'"); >
