🐛 An invalid predicates expression warns instead of ending the build

[chrisjsewell]

A predicates match expression on a needs_fields or needs_links default that
cannot be evaluated ends the entire build with a traceback, rather than being
reported like every other bad expression in the project.

What happens today

# conf.py
needs_fields = {
    "p_section": {"default": "no", "predicates": [["section_name == 'Alpha'", "yes"]]},
}

Alpha
=====

.. spec:: Specification 1
    :id: SPEC_1

Sphinx error!
sphinx_needs.exceptions.NeedsInvalidFilter: Predicate "section_name == 'Alpha'" not valid. Error: name 'section_name' is not defined.

The full traceback has been saved in: /tmp/sphinx-err-....log
To report this error to the developers, please open an issue at <https://github.com/sphinx-doc/sphinx/issues/>. Thanks!

No need is created, no other warning in the project is ever reported, nothing names
the document the expression failed on, and the user is pointed at Sphinx's issue
tracker for what is a typo in their own conf.py. needs_links predicates fail the
same way ("'body' in content" → name 'content' is not defined).

The trap is easy to fall into, because the names most likely to be reached for —
section_name, sections, content, lineno — all work in ordinary filters, and
none of them is available to a predicate.

What this changes

NeedsInvalidFilter is caught where the predicate is applied, in
_get_field_default / _get_links_default, and reported instead:

index.rst:4: WARNING: needs_fields['p_section']['predicates']: Predicate "section_name == 'Alpha'" not valid. Error: name 'section_name' is not defined. The predicate is skipped. [needs.config]

The predicate is then skipped: the remaining predicates are still evaluated, and if
none of them matches, the plain default applies — exactly as it does for a
predicate that simply did not match. The need is created and the build completes.

apply_default_predicate keeps its documented :raises NeedsInvalidFilter:
contract; the catch is at the call sites, which are the narrowest seam that has a
need location to report against.

The warning subtype

needs.config, reusing an existing subtype rather than adding one:

• a statically malformed predicates value is already reported as
  needs_fields['x']['predicates'] value is incorrect: ... [needs.config] by
  _set_predicates_on_field. This is the runtime half of the same mistake on the
  same configuration key, and now reads as its sibling;
• 🐛 needflow: fix the graphviz label escaper, and stop bad configuration ending the build #1780 took the same route for an unknown needs_flow_engine value — a bad
  configuration value, warned as needs.config at the use site with once=True,
  falling back to the sane default.

It is deduplicated once=True on its full message: a single configuration mistake —
like the name typo above, which fails identically for every need — is reported once
rather than once per need. An expression that fails only for some needs, or for
different reasons on different needs (say int(status) > 1 across varying status
values), is reported once per distinct error, which keeps each real failure visible.

One recorded limitation: the message names the canonical needs_fields /
needs_links key, so a predicate configured through the deprecated
needs_global_options, a needs_extra_links entry, or add_field(...) is labelled
with the canonical key rather than the one the project wrote. The field name in the
message always identifies the culprit, and threading true origins through the schema
is out of scope for a single-purpose fix.

Tests

test_invalid_predicate_default in tests/test_field_defaults.py, one build
covering: an invalid predicate on a field default (warning subtype, location and
text asserted; the plain default applies), the same on a link default, a field
whose plain default is absent (stays unset — current semantics pinned, not
changed), a valid predicate that still applies, and a valid predicate after a
broken one, which still wins over the plain default.

The test was written first and fails on master by killing the build out of
app.build(), which is the defect itself. The two existing tests in that module,
including their verbatim warning lists, are unchanged.

Related

Same class as #1537 — one bad value costing far more than itself — though a
different cause; this is not a duplicate of it.

The predicates documentation did not say what happens to an expression that
cannot be evaluated, so one sentence has been added to the "Default values"
subsections of both needs_fields and needs_links.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.05%. Comparing base (4e10030) to head (e836a54).
⚠️ Report is 335 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1791      +/-   ##
==========================================
+ Coverage   86.87%   91.05%   +4.17%     
==========================================
  Files          56       77      +21     
  Lines        6532    11688    +5156     
==========================================
+ Hits         5675    10643    +4968     
- Misses        857     1045     +188     

Flag	Coverage Δ
pytests	91.05% <100.00%> (+4.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.