fix: batch close #125, #126, #127, #128, #129 (parser + instance)

crates/spar-base-db/src/lib.rs

@@ -73,6 +73,20 @@ pub struct ParseError {
     pub offset: usize,
 }
 
+/// Convert a byte offset into source text to a 1-based `(line, column)` pair.
+///
+/// Lines are separated by `\n`. Column counts Unicode scalar values (chars),
+/// not bytes, so multibyte characters don't inflate the column number. If
+/// `offset` exceeds `source.len()` it is clamped to the end.
+pub fn offset_to_line_col(source: &str, offset: usize) -> (u32, u32) {
+    let clamped = offset.min(source.len());
+    let prefix = &source[..clamped];
+    let line = prefix.bytes().filter(|&b| b == b'\n').count() as u32 + 1;
+    let line_start = prefix.rfind('\n').map(|i| i + 1).unwrap_or(0);
+    let col = source[line_start..clamped].chars().count() as u32 + 1;
+    (line, col)
+}
+
 /// A parsed annex within a file.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct AnnexResult {
@@ -207,6 +221,33 @@ end ErrorLib;
         assert_eq!(result.annexes()[0].name, "EMV2");
     }
 
+    #[test]
+    fn offset_to_line_col_basic() {
+        let src = "abc\ndef\nghi";
+        assert_eq!(offset_to_line_col(src, 0), (1, 1));
+        assert_eq!(offset_to_line_col(src, 2), (1, 3));
+        assert_eq!(offset_to_line_col(src, 3), (1, 4)); // column after "abc"
+        assert_eq!(offset_to_line_col(src, 4), (2, 1)); // 'd'
+        assert_eq!(offset_to_line_col(src, 8), (3, 1)); // 'g'
+        assert_eq!(offset_to_line_col(src, 10), (3, 3)); // 'i'
+    }
+
+    #[test]
+    fn offset_to_line_col_clamps_past_end() {
+        let src = "one";
+        assert_eq!(offset_to_line_col(src, 9999), (1, 4));
+    }
+
+    #[test]
+    fn offset_to_line_col_multibyte() {
+        // 'é' is 2 bytes in UTF-8; offsets must land on char boundaries.
+        let src = "héllo\nworld";
+        let e_offset = 3; // byte offset after "hé" (1 + 2)
+        assert_eq!(offset_to_line_col(src, e_offset), (1, 3));
+        let w_offset = src.find('w').unwrap();
+        assert_eq!(offset_to_line_col(src, w_offset), (2, 1));
+    }
+
     #[test]
     fn incremental_reparse() {
         use salsa::Setter;

crates/spar-cli/src/main.rs

@@ -124,7 +124,8 @@ fn cmd_parse(args: &[String]) {
         } else {
             has_errors = true;
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
         }
     }
@@ -439,7 +440,8 @@ fn cmd_analyze(args: &[String]) {
         let parsed = spar_syntax::parse(&source);
         if !parsed.ok() {
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
             eprintln!("Cannot analyze: parse errors in {}", file_path);
             process::exit(1);
@@ -586,7 +588,8 @@ fn cmd_allocate(args: &[String]) {
         let parsed = spar_syntax::parse(&source);
         if !parsed.ok() {
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
             eprintln!("Cannot allocate: parse errors in {}", file_path);
             process::exit(1);
@@ -1287,7 +1290,8 @@ fn cmd_verify(args: &[String]) {
         let parsed = spar_syntax::parse(&source);
         if !parsed.ok() {
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
             eprintln!("Cannot verify: parse errors in {}", file_path);
             process::exit(1);
@@ -1581,7 +1585,8 @@ fn cmd_codegen(args: &[String]) {
         let parsed = spar_syntax::parse(&source);
         if !parsed.ok() {
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
             eprintln!("Cannot codegen: parse errors in {}", file_path);
             process::exit(1);
@@ -1715,7 +1720,8 @@ fn cmd_sysml2_parse(args: &[String]) {
         } else {
             has_errors = true;
             for err in parsed.errors() {
-                eprintln!("{}:{}: {}", file_path, err.offset, err.msg);
+                let (line, col) = spar_base_db::offset_to_line_col(&source, err.offset);
+                eprintln!("{}:{}:{}: {}", file_path, line, col, err.msg);
             }
         }
     }
@@ -1768,7 +1774,8 @@ fn cmd_sysml2_lower(args: &[String]) {
     let parsed = spar_sysml2::parse(&all_source);
     if !parsed.ok() {
         for err in parsed.errors() {
-            eprintln!("parse error at {}: {}", err.offset, err.msg);
+            let (line, col) = spar_base_db::offset_to_line_col(&all_source, err.offset);
+            eprintln!("parse error at {}:{}: {}", line, col, err.msg);
         }
         process::exit(1);
     }
@@ -1838,7 +1845,8 @@ fn cmd_sysml2_extract(args: &[String]) {
     let parsed = spar_sysml2::parse(&all_source);
     if !parsed.ok() {
         for err in parsed.errors() {
-            eprintln!("parse error at {}: {}", err.offset, err.msg);
+            let (line, col) = spar_base_db::offset_to_line_col(&all_source, err.offset);
+            eprintln!("parse error at {}:{}: {}", line, col, err.msg);
         }
         process::exit(1);
     }

crates/spar-cli/tests/applies_to_nested.rs

@@ -0,0 +1,176 @@
+//! Integration test for GitHub issues #128 and #129:
+//!
+//!   #128 — `binding_rules` analyzer missed `Actual_Processor_Binding`
+//!         declared with `applies to fw.firmware` (dotted path).
+//!   #129 — `spar instance --format json` omitted all `properties`.
+//!
+//! Both have the same root cause: property associations with a non-empty
+//! `applies to <path>` clause were dropped when converting
+//! PropertyAssociationItem → PropertyValue during instantiation. The fix
+//! attaches them eagerly to the resolved target instance.
+
+use std::env;
+use std::fs;
+use std::process::Command;
+
+fn spar() -> Command {
+    Command::new(env!("CARGO_BIN_EXE_spar"))
+}
+
+/// AS5506B §8.3 reproducer from issue #128: `applies to fw.firmware`
+/// where `fw` is a process subcomponent containing thread `firmware`.
+const MODEL: &str = "\
+package Test_Applies_To
+public
+  processor NRF52840
+  end NRF52840;
+
+  thread DoorFirmware
+  end DoorFirmware;
+
+  process DoorFirmwareProcess
+  end DoorFirmwareProcess;
+
+  process implementation DoorFirmwareProcess.Impl
+    subcomponents
+      firmware: thread DoorFirmware;
+  end DoorFirmwareProcess.Impl;
+
+  system DoorNode
+  end DoorNode;
+
+  system implementation DoorNode.Battery
+    subcomponents
+      mcu: processor NRF52840;
+      fw: process DoorFirmwareProcess.Impl;
+    properties
+      Actual_Processor_Binding => (reference (mcu)) applies to fw.firmware;
+  end DoorNode.Battery;
+end Test_Applies_To;
+";
+
+fn write_model() -> std::path::PathBuf {
+    let path = env::temp_dir().join(format!(
+        "spar_applies_to_nested_{}.aadl",
+        std::process::id()
+    ));
+    fs::write(&path, MODEL).expect("write temp AADL");
+    path
+}
+
+/// #128: binding_rules must see the binding on the thread instance.
+#[test]
+fn issue_128_binding_rules_accepts_nested_applies_to() {
+    let path = write_model();
+    let output = spar()
+        .arg("analyze")
+        .arg("--root")
+        .arg("Test_Applies_To::DoorNode.Battery")
+        .arg(&path)
+        .output()
+        .expect("failed to run spar");
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    let combined = format!("{stdout}\n{stderr}");
+
+    assert!(
+        !combined.contains("missing required Actual_Processor_Binding"),
+        "binding_rules still reports the thread as unbound — #128 regression.\n\
+         combined output:\n{combined}"
+    );
+
+    let _ = fs::remove_file(&path);
+}
+
+/// Unresolvable-path fallback: a property association whose `applies to`
+/// path does not resolve to a real subcomponent must not crash the
+/// pipeline. The property stays on the declaring component and a
+/// diagnostic is emitted; `spar instance` still completes.
+#[test]
+fn applies_to_unresolvable_path_emits_diagnostic() {
+    let src = "\
+package Test_Unresolvable
+public
+  processor Proc
+  end Proc;
+
+  system Sys
+  end Sys;
+
+  system implementation Sys.Impl
+    subcomponents
+      cpu: processor Proc;
+    properties
+      Actual_Processor_Binding => (reference (cpu)) applies to no_such_sub.no_such_thread;
+  end Sys.Impl;
+end Test_Unresolvable;
+";
+    let path = env::temp_dir().join(format!("spar_applies_to_bad_{}.aadl", std::process::id()));
+    fs::write(&path, src).expect("write temp AADL");
+
+    let output = spar()
+        .arg("instance")
+        .arg("--root")
+        .arg("Test_Unresolvable::Sys.Impl")
+        .arg(&path)
+        .output()
+        .expect("failed to run spar");
+
+    assert!(
+        output.status.success(),
+        "spar instance must not crash on unresolvable applies_to; stderr:\n{}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("applies_to path")
+            && stderr.contains("no_such_sub.no_such_thread")
+            && stderr.contains("could not be resolved"),
+        "expected unresolved-path diagnostic in stderr, got:\n{stderr}"
+    );
+
+    let _ = fs::remove_file(&path);
+}
+
+/// #129: `spar instance --format json` must emit the property on the
+/// target instance, not the declaring system.
+#[test]
+fn issue_129_instance_json_includes_applies_to_properties() {
+    let path = write_model();
+    let output = spar()
+        .arg("instance")
+        .arg("--root")
+        .arg("Test_Applies_To::DoorNode.Battery")
+        .arg("--format")
+        .arg("json")
+        .arg(&path)
+        .output()
+        .expect("failed to run spar");
+
+    assert!(
+        output.status.success(),
+        "spar instance failed; stderr:\n{}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    // The property is attached to the thread instance `firmware` that
+    // lives under `fw`. A serialized snippet would look like:
+    //   "name": "firmware", ..., "properties": { "Actual_Processor_Binding": ... }
+    // We do a shape-insensitive assertion against the raw text.
+    assert!(
+        stdout.contains("Actual_Processor_Binding"),
+        "instance JSON does not contain the expected property name — #129 regression.\n\
+         stdout:\n{stdout}"
+    );
+    // Bonus guard: the reference text should be present too.
+    assert!(
+        stdout.contains("reference") && stdout.contains("mcu"),
+        "instance JSON does not contain the bound target — #129 regression.\n\
+         stdout:\n{stdout}"
+    );
+
+    let _ = fs::remove_file(&path);
+}

crates/spar-cli/tests/parse_error_line_col.rs

@@ -0,0 +1,54 @@
+use std::env;
+use std::fs;
+use std::process::Command;
+
+fn spar() -> Command {
+    Command::new(env!("CARGO_BIN_EXE_spar"))
+}
+
+/// Regression test for GitHub issues #125/#126/#127 side-note:
+/// `spar parse` must print 1-based `line:col` in error messages, not a raw
+/// byte offset.
+#[test]
+fn parse_error_prints_line_and_col() {
+    // AADL with a deliberate syntax error on line 3 (`let` is not valid AADL).
+    let src = "package Broken\n\
+public\n\
+let x;\n\
+end Broken;\n";
+
+    let path = env::temp_dir().join(format!(
+        "spar_parse_error_{}_{}.aadl",
+        std::process::id(),
+        line!()
+    ));
+    fs::write(&path, src).expect("write temp AADL");
+
+    let output = spar()
+        .arg("parse")
+        .arg(&path)
+        .output()
+        .expect("failed to run spar");
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+
+    // Error must be reported on line 3 (where `let x;` is), with a column.
+    // Format: "<path>:<line>:<col>: <msg>"
+    let expected_prefix = format!("{}:3:", path.display());
+    assert!(
+        stderr.contains(&expected_prefix),
+        "expected stderr to contain {:?} (line:col for line-3 error); got:\n{}",
+        expected_prefix,
+        stderr
+    );
+
+    // Guard against the pre-fix regression where a raw byte offset (>= file len)
+    // was printed as the line number.
+    assert!(
+        !stderr.contains(":20:") && !stderr.contains(":30:") && !stderr.contains(":40:"),
+        "stderr looks like it still contains a raw byte offset (file is ~38 bytes):\n{}",
+        stderr
+    );
+
+    let _ = fs::remove_file(&path);
+}