Various fixes to the cnfvar module
authorSamir Aguiar <samir.aguiar@intra2net.com>
Mon, 23 Aug 2021 03:26:48 +0000 (00:26 -0300)
committerChristian Herdtweck <christian.herdtweck@intra2net.com>
Mon, 4 Apr 2022 12:24:58 +0000 (14:24 +0200)
- fix infinite loop with blank lines
- do not drop parent and comment values when normalizing
- fix error when first child begins with whitespaces
- format, restructure and document the module

src/cnfvar.py

index e0618fc..a8b0fef 100644 (file)
@@ -106,14 +106,87 @@ import json
 import re
 import io
 
-#
-#                                   helpers
-#
 
-# Sadly, the Intranator is still stuck with one leg in the 90s.
-#
+###############################################################################
+# CONSTANTS
+###############################################################################
+
 
+CNF_FIELD_MANDATORY = set ([ "varname", "data", "instance" ])
+CNF_FIELD_OPTIONAL  = set ([ "parent", "children", "comment", "number" ])
+CNF_FIELD_KNOWN     = CNF_FIELD_MANDATORY | CNF_FIELD_OPTIONAL
+
+grab_parent_pattern = re.compile(b"""
+                                    ^            # match from start
+                                    \s*          # optional spaces
+                                    \d+          # line number
+                                    \s+          # spaces
+                                    \((\d+)\)    # parent
+                                 """,
+                                 re.VERBOSE)
 
+base_line_pattern = re.compile(b"""
+                                    ^                    # match from start
+                                    \s*                  # optional spaces
+                                    (\d+)                # line number
+                                    \s+                  # spaces
+                                    ([A-Z][A-Z0-9_]*)    # varname
+                                    \s*                  # optional spaces
+                                    ,                    # delimiter
+                                    \s*                  # optional spaces
+                                    (-1|\d+)             # instance
+                                    \s*                  # optional spaces
+                                    :                    # delimiter
+                                    \s*                  # optional spaces
+                                    \"(                  # quoted string (data)
+                                       (?:  \\\"         #  (of escaped dquote
+                                           |[^\"])*      #   or anything not a
+                                      )\"                #   literal quote)
+                                    \s*                  # optional spaces
+                                    (                    # bgroup
+                                     \#                  # comment leader
+                                     \s*                 # optional spaces
+                                     .*                  # string (comment)
+                                    )?                   # egroup, optional
+                                    $                    # eol
+                               """,
+                               re.VERBOSE)
+
+child_line_pattern = re.compile(b"""
+                                     ^                    # match from start
+                                     \s*                  # optional spaces
+                                     (\d+)                # line number
+                                     \s+                  # spaces
+                                     \((\d+)\)            # parent
+                                     \s+                  # spaces
+                                     ([A-Z][A-Z0-9_]*)    # varname
+                                     \s*                  # optional spaces
+                                     ,                    # delimiter
+                                     \s*                  # optional spaces
+                                     (-1|\d+)             # instance
+                                     \s*                  # optional spaces
+                                     :                    # delimiter
+                                     \s*                  # optional spaces
+                                     \"([^\"]*)\"         # quoted string (data)
+                                     \s*                  # optional spaces
+                                     (                    # bgroup
+                                      \#                  # comment leader
+                                      \s*                 # optional spaces
+                                      .*                  # string (comment)
+                                     )?                   # egroup, optional
+                                     $                    # eol
+                                """,
+                                re.VERBOSE)
+
+
+###############################################################################
+# HELPERS
+###############################################################################
+
+
+#
+# Sadly, the Intranator is still stuck with one leg in the 90s.
+#
 def to_latin1(s):
     """Take given unicode str and convert it to a latin1-encoded `bytes`."""
     return s.encode("latin-1")
@@ -123,10 +196,47 @@ def from_latin1(s):
     """Take given latin1-encoded `bytes` value and convert it to `str`."""
     return s.decode("latin-1")
 
+
+#
+# Conversion functions
+#
+
+def marshal_in_number(number):
+    return int(number)
+
+
+def marshal_in_parent(parent):
+    return int(parent)
+
+
+def marshal_in_instance(instance):
+    return int(instance)
+
+
+def marshal_in_varname(varname):
+    return from_latin1(varname).lower()
+
+
+def marshal_in_data(data):
+    return from_latin1(data) if data is not None else ""
+
+
+def marshal_in_comment(comment):
+    return comment and from_latin1(comment[1:].strip()) or None
+
+
 #
-#                                  traversal
+# Type checking
 #
 
