Add JSON output for review findings

Author: olwang

README.md

@@ -15,7 +15,7 @@ It currently implements:
 - `rss check --json <file.rss>` with serde-backed machine-readable diagnostics
 - `rss check --explain <code>` for diagnostic code explanations
 - `rss fmt <file.rss>` as a parse/check gate that prints the source unchanged when valid
-- `rss review <old.rss> <new.rss>` for a first-pass API/type/effect/freshness diff with structured risk categories and path-aware local/manage boundary summaries
+- `rss review <old.rss> <new.rss>` and `rss review --json <old.rss> <new.rss>` for first-pass API/type/effect/freshness diffs with structured risk categories and path-aware local/manage boundary summaries
 - Lexer, lightweight parser, syntax AST, HIR signature table, and semantic checks for the review-critical v0.4.1 rules
 - Builtin signatures for the current fixture stdlib surface, including `Image`, `File`, `Map`, `ResourcePool`, `Json`, `Csv`, database/resource helpers, and cache/config helpers
 - HIR type and field tables for class/struct/resource declarations and handle fields
@@ -61,7 +61,7 @@ Implemented diagnostic classes include:
 - local capture by managed closures
 - local capture by closures passed to retaining APIs
 - `take` of handle fields
-- API, type layout, path-aware local/manage boundary, effect, mode, and freshness changes in `rss review`, tagged with structured risk categories
+- API, type layout, path-aware local/manage boundary, effect, mode, and freshness changes in `rss review`, tagged with structured risk categories in human and JSON output
 - likely operator overload attempts
 
 Non-goals for this stage:
@@ -79,7 +79,7 @@ Non-goals for this stage:
 Near-term roadmap:
 
 1. Expand local/resource dataflow precision around typed `ResourcePool<T>` lease propagation and expression-order coverage for short-circuiting/control expressions.
-2. Expand `rss review` risk categories into machine-readable JSON output.
+2. Expand `rss review` JSON with spans and structured before/after fields.
 
 Run:
 
@@ -91,4 +91,5 @@ cargo run --bin rss -- check path/to/file.rss
 cargo run --bin rss -- check --json path/to/file.rss
 cargo run --bin rss -- check --explain RS0401
 cargo run --bin rss -- review old.rss new.rss
