pyyaml icon indicating copy to clipboard operation
pyyaml copied to clipboard

Add method to construct mapping key

Open blag opened this issue 4 years ago • 1 comments

This PR makes it much easier to generate a safe loader (or a normal loader) that disallows duplicate keys in mappings.

This follows a few points of the Zen of Python:

  • Beautiful is better than ugly.
  • Simple is better than complex.
  • If the implementation is easy to explain, it may be a good idea.

Before (which I'm pretty sure mutates global state):

data = '''
---
foo: bar
foo: qux
'''

def no_duplicates_constructor(loader, node, deep=False):
    """Check for duplicate keys."""

    mapping = {}
    for key_node, value_node in node.value:
        key = loader.construct_object(key_node, deep=deep)
        value = loader.construct_object(value_node, deep=deep)
        if key in mapping:
            raise ConstructorError("while constructing a mapping", node.start_mark,
                                   "found duplicate key (%s)" % key, key_node.start_mark)
        mapping[key] = value

    return loader.construct_mapping(node, deep)

yaml.add_constructor(yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, no_duplicates_constructor, yaml.SafeLoader)

yaml.safe_load(data)

After (does not mutate global state):

import yaml

data = '''
---
foo: bar
foo: qux
'''

class SafeUniqueKeyLoader(yaml.loader.SafeLoader):
    def construct_mapping_key(self, key, mapping):
        if key in mapping:
            raise ValueError(f"The '{key}' key already exists in {mapping}")

yaml.load(data, SafeUniqueKeyLoader)

IMO, the second one is a lot easier to explain to somebody who is new to PyYAML.

A few points for your consideration:

  • I have intentionally omitted any parsing data from the parameters to construct_mapping_key and subsequently from the raised exception in the example method, because it isn't technically a parsing error, and it should be obvious from the exception message which dictionary/object the duplicate key appeared in. I have an alternative PR with a slightly different implementation: #503.
  • I have no idea where to document this, so the quick docstrings are the only documentation I have made for this. I'm happy to write better and more thorough documentation if you point me to it.

blag avatar Feb 23 '21 20:02 blag

Not sure why tests are failing. They may need to be kicked again.

blag avatar Feb 24 '21 22:02 blag