Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃憣 Fail and emit warning on filters that do not return a boolean result #964

Merged
merged 20 commits into from Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/changelog.rst
Expand Up @@ -30,6 +30,8 @@ Released: 16.08.2023
for debugging purposes.
(`#917 <https://github.com/useblocks/sphinx-needs/pull/917>`_)

* Bugfix: Check filter strings for correctness.
(`#964 <https://github.com/useblocks/sphinx-needs/pull/964>`_)
* Bugfix: Replace hardcoded `index` with config value `root_doc`.
(`#877 <https://github.com/useblocks/sphinx-needs/pull/877>`_)
* Bugfix: Fix unbounded memory usage in pickle environment.
Expand Down
15 changes: 12 additions & 3 deletions sphinx_needs/filter_common.py
Expand Up @@ -299,6 +299,10 @@ def filter_needs(
return found_needs


def need_search(*args, **kwargs):
big1hc marked this conversation as resolved.
Show resolved Hide resolved
return bool(re.search(*args, **kwargs))
big1hc marked this conversation as resolved.
Show resolved Hide resolved


@measure_time("filtering")
def filter_single_need(
app: Sphinx,
Expand Down Expand Up @@ -330,15 +334,20 @@ def filter_single_need(
# Get needs external filter data and merge to filter_context
filter_context.update(NeedsSphinxConfig(app.config).filter_data)

filter_context["search"] = re.search
filter_context["search"] = need_search
result = False
try:
# Set filter_context as globals and not only locals in eval()!
# Otherwise, the vars not be accessed in list comprehensions.
if filter_compiled:
result = bool(eval(filter_compiled, filter_context))
result = eval(filter_compiled, filter_context)
else:
result = bool(eval(filter_string, filter_context))
result = eval(filter_string, filter_context)
if not isinstance(result, bool):
# Raises NeedsInvalidFilter if the result is a string type
raise NeedsInvalidFilter(
f"Error when evaluating filter: expected output to have True/False but got a string <{result}>"
)
big1hc marked this conversation as resolved.
Show resolved Hide resolved
big1hc marked this conversation as resolved.
Show resolved Hide resolved
except Exception as e:
raise NeedsInvalidFilter(f"Filter {filter_string} not valid. Error: {e}.")
return result