From 9099240351897d87240ea64940b234af2fd68d5e Mon Sep 17 00:00:00 2001 From: Andreas Zwinkau Date: Tue, 9 Jun 2026 17:43:19 +0200 Subject: [PATCH 1/3] test: replicate bug 588 --- .../tests/test_check_options.py | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/extensions/score_metamodel/tests/test_check_options.py b/src/extensions/score_metamodel/tests/test_check_options.py index 0e689919a..6519d6d5e 100644 --- a/src/extensions/score_metamodel/tests/test_check_options.py +++ b/src/extensions/score_metamodel/tests/test_check_options.py @@ -21,6 +21,7 @@ check_extra_options, check_options, parse_milestone, + validate_links, ) from score_metamodel.tests import fake_check_logger, need from sphinx.application import Sphinx # type: ignore[import-untyped] @@ -124,3 +125,125 @@ def test_milestone_parsing(): assert parse_milestone("v0.5") == (0, 5, 0) assert parse_milestone("v1.0") == (1, 0, 0) assert parse_milestone("v1.0.1") == (1, 0, 1) + + +class TestValidateLinks: + """Tests for validate_links() — specifically the prefix-stripping bug (#588).""" + + NEED_TYPE_WITH_LINK_BY_NAME: ScoreNeedType = { + "title": "Test Type", + "prefix": "TT", + "tags": [], + "parts": 1, + "directive": "test_type", + "mandatory_options": { + "id": "^test_type__.*$", + "status": "^(draft|valid)$", + }, + "optional_options": {}, + "mandatory_links": {}, + "optional_links": { + "satisfies": ["product"], + }, + } + + NEED_TYPE_WITH_LINK_BY_REGEX: ScoreNeedType = { + "title": "Test Type", + "prefix": "TT", + "tags": [], + "parts": 1, + "directive": "test_type", + "mandatory_options": { + "id": "^test_type__.*$", + "status": "^(draft|valid)$", + }, + "optional_options": {}, + "mandatory_links": {}, + "optional_links": { + "satisfies": ["^product__.*$"], + }, + } + + def test_link_matches_by_type_name_no_prefix(self): + """A link value that starts with the type name should pass validation.""" + need_1 = need( + id="test_type__001", + type="test_type", + satisfies="product__foo", + ) + logger = fake_check_logger() + validate_links( + cast(CheckLogger, logger), + self.NEED_TYPE_WITH_LINK_BY_NAME, + need_1, + ) + logger.assert_no_warnings() + + def test_link_with_uppercase_prefix_no_longer_stripped(self): + """ + After the fix for #588, uppercase prefixes are no longer stripped. + P_FOO stays as P_FOO and correctly fails to match the 'product' type pattern. + """ + need_1 = need( + id="test_type__001", + type="test_type", + satisfies="P_FOO", + ) + logger = fake_check_logger() + validate_links( + cast(CheckLogger, logger), + self.NEED_TYPE_WITH_LINK_BY_NAME, + need_1, + ) + # P_FOO is preserved as-is (no stripping) and doesn't match ^product__ + logger.assert_warning("references 'P_FOO' as 'satisfies'") + + def test_link_with_uppercase_prefix_not_stripped_for_regex(self): + """ + After the fix for #588, uppercase prefixes are no longer stripped. + P_product__foo stays as P_product__foo and does NOT match ^product__.*$ + because the P_ prefix is preserved. + """ + need_1 = need( + id="test_type__001", + type="test_type", + satisfies="P_product__foo", + ) + logger = fake_check_logger() + validate_links( + cast(CheckLogger, logger), + self.NEED_TYPE_WITH_LINK_BY_REGEX, + need_1, + ) + # P_product__foo is preserved as-is and doesn't match ^product__.*$ + logger.assert_warning("references 'P_product__foo' as 'satisfies'") + + def test_link_without_prefix_matches_by_name(self): + """A link value without any prefix that starts with the type name passes.""" + need_1 = need( + id="test_type__001", + type="test_type", + satisfies="product__bar", + ) + logger = fake_check_logger() + validate_links( + cast(CheckLogger, logger), + self.NEED_TYPE_WITH_LINK_BY_NAME, + need_1, + ) + logger.assert_no_warnings() + + def test_link_not_matching_any_type_warns(self): + """A link value that doesn't match any allowed type should warn.""" + need_1 = need( + id="test_type__001", + type="test_type", + satisfies="unknown__thing", + ) + logger = fake_check_logger() + validate_links( + cast(CheckLogger, logger), + self.NEED_TYPE_WITH_LINK_BY_NAME, + need_1, + ) + logger.assert_warning("references 'unknown__thing' as 'satisfies'") From 96850739df4e2e439b5fa0f2c0e6defb2f921e7e Mon Sep 17 00:00:00 2001 From: Andreas Zwinkau Date: Tue, 9 Jun 2026 17:45:51 +0200 Subject: [PATCH 2/3] fix: do not strip uppercase prefixes --- .../score_metamodel/checks/check_options.py | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/extensions/score_metamodel/checks/check_options.py b/src/extensions/score_metamodel/checks/check_options.py index dfc44ba22..8f1ae677c 100644 --- a/src/extensions/score_metamodel/checks/check_options.py +++ b/src/extensions/score_metamodel/checks/check_options.py @@ -32,14 +32,12 @@ def get_need_type(needs_types: list[ScoreNeedType], directive: str) -> ScoreNeed raise ValueError(f"Need type {directive} not found in needs_types") -def _get_normalized(need: NeedItem, key: str, remove_prefix: bool = False) -> list[str]: +def _get_normalized(need: NeedItem, key: str) -> list[str]: """Normalize a raw value into a list of strings.""" 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): # Verify all elements are strings @@ -47,10 +45,7 @@ def _get_normalized(need: NeedItem, key: str, remove_prefix: bool = False) -> li for item in raw_list: if not isinstance(item, str): raise ValueError - str_list = cast(list[str], raw_value) - if remove_prefix: - return [_remove_namespace_prefix_(v) for v in str_list] - return str_list + return cast(list[str], raw_value) raise ValueError @@ -73,11 +68,6 @@ def _validate_value_pattern( ) from e -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, need_type: ScoreNeedType, @@ -159,13 +149,17 @@ def _validate( treat_as_info: bool = False, ): for attribute, allowed_values in attributes_to_allowed_values.items(): - values = _get_normalized(need, attribute, remove_prefix=True) + values = _get_normalized(need, attribute) 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"] + v + if isinstance(v, str) and v.startswith("^") + else f"^{v}__" + if isinstance(v, str) + else v["mandatory_options"]["id"] for v in allowed_values ] ) From 0209fc8e317d3fec549e38057d87b32891d05e55 Mon Sep 17 00:00:00 2001 From: Andreas Zwinkau Date: Tue, 9 Jun 2026 17:53:57 +0200 Subject: [PATCH 3/3] refactor: clarify code --- .../score_metamodel/checks/check_options.py | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/extensions/score_metamodel/checks/check_options.py b/src/extensions/score_metamodel/checks/check_options.py index 8f1ae677c..d8d2e7c6e 100644 --- a/src/extensions/score_metamodel/checks/check_options.py +++ b/src/extensions/score_metamodel/checks/check_options.py @@ -134,6 +134,22 @@ def _validate(attributes_to_allowed_values: dict[str, str], mandatory: bool): # ) +def _to_link_pattern(value: "str | ScoreNeedType") -> str: + """Convert a link constraint to a regex pattern. + + - A plain type name like ``"stkh_req"`` becomes ``^stkh_req__`` + (matching IDs that start with the type name followed by ``__``). + - A string already starting with ``^`` is treated as an explicit regex + and returned unchanged. + - A ScoreNeedType dict uses its ``mandatory_options.id`` pattern. + """ + if isinstance(value, str): + if value.startswith("^"): + return value + return f"^{value}__" + return value["mandatory_options"]["id"] + + def validate_links( log: CheckLogger, need_type: ScoreNeedType, @@ -153,16 +169,7 @@ def _validate( if mandatory and not values: log.warning_for_need(need, f"is missing required link: `{attribute}`.") - allowed_regex = "|".join( - [ - v - if isinstance(v, str) and v.startswith("^") - else f"^{v}__" - if isinstance(v, str) - else v["mandatory_options"]["id"] - for v in allowed_values - ] - ) + allowed_regex = "|".join(_to_link_pattern(v) for v in allowed_values) # regex based validation for value in values: