You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@royale.apache.org by ah...@apache.org on 2018/09/28 03:34:57 UTC
[royale-compiler] 01/03: fix bracket access of XML/XMLList
This is an automated email from the ASF dual-hosted git repository.
aharui pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
Author: Alex Harui <ah...@apache.org>
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'");
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Harbs <ha...@gmail.com>.
I don’t have time right now, but I added an issue so we shouldn’t forget about it…
> On Oct 7, 2018, at 5:38 PM, Alex Harui <ah...@adobe.com.INVALID> wrote:
>
> Then add tests for string expressions. I thought I'd already done that. I would expect that Tour De Flex is now broken. I've been trying to fix the compile to get TDF to work.
>
> IExpressionNode.resolveType() should return the type of a function. See the Javadoc for it.
>
> -Alex
>
> On 10/7/18, 4:59 AM, "Harbs" <harbs.lists@gmail.com <ma...@gmail.com>> wrote:
>
> 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 <ha...@gmail.com> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID> <mailto:aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>>> 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" <harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>>> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID> <mailto:aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>>> 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" <harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>> <mailto:harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>>>> 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, aharui@apache.org <ma...@apache.org> <mailto:aharui@apache.org <ma...@apache.org>> 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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460602827&sdata=pUyokCJAe7aUUdpXOd72VsvnnD8gKU4zw%2FmFd1AG6Ss%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460602827&sdata=pUyokCJAe7aUUdpXOd72VsvnnD8gKU4zw%2FmFd1AG6Ss%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%3D&reserved=0>>>
>>>>>
>>>>> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
>>>>> Author: Alex Harui <aharui@apache.org <ma...@apache.org> <mailto:aharui@apache.org <ma...@apache.org>>>
>>>>> 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'");
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Alex Harui <ah...@adobe.com.INVALID>.
Then add tests for string expressions. I thought I'd already done that. I would expect that Tour De Flex is now broken. I've been trying to fix the compile to get TDF to work.
IExpressionNode.resolveType() should return the type of a function. See the Javadoc for it.
-Alex
On 10/7/18, 4:59 AM, "Harbs" <ha...@gmail.com> wrote:
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 <ha...@gmail.com> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>> 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" <harbs.lists@gmail.com <ma...@gmail.com>> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>> 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" <harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>>> 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, aharui@apache.org <ma...@apache.org> 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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460602827&sdata=pUyokCJAe7aUUdpXOd72VsvnnD8gKU4zw%2FmFd1AG6Ss%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%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%7C21d3c40fc0b8481819e608d62c4c4787%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636745103460612841&sdata=Ejnj9JrGz%2Fq0oPoHMbWSSYGjDx7UuMXeJGMBYljxT10%3D&reserved=0>>
>>>>
>>>> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
>>>> Author: Alex Harui <aharui@apache.org <ma...@apache.org>>
>>>> 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'");
>
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Harbs <ha...@gmail.com>.
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 <ha...@gmail.com> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>> 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" <harbs.lists@gmail.com <ma...@gmail.com>> 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 <aharui@adobe.com.INVALID <ma...@adobe.com.INVALID>> 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" <harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>>> 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, aharui@apache.org <ma...@apache.org> 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 <aharui@apache.org <ma...@apache.org>>
>>>> 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'");
>
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Harbs <ha...@gmail.com>.
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 <ah...@adobe.com.INVALID> 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" <harbs.lists@gmail.com <ma...@gmail.com>> 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 <ah...@adobe.com.INVALID> 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" <harbs.lists@gmail.com <ma...@gmail.com> <mailto:harbs.lists@gmail.com <ma...@gmail.com>>> 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, aharui@apache.org <ma...@apache.org> 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 <ah...@apache.org>
>>> 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'");
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Alex Harui <ah...@adobe.com.INVALID>.
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" <ha...@gmail.com> 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 <ah...@adobe.com.INVALID> 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" <harbs.lists@gmail.com <ma...@gmail.com>> 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, aharui@apache.org 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>
>>
>> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
>> Author: Alex Harui <ah...@apache.org>
>> 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'");
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Harbs <ha...@gmail.com>.
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 <ah...@adobe.com.INVALID> 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" <harbs.lists@gmail.com <ma...@gmail.com>> 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, aharui@apache.org 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%7Cea3bf6b27b1d4c681a5d08d6283d7b9a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636740641870109874&sdata=6nGbwmtGYJKnYRUUPdFKixN%2BVUpEG0Ri9VY8Lp8Ib2k%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%7Cea3bf6b27b1d4c681a5d08d6283d7b9a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636740641870109874&sdata=6nGbwmtGYJKnYRUUPdFKixN%2BVUpEG0Ri9VY8Lp8Ib2k%3D&reserved=0>
>>
>> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
>> Author: Alex Harui <ah...@apache.org>
>> 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'");
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Alex Harui <ah...@adobe.com.INVALID>.
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" <ha...@gmail.com> 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, aharui@apache.org 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%7Cea3bf6b27b1d4c681a5d08d6283d7b9a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636740641870109874&sdata=6nGbwmtGYJKnYRUUPdFKixN%2BVUpEG0Ri9VY8Lp8Ib2k%3D&reserved=0
>
> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
> Author: Alex Harui <ah...@apache.org>
> 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'");
>
Re: [royale-compiler] 01/03: fix bracket access of XML/XMLList
Posted by Harbs <ha...@gmail.com>.
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, aharui@apache.org wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> aharui pushed a commit to branch develop
> in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
>
> commit 86bfe0a6ef8789e8ce065c856ce6f888f7562776
> Author: Alex Harui <ah...@apache.org>
> 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'");
>