diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index fc0b75908..5897cfec1 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -13,10 +13,8 @@ import importlib import os import pkgutil -import re from collections.abc import Callable from pathlib import Path -from typing import Any from sphinx.application import Sphinx from sphinx_needs import logging @@ -102,6 +100,13 @@ def _run_checks(app: Sphinx, exception: Exception | None) -> None: if exception: return + # First of all postprocess the need links to convert + # type names into actual need types. + # This must be done before any checks are run. + # And it must be done after config was hashed, otherwise + # the config hash would include recusive linking between types. + postprocess_need_links(app.config.needs_types) + # Filter out external needs, as checks are only intended to be run # on internal needs. needs_all_needs = SphinxNeedsData(app.env).get_needs_view() @@ -171,59 +176,53 @@ def _get_need_type_for_need(app: Sphinx, need: NeedsInfoType) -> ScoreNeedType: raise ValueError(f"Need type {need['type']} not found in needs_types") -def _validate_external_need_opt_links( - need: NeedsInfoType, - opt_links: dict[str, str], - allowed_prefixes: list[str], - log: CheckLogger, -) -> None: - for link_field, pattern in opt_links.items(): - raw_value: str | list[str] | None = need.get(link_field, None) - if raw_value in [None, [], ""]: - continue - - values: list[str | Any] = ( - raw_value if isinstance(raw_value, list) else [raw_value] - ) - for value in values: - v: str | Any - if isinstance(value, str): - v = _remove_prefix(value, allowed_prefixes) - else: - v = value - - try: - if not isinstance(v, str) or not re.match(pattern, v): - log.warning_for_option( - need, - link_field, - f"does not follow pattern `{pattern}`.", - is_new_check=True, - ) - except TypeError: - log.warning_for_option( - need, - link_field, - f"pattern `{pattern}` is not a valid regex pattern.", - is_new_check=True, - ) - - -def _check_external_optional_link_patterns(app: Sphinx, log: CheckLogger) -> None: - """Validate optional link patterns on external needs and log as info-only. - - Mirrors the original inline logic from ``_run_checks`` without changing behavior. +def _resolve_linkable_types( + link_name: str, + link_value: str, + current_need_type: ScoreNeedType, + needs_types: list[ScoreNeedType], +) -> list[ScoreNeedType]: + needs_types_dict = {nt["directive"]: nt for nt in needs_types} + link_values = [v.strip() for v in link_value.split(",")] + linkable_types: list[ScoreNeedType] = [] + for v in link_values: + target_need_type = needs_types_dict.get(v) + if target_need_type is None: + logger.error( + f"In metamodel.yaml: {current_need_type['directive']}, " + f"link '{link_name}' references unknown type '{v}'." + ) + else: + linkable_types.append(target_need_type) + return linkable_types + + +def postprocess_need_links(needs_types_list: list[ScoreNeedType]): + """Convert link option strings into lists of target need types. + + If a link value starts with '^' it is treated as a regex and left + unchanged. Otherwise it is a comma-separated list of type names which + are resolved to the corresponding ScoreNeedTypes. """ - needs_external_needs = ( - SphinxNeedsData(app.env).get_needs_view().filter_is_external(True) - ) + for need_type in needs_types_list: + try: + link_dicts = ( + need_type["mandatory_links"], + need_type["optional_links"], + ) + except KeyError: + # TODO: remove the Sphinx-Needs defaults from our metamodel + # Example: {'directive': 'issue', 'title': 'Issue', 'prefix': 'IS_'} + continue - for need in needs_external_needs.values(): - need_type = _get_need_type_for_need(app, need) + for link_dict in link_dicts: + for link_name, link_value in link_dict.items(): + assert isinstance(link_value, str) # so far all of them are strings - if opt_links := need_type["optional_links"]: - allowed_prefixes = app.config.allowed_external_prefixes - _validate_external_need_opt_links(need, opt_links, allowed_prefixes, log) + if not link_value.startswith("^"): + link_dict[link_name] = _resolve_linkable_types( # pyright: ignore[reportArgumentType] + link_name, link_value, need_type, needs_types_list + ) def setup(app: Sphinx) -> dict[str, str | bool]: diff --git a/src/extensions/score_metamodel/checks/check_options.py b/src/extensions/score_metamodel/checks/check_options.py index 2c23c0a6c..42953c672 100644 --- a/src/extensions/score_metamodel/checks/check_options.py +++ b/src/extensions/score_metamodel/checks/check_options.py @@ -60,10 +60,49 @@ def _validate_value_pattern( ) from e +def _log_option_warning( + need: NeedsInfoType, + log: CheckLogger, + field_type: str, + allowed_directives: list[ScoreNeedType] | None, + field: str, + value: str | list[str], + allowed_value: str | list[str], + required: bool, +): + if field_type == "link": + if allowed_directives: + dirs = " or ".join( + f"{d['title']} ({d['directive']})" for d in allowed_directives + ) + msg = f"but it must reference {dirs}." + else: + msg = f"which does not follow pattern `{allowed_value}`." + + # warning_for_option will print all the values. This way the specific + # problematic value is highlighted in the message. + # This is especially useful if multiple values are given. + msg = f"references '{value}' as '{field}', {msg}" + log.warning_for_need( + need, + msg, + # TODO: Errors in optional links are non fatal for now + is_new_check=not required, + ) + else: + msg = f"does not follow pattern `{allowed_value}`." + log.warning_for_option( + need, + field, + msg, + is_new_check=False, + ) + + def validate_fields( need: NeedsInfoType, log: CheckLogger, - fields: dict[str, str], + fields: dict[str, str] | dict[str, list[ScoreNeedType]], required: bool, field_type: str, allowed_prefixes: list[str], @@ -83,8 +122,6 @@ def remove_prefix(word: str, prefixes: list[str]) -> str: # Removes any prefix allowed by configuration, if prefix is there. return [word.removeprefix(prefix) for prefix in prefixes][0] - optional_link_as_info = (not required) and (field_type == "link") - for field, allowed_value in fields.items(): raw_value: str | list[str] | None = need.get(field, None) if raw_value in [None, [], ""]: @@ -96,17 +133,37 @@ def remove_prefix(word: str, prefixes: list[str]) -> str: values = _normalize_values(raw_value) + # Links can be configured to reference other need types instead of regex. + # However, in order to not "load" the other need, we'll check the regex as + # it does encode the need type (at least in S-CORE metamodel). + # Therefore this can remain a @local_check! + # TypedDicts cannot be used with isinstance, so check for dict and required keys + if isinstance(allowed_value, list): + assert field_type == "link" # sanity check + # patterns holds a list of allowed need types + allowed_directives = allowed_value + allowed_value = ( + "(" + + "|".join(d["mandatory_options"]["id"] for d in allowed_directives) + + ")" + ) + else: + allowed_directives = None + # regex based validation for value in values: if allowed_prefixes: value = remove_prefix(value, allowed_prefixes) if not _validate_value_pattern(value, allowed_value, need, field): - msg = f"does not follow pattern `{allowed_value}`." - log.warning_for_option( + _log_option_warning( need, + log, + field_type, + allowed_directives, field, - msg, - is_new_check=optional_link_as_info, + value, + allowed_value, + required, ) @@ -132,7 +189,9 @@ def check_options( allowed_prefixes = app.config.allowed_external_prefixes # Validate Options and Links - field_validations = [ + field_validations: list[ + tuple[str, dict[str, str] | dict[str, list[ScoreNeedType]], bool] + ] = [ ("option", need_type["mandatory_options"], True), ("option", need_type["optional_options"], False), ("link", need_type["mandatory_links"], True), diff --git a/src/extensions/score_metamodel/metamodel.yaml b/src/extensions/score_metamodel/metamodel.yaml index b2a28065f..219afeded 100644 --- a/src/extensions/score_metamodel/metamodel.yaml +++ b/src/extensions/score_metamodel/metamodel.yaml @@ -108,12 +108,12 @@ needs_types: mandatory_options: status: ^(valid|draft)$ mandatory_links: - input: ^wp__.*$ - output: ^wp__.*$ - approved_by: ^rl__.*$ - responsible: ^rl__.*$ + input: workproduct + output: workproduct + approved_by: role + responsible: role optional_links: - supported_by: ^rl__.*$ + supported_by: role contains: ^gd_(req|temp|chklst|guidl|meth)__.*$ has: ^doc_(getstrt|concept)__.*$ parts: 2 @@ -128,7 +128,7 @@ needs_types: optional_links: # req-Id: tool_req__docs_req_link_satisfies_allowed # TODO: fix once process_description is fixed - satisfies: ^wf__.*$ + satisfies: workflow complies: ^std_req__(iso26262|isosae21434|isopas8926|aspice_40)__.*$ tags: - requirement @@ -182,7 +182,7 @@ needs_types: title: Role prefix: rl__ optional_links: - contains: ^rl__.*$ + contains: role parts: 2 # Documents, process_description only @@ -212,7 +212,7 @@ needs_types: approver: ^.*$ reviewer: ^.*$ optional_links: - realizes: "^wp__.+$" + realizes: workproduct parts: 2 # req-Id: tool_req__docs_doc_types @@ -233,7 +233,7 @@ needs_types: approver: ^.*$ reviewer: ^.*$ optional_links: - realizes: "^wp__.+$" + realizes: workproduct parts: 2 # Requirements @@ -282,7 +282,7 @@ needs_types: content: ^[\s\S]+$ mandatory_links: # req-Id: tool_req__docs_req_link_satisfies_allowed - satisfies: ^stkh_req__.*$ + satisfies: stkh_req optional_options: codelink: ^.*$ testlink: ^.*$ @@ -311,7 +311,7 @@ needs_types: content: ^[\s\S]+$ mandatory_links: # req-Id: tool_req__docs_req_link_satisfies_allowed - satisfies: ^feat_req__.*$ + satisfies: feat_req optional_options: codelink: ^.*$ testlink: ^.*$ @@ -339,7 +339,7 @@ needs_types: optional_links: # req-Id: tool_req__docs_req_link_satisfies_allowed # TODO: make it mandatory - satisfies: ^gd_req__.*$ + satisfies: gd_req optional_options: codelink: ^.*$ tags: ^.*$ @@ -400,7 +400,7 @@ needs_types: mandatory_links: includes: ^logic_arc_int(_op)*__.+$ optional_links: - fulfils: ^feat_req__.+$ + fulfils: feat_req tags: - architecture_element - architecture_view @@ -420,7 +420,7 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ mandatory_links: - fulfils: ^feat_req__.+$ + fulfils: feat_req tags: - architecture_view - architecture_element @@ -441,8 +441,8 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ optional_links: - includes: ^logic_arc_int_op__.+$ - fulfils: ^comp_req__.+$ + includes: logic_arc_int_op + fulfils: comp_req tags: - architecture_element - architecture_view @@ -462,7 +462,7 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ mandatory_links: - included_by: ^logic_arc_int__.+$ + included_by: logic_arc_int tags: - architecture_element parts: 3 @@ -474,7 +474,7 @@ needs_types: color: #FEDCD2 style: card mandatory_links: - includes: ^comp_arc_sta__.+$ + includes: comp_arc_sta tags: - architecture_view parts: 3 @@ -501,10 +501,10 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ optional_links: - implements: ^real_arc_int(_op)*__.+$ - includes: ^comp_arc_sta__.+$ - uses: ^real_arc_int(_op)*__.+$ - fulfils: ^comp_req__.+$ + implements: real_arc_int, real_arc_int_op + includes: comp_arc_sta + uses: real_arc_int, real_arc_int_op + fulfils: comp_req tags: - architecture_element - architecture_view @@ -524,7 +524,7 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ optional_links: - fulfils: ^comp_req__.+$ + fulfils: comp_req tags: - architecture_view - architecture_element @@ -546,7 +546,7 @@ needs_types: status: ^(valid|invalid)$ language: ^(cpp|rust)$ optional_links: - fulfils: ^comp_req__.+$ + fulfils: comp_req tags: - architecture_element - architecture_view @@ -566,9 +566,9 @@ needs_types: # req-Id: tool_req__docs_common_attr_status status: ^(valid|invalid)$ mandatory_links: - included_by: ^real_arc_int__.+$ + included_by: real_arc_int optional_links: - implements: ^logic_arc_int_op__.+$ + implements: logic_arc_int_op tags: - architecture_element parts: 3 @@ -594,10 +594,10 @@ needs_types: safety: ^(QM|ASIL_B)$ status: ^(valid|invalid)$ mandatory_links: - implements: ^comp_req__.*$ - satisfies: ^comp_arc_sta__.*$ + implements: comp_req + satisfies: comp_arc_sta optional_links: - includes: ^sw_unit__.*$ + includes: sw_unit parts: 3 dd_dyn: @@ -609,8 +609,8 @@ needs_types: safety: ^(QM|ASIL_B)$ status: ^(valid|invalid)$ mandatory_links: - implements: ^comp_req__.*$ - satisfies: ^comp_arc_sta__.*$ + implements: comp_req + satisfies: comp_arc_sta parts: 3 sw_unit: @@ -642,11 +642,11 @@ needs_types: status: ^(valid|invalid)$ content: ^[\s\S]+$ mandatory_links: - violates: ^feat_arc_sta__[0-9a-z_]+$ + violates: feat_arc_sta optional_options: mitigation_issue: ^https://github.com/.*$ optional_links: - mitigated_by: ^(feat_req__.*|aou_req__.*)$ + mitigated_by: feat_req, aou_req parts: 3 # req-Id: tool_req__docs_saf_types @@ -663,14 +663,14 @@ needs_types: content: ^[\s\S]+$ mandatory_links: # req-Id: tool_req__docs_saf_attrs_violates - violates: ^feat_arc_sta__[0-9a-z_]+$ + violates: feat_arc_sta optional_options: # req-Id: tool_req__docs_saf_attrs_mitigation_issue mitigation_issue: ^https://github.com/.*$ optional_links: # req-Id: tool_req__docs_saf_attrs_mitigated_by # (only mandatory once valid status == valid) - mitigated_by: ^(feat_req__.*|aou_req__.*)$ + mitigated_by: feat_req, aou_req tags: - dependent_failure_analysis - safety_analysis @@ -693,11 +693,11 @@ needs_types: mitigation_issue: ^https://github.com/.*$ mandatory_links: # req-Id: tool_req__docs_saf_attrs_violates - violates: ^comp_arc_sta__[0-9a-z_]+$ + violates: comp_arc_sta optional_links: # req-Id: tool_req__docs_saf_attrs_mitigated_by # (only mandatory once valid status == valid) - mitigated_by: ^(comp_req__.*|aou_req__.*)$ + mitigated_by: comp_req, aou_req tags: - dependent_failure_analysis - safety_analysis @@ -721,11 +721,11 @@ needs_types: mitigation_issue: ^https://github.com/.*$ mandatory_links: # req-Id: tool_req__docs_saf_attrs_violates - violates: ^feat_arc_dyn__[0-9a-z_]+$ + violates: feat_arc_dyn optional_links: # req-Id: tool_req__docs_saf_attrs_mitigated_by # (only mandatory once valid status == valid) - mitigated_by: ^(feat_req__.*|aou_req__.*)$ + mitigated_by: feat_req, aou_req tags: - failure_mode_effects_analysis - safety_analysis @@ -748,9 +748,9 @@ needs_types: mitigation_issue: ^https://github.com/.*$ mandatory_links: # req-Id: tool_req__docs_saf_attrs_violates - violates: ^comp_arc_dyn__[0-9a-z_]+$ + violates: comp_arc_dyn optional_links: - mitigated_by: ^(comp_req__.*|aou_req__.*)$ + mitigated_by: comp_req, aou_req tags: - failure_mode_effects_analysis - safety_analysis diff --git a/src/extensions/score_metamodel/metamodel_types.py b/src/extensions/score_metamodel/metamodel_types.py index 8eee3f062..8a1cf7c94 100644 --- a/src/extensions/score_metamodel/metamodel_types.py +++ b/src/extensions/score_metamodel/metamodel_types.py @@ -11,6 +11,8 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +from __future__ import annotations + from dataclasses import dataclass, field from sphinx_needs.config import NeedType @@ -32,6 +34,7 @@ class ScoreNeedType(NeedType): mandatory_options: dict[str, str] optional_options: dict[str, str] - # Holds a regex (str) - mandatory_links: dict[str, str] - optional_links: dict[str, str] + # Holds either regexes (str) or a list of other need types (list of ScoreNeedType). + # One or the other for simplicity, no mixing. + mandatory_links: dict[str, str] | dict[str, list[ScoreNeedType]] + optional_links: dict[str, str] | dict[str, list[ScoreNeedType]] diff --git a/src/extensions/score_metamodel/tests/rst/options/test_options_options.rst b/src/extensions/score_metamodel/tests/rst/options/test_options_options.rst index a3cd9c07a..32203f36e 100644 --- a/src/extensions/score_metamodel/tests/rst/options/test_options_options.rst +++ b/src/extensions/score_metamodel/tests/rst/options/test_options_options.rst @@ -33,7 +33,7 @@ .. Required link `satisfies` refers to wrong requirement type -#EXPECT: feat_req__abce.satisfies (['std_wp__test__abce']): does not follow pattern `^stkh_req__.*$`. +#EXPECT: feat_req__abce: references 'std_wp__test__abce' as 'satisfies', but it must reference Stakeholder Requirement (stkh_req). .. feat_req:: Child requirement :id: feat_req__abce @@ -176,7 +176,7 @@ -.. +.. This Test can not be tested at the moment without enabeling that optional checks are also linked. TODO: Re-enable this check .. Negative Test: Linked to a non-allowed requirement type. @@ -191,7 +191,7 @@ .. Negative Test: Linked to a non-allowed requirement type. -#EXPECT: feat_saf_fmea__child__26.violates (['comp_req__child__ASIL_B']): does not follow pattern `^feat_arc_dyn__[0-9a-z_]+$`. +#EXPECT: feat_saf_fmea__child__26: references 'comp_req__child__ASIL_B' as 'violates', but it must reference Feature Sequence Diagram (feat_arc_dyn). .. feat_saf_fmea:: Child requirement 26 :id: feat_saf_fmea__child__26 @@ -504,4 +504,3 @@ .. std_wp:: This is a test :id: std_wp__test_content - diff --git a/src/extensions/score_metamodel/yaml_parser.py b/src/extensions/score_metamodel/yaml_parser.py index a0a912879..de89fd9d9 100644 --- a/src/extensions/score_metamodel/yaml_parser.py +++ b/src/extensions/score_metamodel/yaml_parser.py @@ -144,9 +144,7 @@ def _parse_needs_types( return needs_types -def _parse_links( - links_dict: dict[str, Any], -) -> list[dict[str, str]]: +def _parse_links(links_dict: dict[str, dict[str, str]]) -> list[dict[str, str]]: """ Generate 'needs_extra_links' for sphinx-needs.