Skip to content

Commit 76781a7

Browse files
babywormclaude
andcommitted
Harden YAML nested map shape, add direct STL includes
Codex Round 4 cross-review: - ConventionChecker.cpp: nested input/output/instance prefix access threw YAML::BadSubscript before the inner try when the parent node was a scalar instead of a map. Now the parent shape is validated before child indexing, with the whole nested access guarded. - StyleSyntaxScanner.cpp: added direct <vector> and <unordered_map> includes; previously transitively available via slang headers but a portability trap on stricter libc++. Tests extended: malformed-parent-shape YAML now exercised. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0cffc9c commit 76781a7

3 files changed

Lines changed: 60 additions & 6 deletions

File tree

src/ConventionChecker.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,29 +80,52 @@ ConventionRules loadConventionRules(const std::string& yamlPath) {
8080
rules.inputPrefix = *value;
8181
else if (auto value = getString(root, "inputPrefix"); value)
8282
rules.inputPrefix = *value;
83-
else if (root["input"] && root["input"]["prefix"]) {
83+
else {
84+
// Codex Round 4 cross-review: guard the parent-shape check too.
85+
// If `input` is a scalar (e.g. `input: scalar_not_a_map`) then
86+
// `root["input"]["prefix"]` throws YAML::BadSubscript before the
87+
// inner try fires. Wrap the entire parent+child access so a
88+
// non-map parent node is silently ignored.
8489
try {
85-
rules.inputPrefix = root["input"]["prefix"].as<std::string>();
90+
if (auto in = root["input"]; in && in.IsMap()) {
91+
if (auto p = in["prefix"]; p && p.IsScalar()) {
92+
try {
93+
rules.inputPrefix = p.as<std::string>();
94+
} catch (const YAML::Exception&) {}
95+
}
96+
}
8697
} catch (const YAML::Exception&) {}
8798
}
8899

89100
if (auto value = getString(root, "output_prefix"); value)
90101
rules.outputPrefix = *value;
91102
else if (auto value = getString(root, "outputPrefix"); value)
92103
rules.outputPrefix = *value;
93-
else if (root["output"] && root["output"]["prefix"]) {
104+
else {
94105
try {
95-
rules.outputPrefix = root["output"]["prefix"].as<std::string>();
106+
if (auto out = root["output"]; out && out.IsMap()) {
107+
if (auto p = out["prefix"]; p && p.IsScalar()) {
108+
try {
109+
rules.outputPrefix = p.as<std::string>();
110+
} catch (const YAML::Exception&) {}
111+
}
112+
}
96113
} catch (const YAML::Exception&) {}
97114
}
98115

99116
if (auto value = getString(root, "instance_prefix"); value)
100117
rules.instancePrefix = *value;
101118
else if (auto value = getString(root, "instancePrefix"); value)
102119
rules.instancePrefix = *value;
103-
else if (root["instance"] && root["instance"]["prefix"]) {
120+
else {
104121
try {
105-
rules.instancePrefix = root["instance"]["prefix"].as<std::string>();
122+
if (auto inst = root["instance"]; inst && inst.IsMap()) {
123+
if (auto p = inst["prefix"]; p && p.IsScalar()) {
124+
try {
125+
rules.instancePrefix = p.as<std::string>();
126+
} catch (const YAML::Exception&) {}
127+
}
128+
}
106129
} catch (const YAML::Exception&) {}
107130
}
108131

src/StyleSyntaxScanner.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010

1111
#include <string>
1212
#include <string_view>
13+
#include <unordered_map>
1314
#include <unordered_set>
15+
#include <vector>
1416

1517
namespace connect {
1618

tests/test_convention_checker.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,35 @@ TEST_CASE("ConventionChecker: malformed nested prefix field keeps default and do
203203
std::remove(yamlPath);
204204
}
205205

206+
TEST_CASE("ConventionChecker: malformed parent-shape YAML keeps default and does not throw") {
207+
// Codex Round 4 cross-review: if a parent node like `input` is a
208+
// scalar rather than a map (e.g. `input: scalar_not_a_map`), then
209+
// `root["input"]["prefix"]` throws YAML::BadSubscript before the
210+
// inner try/catch fires. The outer try added in R4 must absorb this
211+
// so load completes and the field keeps its struct default.
212+
const char* yamlPath = "test_convention_parent_shape.yaml";
213+
{
214+
std::ofstream ofs(yamlPath);
215+
REQUIRE(ofs.good());
216+
ofs << "input: scalar_not_a_map\n"; // parent is scalar, not map
217+
ofs << "output: scalar_not_a_map\n"; // same for output
218+
ofs << "instance: scalar_not_a_map\n"; // same for instance
219+
ofs << "output_prefix: out_\n"; // valid flat key after bad parents
220+
}
221+
222+
ConventionRules rules;
223+
REQUIRE_NOTHROW(rules = loadConventionRules(yamlPath));
224+
225+
// All nested-prefix fields keep struct defaults (no throw, no crash).
226+
ConventionRules defaultRules;
227+
CHECK(rules.inputPrefix == defaultRules.inputPrefix);
228+
CHECK(rules.instancePrefix == defaultRules.instancePrefix);
229+
// The valid flat key after the bad parents must still apply.
230+
CHECK(rules.outputPrefix == "out_");
231+
232+
std::remove(yamlPath);
233+
}
234+
206235
TEST_CASE("ConventionChecker: bad scalar does not skip later valid keys") {
207236
// Codex Round 2 cross-review: the previous whole-block try/catch
208237
// aborted on the first YAML::BadConversion, silently dropping any

0 commit comments

Comments
 (0)