From 4bd8df600f7c0dbd0c4519163d5ad66d468ed6c7 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Mon, 6 Mar 2023 11:54:49 -0700 Subject: [PATCH] fix: avoid recursion errors when using `lists` functions together (#228) --- bzllib/private/lists.bzl | 99 +++++++++++++++++++++++++++--------- bzllib/tests/lists_tests.bzl | 26 ++++++++++ doc/bzllib/lists.md | 72 ++++++++++++++++++++------ 3 files changed, 156 insertions(+), 41 deletions(-) diff --git a/bzllib/private/lists.bzl b/bzllib/private/lists.bzl index 201e12f9..17d6619c 100644 --- a/bzllib/private/lists.bzl +++ b/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. diff --git a/bzllib/tests/lists_tests.bzl b/bzllib/tests/lists_tests.bzl index 00f20461..b123202f 100644 --- a/bzllib/tests/lists_tests.bzl +++ b/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, ) diff --git a/doc/bzllib/lists.md b/doc/bzllib/lists.md index 9a54df94..d735adfe 100755 --- a/doc/bzllib/lists.md +++ b/doc/bzllib/lists.md @@ -10,7 +10,7 @@ lists.compact(items) -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. @@ -32,7 +32,20 @@ A `list` of items with the `None` values removed. lists.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`. + **PARAMETERS** @@ -40,11 +53,11 @@ Determines if the provide value is found in a list. | Name | Description | Default Value | | :------------- | :------------- | :------------- | | items | A list of items to evaluate. | none | -| 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. | none | +| 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. | 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. @@ -55,7 +68,7 @@ A `bool` indicating whether the target item was found in the list. lists.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`. **PARAMETERS** @@ -63,11 +76,12 @@ Returns a new `list` with the items that satisfy the boolean function. | Name | Description | Default Value | | :------------- | :------------- | :------------- | | items | A list of items to evaluate. | none | -| bool_fn | A function that takes a single parameter (list item) and returns a bool indicating whether the meets the criteria. | none | +| bool_fn | A function that takes a single parameter returns a bool 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`. @@ -78,7 +92,13 @@ A `list` of the provided items that satisfy the boolean function. lists.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`. + **PARAMETERS** @@ -86,7 +106,7 @@ Returns the list item that satisfies the provide boolean function. | Name | Description | Default Value | | :------------- | :------------- | :------------- | | items | A list of items to evaluate. | none | -| bool_fn | A function that takes a single parameter (list item) and returns a bool indicating whether the meets the criteria. | none | +| bool_fn | A function that takes a single parameter and returns a bool value. | none | **RETURNS** @@ -98,13 +118,32 @@ A list item or `None`. ## lists.flatten
-lists.flatten(items)
+lists.flatten(items, max_iterations)
 
-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 | | :------------- | :------------- | :------------- | | items | A list or a single item. | none | +| max_iterations | Optional. The maximum number of processing iterations. | 10000 | **RETURNS** @@ -128,7 +168,7 @@ A `list` with all of the items flattened (i.e., no items in the result lists.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`. +Returns a new `list` where each item is the result of calling the map `function` on each item in the original `list`. **PARAMETERS** @@ -136,7 +176,7 @@ Returns a new `list` where each item is the result of calling the map functi | Name | Description | Default Value | | :------------- | :------------- | :------------- | | items | A list of items to evaluate. | none | -| 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. | none | +| map_fn | A function that takes a single parameter returns a value that will be added to the new list at the correspnding location. | none | **RETURNS**