diff --git a/docs/internals/extensions/rst_filebased_testing.md b/docs/internals/extensions/rst_filebased_testing.md index 8016989bc..3fa57fee5 100644 --- a/docs/internals/extensions/rst_filebased_testing.md +++ b/docs/internals/extensions/rst_filebased_testing.md @@ -43,11 +43,11 @@ One or more Sphinx-Needs directives needed for the **Example:** #CHECK: check_options - #EXPECT: std_wp__test__abcd: is missing required option: `status`. + #EXPECT: std_wp__test__abcd: is missing required attribute: `status`. .. std_wp:: Test requirement :id: std_wp__test__abcd This example verifies that the warning message -*std_wp__test__abcd: is missing required option: \`status\`* +*std_wp__test__abcd: is missing required attribute: \`status\`* is shown during the Sphinx build. Only the *check_options* check is enabled. diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index 5897cfec1..89a27a8ff 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -181,19 +181,22 @@ def _resolve_linkable_types( link_value: str, current_need_type: ScoreNeedType, needs_types: list[ScoreNeedType], -) -> list[ScoreNeedType]: +) -> list[ScoreNeedType | str]: 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] = [] + linkable_types: list[ScoreNeedType | str] = [] 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}'." - ) + if v.startswith("^"): + linkable_types.append(v) # keep regex as-is else: - linkable_types.append(target_need_type) + 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 @@ -219,10 +222,9 @@ def postprocess_need_links(needs_types_list: list[ScoreNeedType]): for link_name, link_value in link_dict.items(): assert isinstance(link_value, str) # so far all of them are strings - if not link_value.startswith("^"): - link_dict[link_name] = _resolve_linkable_types( # pyright: ignore[reportArgumentType] - link_name, link_value, need_type, needs_types_list - ) + 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 42953c672..a182320d5 100644 --- a/src/extensions/score_metamodel/checks/check_options.py +++ b/src/extensions/score_metamodel/checks/check_options.py @@ -18,6 +18,7 @@ default_options, local_check, ) +from score_metamodel.metamodel_types import AllowedLinksType from sphinx.application import Sphinx from sphinx_needs.data import NeedsInfoType @@ -30,13 +31,20 @@ def get_need_type(needs_types: list[ScoreNeedType], directive: str) -> ScoreNeed raise ValueError(f"Need type {directive} not found in needs_types") -def _normalize_values(raw_value: str | list[str] | None) -> list[str]: +def _get_normalized( + need: NeedsInfoType, key: str, remove_prefix: bool = False +) -> list[str]: """Normalize a raw value into a list of strings.""" - if raw_value is None: + raw_value = need.get(key, None) + if not raw_value: return [] if isinstance(raw_value, str): + if remove_prefix: + return [_remove_namespace_prefix_(raw_value)] return [raw_value] if isinstance(raw_value, list) and all(isinstance(v, str) for v in raw_value): + if remove_prefix: + return [_remove_namespace_prefix_(v) for v in raw_value] return raw_value raise ValueError @@ -53,118 +61,90 @@ def _validate_value_pattern( """ try: return re.match(pattern, value) is not None - except TypeError as e: + except Exception as e: raise TypeError( f"Error in metamodel.yaml at {need['type']}->{field}: " f"pattern `{pattern}` is not a valid regex pattern." ) from e -def _log_option_warning( - need: NeedsInfoType, +def _remove_namespace_prefix_(word: str) -> str: + # If the word starts with uppercase letters followed by an underscore, remove them. + return re.sub(r"^[A-Z]+_", "", word) + + +def validate_options( 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_type: ScoreNeedType, need: NeedsInfoType, - log: CheckLogger, - fields: dict[str, str] | dict[str, list[ScoreNeedType]], - required: bool, - field_type: str, - allowed_prefixes: list[str], ): """ - Validates that fields (options or links) in a need match their expected patterns. - - :param need: The need object containing the data. - :param log: Logger for warnings. - :param fields: A dictionary of field names and their regex patterns. - :param required: Whether the fields are required (True) or optional (False). - :param field_type: A string indicating the field type ('option' or 'link'). + Validates that options in a need match their expected patterns. """ - def remove_prefix(word: str, prefixes: list[str]) -> str: - # Memory and allocation wise better to use a generator here. - # Removes any prefix allowed by configuration, if prefix is there. - return [word.removeprefix(prefix) for prefix in prefixes][0] - - for field, allowed_value in fields.items(): - raw_value: str | list[str] | None = need.get(field, None) - if raw_value in [None, [], ""]: - if required: + def _validate(attributes_to_allowed_values: dict[str, str], mandatory: bool): + for attribute, allowed_regex in attributes_to_allowed_values.items(): + values = _get_normalized(need, attribute) + if mandatory and not values: log.warning_for_need( - need, f"is missing required {field_type}: `{field}`." + need, f"is missing required attribute: `{attribute}`." ) - continue # Nothing to validate if not present - - 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) - + ")" + + for value in values: + if not _validate_value_pattern(value, allowed_regex, need, attribute): + log.warning_for_option( + need, attribute, f"does not follow pattern `{allowed_regex}`." + ) + + _validate(need_type["mandatory_options"], True) + _validate(need_type["optional_options"], False) + + +def validate_links( + log: CheckLogger, + need_type: ScoreNeedType, + need: NeedsInfoType, +): + """ + Validates that links in a need match the expected types or regexes. + """ + + def _validate( + attributes_to_allowed_values: AllowedLinksType, + mandatory: bool, + treat_as_info: bool = False, + ): + for attribute, allowed_values in attributes_to_allowed_values.items(): + values = _get_normalized(need, attribute, remove_prefix=True) + if mandatory and not values: + log.warning_for_need(need, f"is missing required link: `{attribute}`.") + + allowed_regex = "|".join( + [ + v if isinstance(v, str) else v["mandatory_options"]["id"] + for v in allowed_values + ] ) - 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): - _log_option_warning( - need, - log, - field_type, - allowed_directives, - field, - value, - allowed_value, - required, - ) + + # regex based validation + for value in values: + if not _validate_value_pattern(value, allowed_regex, need, attribute): + log.warning_for_link( + need, + attribute, + value, + [ + av + if isinstance(av, str) + else f"{av['title']} ({av['directive']})" + for av in allowed_values + ], + allowed_regex, + is_new_check=treat_as_info, + ) + + _validate(need_type["mandatory_links"], True) + _validate(need_type["optional_links"], False, treat_as_info=True) # req-Id: tool_req__docs_req_attr_reqtype @@ -185,28 +165,8 @@ def check_options( """ need_type = get_need_type(app.config.needs_types, need["type"]) - # If undefined this is an empty list - allowed_prefixes = app.config.allowed_external_prefixes - - # Validate Options and Links - 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), - ("link", need_type["optional_links"], False), - ] - - for field_type, field_values, is_required in field_validations: - validate_fields( - need, - log, - field_values, - required=is_required, - field_type=field_type, - allowed_prefixes=allowed_prefixes, - ) + validate_options(log, need_type, need) + validate_links(log, need_type, need) @local_check diff --git a/src/extensions/score_metamodel/log.py b/src/extensions/score_metamodel/log.py index d2953decb..7f433053d 100644 --- a/src/extensions/score_metamodel/log.py +++ b/src/extensions/score_metamodel/log.py @@ -55,6 +55,26 @@ def warning_for_option( location = CheckLogger._location(need, self._prefix) self._log_message(full_msg, location, is_new_check) + def warning_for_link( + self, + need: NeedsInfoType, + option: str, + problematic_value: str, + allowed_values: list[str], + allowed_regex: str, + is_new_check: bool = False, + ): + msg = ( + f"references '{problematic_value}' as '{option}', " + f"but it must reference {' or '.join(allowed_values)}." + ) + # Sometimes printing this helps, but most often it just clutters the log. + # Not sure yet. + # if allowed_regex: + # msg += f" (allowed pattern: `{allowed_regex}`)" + + self.warning_for_need(need, msg, is_new_check=is_new_check) + def warning_for_need( self, need: NeedsInfoType, msg: str, is_new_check: bool = False ): diff --git a/src/extensions/score_metamodel/metamodel.yaml b/src/extensions/score_metamodel/metamodel.yaml index 219afeded..83e4a1263 100644 --- a/src/extensions/score_metamodel/metamodel.yaml +++ b/src/extensions/score_metamodel/metamodel.yaml @@ -129,7 +129,7 @@ needs_types: # req-Id: tool_req__docs_req_link_satisfies_allowed # TODO: fix once process_description is fixed satisfies: workflow - complies: ^std_req__(iso26262|isosae21434|isopas8926|aspice_40)__.*$ + complies: std_req tags: - requirement parts: 2 @@ -139,7 +139,7 @@ needs_types: mandatory_options: status: ^(valid|draft)$ optional_links: - complies: ^std_req__(iso26262|isodae21434|isopas8926|aspice_40)__.*$ + complies: std_req parts: 2 gd_chklst: @@ -147,7 +147,7 @@ needs_types: mandatory_options: status: ^(valid|draft)$ optional_links: - complies: ^std_req__(iso26262|isodae21434|isopas8926|aspice_40)__.*$ + complies: std_req parts: 2 gd_guidl: @@ -155,7 +155,7 @@ needs_types: mandatory_options: status: ^(valid|draft)$ optional_links: - complies: ^std_req__(iso26262|isosae21434|isopas8926|aspice_40)__.*$ + complies: std_req parts: 2 gd_method: @@ -164,7 +164,7 @@ needs_types: mandatory_options: status: ^(valid|draft)$ optional_links: - complies: ^std_req__(iso26262|isosae21434|isopas8926|aspice_40)__.*$ + complies: std_req parts: 2 # S-CORE Workproduct @@ -174,7 +174,7 @@ needs_types: mandatory_options: status: ^(valid|draft)$ optional_links: - complies: ^std_(wp__iso26262|wp__isosae21434|wp__isopas8926|iic_aspice_40)__.*$ + complies: std_wp, ^std_req__aspice_40__iic.*$ parts: 2 # Role diff --git a/src/extensions/score_metamodel/metamodel_types.py b/src/extensions/score_metamodel/metamodel_types.py index 8a1cf7c94..15a405a87 100644 --- a/src/extensions/score_metamodel/metamodel_types.py +++ b/src/extensions/score_metamodel/metamodel_types.py @@ -27,6 +27,10 @@ class ProhibitedWordCheck: types: list[str] = field(default_factory=list) +# links to either regexes (str) or a other need types (list of ScoreNeedType). +AllowedLinksType = dict[str, list["str | ScoreNeedType"]] + + class ScoreNeedType(NeedType): tags: list[str] parts: int @@ -34,7 +38,5 @@ class ScoreNeedType(NeedType): mandatory_options: dict[str, str] optional_options: 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]] + mandatory_links: AllowedLinksType + optional_links: AllowedLinksType diff --git a/src/extensions/score_metamodel/tests/rst/options/gd_req_comp.rst b/src/extensions/score_metamodel/tests/rst/options/gd_req_comp.rst new file mode 100644 index 000000000..ddc4ad808 --- /dev/null +++ b/src/extensions/score_metamodel/tests/rst/options/gd_req_comp.rst @@ -0,0 +1,39 @@ +.. + # ******************************************************************************* + # Copyright (c) 2025 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* + +#CHECK: check_options + +.. std_req:: Standard requirement + :id: std_req__iso26262__001 + +# Expect to warning with "complies" +#EXPECT-NOT: complies + +.. gd_req:: No Link is ok, since complies is optional + :id: gd_req__001 + +# Expect to warning with "complies" +#EXPECT-NOT: complies + +.. gd_req:: Correct link to std_req + :id: gd_req__002 + :complies: std_req__iso26262__001 + +#FIXME: this will currently be printed as an INFO, and not as a warning. +# Re-enable EXCPECT once we can enable that as a warning. +#EXP-ECT: gd_req__003: references 'gd_req__001' as 'complies', but it must reference Standard Requirement (std_req). + +.. gd_req:: Cannot refer to non std_req element + :id: gd_req__003 + :complies: gd_req__001 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 32203f36e..789238171 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 @@ -16,7 +16,7 @@ .. Required option: `status` is missing -#EXPECT: std_wp__test__abcd: is missing required option: `status`. +#EXPECT: std_wp__test__abcd: is missing required attribute: `status`. .. std_wp:: This is a test :id: std_wp__test__abcd @@ -24,7 +24,7 @@ .. All required options are present -#EXPECT-NOT: std_wp__test__abcd: is missing required option +#EXPECT-NOT: std_wp__test__abcd: is missing required attribute .. std_wp:: This is a test :id: std_wp__test__abce @@ -478,7 +478,7 @@ .. Ensuring that empty content is detected correctly -.. #EXPECT: stkh_req__test_no_content: is missing required option: `content` +.. #EXPECT: stkh_req__test_no_content: is missing required attribute: `content` .. .. .. stkh_req:: This is a test .. :id: stkh_req__test_no_content @@ -488,7 +488,7 @@ .. Ensuring that non empty content is detected correctly -#EXPECT-NOT: stkh_req__test_content: is missing required option: `content` +#EXPECT-NOT: stkh_req__test_content: is missing required attribute: `content` .. stkh_req:: This is a test :id: stkh_req__test_content @@ -500,7 +500,7 @@ .. This should not trigger, as 'std_wp' is not checked for content -#EXPECT-NOT: std_wp__test_content: is missing required option: `content` +#EXPECT-NOT: std_wp__test_content: is missing required attribute: `content` .. std_wp:: This is a test :id: std_wp__test_content diff --git a/src/extensions/score_metamodel/tests/rst/options/wp_comp.rst b/src/extensions/score_metamodel/tests/rst/options/wp_comp.rst new file mode 100644 index 000000000..f3c7dea52 --- /dev/null +++ b/src/extensions/score_metamodel/tests/rst/options/wp_comp.rst @@ -0,0 +1,63 @@ +.. + # ******************************************************************************* + # Copyright (c) 2025 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* + +#CHECK: check_options + +.. std_wp:: Standard work product + :id: std_wp__iso26262__001 + +.. std_req:: Standard requirement + :id: std_req__iso26262__001 + +.. std_req:: Standard IIC requirement + :id: std_req__aspice_40__iic_001 + +---- + +# Expect no warning with "complies" +#EXPECT-NOT: complies + +.. workproduct:: No Link is ok, since complies is optional + :id: wp__001 + +--- + +# Expect no warning with "complies" +#EXPECT-NOT: complies + +.. workproduct:: Linking to std_wp is allowed + :id: wp__002 + :complies: std_wp__iso26262__001 + +--- + +#FIXME: this will currently be printed as an INFO, and not as a warning. +# Re-enable EXCPECT once we can enable that as a warning. +#EXP-ECT: wp__003: references 'std_req__iso26262__001' as 'complies', but it must reference Standard Work Product (std_wp) or ^std_req__aspice_40__iic.*$. + +.. workproduct:: Cannot refer to std_req element + :id: wp__003 + :complies: std_req__iso26262__001 + +--- + + +# Expect no warning with "complies" +#EXPECT-NOT: complies + +.. workproduct:: But it can refer to std_req if it is an IIC requirement + :id: wp__003 + :complies: std_req__aspice_40__iic_001 + +--- diff --git a/src/extensions/score_metamodel/tests/test_rules_file_based.py b/src/extensions/score_metamodel/tests/test_rules_file_based.py index 7dbd4c382..050a60b52 100644 --- a/src/extensions/score_metamodel/tests/test_rules_file_based.py +++ b/src/extensions/score_metamodel/tests/test_rules_file_based.py @@ -162,13 +162,13 @@ def warning_matches( warning_info: WarningInfo, expected_message: str, warnings: list[str], -) -> bool: +) -> str | None: ### Checks if any element of the warning list is includes the given warning info. - # It returns True if found otherwise False. + # It returns the matched warning or None if no match is found. for warning in filter_warnings_by_position(rst_data, warning_info, warnings): if expected_message in warning: - return True - return False + return warning + return None @pytest.mark.parametrize("rst_file", RST_FILES) @@ -193,16 +193,24 @@ def test_rst_files( # Collect the warnings warnings = app.warning.getvalue().splitlines() - # print(f"Warnings: {warnings}") # Check if the expected warnings are present for warning_info in rst_data.warning_infos: for w in warning_info.expected: if not warning_matches(rst_data, warning_info, w, warnings): actual = filter_warnings_by_position(rst_data, warning_info, warnings) - raise AssertionError( - f"Expected warning: '{w}' not found. Received: {actual}" - ) + loc = f"{rst_data.filename}:{warning_info.lineno}" + msg = f"{loc} Expected warning not found:\n" + msg += f" Expected: '{w}'\n" + msg += " Actual:\n" + for a in actual: + msg += f" - {a}\n" + pytest.fail(msg, pytrace=False) + for w in warning_info.not_expected: - if warning_matches(rst_data, warning_info, w, warnings): - raise AssertionError(f"Unexpected warning: '{w}' found") + if unexpected := warning_matches(rst_data, warning_info, w, warnings): + loc = f"{rst_data.filename}:{warning_info.lineno}" + msg = f"{loc} Unexpected warning found:\n" + msg += f" Not Expected: '{w}'\n" + msg += f" Actual: '{unexpected}'\n" + pytest.fail(msg, pytrace=False)