Skip to content

Commit 1aab94d

Browse files
committed
Report unsafe native additions in review
1 parent 47fbb54 commit 1aab94d

3 files changed

Lines changed: 81 additions & 0 deletions

File tree

src/diagnostic.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub mod code {
5050
pub const REVIEW_TYPE_KIND_CHANGED: &str = "RSR009";
5151
pub const REVIEW_TYPE_FIELDS_CHANGED: &str = "RSR010";
5252
pub const REVIEW_BOUNDARY_CHANGED: &str = "RSR011";
53+
pub const REVIEW_UNSAFE_NATIVE_ADDED: &str = "RSR012";
5354
}
5455

5556
#[derive(Debug, Clone, PartialEq, Eq)]

src/review.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ pub enum ReviewRisk {
4646
TypeLayout,
4747
Effect,
4848
Boundary,
49+
Unsafe,
4950
}
5051

5152
impl ReviewRisk {
@@ -56,6 +57,7 @@ impl ReviewRisk {
5657
Self::TypeLayout => "type-layout",
5758
Self::Effect => "effect",
5859
Self::Boundary => "boundary",
60+
Self::Unsafe => "unsafe",
5961
}
6062
}
6163
}
@@ -232,6 +234,10 @@ fn review_fixes(code: &str) -> Vec<ReviewFix> {
232234
"review_local_manage_boundary",
233235
"Review the changed local ownership and manage boundary.",
234236
),
237+
code::REVIEW_UNSAFE_NATIVE_ADDED => (
238+
"review_unsafe_native_boundary",
239+
"Review the new unsafe or native boundary and require explicit justification.",
240+
),
235241
_ => ("review_change", "Review this source-level contract change."),
236242
};
237243
vec![ReviewFix {
@@ -383,6 +389,31 @@ fn compare_function(old: &FunctionSig, new: &FunctionSig, findings: &mut Vec<Rev
383389
Some(effects_contract(&new.effects)),
384390
));
385391
}
392+
let old_unsafe_native = unsafe_native_effects(&old.effects);
393+
let new_unsafe_native = unsafe_native_effects(&new.effects);
394+
let added_unsafe_native: BTreeSet<_> = new_unsafe_native
395+
.difference(&old_unsafe_native)
396+
.cloned()
397+
.collect();
398+
if !added_unsafe_native.is_empty() {
399+
findings.push(review_finding(
400+
code::REVIEW_UNSAFE_NATIVE_ADDED,
401+
ReviewRisk::Unsafe,
402+
format!(
403+
"function `{}` added unsafe/native boundary: {}.",
404+
old.name,
405+
effects_contract(&added_unsafe_native)
406+
),
407+
paired_spans(
408+
&old.span,
409+
&new.span,
410+
"old function",
411+
"new unsafe/native boundary",
412+
),
413+
Some(effects_contract(&old_unsafe_native)),
414+
Some(effects_contract(&new_unsafe_native)),
415+
));
416+
}
386417
if old.boundary != new.boundary {
387418
findings.push(review_finding(
388419
code::REVIEW_BOUNDARY_CHANGED,
@@ -513,6 +544,14 @@ fn effects_contract(effects: &BTreeSet<String>) -> String {
513544
effects.iter().cloned().collect::<Vec<_>>().join(", ")
514545
}
515546

547+
fn unsafe_native_effects(effects: &BTreeSet<String>) -> BTreeSet<String> {
548+
effects
549+
.iter()
550+
.filter(|effect| matches!(effect.as_str(), "unsafe" | "native"))
551+
.cloned()
552+
.collect()
553+
}
554+
516555
fn type_contract(ty: &TypeSig) -> String {
517556
format!(
518557
"{} {} {{ {} }}",

tests/checker.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,47 @@ fn inspect(image: read Image) -> Unit {
228228
assert!(format_review_human(&findings).contains("RSR006[effect]:"));
229229
}
230230

231+
#[test]
232+
fn review_reports_new_unsafe_native_usage() {
233+
let old_source = r#"
234+
mode: managed
235+
236+
fn checksum(data: read Bytes) -> UInt64
237+
effects(no_panic)
238+
{
239+
Bytes.checksum(data: read data)
240+
}
241+
"#;
242+
let new_source = r#"
243+
mode: managed
244+
245+
fn checksum(data: read Bytes) -> UInt64
246+
effects(no_panic, unsafe, native)
247+
{
248+
Native.checksum(data: read data)
249+
}
250+
"#;
251+
252+
let findings = review_sources("old.rss", old_source, "new.rss", new_source);
253+
let unsafe_finding = findings
254+
.iter()
255+
.find(|finding| finding.code == "RSR012")
256+
.expect("expected unsafe/native review finding");
257+
258+
assert_eq!(unsafe_finding.risk, ReviewRisk::Unsafe);
259+
assert_eq!(unsafe_finding.before.as_deref(), Some("<none>"));
260+
assert_eq!(unsafe_finding.after.as_deref(), Some("native, unsafe"));
261+
assert!(format_review_human(&findings).contains("RSR012[unsafe]: function `checksum` added"));
262+
263+
let json = format_review_json(&findings);
264+
let value: Value = serde_json::from_str(&json).expect("review JSON should parse");
265+
assert!(value.as_array().is_some_and(|items| {
266+
items
267+
.iter()
268+
.any(|item| item["code"] == "RSR012" && item["risk"] == "unsafe")
269+
}));
270+
}
271+
231272
#[test]
232273
fn review_reports_type_layout_changes() {
233274
let old_source = r#"

0 commit comments

Comments
 (0)