fix: avoid recursion errors when using lists functions together (#228)

bzllib/private/lists.bzl

@@ -1,41 +1,68 @@
 """Module for managing Starlark `list` values."""
 
 def _compact(items):
-    """Returns the provide items with any `None` values removed.
+    """Returns a new `list` with any `None` values removed.
 
     Args:
         items: A `list` of items to evaluate.
 
     Returns:
-        A `list` of items with the `None` values removed.
+        A new `list` of items with the `None` values removed.
     """
-    return _filter(items, lambda x: x != None)
+
+    # We are intentionally not calling _filter(). We want to avoid recursion
+    # errors, if these functions are used together.
+    return [item for item in items if item != None]
 
 def _contains(items, target_or_fn):
-    """Determines if the provide value is found in a list.
+    """Determines if a value exists in the provided `list`.
+
+    If a boolean function is provided as the second argument, the function is
+    evaluated against the items in the list starting from the first item. If
+    the result of the boolean function call is `True`, processing stops and
+    this function returns `True`. If no items satisfy the boolean function,
+    this function returns `False`.
+
+    If the second argument is not a `function` (i.e., the target), each item in
+    the list is evaluated for equality (==) with the target. If the equality
+    evaluation returns `True` for an item in the list, processing stops and
+    this function returns `True`. If no items are found to be equal to the
+    target, this function returns `False`.
 
     Args:
         items: A `list` of items to evaluate.
-        target_or_fn: The item that may be contained in the items list or a
-            `function` that takes a single value and returns a `bool`.
+        target_or_fn: An item to be evaluated for equality or a boolean
+            `function`. A boolean `function` is defined as one that takes a
+            single argument and returns a `bool` value.
 
     Returns:
-        A `bool` indicating whether the target item was found in the list.
+        A `bool` indicating whether an item was found in the list.
     """
     if type(target_or_fn) == "function":
         bool_fn = target_or_fn
     else:
         bool_fn = lambda x: x == target_or_fn
-    item = _find(items, bool_fn)
-    return item != None
+
+    # We are intentionally not calling _find(). We want to be able to use the
+    # lists functions together. For instance, we want to be able to use
+    # lists.contains inside the lambda for lists.find.
+    for item in items:
+        if bool_fn(item):
+            return True
+    return False
 
 def _find(items, bool_fn):
-    """Returns the list item that satisfies the provide boolean function.
+    """Returns the list item that satisfies the provided boolean `function`.
+
+    The boolean `function` is evaluated against the items in the list starting
+    from the first item. If the result of the boolean function call is `True`,
+    processing stops and this function returns item. If no items satisfy the
+    boolean function, this function returns `None`.
 
     Args:
         items: A `list` of items to evaluate.
-        bool_fn: A `function` that takes a single parameter (list item) and
-            returns a `bool` indicating whether the meets the criteria.
+        bool_fn: A `function` that takes a single parameter and returns a `bool`
+            value.
 
     Returns:
         A list item or `None`.
@@ -45,14 +72,35 @@ def _find(items, bool_fn):
             return item
     return None
 
-def _flatten(items):
-    """Flattens the items to a single list.
+def _flatten(items, max_iterations = 10000):
+    """Flattens a `list` containing an arbitrary number of child `list` values \
+    to a new `list` with the items from the original `list` values.
+
+    Every effort is made to preserve the order of the flattened list items
+    relative to their order in the child `list` values. For instance, an input
+    of `["foo", ["alpha", ["omega"]], ["chicken", "cow"]]` to this function
+    returns `["foo", "alpha", "omega", "chicken", "cow"]`.
+
+    If provided a `list` value, each item in the `list` is evaluated for
+    inclusion in the result.  If the item is not a `list`, the item is added to
+    the result. If the item is a `list`, the items in the child `list` are
+    added to the result and the result is marked for another round of
+    processing. Once the result has been processed without detecting any child
+    `list` values, the result is returned.
+
+    If provided a value that is not a `list`, the value is wrapped in a list
+    and returned.
 
-    If provided a single item, it is wrapped in a list and processed as if
-    provided as a `list`.
+    Because Starlark does not support recursion or boundless looping, the
+    processing of the input is restricted to a fixed number of processing
+    iterations. The default for the maximum number of iterations should be
+    sufficient for processing most multi-level `list` values. However, if you
+    need to change this value, you can specify the `max_iterations` value to
+    suit your needs.
 
     Args:
         items: A `list` or a single item.
+        max_iterations: Optional. The maximum number of processing iterations.
 
     Returns:
         A `list` with all of the items flattened (i.e., no items in the result
@@ -64,7 +112,7 @@ def _flatten(items):
         results = [items]
 
     finished = False
-    for _ in range(1000):
+    for _ in range(max_iterations):
         if finished:
             break
         finished = True
@@ -83,27 +131,28 @@ def _flatten(items):
     return results
 
 def _filter(items, bool_fn):
-    """Returns a new `list` with the items that satisfy the boolean function.
+    """Returns a new `list` containing the items from the original that \
+    satisfy the specified boolean `function`.
 
     Args:
         items: A `list` of items to evaluate.
-        bool_fn: A `function` that takes a single parameter (list item) and
-            returns a `bool` indicating whether the meets the criteria.
+        bool_fn: A `function` that takes a single parameter returns a `bool`
+            value.
 
     Returns:
-        A `list` of the provided items that satisfy the boolean function.
+        A new `list` containing the items that satisfy the provided boolean
+        `function`.
     """
     return [item for item in items if bool_fn(item)]
 
 def _map(items, map_fn):
     """Returns a new `list` where each item is the result of calling the map \
-    function on each item in the original `list`.
+    `function` on each item in the original `list`.
 
     Args:
         items: A `list` of items to evaluate.
-        map_fn: A `function` that takes a single parameter (list item) and
-            returns a value that will be added to the new list at the
-            correspnding location.
+        map_fn: A `function` that takes a single parameter returns a value that
+            will be added to the new list at the correspnding location.
 
     Returns:
         A `list` with the transformed values.

bzllib/tests/lists_tests.bzl

@@ -193,6 +193,31 @@ def _map_test(ctx):
 
 map_test = unittest.make(_map_test)
 
+def _avoid_recursion_test(ctx):
+    env = unittest.begin(ctx)
+
+    fruits = ["apple", "pear", "cherry"]
+    fav_fruits = ["apple", "cherry", "banana"]
+
+    # Find the first favorite fruit
+    actual = lists.find(
+        fruits,
+        lambda x: lists.contains(fav_fruits, x),
+    )
+    asserts.equals(env, "apple", actual)
+
+    # This is admittedly a very inefficient way to use these functions
+    # together. It is here to ensure that a recursion error does not occur.
+    actual = lists.filter(
+        fruits,
+        lambda x: lists.contains(lists.compact(fav_fruits), x),
+    )
+    asserts.equals(env, ["apple", "cherry"], actual)
+
+    return unittest.end(env)
+
+avoid_recursion_test = unittest.make(_avoid_recursion_test)
+
 def lists_test_suite():
     return unittest.suite(
         "lists_tests",
@@ -202,4 +227,5 @@ def lists_test_suite():
         flatten_test,
         filter_test,
         map_test,
+        avoid_recursion_test,
     )

doc/bzllib/lists.md

@@ -10,7 +10,7 @@
 lists.compact(<a href="#lists.compact-items">items</a>)
 </pre>
 
-Returns the provide items with any `None` values removed.
+Returns a new `list` with any `None` values removed.
 
 **PARAMETERS**
 
@@ -21,7 +21,7 @@ Returns the provide items with any `None` values removed.
 
 **RETURNS**
 
-A `list` of items with the `None` values removed.
+A new `list` of items with the `None` values removed.
 
 
 <a id="lists.contains"></a>
@@ -32,19 +32,32 @@ A `list` of items with the `None` values removed.
 lists.contains(<a href="#lists.contains-items">items</a>, <a href="#lists.contains-target_or_fn">target_or_fn</a>)
 </pre>
 
-Determines if the provide value is found in a list.
+Determines if a value exists in the provided `list`.
+
+If a boolean function is provided as the second argument, the function is
+evaluated against the items in the list starting from the first item. If
+the result of the boolean function call is `True`, processing stops and
+this function returns `True`. If no items satisfy the boolean function,
+this function returns `False`.
+
+If the second argument is not a `function` (i.e., the target), each item in
+the list is evaluated for equality (==) with the target. If the equality
+evaluation returns `True` for an item in the list, processing stops and
+this function returns `True`. If no items are found to be equal to the
+target, this function returns `False`.
+
 
 **PARAMETERS**
 
 
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="lists.contains-items"></a>items |  A <code>list</code> of items to evaluate.   |  none |
-| <a id="lists.contains-target_or_fn"></a>target_or_fn |  The item that may be contained in the items list or a <code>function</code> that takes a single value and returns a <code>bool</code>.   |  none |
+| <a id="lists.contains-target_or_fn"></a>target_or_fn |  An item to be evaluated for equality or a boolean <code>function</code>. A boolean <code>function</code> is defined as one that takes a single argument and returns a <code>bool</code> value.   |  none |
 
 **RETURNS**
 
-A `bool` indicating whether the target item was found in the list.
+A `bool` indicating whether an item was found in the list.
 
 
 <a id="lists.filter"></a>
@@ -55,19 +68,20 @@ A `bool` indicating whether the target item was found in the list.
 lists.filter(<a href="#lists.filter-items">items</a>, <a href="#lists.filter-bool_fn">bool_fn</a>)
 </pre>
 
-Returns a new `list` with the items that satisfy the boolean function.
+Returns a new `list` containing the items from the original that     satisfy the specified boolean `function`.
 
 **PARAMETERS**
 
 
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="lists.filter-items"></a>items |  A <code>list</code> of items to evaluate.   |  none |
-| <a id="lists.filter-bool_fn"></a>bool_fn |  A <code>function</code> that takes a single parameter (list item) and returns a <code>bool</code> indicating whether the meets the criteria.   |  none |
+| <a id="lists.filter-bool_fn"></a>bool_fn |  A <code>function</code> that takes a single parameter returns a <code>bool</code> value.   |  none |
 
 **RETURNS**
 
-A `list` of the provided items that satisfy the boolean function.
+A new `list` containing the items that satisfy the provided boolean
+  `function`.
 
 
 <a id="lists.find"></a>
@@ -78,15 +92,21 @@ A `list` of the provided items that satisfy the boolean function.
 lists.find(<a href="#lists.find-items">items</a>, <a href="#lists.find-bool_fn">bool_fn</a>)
 </pre>
 
-Returns the list item that satisfies the provide boolean function.
+Returns the list item that satisfies the provided boolean `function`.
+
+The boolean `function` is evaluated against the items in the list starting
+from the first item. If the result of the boolean function call is `True`,
+processing stops and this function returns item. If no items satisfy the
+boolean function, this function returns `None`.
+
 
 **PARAMETERS**
 
 
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="lists.find-items"></a>items |  A <code>list</code> of items to evaluate.   |  none |
-| <a id="lists.find-bool_fn"></a>bool_fn |  A <code>function</code> that takes a single parameter (list item) and returns a <code>bool</code> indicating whether the meets the criteria.   |  none |
+| <a id="lists.find-bool_fn"></a>bool_fn |  A <code>function</code> that takes a single parameter and returns a <code>bool</code> value.   |  none |
 
 **RETURNS**
 
@@ -98,13 +118,32 @@ A list item or `None`.
 ## lists.flatten
 
 <pre>
-lists.flatten(<a href="#lists.flatten-items">items</a>)
+lists.flatten(<a href="#lists.flatten-items">items</a>, <a href="#lists.flatten-max_iterations">max_iterations</a>)
 </pre>
 
-Flattens the items to a single list.
+Flattens a `list` containing an arbitrary number of child `list` values     to a new `list` with the items from the original `list` values.
+
+Every effort is made to preserve the order of the flattened list items
+relative to their order in the child `list` values. For instance, an input
+of `["foo", ["alpha", ["omega"]], ["chicken", "cow"]]` to this function
+returns `["foo", "alpha", "omega", "chicken", "cow"]`.
+
+If provided a `list` value, each item in the `list` is evaluated for
+inclusion in the result.  If the item is not a `list`, the item is added to
+the result. If the item is a `list`, the items in the child `list` are
+added to the result and the result is marked for another round of
+processing. Once the result has been processed without detecting any child
+`list` values, the result is returned.
+
+If provided a value that is not a `list`, the value is wrapped in a list
+and returned.
 
-If provided a single item, it is wrapped in a list and processed as if
-provided as a `list`.
+Because Starlark does not support recursion or boundless looping, the
+processing of the input is restricted to a fixed number of processing
+iterations. The default for the maximum number of iterations should be
+sufficient for processing most multi-level `list` values. However, if you
+need to change this value, you can specify the `max_iterations` value to
+suit your needs.
 
 
 **PARAMETERS**
@@ -113,6 +152,7 @@ provided as a `list`.
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="lists.flatten-items"></a>items |  A <code>list</code> or a single item.   |  none |
+| <a id="lists.flatten-max_iterations"></a>max_iterations |  Optional. The maximum number of processing iterations.   |  <code>10000</code> |
 
 **RETURNS**
 
@@ -128,15 +168,15 @@ A `list` with all of the items flattened (i.e., no items in the result
 lists.map(<a href="#lists.map-items">items</a>, <a href="#lists.map-map_fn">map_fn</a>)
 </pre>
 
-Returns a new `list` where each item is the result of calling the map     function on each item in the original `list`.
+Returns a new `list` where each item is the result of calling the map     `function` on each item in the original `list`.
 
 **PARAMETERS**
 
 
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="lists.map-items"></a>items |  A <code>list</code> of items to evaluate.   |  none |
-| <a id="lists.map-map_fn"></a>map_fn |  A <code>function</code> that takes a single parameter (list item) and returns a value that will be added to the new list at the correspnding location.   |  none |
+| <a id="lists.map-map_fn"></a>map_fn |  A <code>function</code> that takes a single parameter returns a value that will be added to the new list at the correspnding location.   |  none |
 
 **RETURNS**