+def is_string(s):
+    return isinstance(s, str)
+
+
+###############################################################################
+# EXCEPTIONS
+###############################################################################
+
 
 class InvalidCNF(Exception):
 
@@ -137,35 +247,18 @@ class InvalidCNF(Exception):
         return "Malformed CNF_VAR: \"%s\"" % self.msg
 
 
-def walk_cnf(cnf, nested, fun, acc):
-    """
-    :type cnf: cnf list
-    :type nested: bool
-    :type fun: 'a -> bool -> (cnf stuff) -> 'a
-    :type acc: 'a
-    :rtype: 'a
-    """
-    for var in cnf:
-        acc = fun(acc,
-                  nested,
-                  comment=var.get("comment", None),
-                  data=var.get("data", None),
-                  instance=var.get("instance", None),
-                  number=var.get("number", None),
-                  parent=var.get("parent", None),
-                  varname=var.get("varname", None))
-        children = var.get("children", None)
-        if children is not None:
-            acc = walk_cnf(children, True, fun, acc)
-    return acc
+class MalformedCNF(Exception):
 
+    def __init__(self, msg):
+        self.msg = msg
 
-#
-#                                 validation
-#
+    def __str__(self):
+        return "Malformed CNF file: \"%s\"" % self.msg
 
-def is_string(s):
-    return isinstance(s, str)
+
+###############################################################################
+# VALIDATION
+###############################################################################
 
 
 def is_valid(acc,
@@ -246,27 +339,43 @@ def is_cnf(root):
     well-formed member of the argument will cause the predicate to bail out
     with an exception during traversal.
     """
-    cnf = cnf_root (root)
+    cnf = cnf_root(root)
     if cnf is None:
-        raise InvalidCNF (root)
+        raise InvalidCNF(root)
     return walk_cnf(cnf, False, is_valid, {}) is not None
 
 
-def count_vars (root):
+def is_cnf_var(obj):
     """
-    Traverse the cnf structure recursively, counting VAR objects (CNF lines).
+    Check whether a dictionary is a valid CNF.
+
+    :param dict obj: dictionary to check
+    :returns: True if the dictionary has all the mandatory fields and no
+              unknown fields, False otherwise
+    :rtype: bool
     """
-    cnf = cnf_root (root)
-    if cnf is None:
-        raise InvalidCNF (root)
-    return walk_cnf (cnf, True, lambda n, _nested, **_kwa: n + 1, 0)
+    assert isinstance (obj, dict)
+
+    for f in CNF_FIELD_MANDATORY:
+        if obj.get(f, None) is None:
+            return False
+
+    for f in obj:
+        if f not in CNF_FIELD_KNOWN:
+            return False
+
+    return True
+
+
+###############################################################################
+# DESERIALIZATION
+###############################################################################
+
 
 #
-#                               deserialization
+# JSON reader for get_cnf -j (the easy part)
 #
 
-# the easy part: JSON reader for get_cnf -j
-
 def make_varname_lowercase(cnfvar):
     """
     Custom hook for json decoder: convert variable name to lowercase.
@@ -307,29 +416,63 @@ def read_cnf_json(cnfdata):
         raise TypeError("Invalid CNF_VAR.")
     return cnf_json
 
-# the moderately more complicated part: CNF reader
 
+#
+# CNF reader (the moderately more complicated part)
+#
+# Parsing usually starts from the `read_cnf`, which accepts a string containing
+# the variables to parse in the same structure as returned by `get_cnf`.
+#
+# In the `prepare` function the string is split into lines, and a 3-element
+# tuple is built. The first (named `current`) and second (named `next`)
+# elements of this tuple are respectively the first and second non-empty lines
+# of the input, while the third is a list of the remaining lines. This tuple is
+# named `state` in the implementation below, and it is passed around during
+# parsing. The `get` and `peek` functions are used to easily retrieve the
+# `current` and `next` items from the "state".
+#
+# When we "advance" the state, we actually drop the "current" element,
+# replacing it with the "next", while a new "next" is popped from the list of
+# remaining lines. Parsing is done this way because we need to look ahead at
+# the next line -- if it is a child it needs to be appended to the `children`
+# property of the current line.
+#
+# Regular expressions are used to extract important information from the CNF
+# lines. Finally, once parsing is completed, a dictionary is returned. The dict
+# has the same structure as the serialized JSON output returned by
+# `get_cnf -j`.
+#
 
-def advance(cns):
-    current, next, stream = cns
-    if next is None:  # reached end of stream
-        return None
-    current = next
 
-    try:
-        next = stream.pop()
-        next = next.strip()
-    except IndexError:
-        next = None
+def read_cnf(data):
+    """
+    Read cnf data from data bytes.
 
-    if current == "":  # skip blank lines
-        return advance((current, next, stream))
-    return (current, next, stream)
+    :param bytes data: raw data
+    :return: the parsed cnf data
+    :rtype: {str, {str, str or int}}
+    """
+    if isinstance(data, str):
+        data = to_latin1(data)
+    state = prepare(data)
+    if state is None:
+        raise InvalidCNF("Empty input string.")
+
+    cnf = parse_cnf_root(state)
+    if is_cnf(cnf) is False:
+        raise TypeError("Invalid CNF_VAR.")
+    return {"cnf": cnf}
 
 
 def prepare(raw):
     """
+    Build 3-element iterable from a CNF string dump.
+
+    :param raw: string content as returned by `get_cnf`
     :type raw: bytes
+    :returns: 3-element tuple, where the first two elements are the first two
+              lines of the output and the third is a list containing the rest
+              of the lines in reverse.
     :rtype: (str * str option * str list) option
     """
     lines = raw.splitlines()
@@ -351,81 +494,214 @@ def prepare(raw):
     return (first, second, lines)
 
 
-def peek(cns):
-    _, next, _ = cns
-    return next
+def advance(cns):
+    """
+    Pop the next line from the stream, advancing the tuple.
+
+    :param cns: a 3-element tuple containing two CNF lines and a list of the
+                remaining lines
+    :type cnd: (str, str, [str])
+    :returns: a new tuple with a new item popped from the list of lines
+    :rtype cnd: (str, str, [str])
+    """
+    current, next, stream = cns
+    if next is None:  # reached end of stream
+        return None
+    current = next
+
+    try:
+        next = stream.pop()
+        next = next.strip()
+    except IndexError:
+        next = None
+
+    if current == "":  # skip blank lines
+        return advance((current, next, stream))
+    return (current, next, stream)
 
 
 def get(cns):
+    """
+    Get the current line from the state without advancing it.
+
+    :param cns: a 3-element tuple containing two CNF lines and a list of the
+                remaining lines
+    :type cnd: (str, str, [str])
+    :returns: the CNF line stored as `current`
+    :rtype: str
+    """
     current, _, _ = cns
     return current
 
 
-class MalformedCNF(Exception):
+def peek(cns):
+    """
+    Get the next line from the state without advancing it.
 
-    def __init__(self, msg):
-        self.msg = msg
+    :param cns: a 3-element tuple containing two CNF lines and a list of the
+                remaining lines
+    :type cnd: (str, str, [str])
+    :returns: the CNF line stored as `next`
+    :rtype: str
+    """
+    _, next, _ = cns
+    return next
 
-    def __str__(self):
-        return "Malformed CNF file: \"%s\"" % self.msg
 
-grab_parent_pattern = re.compile(b"""
-                                    ^            # match from start
-                                    \d+          # line number
-                                    \s+          # spaces
-                                    \((\d+)\)    # parent
-                                 """,
-                                 re.VERBOSE)
+def parse_cnf_root(state):
+    """
+    Iterate over and parse a list of CNF lines.
+
+    :param state: a 3-element tuple containing two lines and a list of the
+                  remaining lines
+    :type state: (str, str, [str])
+    :returns: a list of parsed CNF variables
+    :rtype: [dict]
+
+    The function will parse the first element from the `state` tuple, then read
+    the next line to see if it is a child variable. If it is, it will be
+    appended to the last parsed CNF, otherwise top-level parsing is done
+    normally.
+    """
+    lines = []
+    current = get(state)
+    while state:
+        cnf_line = read_base_line(current)
+        if cnf_line is not None:
+            lines.append(cnf_line)
+            state = advance(state)
+            if state is None:  # -> nothing left to do
+                break
+            current = get(state)
+            parent = get_parent(current)  # peek at next line
+            if parent is not None:  # -> recurse into children
+                (state, children, _parent) = parse_cnf_children(state, parent)
+                cnf_line["children"] = children
+                if state is None:
+                    break
+                current = get(state)
+        else:
+            state = advance(state)
+            if state is None:
+                break
+            current = get(state)
+    return lines
+
+
+def parse_cnf_children(state, parent):
+    """
+    Read and parse child CNFs of a given parent until there is none left.
+
+    :param state: a 3-element tuple containing two lines and a list of the
+                  remaining lines
+    :type state: (str, str, [str])
+    :param parent: id of the parent whose children we are looking for
+    :type parent: int
+    :returns: a 3-element tuple with the current state, a list of children of
+              the given parent and the parent ID
+    :rtype: (tuple, [str], int)
+
+    The function will recursively parse child lines from the `state` tuple
+    until one of these conditions is satisfied:
+
+    1. the input is exhausted
+    2. the next CNF line
+        2.1. is a toplevel line
+        2.2. is a child line whose parent has a lower parent number
+
+    Conceptually, 2.1 is a very similar to 2.2 but due to the special status of
+    toplevel lines in CNF we need to handle them separately.
+
+    Note that since nesting of CNF vars is achieved via parent line numbers,
+    lines with different parents could appear out of order. libcnffile will
+    happily parse those and still assign children to the specified parent:
+
+    ::
+        # set_cnf <<THATSALL
+        1 USER,1337: "l33t_h4x0r"
+        2    (1) USER_GROUP_MEMBER_REF,0: "2"
+        4 USER,1701: "picard"
+        5    (4) USER_GROUP_MEMBER_REF,0: "2"
+        6    (4) USER_PASSWORD,0: "engage"
+        3    (1) USER_PASSWORD,0: "hacktheplanet"
+        THATSALL
+        # get_cnf user 1337
+        1 USER,1337: "l33t_h4x0r"
+        2    (1) USER_GROUP_MEMBER_REF,0: "2"
+        3    (1) USER_PASSWORD,0: "hacktheplanet"
+        # get_cnf user 1701
+        1 USER,1701: "picard"
+        2    (1) USER_GROUP_MEMBER_REF,0: "2"
+        3    (1) USER_PASSWORD,0: "engage"
+
+    It is a limitation of ``cnfvar.py`` that it cannot parse CNF data
+    structured like the above example: child lists are only populated from
+    subsequent CNF vars using the parent number solely to track nesting levels.
+    The parser does not keep track of line numbers while traversing the input
+    so it doesn’t support retroactively assigning a child to anything else but
+    the immediate parent.
+    """
+    lines = []
+    current = get(state)
+    while True:
+        cnf_line = read_child_line(current)
+        if cnf_line is not None:
+            lines.append(cnf_line)
+            state = advance(state)
+            if state is None:
+                break
+            current = get(state)
+            new_parent = get_parent(current)
+            if new_parent is None:
+                # drop stack
+                return (state, lines, None)
+            if new_parent > parent:
+                # parent is further down in hierarchy -> new level
+                (state, children, new_parent) = \
+                    parse_cnf_children (state, new_parent)
+                if state is None:
+                    break
+                cnf_line["children"] = children
+                current = get(state)
+                new_parent = get_parent(current)
+                if new_parent is None:
+                    # drop stack
+                    return (state, lines, None)
+            if new_parent < parent:
+                # parent is further up in hierarchy -> pop level
+                return (state, lines, new_parent)
+            # new_parent == parent -> continue parsing on same level
+    return (state, lines, parent)
 
 
 def get_parent(line):
+    """
+    Extract the ID of the parent for a given CNF line.
+
+    :param str line: CNF line
+    :returns: parent ID or None if no parent is found
+    :rtype: int or None
+    """
     match = re.match(grab_parent_pattern, line)
     if match is None:  # -> no parent
         return None
     return int(match.groups()[0])
 
 
-def marshal_in_number   (number):   return int (number)
-
-def marshal_in_parent   (parent):   return int (parent)
-
-def marshal_in_varname  (varname):  return from_latin1 (varname).lower ()
-
-def marshal_in_instance (instance): return int (instance)
+def read_base_line(line):
+    """
+    Turn one top-level CNF line into a dictionary.
 
-def marshal_in_data     (data):     return from_latin1 (data) if data is not None else ""
+    :param str line: CNF line
+    :rtype: {str: Any}
 
-def marshal_in_comment  (comment):  return comment and from_latin1(comment[1:].strip()) or None
+    This performs the necessary decoding on values to obtain proper Python
+    strings from 8-bit encoded CNF data.
 
-base_line_pattern = re.compile(b"""
-                                    ^                    # match from start
-                                    \s*                  # optional spaces
-                                    (\d+)                # line number
-                                    \s+                  # spaces
-                                    ([A-Z][A-Z0-9_]*)    # varname
-                                    \s*                  # optional spaces
-                                    ,                    # delimiter
-                                    \s*                  # optional spaces
-                                    (-1|\d+)             # instance
-                                    \s*                  # optional spaces
-                                    :                    # delimiter
-                                    \s*                  # optional spaces
-                                    \"(                  # quoted string (data)
-                                       (?:  \\\"         #  (of escaped dquote
-                                           |[^\"])*      #   or anything not a
-                                      )\"                #   literal quote)
-                                    \s*                  # optional spaces
-                                    (                    # bgroup
-                                     \#                  # comment leader
-                                     \s*                 # optional spaces
-                                     .*                  # string (comment)
-                                    )?                   # egroup, optional
-                                    $                    # eol
-                               """,
-                               re.VERBOSE)
-
-
-def read_base_line(line):
+    The function only operates on individual lines. Argument strings that
+    contain data for multiple lines – this includes child lines of the current
+    CNF var! – will trigger a parsing exception.
+    """
     if len(line.strip()) == 0:
         return None  # ignore empty lines
     if line[0] == b"#":
@@ -443,34 +719,18 @@ def read_base_line(line):
         "comment"  : marshal_in_comment  (comment),
     }
 
-child_line_pattern = re.compile(b"""
-                                     ^                    # match from start
-                                     \s*                  # optional spaces
-                                     (\d+)                # line number
-                                     \s+                  # spaces
-                                     \((\d+)\)            # parent
-                                     \s+                  # spaces
-                                     ([A-Z][A-Z0-9_]*)    # varname
-                                     \s*                  # optional spaces
-                                     ,                    # delimiter
-                                     \s*                  # optional spaces
-                                     (-1|\d+)             # instance
-                                     \s*                  # optional spaces
-                                     :                    # delimiter
-                                     \s*                  # optional spaces
-                                     \"([^\"]*)\"         # quoted string (data)
-                                     \s*                  # optional spaces
-                                     (                    # bgroup
-                                      \#                  # comment leader
-                                      \s*                 # optional spaces
-                                      .*                  # string (comment)
-                                     )?                   # egroup, optional
-                                     $                    # eol
-                                """,
-                                re.VERBOSE)
-
 
 def read_child_line(line):
+    """
+    Turn one child CNF line into a dictionary.
+
+    :param str line: CNF line
+    :rtype: {str: Any}
+
+    This function only operates on individual lines. If the argument string is
+    syntactically valid but contains input representing multiple CNF vars, a
+    parse error will be thrown.
+    """
     if len(line.strip()) == 0:
         return None  # ignore empty lines
     if line[0] == "#":
@@ -491,122 +751,35 @@ def read_child_line(line):
     }
 
 
