feat!: needextend effects must be local - #689
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //src:license-checkStatus: Click to expand output |
There was a problem hiding this comment.
Pull request overview
This PR activates and refactors the SCORE “needextend must be local to the current document” policy enforcement, updates documentation to reflect the now-active constraint, and adds/adjusts tests so warnings can be asserted for both needs and needextend directives.
Changes:
- Refactors
check_needs_extendsinto smaller helpers and enforces explicitc.this_doc()usage plus cross-document match detection. - Adds unit tests for cross-document needextend matches and updates the file-based RST test case to expect the new policy warning.
- Extends the file-based warning expectation runner to read
:expect:/:expect_not:annotations from needextend directives (via modifications).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extensions/score_metamodel/tests/test_rules_file_based.py | Adds helper to read expectations from needextend modifications and uses it in warning checks. |
| src/extensions/score_metamodel/tests/test_check_needs_extends.py | New mocked unit tests verifying cross-document matching produces the expected warning. |
| src/extensions/score_metamodel/tests/rst/options/test_need_extends.rst | Activates an RST-level expectation for missing c.this_doc() on a needextend filter. |
| src/extensions/score_metamodel/checks/check_needs_extends.py | Refactors and strengthens the needextend policy checks (explicit c.this_doc() + cross-document match detection). |
| docs/how-to/write_docs.rst | Updates documentation note to reflect the now-enforced local-only needextend rule. |
| try: | ||
| return filter_needs_mutable( | ||
| all_needs, | ||
| needs_config, | ||
| need_filter, | ||
| location=location, | ||
| origin_docname=location[0], | ||
| ) | ||
| except Exception as e: | ||
| _warn(f"Invalid filter {need_filter!r}: {e}", location) | ||
| return [] |
| need_id = needextend["filter"] | ||
| try: | ||
| return [all_needs[need_id]] | ||
| except KeyError: | ||
| _warn( | ||
| f"Provided id {need_id!r} for needextend does not exist.", | ||
| location, | ||
| ) | ||
| return [] |
|
|
||
| line_nr = need.get("lineno") | ||
|
|
||
| for raw in cast("list[str]", need.get("expect") or []): | ||
| for raw in _get_expectations(need, "expect"): |
There was a problem hiding this comment.
Looks ok to me. No need to fix Copilot's remarks.
We need to fix process though:
/home/runner/.bazel/sandbox/processwrapper-sandbox/5/execroot/_main/external/score_process+/process/standards/isosae21434/isosae21434.rst:592: WARNING: needextend in S-CORE must always be used per document only. Please add 'c.this_doc()' to the needextend to limit its effects to the correct document. See https://eclipse-score.github.io/docs-as-code/main/how-to/write_docs.html#needextend for more information. [needs.needextend]
We wanted to ensure local needextend for a while, but it seems we forgot to actually activate the check. Besides activating, this PR also improves and fixes the check itself.
Downstream PRs: