Skip to content

Commit 6bb290d

Browse files
committed
Address review comments
1 parent 90960e8 commit 6bb290d

7 files changed

Lines changed: 224 additions & 17 deletions

File tree

‎bazel/rules/rules_score/private/rst_to_trlc.bzl‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,8 @@ def rst_srcs_to_trlc(name, srcs, deps = [], ref_package = ""):
6262
def _rst_to_trlc_impl(ctx):
6363
"""Convert each .rst source file to a .trlc file via the Python converter."""
6464
outs = []
65-
for src in ctx.files.srcs:
66-
# `src` may come from a different repository (E.g.: a cross-repo RST label)
67-
# Nest under the target's own name so two rst_to_trlc targets in the
68-
# same package converting same-named files don't clash on the output path.
69-
out = ctx.actions.declare_file(ctx.label.name + "/" + src.basename[:-4] + ".trlc")
65+
for i, src in enumerate(ctx.files.srcs):
66+
out = ctx.actions.declare_file(ctx.label.name + "/" + str(i) + "/" + src.basename.removesuffix(".rst") + ".trlc")
7067
outs.append(out)
7168

7269
args = ctx.actions.args()

‎bazel/rules/rules_score/src/rst_to_trlc.py‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,20 +141,40 @@ def _escape(text: str) -> str:
141141
return text.replace("\\", "\\\\").replace('"', '\\"')
142142

143143

144+
def _split_top_level(value: str, sep: str = ",") -> list[str]:
145+
"""Split on ``sep``, ignoring occurrences nested inside ``[...]`` brackets."""
146+
parts: list[str] = []
147+
current: list[str] = []
148+
depth = 0
149+
for ch in value:
150+
if ch == "[":
151+
depth += 1
152+
elif ch == "]":
153+
depth = max(depth - 1, 0)
154+
if ch == sep and depth == 0:
155+
parts.append("".join(current))
156+
current = []
157+
else:
158+
current.append(ch)
159+
parts.append("".join(current))
160+
return parts
161+
162+
144163
def _collect_refs(fields: dict[str, str]) -> list[str]:
145-
"""Extract all cross-reference IDs from relationship fields.
164+
"""Extract cross-reference IDs from relationship fields.
146165
147-
Strips one or more trailing sphinx-needs filter expressions (E.g.:
148-
[version==1] or [version==1][status==valid]), which are not
149-
valid in TRLC reference syntax.
166+
Strips trailing sphinx-needs filter expressions (E.g. [version==1]),
167+
which aren't valid TRLC reference syntax.
150168
"""
151-
return [
152-
_RE_NEEDS_FILTER.sub("", r.strip()).strip()
153-
for k in _REF_FIELDS
154-
if k in fields
155-
for r in fields[k].split(",")
156-
if r.strip()
157-
]
169+
refs: list[str] = []
170+
for k in _REF_FIELDS:
171+
if k not in fields:
172+
continue
173+
for r in _split_top_level(fields[k]):
174+
ref = _RE_NEEDS_FILTER.sub("", r.strip()).strip()
175+
if ref:
176+
refs.append(ref)
177+
return refs
158178

159179

160180
def _collect_root_causes(fields: dict[str, str]) -> list[str]:
@@ -206,7 +226,7 @@ def parse_directives(content: str) -> list[dict[str, Any]]:
206226
# allows this); consume any such lines here before the field list,
207227
# otherwise they get mis-parsed as body text and the real :id: (and
208228
# other fields) on the following lines are missed entirely.
209-
while i < len(lines) and lines[i].strip() and not _RE_FIELD.match(lines[i]):
229+
while i < len(lines) and lines[i].strip() and lines[i][0].isspace() and not _RE_FIELD.match(lines[i]):
210230
title += " " + lines[i].strip()
211231
i += 1
212232

‎bazel/rules/rules_score/test/BUILD‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ load(
3434
"unit_design",
3535
)
3636
load("@score_tooling//bazel/rules/rules_score:sphinx_toolchain.bzl", "score_sphinx_toolchain")
37+
load("@score_tooling//bazel/rules/rules_score/private:rst_to_trlc.bzl", "rst_to_trlc")
3738
load("@score_tooling//cpp/libclang:libclang_toolchain.bzl", "libclang_toolchain")
3839
load("@trlc//:trlc.bzl", "trlc_requirements", "trlc_requirements_test", "trlc_specification")
3940
load(
@@ -83,6 +84,12 @@ load(
8384
"feat_req_multi_spec_provider_test",
8485
"requirements_multi_spec_test_suite",
8586
)
87+
load(
88+
":requirements_package_test.bzl",
89+
"asr_package_forwarded_test",
90+
"requirements_package_test_suite",
91+
"rst_to_trlc_no_collision_test",
92+
)
8693
load(
8794
":requirements_rst_test.bzl",
8895
"aous_rst_provider_test",
@@ -2171,6 +2178,38 @@ aous_rst_sphinx_test(
21712178

21722179
requirements_rst_test_suite(name = "requirements_rst_tests")
21732180

2181+
# ============================================================================
2182+
# package attr forwarding / rst_to_trlc output-path collision tests
2183+
# ============================================================================
2184+
2185+
# Fixture: package attr must forward to the generated rst_to_trlc target.
2186+
assumed_system_requirements(
2187+
name = "asr_custom_package",
2188+
srcs = ["fixtures/rst_requirements/assumed_system_requirements.rst"],
2189+
package = "CustomAsrPackage",
2190+
)
2191+
2192+
asr_package_forwarded_test(
2193+
name = "asr_package_forwarded_test",
2194+
target_under_test = ":_asr_custom_package_rst_gen_0",
2195+
)
2196+
2197+
# Fixture: same-basename .rst srcs in one target must not collide.
2198+
rst_to_trlc(
2199+
name = "rst_collision_fixture",
2200+
srcs = [
2201+
"fixtures/rst_collision/a/req.rst",
2202+
"fixtures/rst_collision/b/req.rst",
2203+
],
2204+
)
2205+
2206+
rst_to_trlc_no_collision_test(
2207+
name = "rst_to_trlc_no_collision_test",
2208+
target_under_test = ":rst_collision_fixture",
2209+
)
2210+
2211+
requirements_package_test_suite(name = "requirements_package_tests")
2212+
21742213
# ============================================================================
21752214
# Multi-spec Requirements Tests
21762215
# ============================================================================
@@ -2361,6 +2400,7 @@ test_suite(
23612400
":measure_deps_are_context_only_test",
23622401
":measure_spec_is_merged_test",
23632402
":requirements_multi_spec_tests",
2403+
":requirements_package_tests",
23642404
":requirements_rst_tests",
23652405
":seooc_tests",
23662406
":sphinx_module_tests",
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
..
2+
# *******************************************************************************
3+
# Copyright (c) 2026 Contributors to the Eclipse Foundation
4+
#
5+
# See the NOTICE file(s) distributed with this work for additional
6+
# information regarding copyright ownership.
7+
#
8+
# This program and the accompanying materials are made available under the
9+
# terms of the Apache License Version 2.0 which is available at
10+
# https://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# SPDX-License-Identifier: Apache-2.0
13+
# *******************************************************************************
14+
15+
Collision Fixture A
16+
====================
17+
18+
.. comp_req:: From A
19+
:id: comp_req__collision_test__a
20+
:safety: QM
21+
22+
Requirement sourced from a/req.rst.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
..
2+
# *******************************************************************************
3+
# Copyright (c) 2026 Contributors to the Eclipse Foundation
4+
#
5+
# See the NOTICE file(s) distributed with this work for additional
6+
# information regarding copyright ownership.
7+
#
8+
# This program and the accompanying materials are made available under the
9+
# terms of the Apache License Version 2.0 which is available at
10+
# https://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# SPDX-License-Identifier: Apache-2.0
13+
# *******************************************************************************
14+
15+
Collision Fixture B
16+
====================
17+
18+
.. comp_req:: From B
19+
:id: comp_req__collision_test__b
20+
:safety: QM
21+
22+
Requirement sourced from b/req.rst.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# *******************************************************************************
2+
# Copyright (c) 2026 Contributors to the Eclipse Foundation
3+
#
4+
# See the NOTICE file(s) distributed with this work for additional
5+
# information regarding copyright ownership.
6+
#
7+
# This program and the accompanying materials are made available under the
8+
# terms of the Apache License Version 2.0 which is available at
9+
# https://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# SPDX-License-Identifier: Apache-2.0
12+
# *******************************************************************************
13+
"""Tests for the ``package`` attr and the rst_to_trlc output-path fix."""
14+
15+
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
16+
17+
# ============================================================================
18+
# assumed_system_requirements(package = ...) forwards to rst_to_trlc
19+
# ============================================================================
20+
21+
def _asr_package_forwarded_test_impl(ctx):
22+
env = analysistest.begin(ctx)
23+
actions = analysistest.target_actions(env)
24+
rst_actions = [a for a in actions if a.mnemonic == "RstToTrlc"]
25+
26+
asserts.true(env, len(rst_actions) == 1, "expected exactly one RstToTrlc action")
27+
if rst_actions:
28+
argv = rst_actions[0].argv
29+
asserts.true(env, "--package" in argv, "expected --package in RstToTrlc argv")
30+
asserts.equals(env, "CustomAsrPackage", argv[argv.index("--package") + 1])
31+
32+
return analysistest.end(env)
33+
34+
asr_package_forwarded_test = analysistest.make(_asr_package_forwarded_test_impl)
35+
36+
# ============================================================================
37+
# rst_to_trlc: same-basename srcs in one target must not collide
38+
# ============================================================================
39+
40+
def _rst_to_trlc_no_collision_test_impl(ctx):
41+
env = analysistest.begin(ctx)
42+
target_under_test = analysistest.target_under_test(env)
43+
outs = target_under_test[DefaultInfo].files.to_list()
44+
45+
asserts.equals(env, 2, len(outs))
46+
asserts.true(env, outs[0].path != outs[1].path, "same-basename srcs must produce distinct output paths")
47+
48+
return analysistest.end(env)
49+
50+
rst_to_trlc_no_collision_test = analysistest.make(_rst_to_trlc_no_collision_test_impl)
51+
52+
# ============================================================================
53+
# Test Suite
54+
# ============================================================================
55+
56+
def requirements_package_test_suite(name):
57+
"""Register the package-forwarding and output-collision tests."""
58+
native.test_suite(
59+
name = name,
60+
tests = [
61+
":asr_package_forwarded_test",
62+
":rst_to_trlc_no_collision_test",
63+
],
64+
)

‎bazel/rules/rules_score/test/rst_to_trlc_test.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,18 @@ def test_single_line_title_unaffected_by_continuation_handling(self):
368368
result = parse_directives(rst)
369369
self.assertEqual(result[0]["title"], "Single Line Title")
370370

371+
def test_unindented_next_directive_not_swallowed_as_continuation(self):
372+
rst = _rst(
373+
".. comp_req:: A",
374+
".. comp_req:: B",
375+
" :id: comp_req__test__b",
376+
)
377+
result = parse_directives(rst)
378+
self.assertEqual(len(result), 2)
379+
self.assertEqual(result[0]["title"], "A")
380+
self.assertEqual(result[1]["title"], "B")
381+
self.assertEqual(result[1]["fields"]["id"], "comp_req__test__b")
382+
371383

372384
# ---------------------------------------------------------------------------
373385
# render_trlc – TRLC output for each S-CORE type
@@ -428,6 +440,13 @@ def test_feat_req_imports_ref_package_when_refs_present(self):
428440
out = render_trlc(items, "FeatPkg", "AsrPkg")
429441
self.assertIn("import AsrPkg", out)
430442

443+
def test_feat_req_satisfies_that_is_only_a_filter_produces_no_derived_from(self):
444+
"""A satisfies value like "[version==1]" has no id before the filter;
445+
it must not render a malformed "derived_from = [AsrPkg.@1]" entry."""
446+
items = self._single("feat_req", {"satisfies": "[version==1]"})
447+
out = render_trlc(items, "FeatPkg", "AsrPkg")
448+
self.assertNotIn("derived_from", out)
449+
431450
# --- CompReq ---
432451

433452
def test_comp_req_produces_comp_req_type(self):
@@ -632,6 +651,29 @@ def test_strips_needs_filter_from_each_comma_separated_ref(self):
632651
["req_001", "req_002"],
633652
)
634653

654+
def test_ref_that_is_only_a_filter_expression_is_dropped(self):
655+
"""A ref field containing only a filter (no id before the bracket)
656+
must not render as an empty derived_from entry."""
657+
self.assertEqual(_collect_refs({"satisfies": "[version==1]"}), [])
658+
659+
def test_filter_containing_comma_is_not_split_into_two_refs(self):
660+
"""sphinx-needs filters may contain commas of their own (E.g. a
661+
top-level ``,`` inside the filter's argument list); only commas
662+
outside of the trailing ``[...]`` separate distinct refs."""
663+
self.assertEqual(
664+
_collect_refs({"satisfies": "req_001[version==1, status==x]"}),
665+
["req_001"],
666+
)
667+
668+
def test_filter_with_bracketed_list_argument_is_not_split(self):
669+
"""A ``,`` inside a nested ``[...]`` filter argument must not be
670+
treated as a ref separator, even though the top-level filter regex
671+
does not itself strip nested brackets."""
672+
self.assertEqual(
673+
_collect_refs({"satisfies": 'req_001[tags in ["a", "b"]]'}),
674+
['req_001[tags in ["a", "b"]]'],
675+
)
676+
635677

636678
# ---------------------------------------------------------------------------
637679
# _collect_body – blank-line handling consistency

0 commit comments

Comments
 (0)