id,summary,reporter,owner,description,type,status,priority,component,severity,resolution,keywords,cc
43,path resolver has bug,jstroud@…,xi,"== Path resolving does not work for Nodes within Sequence Nodes ==

[This is current for pyyaml 3.04.]

For example:

{{{
yaml.add_path_resolver(u'!MyObject', (u'myobjs', [list, False]))
}}}

should match all of the items in the myobjs sequence in this yaml:

{{{
""""""
--- !MyConfig
myobjs :
    - name: x
      root: z
    - name: p
      root: q
...
""""""
}}}

The problem is here in resolver.py (line 210):

{{{
elif isinstance(index_check, int):
    if index_check != current_index:
         return
}}}

If I'm infering the intent of the code correctly

{{{
(u'myobjs', (list, False))
}}}

should match all elements of the sequence keyed by ""myobjs"". However,

{{{
isinstance(False, int)
}}}

always returns True because bool is a subclass of int, so index_check will only equal current_index for the first element when False is specified as the index_check.

----

Additionally, I believe another bug exists in resolver.py. According to the code in add_path_resolver(), index_check defaults to True when not specified (resolver.py, line 39):

{{{
if isinstance(element, (list, tuple)):
    if len(element) == 2:
        node_check, index_check = element
    elif len(element) == 1:
        node_check = element[0]
        index_check = True
}}}

The intendend meaning of True here is not clear because True causes every element in the sequence to fail the match. The relevant test is in check_resolver_prefix() (resolver.py, line 116):

{{{
if index_check is True and current_index is not None:
    return
}}}

Thus, any path not specifying an index_check will never match. Most insidious to the user is that  this case is never reported nor an exception thrown. If index_check is True, this code will only continue to test if current_index is None, which should never happen for any node of any sequence, because any node of a sequence must, by virtue of its existence, have an index.

----

To fix these problems, I propose the following patch to resolver.py:

{{{
44c44
<                     index_check = True
---
>                     index_check = False
122c122
<         elif isinstance(index_check, int):
---
>         elif isinstance(index_check, int) and index_check is not False:
}}}

This will set the default value of index_check to match all elements of a sequence if no index_check is given and will not attempt to compare False to the value of index_check.

At the bare minimum, the following patch should be strongly considered if a good reason exists to leave the defalut of index_check to True.

{{{
122c122
<         elif isinstance(index_check, int):
---
>         elif isinstance(index_check, int) and index_check is not False:
}}}

Moreover, the code should provide some indication for what an index_check value of True actually means.

In fact, I propose re-writing the code to use None to match all elements, because

{{{
isinstance(None, int)
}}}

will always evaluate to False. This will also remove the question as to what the ""other"" bool value means because bools will not be used. I can provide a patch if this latter proposal sounds good to the developers.",defect,closed,high,pyyaml,major,fixed,Path Resolver,