+cargo run --bin rss -- review --json old.rss new.rss
 ```

src/lib.rs

@@ -11,4 +11,6 @@ pub use diagnostic::{
     Diagnostic, DiagnosticExplanation, Severity, explain_diagnostic_code,
     format_diagnostic_explanation, format_diagnostics_human, format_diagnostics_json,
 };
-pub use review::{ReviewFinding, ReviewRisk, format_review_human, review_sources};
+pub use review::{
+    ReviewFinding, ReviewRisk, format_review_human, format_review_json, review_sources,
+};

src/main.rs

@@ -4,7 +4,8 @@ use std::process::ExitCode;
 
 use rsscript::{
     analyze_source, explain_diagnostic_code, format_diagnostic_explanation,
-    format_diagnostics_human, format_diagnostics_json, format_review_human, review_sources,
+    format_diagnostics_human, format_diagnostics_json, format_review_human, format_review_json,
+    review_sources,
 };
 
 fn main() -> ExitCode {
@@ -97,7 +98,8 @@ fn run_fmt(args: &[String]) -> ExitCode {
 }
 
 fn run_review(args: &[String]) -> ExitCode {
-    let [old_path, new_path] = args else {
+    let (json, old_path, new_path) = parse_review_args(args);
+    let (Some(old_path), Some(new_path)) = (old_path, new_path) else {
         print_usage();
         return ExitCode::from(2);
     };
@@ -124,13 +126,23 @@ fn run_review(args: &[String]) -> ExitCode {
         .chain(new_diagnostics.iter())
         .any(|diagnostic| diagnostic.severity.is_error());
     if has_errors {
-        print!("{}", format_diagnostics_human(&old_diagnostics));
-        print!("{}", format_diagnostics_human(&new_diagnostics));
+        if json {
+            let mut diagnostics = old_diagnostics;
+            diagnostics.extend(new_diagnostics);
+            println!("{}", format_diagnostics_json(&diagnostics));
+        } else {
+            print!("{}", format_diagnostics_human(&old_diagnostics));
+            print!("{}", format_diagnostics_human(&new_diagnostics));
+        }
         return ExitCode::from(1);
     }
 
     let findings = review_sources(old_path, &old_source, new_path, &new_source);
-    print!("{}", format_review_human(&findings));
+    if json {
+        println!("{}", format_review_json(&findings));
+    } else {
+        print!("{}", format_review_human(&findings));
+    }
     ExitCode::SUCCESS
 }
 
@@ -156,10 +168,28 @@ fn parse_path_args(args: &[String]) -> (bool, Option<&str>) {
     (json, path)
 }
 
+fn parse_review_args(args: &[String]) -> (bool, Option<&str>, Option<&str>) {
+    let mut json = false;
+    let mut paths = Vec::new();
+
+    for arg in args {
+        if arg == "--json" {
+            json = true;
+        } else {
+            paths.push(arg.as_str());
+        }
+    }
+
+    if paths.len() != 2 {
+        return (json, None, None);
+    }
+    (json, Some(paths[0]), Some(paths[1]))
+}
+
 fn print_usage() {
     eprintln!("usage:");
     eprintln!("  rsscript check [--json] <file.rss>");
     eprintln!("  rsscript check --explain <code>");
     eprintln!("  rsscript fmt <file.rss>");
-    eprintln!("  rsscript review <old.rss> <new.rss>");
+    eprintln!("  rsscript review [--json] <old.rss> <new.rss>");
 }

src/review.rs

@@ -1,20 +1,23 @@
 use std::collections::{BTreeMap, BTreeSet};
 
+use serde::Serialize;
+
 use crate::diagnostic::code;
 use crate::syntax::ast::{
     Block, CallArg, DataEffect, EffectDecl, Expr, FieldDecl, FileMode, FunctionDecl, Item, LetKind,
     Param, Stmt, TypeDecl, TypeKind, TypeRef,
 };
 use crate::syntax::parse_source;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 pub struct ReviewFinding {
     pub code: String,
     pub risk: ReviewRisk,
     pub summary: String,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
+#[serde(rename_all = "kebab-case")]
 pub enum ReviewRisk {
     Mode,
     Api,
@@ -123,6 +126,10 @@ pub fn format_review_human(findings: &[ReviewFinding]) -> String {
     output
 }
 
+pub fn format_review_json(findings: &[ReviewFinding]) -> String {
+    serde_json::to_string(findings).expect("review JSON serialization should not fail")
+}
+
 fn review_finding(code: &str, risk: ReviewRisk, summary: impl Into<String>) -> ReviewFinding {
     ReviewFinding {
         code: code.to_string(),

tests/checker.rs

@@ -5,7 +5,7 @@ use rsscript::syntax::ast::Item;
 use rsscript::syntax::parse_source;
 use rsscript::{
     ReviewRisk, analyze_source, explain_diagnostic_code, format_diagnostic_explanation,
-    format_diagnostics_json, format_review_human, review_sources,
+    format_diagnostics_json, format_review_human, format_review_json, review_sources,
 };
 use serde_json::Value;
 
@@ -77,6 +77,46 @@ fn diagnostic_explanations_are_available_by_code() {
     assert!(explain_diagnostic_code("RS9999").is_none());
 }
 
+#[test]
+fn review_json_uses_protocol_shape() {
+    let old_source = r#"
+mode: managed
+
+fn render(path: read Path) -> Image {
+    Image.load(path: read path)
+}
+"#;
+    let new_source = r#"
+mode: uses-local
+
+fn render(path: take Path) -> fresh Image
+    effects(retains(path))
+{
+    Image.load(path: read path)
+}
+"#;
+    let findings = review_sources("old.rss", old_source, "new.rss", new_source);
+    let json = format_review_json(&findings);
+    let value: Value = serde_json::from_str(&json).expect("review JSON should parse");
+    let items = value.as_array().expect("review JSON should be an array");
+
+    assert!(
+        items
+            .iter()
+            .any(|item| item["code"] == "RSR001" && item["risk"] == "mode")
+    );
+    assert!(
+        items
+            .iter()
+            .any(|item| item["code"] == "RSR006" && item["risk"] == "effect")
+    );
+    assert!(items.iter().all(|item| {
+        item["summary"]
+            .as_str()
+            .is_some_and(|summary| !summary.is_empty())
+    }));
+}
+
 #[test]
 fn syntax_parser_accepts_all_fixtures() {
     let mut paths = fixture_paths("tests/fixtures/pass");