-def parse_cnf_children(state, parent):
-    lines = []
-    current = get(state)
-    while True:
-        cnf_line = read_child_line(current)
-        if cnf_line is not None:
-            lines.append(cnf_line)
-            state = advance(state)
-            if state is None:
-                break
-            current = get(state)
-            new_parent = get_parent(current)
-            if new_parent is None:
-                # drop stack
-                return (state, lines, None)
-            if new_parent > parent:
-                # parent is further down in hierarchy -> new level
-                (state, children, new_parent) = \
-                    parse_cnf_children (state, new_parent)
-                if state is None:
-                    break
-                cnf_line["children"] = children
-                current = get(state)
-                new_parent = get_parent(current)
-                if new_parent is None:
-                    # drop stack
-                    return (state, lines, None)
-            if new_parent < parent:
-                # parent is further up in hierarchy -> pop level
-                return (state, lines, new_parent)
-            # new_parent == parent -> continue parsing on same level
-    return (state, lines, parent)
-
-
-def parse_cnf_root(state):
-    lines = []
-    current = get(state)
-    while state:
-        cnf_line = read_base_line(current)
-        if cnf_line is not None:
-            lines.append(cnf_line)
-            state = advance(state)
-            if state is None:  # -> nothing left to do
-                break
-            current = get(state)
-            parent = get_parent(current)  # peek at next line
-            if parent is not None:  # -> recurse into children
-                (state, children, _parent) = parse_cnf_children(state, parent)
-                cnf_line["children"] = children
-                if state is None:
-                    break
-                current = get(state)
-    return lines
-
-
-def read_cnf(data):
-    """
-    Read cnf data from data bytes.
-
-    :param bytes data: raw data
-    :return: the parsed cnf data
-    :rtype: {str, {str, str or int}}
-    """
-    if isinstance (data, str):
-        data = to_latin1 (data)
-    state = prepare(data)
-    if state is None:
-        raise InvalidCNF("Empty input string.")
-
-    cnf = parse_cnf_root(state)
-    if is_cnf(cnf) is False:
-        raise TypeError("Invalid CNF_VAR.")
-    return {"cnf": cnf}
-
-
-def renumber_vars(root, parent=None, toplevel=False):
-    """
-    Number cnfvars linearly.
+###############################################################################
+# SERIALIZATION
+###############################################################################
 
-    If *parent* is specified, numbering will start at this offset. Also, the
-    VAR *root* will be assigned this number as a parent lineno unless
-    *toplevel* is set (the root var in a CNF tree obviously can’t have a
-    parent).
-
-    The *toplevel* parameter is useful when renumbering an existing variable
-    starting at a given offset without at the same time having that offset
-    assigned as a parent.
-    """
-    root = cnf_root (root)
-    i = parent or 0
-    for var in root:
-        i += 1
-        var["number"] = i
-        if toplevel is False and parent is not None:
-            var["parent"] = parent
-        children = var.get("children", None)
-        if children is not None:
-            i = renumber_vars(children, parent=i, toplevel=False)
-    return i
-
-#
-#                                serialization
-#
 
 cnf_line_nest_indent = "  "
 cnf_line_base_fmt    = "%d %s,%d: \"%s\""
 cnf_line_child_fmt   = "%d %s(%d) %s,%d: \"%s\""
 
+
 def format_cnf_vars(da, var):
     """
     Return a list of formatted cnf_line byte strings.
 
+    :param da: a tuple where the first element is the depth (0 = top-level,
+               >1 = child CNF) and the second is the string being built.
+    :type da: (int, str)
+    :param var: the CNF element to convert to string in the current iteration
+    :type var: dict
+    :returns: a tuple like `da`, where the second element should contain all
+              converted CNFs
+    :rtype: (int, str)
+
+    This function is meant to be passed to the :py:func:`functools.reduce`
+    function.
+
     The variable names are uppercased unconditionally because while ``get_cnf``
     is case-indifferent for variable names, ``set_cnf`` isn’t.
     """
-
     depth, acc = da
     line = None
     if depth > 0:
@@ -614,21 +787,21 @@ def format_cnf_vars(da, var):
             % (var["number"],
                cnf_line_nest_indent * depth,
                var["parent"],
-               var["varname"].upper (),
+               var["varname"].upper(),
                var["instance"],
                var["data"])
     else:
         line = cnf_line_base_fmt \
             % (var["number"],
-               var["varname"].upper (),
+               var["varname"].upper(),
                var["instance"],
                var["data"])
 
     comment = var.get("comment", None)
-    if comment and len (comment) != 0:
+    if comment and len(comment) != 0:
         line = line + (" # %s" % comment)
 
