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 all 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
6 changes: 6 additions & 0 deletions docs/changelog.rst
Expand Up @@ -8,6 +8,12 @@ License

.. include:: ../LICENSE

Unreleased
----------

* 馃憣 Fail and emit warning on filters that do not return a boolean result
(`#964 <https://github.com/useblocks/sphinx-needs/pull/964>`_)


2.0.0
-----
Expand Down
4 changes: 2 additions & 2 deletions docs/directives/need.rst
Expand Up @@ -267,14 +267,14 @@ are representing some kind of a diagram.
.. code-block:: rst

.. needtable::
:filter: arch
:filter: bool(arch)
:style: table
:columns: id, type, title

|out|

.. needtable::
:filter: arch
:filter: bool(arch)
:style: table
:columns: id, type, title

Expand Down
14 changes: 11 additions & 3 deletions sphinx_needs/filter_common.py
Expand Up @@ -354,6 +354,10 @@ def filter_needs(
return found_needs


def need_search(*args: Any, **kwargs: Any) -> bool:
return re.search(*args, **kwargs) is not None


@measure_time("filtering")
def filter_single_need(
need: NeedsInfoType,
Expand Down Expand Up @@ -385,15 +389,19 @@ def filter_single_need(
# Get needs external filter data and merge to filter_context
filter_context.update(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):
raise NeedsInvalidFilter(
f"Filter did not evaluate to a boolean, instead {type(result)}: {result}"
)
except Exception as e:
raise NeedsInvalidFilter(f"Filter {filter_string!r} not valid. Error: {e}.")
return result
3 changes: 3 additions & 0 deletions tests/doc_test/filter_doc/index.rst
Expand Up @@ -51,6 +51,9 @@ Testing bad filters
.. needfilter::
:filter: xxx

.. needfilter::
:filter: 1

.. needlist::
:filter: yyy

Expand Down
5 changes: 3 additions & 2 deletions tests/test_filter.py
Expand Up @@ -20,8 +20,9 @@ def test_filter_build_html(test_app):
print(w)
assert warnings == [
"srcdir/index.rst:51: WARNING: Filter 'xxx' not valid. Error: name 'xxx' is not defined. [needs.filter]",
"srcdir/index.rst:54: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
"srcdir/index.rst:57: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
"srcdir/index.rst:54: WARNING: Filter '1' not valid. Error: Filter did not evaluate to a boolean, instead <class 'int'>: 1. [needs.filter]",
"srcdir/index.rst:57: WARNING: Filter 'yyy' not valid. Error: name 'yyy' is not defined. [needs.filter]",
"srcdir/index.rst:60: WARNING: Filter 'zzz' not valid. Error: name 'zzz' is not defined. [needs.filter]",
]

html = Path(app.outdir, "index.html").read_text()
Expand Down