Skip to content

Commit c39f5c6

Browse files
committed
test(ir): close the two acceptance gaps review found in a689326
Both were real, and both were the same shape: an assertion that passed for a reason weaker than its own name. SECONDARY SARIF PATH. `Evidence.line` reaches a region by a route the primary control never touches — `steps_for` copies it into a `Step`, `emittable` applies the `>= 1` gate, and `related_locations` and `code_flow` each build a `Region` from it. Measured: a truncation planted in `steps_for` left ALL NINE earlier controls green, because `EvidenceIdentity` inspects the `Evidence` and the primary region never passes through a `Step`. The new control walks the band through evidence and asserts BOTH consumers — they share `emittable` today, and checking only `relatedLocations` would have been leaning on that rather than testing it. DI FALLBACK EXACTNESS. `a_non_positive_site_takes_the_registration_fallback` asserted that the verdict was NOT anchored at the negative site. That catches the `>= 1` -> `!= 0` refactor and nothing else: a fallback landing on any third coordinate passed it, while the test's name claimed otherwise. Now filtered to DI004 and asserted exactly — `("reg.cs", 5)` for i64::MIN / -1 / 0, and `("call.cs", line)` for 1 / u32::MAX+1 / i64::MAX. Verified by mutating the fallback to `reg_line + 1`, which the old form passed. Mutations 7 and 8, both caught; 8 of 8 across the representation work. Refs #250, #259. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM
1 parent a689326 commit c39f5c6

2 files changed

Lines changed: 87 additions & 15 deletions

File tree

‎rust/crates/own-analysis/tests/coordinate_band.rs‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,29 +124,43 @@ fn an_effect_verdict_carries_every_line_the_door_accepts() {
124124
/// from `!= 0`.
125125
#[test]
126126
fn a_non_positive_site_takes_the_registration_fallback() {
127+
// Asserted as an EXACT anchor, not as "did not use the site". The weaker
128+
// form catches the `>= 1` -> `!= 0` refactor and would still pass if the
129+
// fallback landed on some third coordinate — an assertion that does not
130+
// say what its own name says.
127131
for line in [i64::MIN, -1, 0] {
128-
for (file, got, code) in di_verdicts(&di004(line)) {
129-
assert_ne!(
130-
(file.as_str(), got.get()),
131-
("call.cs", line),
132-
"{code}: a site at line {line} was used as the anchor; `>= 1` \
133-
must send it to the registration fallback. A refactor to \
134-
`!= 0` passes every previously existing test and fails here — \
135-
which is the whole reason this control exists."
136-
);
137-
}
132+
let di004_verdicts: Vec<_> = di_verdicts(&di004(line))
133+
.into_iter()
134+
.filter(|(_, _, code)| *code == "DI004")
135+
.collect();
136+
let (file, got, _) = di004_verdicts
137+
.first()
138+
.unwrap_or_else(|| panic!("DI004 expected for a site at line {line}"));
139+
assert_eq!(
140+
(file.as_str(), got.get()),
141+
("reg.cs", 5),
142+
"a site at line {line} must anchor at the REGISTRATION site \
143+
(reg.cs:5). A refactor to `!= 0` passes every previously existing \
144+
test and fails here — which is the whole reason this control \
145+
exists — and so does a fallback that picks some other coordinate."
146+
);
138147
}
139148
}
140149

141150
#[test]
142151
fn a_positive_site_beyond_u32_is_still_a_site() {
143152
// The other half: the sentinel must not have become "small enough to fit".
144153
for line in [1_i64, 4_294_967_296, i64::MAX] {
145-
let used_site = di_verdicts(&di004(line))
146-
.iter()
147-
.any(|(file, got, _)| file == "call.cs" && got.get() == line);
148-
assert!(
149-
used_site,
154+
let di004_verdicts: Vec<_> = di_verdicts(&di004(line))
155+
.into_iter()
156+
.filter(|(_, _, code)| *code == "DI004")
157+
.collect();
158+
let (file, got, _) = di004_verdicts
159+
.first()
160+
.unwrap_or_else(|| panic!("DI004 expected for a site at line {line}"));
161+
assert_eq!(
162+
(file.as_str(), got.get()),
163+
("call.cs", line),
150164
"a site at line {line} did not anchor the verdict — the sentinel \
151165
narrowed to something a u32 can hold"
152166
);

‎rust/crates/own-diagnostics/tests/coordinate_band.rs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,61 @@ fn ordering_over_the_band_is_signed_and_total() {
137137
which would reorder every emitted diagnostic on a file that has one"
138138
);
139139
}
140+
141+
/// The **secondary** projection path, which the primary control cannot reach.
142+
///
143+
/// `Evidence.line` travels a different route to SARIF: `steps_for` copies it
144+
/// into a `Step`, `emittable` applies the `>= 1` gate, and `related_locations`
145+
/// and `code_flow` each build a `Region` from it. So a truncation introduced in
146+
/// `steps_for` leaves every other control here green — `EvidenceIdentity`
147+
/// inspects the `Evidence`, and the primary region never passes through a
148+
/// `Step` at all.
149+
///
150+
/// Both consumers are asserted, not just one: they share `emittable` today, and
151+
/// a control that only checked `relatedLocations` would be relying on that.
152+
#[test]
153+
fn the_secondary_sarif_path_carries_evidence_lines_over_the_band() {
154+
let expected: [(i64, Option<i64>); 5] = [
155+
(-1, None),
156+
(0, None),
157+
(1, Some(1)),
158+
(ABOVE_U32, Some(ABOVE_U32)),
159+
(i64::MAX, Some(i64::MAX)),
160+
];
161+
for (line, want) in expected {
162+
let d = diag(1).with_evidence(vec![
163+
Evidence::new(SourceLine(line), "registered here").with_role("registered")
164+
]);
165+
let log = build_sarif(&[d], "A.cs", "error");
166+
let result = &log.runs[0].results[0];
167+
168+
let related: Vec<i64> = result
169+
.related_locations
170+
.iter()
171+
.filter_map(|l| l.physical_location.region.as_ref())
172+
.map(|r| r.start_line.get())
173+
.collect();
174+
let flow: Vec<i64> = result
175+
.code_flows
176+
.iter()
177+
.flat_map(|f| f.thread_flows.iter())
178+
.flat_map(|t| t.locations.iter())
179+
.filter_map(|l| l.location.physical_location.region.as_ref())
180+
.map(|r| r.start_line.get())
181+
.collect();
182+
183+
let want_vec: Vec<i64> = want.into_iter().collect();
184+
assert_eq!(
185+
related, want_vec,
186+
"evidence line {line}: relatedLocations mismatch — below 1 the \
187+
secondary location is dropped entirely, at or above it the value \
188+
rides through verbatim"
189+
);
190+
assert_eq!(
191+
flow, want_vec,
192+
"evidence line {line}: codeFlows mismatch. relatedLocations and \
193+
codeFlows are two separate consumers of the same Step; checking \
194+
only one would lean on them sharing `emittable` today"
195+
);
196+
}
197+
}

0 commit comments

Comments
 (0)