Skip to content

Commit 71a0b6b

Browse files
authored
fix: dont strip uppercase prefixes (#589)
1 parent e28ccd7 commit 71a0b6b

2 files changed

Lines changed: 143 additions & 19 deletions

File tree

src/extensions/score_metamodel/checks/check_options.py

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,25 +32,20 @@ def get_need_type(needs_types: list[ScoreNeedType], directive: str) -> ScoreNeed
3232
raise ValueError(f"Need type {directive} not found in needs_types")
3333

3434

35-
def _get_normalized(need: NeedItem, key: str, remove_prefix: bool = False) -> list[str]:
35+
def _get_normalized(need: NeedItem, key: str) -> list[str]:
3636
"""Normalize a raw value into a list of strings."""
3737
raw_value = need.get(key, None)
3838
if not raw_value:
3939
return []
4040
if isinstance(raw_value, str):
41-
if remove_prefix:
42-
return [_remove_namespace_prefix_(raw_value)]
4341
return [raw_value]
4442
if isinstance(raw_value, list):
4543
# Verify all elements are strings
4644
raw_list = cast(list[object], raw_value)
4745
for item in raw_list:
4846
if not isinstance(item, str):
4947
raise ValueError
50-
str_list = cast(list[str], raw_value)
51-
if remove_prefix:
52-
return [_remove_namespace_prefix_(v) for v in str_list]
53-
return str_list
48+
return cast(list[str], raw_value)
5449
raise ValueError
5550

5651

@@ -73,11 +68,6 @@ def _validate_value_pattern(
7368
) from e
7469

7570

76-
def _remove_namespace_prefix_(word: str) -> str:
77-
# If the word starts with uppercase letters followed by an underscore, remove them.
78-
return re.sub(r"^[A-Z]+_", "", word)
79-
80-
8171
def validate_options(
8272
log: CheckLogger,
8373
need_type: ScoreNeedType,
@@ -144,6 +134,22 @@ def _validate(attributes_to_allowed_values: dict[str, str], mandatory: bool):
144134
# )
145135

146136

137+
def _to_link_pattern(value: "str | ScoreNeedType") -> str:
138+
"""Convert a link constraint to a regex pattern.
139+
140+
- A plain type name like ``"stkh_req"`` becomes ``^stkh_req__``
141+
(matching IDs that start with the type name followed by ``__``).
142+
- A string already starting with ``^`` is treated as an explicit regex
143+
and returned unchanged.
144+
- A ScoreNeedType dict uses its ``mandatory_options.id`` pattern.
145+
"""
146+
if isinstance(value, str):
147+
if value.startswith("^"):
148+
return value
149+
return f"^{value}__"
150+
return value["mandatory_options"]["id"]
151+
152+
147153
def validate_links(
148154
log: CheckLogger,
149155
need_type: ScoreNeedType,
@@ -159,16 +165,11 @@ def _validate(
159165
treat_as_info: bool = False,
160166
):
161167
for attribute, allowed_values in attributes_to_allowed_values.items():
162-
values = _get_normalized(need, attribute, remove_prefix=True)
168+
values = _get_normalized(need, attribute)
163169
if mandatory and not values:
164170
log.warning_for_need(need, f"is missing required link: `{attribute}`.")
165171

166-
allowed_regex = "|".join(
167-
[
168-
v if isinstance(v, str) else v["mandatory_options"]["id"]
169-
for v in allowed_values
170-
]
171-
)
172+
allowed_regex = "|".join(_to_link_pattern(v) for v in allowed_values)
172173

173174
# regex based validation
174175
for value in values:

src/extensions/score_metamodel/tests/test_check_options.py

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
check_extra_options,
2222
check_options,
2323
parse_milestone,
24+
validate_links,
2425
)
2526
from score_metamodel.tests import fake_check_logger, need
2627
from sphinx.application import Sphinx # type: ignore[import-untyped]
@@ -124,3 +125,125 @@ def test_milestone_parsing():
124125
assert parse_milestone("v0.5") == (0, 5, 0)
125126
assert parse_milestone("v1.0") == (1, 0, 0)
126127
assert parse_milestone("v1.0.1") == (1, 0, 1)
128+
129+
130+
class TestValidateLinks:
131+
"""Tests for validate_links() — specifically the prefix-stripping bug (#588)."""
132+
133+
NEED_TYPE_WITH_LINK_BY_NAME: ScoreNeedType = {
134+
"title": "Test Type",
135+
"prefix": "TT",
136+
"tags": [],
137+
"parts": 1,
138+
"directive": "test_type",
139+
"mandatory_options": {
140+
"id": "^test_type__.*$",
141+
"status": "^(draft|valid)$",
142+
},
143+
"optional_options": {},
144+
"mandatory_links": {},
145+
"optional_links": {
146+
"satisfies": ["product"],
147+
},
148+
}
149+
150+
NEED_TYPE_WITH_LINK_BY_REGEX: ScoreNeedType = {
151+
"title": "Test Type",
152+
"prefix": "TT",
153+
"tags": [],
154+
"parts": 1,
155+
"directive": "test_type",
156+
"mandatory_options": {
157+
"id": "^test_type__.*$",
158+
"status": "^(draft|valid)$",
159+
},
160+
"optional_options": {},
161+
"mandatory_links": {},
162+
"optional_links": {
163+
"satisfies": ["^product__.*$"],
164+
},
165+
}
166+
167+
def test_link_matches_by_type_name_no_prefix(self):
168+
"""A link value that starts with the type name should pass validation."""
169+
need_1 = need(
170+
id="test_type__001",
171+
type="test_type",
172+
satisfies="product__foo",
173+
)
174+
logger = fake_check_logger()
175+
validate_links(
176+
cast(CheckLogger, logger),
177+
self.NEED_TYPE_WITH_LINK_BY_NAME,
178+
need_1,
179+
)
180+
logger.assert_no_warnings()
181+
182+
def test_link_with_uppercase_prefix_no_longer_stripped(self):
183+
"""
184+
After the fix for #588, uppercase prefixes are no longer stripped.
185+
P_FOO stays as P_FOO and correctly fails to match the 'product' type pattern.
186+
"""
187+
need_1 = need(
188+
id="test_type__001",
189+
type="test_type",
190+
satisfies="P_FOO",
191+
)
192+
logger = fake_check_logger()
193+
validate_links(
194+
cast(CheckLogger, logger),
195+
self.NEED_TYPE_WITH_LINK_BY_NAME,
196+
need_1,
197+
)
198+
# P_FOO is preserved as-is (no stripping) and doesn't match ^product__
199+
logger.assert_warning("references 'P_FOO' as 'satisfies'")
200+
201+
def test_link_with_uppercase_prefix_not_stripped_for_regex(self):
202+
"""
203+
After the fix for #588, uppercase prefixes are no longer stripped.
204+
P_product__foo stays as P_product__foo and does NOT match ^product__.*$
205+
because the P_ prefix is preserved.
206+
"""
207+
need_1 = need(
208+
id="test_type__001",
209+
type="test_type",
210+
satisfies="P_product__foo",
211+
)
212+
logger = fake_check_logger()
213+
validate_links(
214+
cast(CheckLogger, logger),
215+
self.NEED_TYPE_WITH_LINK_BY_REGEX,
216+
need_1,
217+
)
218+
# P_product__foo is preserved as-is and doesn't match ^product__.*$
219+
logger.assert_warning("references 'P_product__foo' as 'satisfies'")
220+
221+
def test_link_without_prefix_matches_by_name(self):
222+
"""A link value without any prefix that starts with the type name passes."""
223+
need_1 = need(
224+
id="test_type__001",
225+
type="test_type",
226+
satisfies="product__bar",
227+
)
228+
logger = fake_check_logger()
229+
validate_links(
230+
cast(CheckLogger, logger),
231+
self.NEED_TYPE_WITH_LINK_BY_NAME,
232+
need_1,
233+
)
234+
logger.assert_no_warnings()
235+
236+
def test_link_not_matching_any_type_warns(self):
237+
"""A link value that doesn't match any allowed type should warn."""
238+
need_1 = need(
239+
id="test_type__001",
240+
type="test_type",
241+
satisfies="unknown__thing",
242+
)
243+
logger = fake_check_logger()
244+
validate_links(
245+
cast(CheckLogger, logger),
246+
self.NEED_TYPE_WITH_LINK_BY_NAME,
247+
need_1,
248+
)
249+
logger.assert_warning("references 'unknown__thing' as 'satisfies'")

0 commit comments

Comments
 (0)