-    acc.append(to_latin1 (line))
+    acc.append(to_latin1(line))
 
     children = var.get("children", None)
     if children is not None:
@@ -637,84 +810,191 @@ def format_cnf_vars(da, var):
     return (depth, acc)
 
 
-CNF_FIELD_MANDATORY = set ([ "varname", "data", "instance" ])
-CNF_FIELD_OPTIONAL  = set ([ "parent", "children", "comment", "number" ])
-CNF_FIELD_KNOWN     = CNF_FIELD_MANDATORY | CNF_FIELD_OPTIONAL
-
-
-def is_cnf_var (obj):
+def cnf_root(root):
     """
-    Applies if all mandatory fields of a CNF_VAR and no unknown fields are
-    present.
-
-    .. XXX: validate field types.
+    Extract a list of CNFs from a given structure.
+
+    :param root: list of CNFs or a CNF dictionary
+    :type root: [dict] or dict
+    :raises: :py:class:`TypeError` if no CNFs can be extracted
+    :returns: list with one or more CNF objects
+    :rtype: [dict]
+
+    Output varies depending on a few conditions:
+    - If `root` is a list, return it right away
+    - If `root` is a dict corresponding to a valid CNF value, return it wrapped
+      in a list
+    - If `root` is a dict with a `cnf` key containg a list (as the JSON
+      returned by `get_cnf -j`), return the value
+    - Otherwise, raise an error
     """
-    assert isinstance (obj, dict)
-
-    for f in CNF_FIELD_MANDATORY:
-        if obj.get (f, None) is None:
-            return False
-
-    for f in obj:
-        if f not in CNF_FIELD_KNOWN:
-            return False
-
-    return True
-
-
-def cnf_root (root):
-    """
-    Of a given object, return the cnf root. This may be either a list
-    containing the object itself, if it satisfies the criteria for a CNF_VAR,
-    or the object contained in its toplevel field *cnf*.
-    """
-    if isinstance (root, list):
+    if isinstance(root, list):
         return root
     if not isinstance(root, dict):
         raise TypeError(
             "Expected dictionary of CNF_VARs, got %s." % type(root))
-    if is_cnf_var (root):
-        return [ root ]
+    if is_cnf_var(root):
+        return [root]
     cnf = root.get("cnf", None)
     if not isinstance(cnf, list):
         raise TypeError("Expected list of CNF_VARs, got %s." % type(cnf))
     return cnf
 
 
-def normalize_cnf (cnf):
+def normalize_cnf(cnf):
     """
     Ensure the output conforms to set_cnf()’s expectations.
+
+    :param cnf: list of CNF objects to normalize
+    :type cnf: [dict]
+    :returns: normalized list
+    :rtype: [list]
     """
-    if isinstance (cnf, list) is False:
-        raise MalformedCNF ("expected list of CNF_VARs, got [%s]" % type (cnf))
-    def norm (var):
+    if isinstance(cnf, list) is False:
+        raise MalformedCNF("expected list of CNF_VARs, got [%s]" % type(cnf))
+    def norm(var):
         vvar = \
             { "number"   : var ["number"]
-            , "varname"  : var ["varname"].upper ()
+            , "varname"  : var ["varname"].upper()
             , "instance" : var ["instance"]
             , "data"     : var ["data"]
             }
 
-        children = var.get ("children", None)
+        children = var.get("children", None)
         if children is not None:
-            vvar ["children"] = normalize_cnf (children)
+            vvar ["children"] = normalize_cnf(children)
+
+        parent = var.get("parent", None)
+        if parent is not None:
+            vvar ["parent"] = var["parent"]
+
+        comment = var.get("comment", None)
+        if comment is not None:
+            vvar ["comment"] = var["comment"]
 
         return vvar
 
-    return [ norm (var) for var in cnf ]
+    return [norm(var) for var in cnf]
 
 
-def print_cnf_raw(root, out=None):
-    """`if root is not None: out.write(root)`."""
-    if root is not None:
-        out.write(root)
+###############################################################################
+# TRAVERSAL
+###############################################################################
 
 
-def write_cnf_raw(*argv, **kw_argv):
-    """Alias for :py:func:`print_cnf_raw`."""
-    print_cnf_raw(*argv, **kw_argv)
+def walk_cnf(cnf, nested, fun, acc):
+    """
+    Depth-first traversal of a CNF tree.
+
+    :type cnf: cnf list
+    :type nested: bool
+    :type fun: 'a -> bool -> (cnf stuff) -> 'a
+    :type acc: 'a
+    :rtype: 'a
+
+    Executes ``fun`` recursively for each node in the tree. The function
+    receives the accumulator ``acc`` which can be of an arbitrary type as first
+    argument. The second argument is a flag indicating whether the current
+    CNF var is a child (if ``True``) or a parent var. CNF member fields are
+    passed via named optional arguments.
+    """
+    for var in cnf:
+        acc = fun(acc,
+                  nested,
+                  comment=var.get("comment", None),
+                  data=var.get("data", None),
+                  instance=var.get("instance", None),
+                  number=var.get("number", None),
+                  parent=var.get("parent", None),
+                  varname=var.get("varname", None))
+        children = var.get("children", None)
+        if children is not None:
+            acc = walk_cnf(children, True, fun, acc)
+    return acc
 
 
+def renumber_vars(root, parent=None, toplevel=False):
+    """
+    Number cnfvars linearly.
+
+    If *parent* is specified, numbering will start at this offset. Also, the
+    VAR *root* will be assigned this number as a parent lineno unless
+    *toplevel* is set (the root var in a CNF tree obviously can’t have a
+    parent).
+
+    The *toplevel* parameter is useful when renumbering an existing variable
+    starting at a given offset without at the same time having that offset
+    assigned as a parent.
+    """
+    root = cnf_root (root)
+    i = parent or 0
+    for var in root:
+        i += 1
+        var["number"] = i
+        if toplevel is False and parent is not None:
+            var["parent"] = parent
+        children = var.get("children", None)
+        if children is not None:
+            i = renumber_vars(children, parent=i, toplevel=False)
+    return i
+
+
+def count_vars(root):
+    """
+    Traverse the cnf structure recursively, counting VAR objects (CNF lines).
+    """
+    cnf = cnf_root(root)
+    if cnf is None:
+        raise InvalidCNF(root)
+    return walk_cnf(cnf, True, lambda n, _nested, **_kwa: n + 1, 0)
+
+#
+# querying
+#
+
+
+def get_vars(cnf, data=None, instance=None):
+    """
+    get_vars -- Query a CNF_VAR structure. Skims the *toplevel* of the CNF_VAR
+    structure for entries with a matching `data` or `instance` field.
+
+    :type cnf: CNF_VAR
+    :type data: str
+    :type instance: int
+    :rtype: CNF_VAR
+    :returns: The structure containing only references to the
+             matching variables. Containing an empty list of
+             variables in case there is no match.
+
+    Values are compared literally. If both ``instance`` and ``data`` are
+    specified, vars will be compared against both.
+    """
+    cnf = cnf["cnf"]
+    if cnf:
+        criterion = lambda _var: False
+        if data:
+            if instance:
+                criterion = lambda var: var[
+                    "data"] == data and var["instance"] == instance
+            else:
+                criterion = lambda var: var["data"] == data
+        elif instance:
+            criterion = lambda var: var["instance"] == instance
+
+        return {"cnf": [var for var in cnf if criterion(var) is True]}
+
+    return {"cnf": []}
+
+
+###############################################################################
+# PRINTING/DUMPING
+###############################################################################
+
+
+#
+# Print/dump raw CNF values
+#
+
 def output_cnf(root, out, renumber=False):
     """
     Dump a textual representation of given CNF VAR structure to given stream.
@@ -726,6 +1006,9 @@ def output_cnf(root, out, renumber=False):
     :type root: dict or list or anything that :py:func:`cnf_root` accepts
     :param out: file-like object or something with a `write(str)` function
     :param bool renumber: Whether to renumber cnfvars first
+
+    Files are converted to the 8-bit format expected by CNF so they can be fed
+    directly into libcnffile.
     """
     cnf = cnf_root(root)
     if renumber is True:
@@ -785,6 +1068,22 @@ def write_cnf(*argv, **kw_argv):
     print_cnf(*argv, **kw_argv)
 
 
+def print_cnf_raw(root, out=None):
+    """`if root is not None: out.write(root)`."""
+    if root is not None:
+        out.write(root)
+
+
+def write_cnf_raw(*argv, **kw_argv):
+    """Alias for :py:func:`print_cnf_raw`."""
+    print_cnf_raw(*argv, **kw_argv)
+
+
+#
+# Print/dump CNF values in JSON format
+#
+
+
 def output_json(root, out, renumber=False):
     """
     Dump CNF_VAR structure to file-like object in json format.
@@ -833,49 +1132,18 @@ def write_cnf_json(*argv, **kw_argv):
     """Alias for :py:func:`print_cnf_json`."""
     print_cnf_json(*argv, **kw_argv)
 
-#
-#                                  querying
-#
 
 
-def get_vars(cnf, data=None, instance=None):
-    """
-    get_vars -- Query a CNF_VAR structure. Skims the *toplevel* of the CNF_VAR
-    structure for entries with a matching `data` or `instance` field.
+###############################################################################
+# ENTRY POINT FOR DEVELOPMENT
+###############################################################################
 
-    :type cnf: CNF_VAR
-    :type data: str
-    :type instance: int
-    :rtype: CNF_VAR
-    :returns: The structure containing only references to the
-             matching variables. Containing an empty list of
-             variables in case there is no match.
-    """
-    cnf = cnf["cnf"]
-    if cnf:
-        criterion = lambda _var: False
-        if data:
-            if instance:
-                criterion = lambda var: var[
-                    "data"] == data and var["instance"] == instance
-            else:
-                criterion = lambda var: var["data"] == data
-        elif instance:
-            criterion = lambda var: var["instance"] == instance
 
-        return {"cnf": [var for var in cnf if criterion(var) is True]}
-
-    return {"cnf": []}
-
-def usage ():
-    print ("usage: cnfvar.py -"      , file=sys.stderr)
-    print (""                        , file=sys.stderr)
-    print ("    Read CNF from stdin.", file=sys.stderr)
-    print (""                        , file=sys.stderr)
-
-#
-#                         entry point for development
-#
+def usage():
+    print("usage: cnfvar.py -"      , file=sys.stderr)
+    print(""                        , file=sys.stderr)
+    print("    Read CNF from stdin.", file=sys.stderr)
+    print(""                        , file=sys.stderr)
 
 
 def main(argv):
@@ -890,8 +1158,8 @@ def main(argv):
             cnff = get_vars(cnf, instance=2, data="FAX")
             print_cnf(cnff)
             return 0
-    usage ()
+    usage()
     return -1
 
 if __name__ == "__main__":
-    sys.exit (main(sys.argv))
+    sys.exit(main(sys.argv))