🐛 Fix spurious 'Unknown option' warning for needs_string_links fields#1722
Open
cpolzer wants to merge 3 commits into
Open
🐛 Fix spurious 'Unknown option' warning for needs_string_links fields#1722cpolzer wants to merge 3 commits into
cpolzer wants to merge 3 commits into
Conversation
Fields declared via needs_string_links 'options' that are not also registered in needs_fields were rejected by create_need (and flagged by NeedDirective.run) as unknown, emitting a needs.directive warning and silently dropping the need. Fix: in create_schema(), after building the extra field schema from needs_fields, iterate needs_config.string_links and register any 'options' field names not yet in the schema as nullable string extra fields. This is the single source of truth consulted by both NeedDirective.run() and create_need(), so no other changes are needed. Adds tests/test_string_links.py with three regression tests: - no spurious warning when option is only in string_links - field value is stored in needs.json (verified via needs builder) - truly unknown fields still warn (guard test)
ba5a475 to
ddef10e
Compare
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fields declared via
needs_string_linksoptions(e.g.'options': ['github']) that are not also registered inneeds_fields/needs_extra_optionstrigger a spuriousneeds.directivewarning:and the need is silently dropped from the build output.
Root cause
create_schema()insphinx_needs/needs.pyonly populates the schema's extra field registry fromneeds_fieldsandneeds_extra_options. Theoptionsnames insideneeds_string_linksentries are never registered.Both
NeedDirective.run()andcreate_need()validate option keys againstschema.iter_extra_field_names()— any key absent from the registry hits the rejection path.Fix
After building the extra field schema from
needs_fields, iterateneeds_config.string_linksand register anyoptionsfield names not yet in the schema as nullable string extra fields. This is the correct layer — the schema is the single source of truth consulted by both the directive parser andcreate_need().Fields already registered via
needs_fieldsare skipped, so there is no conflict when a project declares the field in both places.Tests
New file
tests/test_string_links.pywith three regression tests:string_links optionsmust not emitUnknown optionneeds.json(verified via theneedsbuilder)needs_fieldsnorstring_links) still warnsAll existing
test_needtabletests (which useneeds_string_linkswith fields also inneeds_fields) continue to pass.Checklist
tox -e ruff-checkpassestox -e mypypassesdocs/changelog.rst, Unreleased > Bug fixes)