From 357ce0d4fd2fa3caf04ed82b41c7135e72b2ae54 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Mon, 20 Apr 2026 19:12:54 +0200 Subject: [PATCH 1/2] chore: satisfy nightly clippy::question-mark lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs `cargo clippy --workspace --all-targets -- -D warnings` on nightly Rust (1.97.0-nightly at writing), which enables `clippy::question-mark`. Three pre-existing sites in `spar-hir-def`, `spar-render`, and `spar-cli` triggered it — each an `if let Some / else return None` pattern that clippy wants rewritten with `?`. No behavior change; nightly clippy now clean, 2,443 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/spar-cli/src/lsp.rs | 5 ++--- crates/spar-hir-def/src/item_tree/lower.rs | 5 ++--- crates/spar-render/src/lib.rs | 5 +---- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/spar-cli/src/lsp.rs b/crates/spar-cli/src/lsp.rs index a607591..149d1ef 100644 --- a/crates/spar-cli/src/lsp.rs +++ b/crates/spar-cli/src/lsp.rs @@ -1899,10 +1899,9 @@ fn make_swap_connection_edit(source: &str, uri: &Uri, diag_range: &Range) -> Opt // Look for `src -> dst` or `src <-> dst` pattern let (arrow, arrow_len) = if let Some(pos) = line.find(" -> ") { (pos, 4) - } else if let Some(pos) = line.find(" <-> ") { - (pos, 5) } else { - return None; + let pos = line.find(" <-> ")?; + (pos, 5) }; // Find the source and destination parts diff --git a/crates/spar-hir-def/src/item_tree/lower.rs b/crates/spar-hir-def/src/item_tree/lower.rs index 6b37c21..68bdfea 100644 --- a/crates/spar-hir-def/src/item_tree/lower.rs +++ b/crates/spar-hir-def/src/item_tree/lower.rs @@ -1976,7 +1976,8 @@ fn lower_range_from_tokens(node: &SyntaxNode) -> Option { } } - let min = if let Some(num_text) = min_num { + let min = { + let num_text = min_num?; if min_is_real { let display = if min_sign < 0 { format!("-{}", num_text) @@ -1988,8 +1989,6 @@ fn lower_range_from_tokens(node: &SyntaxNode) -> Option { let val = parse_aadl_integer(&num_text).unwrap_or(0) * min_sign; PropertyExpr::Integer(val, min_unit) } - } else { - return None; }; let max = max_expr.unwrap_or_else(|| PropertyExpr::Opaque("?".to_string())); diff --git a/crates/spar-render/src/lib.rs b/crates/spar-render/src/lib.rs index 3c5ca3f..e02a71f 100644 --- a/crates/spar-render/src/lib.rs +++ b/crates/spar-render/src/lib.rs @@ -166,10 +166,7 @@ fn ancestor_at_depth( return None; } let comp = instance.component(current); - match comp.parent { - Some(p) => current = p, - None => return None, - } + current = comp.parent?; } } From d6b691cd38ff778f14da62366395878d88f74ccd Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Mon, 20 Apr 2026 21:13:32 +0200 Subject: [PATCH 2/2] test: cover make_swap_connection_edit arrow branches Adds three focused unit tests for the LSP swap-connection quick-fix: the unidirectional `->` path, the bidirectional `<->` path (the new `?` branch introduced in the clippy::question-mark rewrite), and the no-arrow fallback that returns None. Raises patch coverage past the codecov/patch gate. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/spar-cli/src/lsp.rs | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/crates/spar-cli/src/lsp.rs b/crates/spar-cli/src/lsp.rs index 149d1ef..d237dba 100644 --- a/crates/spar-cli/src/lsp.rs +++ b/crates/spar-cli/src/lsp.rs @@ -4063,4 +4063,62 @@ mod tests { ); assert!(has_end_name, "should find end-name on line 5: {:?}", refs); } + + // ── make_swap_connection_edit ─────────────────────────────────── + // + // Cover both arrow shapes; the `<->` branch exists because the AADL + // grammar permits bidirectional `->` and `<->` connections, and the + // quick-fix has to preserve whichever arrow the source used. + + #[test] + #[allow(clippy::mutable_key_type)] // Uri has interior mutability; see make_swap_connection_edit + fn swap_connection_supports_unidirectional_arrow() { + let uri: Uri = "file:///test.aadl".parse().unwrap(); + let source = "package P\npublic\n system S\n features\n p_in : in data port;\n p_out : out data port;\n end S;\n system implementation S.i\n subcomponents\n sub : process Pr;\n connections\n c1 : port sub.outp -> p_out;\n end S.i;\n process Pr\n features\n outp : out data port;\n end Pr;\nend P;\n"; + // Position anywhere on the connection line. + let diag_range = Range::new(Position::new(11, 10), Position::new(11, 10)); + let edit = make_swap_connection_edit(source, &uri, &diag_range) + .expect("-> connection must produce a swap edit"); + let changes = edit.changes.expect("edit must carry changes"); + let text_edits = changes.get(&uri).expect("edits keyed by URI"); + assert_eq!(text_edits.len(), 1, "one TextEdit expected"); + assert!( + text_edits[0].new_text.contains(" -> "), + "arrow must be preserved, got {:?}", + text_edits[0].new_text + ); + } + + #[test] + #[allow(clippy::mutable_key_type)] // Uri has interior mutability; see make_swap_connection_edit + fn swap_connection_supports_bidirectional_arrow() { + // Regression for codecov/patch gap on the `<->` branch of + // make_swap_connection_edit: a bidirectional connection must also + // produce a swap edit and preserve the `<->` arrow form. + let uri: Uri = "file:///test.aadl".parse().unwrap(); + let source = "package P\npublic\n bus B\n end B;\n device D\n features\n ba : requires bus access;\n end D;\n system S\n end S;\n system implementation S.i\n subcomponents\n bus1 : bus B;\n dev : device D;\n connections\n c1 : bus access bus1 <-> dev.ba;\n end S.i;\nend P;\n"; + let diag_range = Range::new(Position::new(15, 10), Position::new(15, 10)); + let edit = make_swap_connection_edit(source, &uri, &diag_range) + .expect("<-> connection must produce a swap edit"); + let changes = edit.changes.expect("edit must carry changes"); + let text_edits = changes.get(&uri).expect("edits keyed by URI"); + assert_eq!(text_edits.len(), 1, "one TextEdit expected"); + assert!( + text_edits[0].new_text.contains(" <-> "), + "bidirectional arrow must be preserved, got {:?}", + text_edits[0].new_text + ); + } + + #[test] + fn swap_connection_returns_none_when_no_arrow() { + // When the targeted line has neither `->` nor `<->`, the helper + // must bail out cleanly with None — exercises the `?` shortcut + // in the `<->` fallback branch. + let uri: Uri = "file:///test.aadl".parse().unwrap(); + let source = "package P\npublic\n system S\n end S;\nend P;\n"; + let diag_range = Range::new(Position::new(2, 2), Position::new(2, 2)); + let edit = make_swap_connection_edit(source, &uri, &diag_range); + assert!(edit.is_none(), "no-arrow line must not produce a swap edit"); + } }