Currently, when a node is being traversed, we check multiple attributes to find its value. To do so, we rely on the assumption that there will be only one value at best, and check, successively:
- The
Name/name field.
- The
Text/text field
- The
Value/value field
- Finally, the
Names attribute. We handle this one differently: if the field is an array of nodes, which it always is, then we join the value of each of these node's Name attribute, if they have one.
In order to avoid duplication, we then ignore the Names attribute in the goDeeper function. While this provides utility, it has 2 risks, one of which I am certain exists:
- If a node has a value held in one of the 3 first attributes and a non-empty
Names attribute, we will not traverse the nodes in it. I have not (yet) found an example for this, but it is a possibility.
2 . If Names contain proper nodes that are nested, thus not having a Name attribute. This is the case in the following example:
from foo import bar as baz
Here, the uast:RuntimeImport node has the following structure:
- a
Path attribute with a single uast:Identifier node, with value foo
- a Names attribute with a single
uast:Alias node, with a Node attribute containing the uast:Identifier node with value bar, and a Name attribute containing uast:Identifier node with value baz
Now, unfortunately this is not a Babelfish bug, as this structure for aliases is always the same: we replace a uast:Identifier node with a uast:Alias node that has a Name and a Node attribute. Also, this makes sense, as in the following snippet the Names attribute would have
2 alias nodes instead of one:
from foo import bar as baz, bar2 as baz2
Anyway, this is a problem, because currently we loose this information. In the first snippet, only the foo identifier is kept. I am going to check out what we can do and will push a PR when a proper solution is found. I think we should start dealing with import nodes in a specific way, and just go deeper on the Names attribute in all other cases, but I've got to look more into this before.
Currently, when a node is being traversed, we check multiple attributes to find its value. To do so, we rely on the assumption that there will be only one value at best, and check, successively:
Name/namefield.Text/textfieldValue/valuefieldNamesattribute. We handle this one differently: if the field is an array of nodes, which it always is, then we join the value of each of these node'sNameattribute, if they have one.In order to avoid duplication, we then ignore the
Namesattribute in thegoDeeperfunction. While this provides utility, it has 2 risks, one of which I am certain exists:Namesattribute, we will not traverse the nodes in it. I have not (yet) found an example for this, but it is a possibility.2 . If
Namescontain proper nodes that are nested, thus not having aNameattribute. This is the case in the following example:Here, the
uast:RuntimeImportnode has the following structure:Pathattribute with a singleuast:Identifiernode, with valuefoouast:Aliasnode, with aNodeattribute containing theuast:Identifiernode with valuebar, and aNameattribute containinguast:Identifiernode with valuebazNow, unfortunately this is not a Babelfish bug, as this structure for aliases is always the same: we replace a
uast:Identifiernode with auast:Aliasnode that has aNameand aNodeattribute. Also, this makes sense, as in the following snippet theNamesattribute would have2 alias nodes instead of one:
Anyway, this is a problem, because currently we loose this information. In the first snippet, only the
fooidentifier is kept. I am going to check out what we can do and will push a PR when a proper solution is found. I think we should start dealing with import nodes in a specific way, and just go deeper on the Names attribute in all other cases, but I've got to look more into this before.