Skip to content

Commit 4914eb9

Browse files
committed
Validate file feature declarations
1 parent ab37b23 commit 4914eb9

10 files changed

Lines changed: 157 additions & 20 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ Files are managed-only by default. Advanced capabilities must be declared with `
122122
features: local
123123
```
124124

125-
The MVP implements only `local`. Future feature names such as `native`, `unsafe`, `async`, and `device` are reserved for capabilities that change review risk or require checker/runtime support. Ordinary libraries like JSON, File, Image, HTTP, Map, and Regex are not features.
125+
The MVP implements only `local`. Future feature names such as `native`, `unsafe`, `async`, `device`, `ffi`, and `reflection` are reserved for capabilities that change review risk or require checker/runtime support. Ordinary libraries like JSON, File, Image, HTTP, Map, and Regex are not features.
126126

127127
### Reviewable at the boundary
128128

RSScript_v0.5_Spec.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2544,7 +2544,7 @@ manage operation
25442544
effects(retains(...))
25452545
with resource
25462546
ResourcePool
2547-
file features such as local/native/unsafe/async/device
2547+
file features such as local/native/unsafe/async/device/ffi/reflection
25482548
native / unsafe
25492549
unknown external call
25502550
writes to managed state
@@ -2562,6 +2562,8 @@ async elevated risk
25622562
native high risk
25632563
unsafe high risk
25642564
device high risk
2565+
ffi high risk
2566+
reflection elevated risk
25652567
```
25662568

25672569
This file-level risk does not require every helper function in the file to be classified as must-review. Region classification still depends on the function's own semantic facts and propagated callee risk.

src/analyzer.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pub(crate) struct Analyzer<'a> {
6666
impl Analyzer<'_> {
6767
fn run(&mut self) {
6868
self.check_single_feature_declaration();
69+
self.check_unknown_file_features();
6970
self.check_removed_profile_declarations();
7071
self.check_unsupported_syntax();
7172
self.check_duplicate_declarations();
@@ -101,6 +102,27 @@ impl Analyzer<'_> {
101102
}
102103
}
103104

105+
fn check_unknown_file_features(&mut self) {
106+
for feature in &self.syntax_program.unknown_features {
107+
self.diagnostics.push(
108+
Diagnostic::error(
109+
code::UNKNOWN_FILE_FEATURE,
110+
format!("Unknown file feature `{}`.", feature.name),
111+
feature.span.clone(),
112+
"unknown feature",
113+
)
114+
.with_cause(
115+
"File features must be review-relevant capabilities recognized by this compiler.",
116+
)
117+
.with_fix(
118+
"remove_or_correct_feature",
119+
"Remove the feature name or replace it with a supported feature such as `local`.",
120+
"manual",
121+
),
122+
);
123+
}
124+
}
125+
104126
fn check_removed_profile_declarations(&mut self) {
105127
for span in &self.syntax_program.profile_spans {
106128
self.diagnostics.push(

src/diagnostic.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub mod code {
1616
pub const INVALID_TRY_OPERATOR: &str = "RS0013";
1717
pub const INVALID_NOALLOC_ALLOCATION: &str = "RS0014";
1818
pub const UNSUPPORTED_SYNTAX: &str = "RS0015";
19+
pub const UNKNOWN_FILE_FEATURE: &str = "RS0016";
1920
pub const FEATURE_VIOLATION: &str = "RS0101";
2021
pub const UNNAMED_ARGUMENT: &str = "RS0201";
2122
pub const MISSING_DATA_EFFECT: &str = "RS0202";
@@ -290,6 +291,11 @@ static DIAGNOSTIC_EXPLANATIONS: &[DiagnosticExplanation] = &[
290291
title: "invalid noalloc allocation",
291292
explanation: "`effects(noalloc)` forbids obvious allocation sites such as value construction and `manage` migration.",
292293
},
294+
DiagnosticExplanation {
295+
code: code::UNKNOWN_FILE_FEATURE,
296+
title: "unknown file feature",
297+
explanation: "A `features:` header may only list review-relevant capabilities known to this compiler version. Unknown feature names are rejected so typos do not silently change review risk.",
298+
},
293299
DiagnosticExplanation {
294300
code: code::FEATURE_VIOLATION,
295301
title: "feature violation",

src/review.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,21 +1101,25 @@ fn feature_name(feature: &FileFeature) -> &'static str {
11011101
FileFeature::Unsafe => "unsafe",
11021102
FileFeature::Async => "async",
11031103
FileFeature::Device => "device",
1104+
FileFeature::Ffi => "ffi",
1105+
FileFeature::Reflection => "reflection",
11041106
}
11051107
}
11061108

11071109
fn review_map_file_risk(features: &[FileFeature]) -> ReviewMapFileRisk {
11081110
if features.iter().any(|feature| {
11091111
matches!(
11101112
feature,
1111-
FileFeature::Native | FileFeature::Unsafe | FileFeature::Device
1113+
FileFeature::Native | FileFeature::Unsafe | FileFeature::Device | FileFeature::Ffi
11121114
)
11131115
}) {
11141116
ReviewMapFileRisk::High
1115-
} else if features
1116-
.iter()
1117-
.any(|feature| matches!(feature, FileFeature::Local | FileFeature::Async))
1118-
{
1117+
} else if features.iter().any(|feature| {
1118+
matches!(
1119+
feature,
1120+
FileFeature::Local | FileFeature::Async | FileFeature::Reflection
1121+
)
1122+
}) {
11191123
ReviewMapFileRisk::Elevated
11201124
} else {
11211125
ReviewMapFileRisk::Low
@@ -1136,6 +1140,8 @@ fn review_map_feature_reason(feature: &str) -> Option<&'static str> {
11361140
"unsafe" => Some("unsafe capability enabled"),
11371141
"async" => Some("async control-flow capability enabled"),
11381142
"device" => Some("device capability enabled"),
1143+
"ffi" => Some("ffi boundary capability enabled"),
1144+
"reflection" => Some("reflection capability enabled"),
11391145
_ => None,
11401146
}
11411147
}

src/rust_lower.rs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,14 @@ impl<'a> RustLowerer<'a> {
318318
out.push_str(
319319
"// Runtime hooks are intentionally explicit while Rust lowering is stabilizing.\n",
320320
);
321-
if self.program.has_feature(FileFeature::Local) {
322-
out.push_str("// RSScript features: local\n");
323-
} else {
321+
let feature_names = lowered_feature_names(&self.program.features);
322+
if feature_names.is_empty() {
324323
out.push_str("// RSScript features: <none>\n");
324+
} else {
325+
out.push_str(&format!(
326+
"// RSScript features: {}\n",
327+
feature_names.join(", ")
328+
));
325329
}
326330
out.push('\n');
327331

@@ -1758,6 +1762,24 @@ fn rust_package_main(program: &Program, package_name: &str) -> Option<String> {
17581762
))
17591763
}
17601764

1765+
fn lowered_feature_names(features: &[FileFeature]) -> Vec<&'static str> {
1766+
let mut names = features
1767+
.iter()
1768+
.map(|feature| match feature {
1769+
FileFeature::Local => "local",
1770+
FileFeature::Native => "native",
1771+
FileFeature::Unsafe => "unsafe",
1772+
FileFeature::Async => "async",
1773+
FileFeature::Device => "device",
1774+
FileFeature::Ffi => "ffi",
1775+
FileFeature::Reflection => "reflection",
1776+
})
1777+
.collect::<Vec<_>>();
1778+
names.sort_unstable();
1779+
names.dedup();
1780+
names
1781+
}
1782+
17611783
fn is_runnable_main(function: &FunctionDecl) -> bool {
17621784
runnable_main_kind(function).is_some()
17631785
}

src/syntax/ast.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,20 @@ pub enum FileFeature {
77
Unsafe,
88
Async,
99
Device,
10+
Ffi,
11+
Reflection,
12+
}
13+
14+
#[derive(Debug, Clone, PartialEq, Eq)]
15+
pub struct UnknownFileFeature {
16+
pub name: String,
17+
pub span: Span,
1018
}
1119

1220
#[derive(Debug, Clone, PartialEq, Eq)]
1321
pub struct Program {
1422
pub features: Vec<FileFeature>,
23+
pub unknown_features: Vec<UnknownFileFeature>,
1524
pub feature_spans: Vec<Span>,
1625
pub profile_spans: Vec<Span>,
1726
pub items: Vec<Item>,

src/syntax/parser.rs

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::lexer::{Token, TokenKind, lex};
22
use crate::syntax::ast::{
33
BinaryOp, Block, CallArg, Callee, DataEffect, EffectDecl, Expr, FieldDecl, FileFeature,
44
FunctionDecl, GenericBound, GenericParam, IfStmt, Item, LetKind, LetStmt, LoopStmt, Param,
5-
Program, ReturnStmt, Stmt, TypeDecl, TypeKind, TypeRef, WithStmt,
5+
Program, ReturnStmt, Stmt, TypeDecl, TypeKind, TypeRef, UnknownFileFeature, WithStmt,
66
};
77

88
pub fn parse_source(file: &str, source: &str) -> Program {
@@ -19,17 +19,25 @@ struct Parser<'a> {
1919
index: usize,
2020
}
2121

22+
struct ParsedFeatures {
23+
features: Vec<FileFeature>,
24+
unknown_features: Vec<UnknownFileFeature>,
25+
}
26+
2227
impl Parser<'_> {
2328
fn parse_program(&mut self) -> Program {
2429
let mut features = Vec::new();
30+
let mut unknown_features = Vec::new();
2531
let mut feature_spans = Vec::new();
2632
let mut profile_spans = Vec::new();
2733
let mut items = Vec::new();
2834

2935
while !self.is_eof() {
3036
if self.at_ident("features") && self.peek_symbol(1, ":") {
3137
feature_spans.push(self.tokens[self.index].span.clone());
32-
features.extend(self.parse_features());
38+
let parsed = self.parse_features();
39+
features.extend(parsed.features);
40+
unknown_features.extend(parsed.unknown_features);
3341
} else if self.at_ident("profile") && self.peek_symbol(1, ":") {
3442
profile_spans.push(self.tokens[self.index].span.clone());
3543
self.index += 1;
@@ -49,27 +57,40 @@ impl Parser<'_> {
4957

5058
Program {
5159
features,
60+
unknown_features,
5261
feature_spans,
5362
profile_spans,
5463
items,
5564
}
5665
}
5766

58-
fn parse_features(&mut self) -> Vec<FileFeature> {
67+
fn parse_features(&mut self) -> ParsedFeatures {
5968
self.index += 2;
6069
let end = declaration_line_end(self.tokens, self.index);
6170
let mut features = Vec::new();
71+
let mut unknown_features = Vec::new();
6272
while self.index < end {
6373
if self.at_symbol(",") {
6474
self.index += 1;
6575
continue;
6676
}
67-
if let Some(feature) = parse_file_feature(self.tokens.get(self.index)) {
77+
let token = self.tokens.get(self.index);
78+
if let Some(feature) = parse_file_feature(token) {
6879
features.push(feature);
80+
} else if let Some(token) = token
81+
&& !matches!(token.kind, TokenKind::Eof)
82+
{
83+
unknown_features.push(UnknownFileFeature {
84+
name: token.text(),
85+
span: token.span.clone(),
86+
});
6987
}
7088
self.index += 1;
7189
}
72-
features
90+
ParsedFeatures {
91+
features,
92+
unknown_features,
93+
}
7394
}
7495

7596
fn parse_type_decl(&mut self) -> Option<TypeDecl> {
@@ -1080,6 +1101,10 @@ fn parse_file_feature(token: Option<&Token>) -> Option<FileFeature> {
10801101
Some(FileFeature::Async)
10811102
} else if token.is_ident_text("device") {
10821103
Some(FileFeature::Device)
1104+
} else if token.is_ident_text("ffi") {
1105+
Some(FileFeature::Ffi)
1106+
} else if token.is_ident_text("reflection") {
1107+
Some(FileFeature::Reflection)
10831108
} else {
10841109
None
10851110
}

tests/checker.rs

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,6 +1642,28 @@ async fn fetch(url: read Url) -> Result<fresh Bytes, NetworkError>
16421642
assert!(analyze_source("net.rssi", source).is_empty());
16431643
}
16441644

1645+
#[test]
1646+
fn checker_rejects_unknown_file_features() {
1647+
let source = r#"
1648+
features: local, locall
1649+
1650+
fn main() -> Unit {
1651+
return Unit
1652+
}
1653+
"#;
1654+
let program = parse_source("features.rss", source);
1655+
1656+
assert_eq!(program.features.len(), 1);
1657+
assert_eq!(program.unknown_features.len(), 1);
1658+
assert_eq!(program.unknown_features[0].name, "locall");
1659+
let diagnostics = analyze_source("features.rss", source);
1660+
assert!(
1661+
diagnostics
1662+
.iter()
1663+
.any(|diagnostic| diagnostic.code == "RS0016")
1664+
);
1665+
}
1666+
16451667
#[test]
16461668
fn review_map_marks_public_rssi_signatures_review_required() {
16471669
let source = r#"
@@ -1886,15 +1908,18 @@ fn update(counter: read Counter) -> Unit {
18861908
#[test]
18871909
fn review_map_reports_file_features() {
18881910
let source = r#"
1889-
features: local, native
1911+
features: local, native, ffi, reflection
18901912
18911913
fn process() -> Unit {
18921914
return Unit
18931915
}
18941916
"#;
18951917
let map = review_map_sources(vec![("features.rss", source)]);
18961918

1897-
assert_eq!(map.files[0].features, vec!["local", "native"]);
1919+
assert_eq!(
1920+
map.files[0].features,
1921+
vec!["ffi", "local", "native", "reflection"]
1922+
);
18981923
assert_eq!(map.files[0].risk, ReviewMapFileRisk::High);
18991924
assert!(
19001925
map.files[0]
@@ -1908,12 +1933,26 @@ fn process() -> Unit {
19081933
.iter()
19091934
.any(|reason| reason == "native boundary capability enabled")
19101935
);
1936+
assert!(
1937+
map.files[0]
1938+
.reasons
1939+
.iter()
1940+
.any(|reason| reason == "ffi boundary capability enabled")
1941+
);
1942+
assert!(
1943+
map.files[0]
1944+
.reasons
1945+
.iter()
1946+
.any(|reason| reason == "reflection capability enabled")
1947+
);
19111948
let human = format_review_map_human(&map);
1912-
assert!(human.contains("features.rss: features local, native; risk high"));
1949+
assert!(human.contains("features.rss: features ffi, local, native, reflection; risk high"));
19131950
let json: Value =
19141951
serde_json::from_str(&format_review_map_json(&map)).expect("review map JSON should parse");
1915-
assert_eq!(json["files"][0]["features"][0], "local");
1916-
assert_eq!(json["files"][0]["features"][1], "native");
1952+
assert_eq!(json["files"][0]["features"][0], "ffi");
1953+
assert_eq!(json["files"][0]["features"][1], "local");
1954+
assert_eq!(json["files"][0]["features"][2], "native");
1955+
assert_eq!(json["files"][0]["features"][3], "reflection");
19171956
assert_eq!(json["files"][0]["risk"], "high");
19181957
assert!(
19191958
json["files"][0]["reasons"]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// expect: RS0016
2+
features: locall
3+
4+
fn main() -> Unit {
5+
return Unit
6+
}

0 commit comments

Comments
 (0)