From 9e67acd9dd16a602aabc4b2542bf529edf9cbf0c Mon Sep 17 00:00:00 2001 From: Teakowa Date: Thu, 22 Oct 2026 04:14:58 +0800 Subject: [PATCH] refactor(mock-provider): state comment-only behavior in code Express behavior that the comment cleanup left unstated: - server: CAPABILITIES entries carry a named min_minor field - symbols: occurrences_in_document_order returns declaration and references sorted at production; symbol_occurrences keeps the declaration-first contract by construction; rename_ranges no longer re-sorts - artifact: share is_valid_identifier and delete is_ident - parse: extract the sort_diagnostics_by_range_start step - edits: each ResolvedEdit carries its request_index, so the failing edit index and the document-sorted application share one index space - text: the phantom-line empty span is stated by a test - methods: the rename parse-error refusal is the named predicate parse_cleanly Fixes #98 --- conformance/mock-provider/src/methods.rs | 6 +- .../mock-provider/src/puzzle/artifact.rs | 13 +-- conformance/mock-provider/src/puzzle/edits.rs | 33 +++++--- conformance/mock-provider/src/puzzle/parse.rs | 8 +- .../mock-provider/src/puzzle/symbols.rs | 41 +++++----- conformance/mock-provider/src/puzzle/tests.rs | 31 ++++--- conformance/mock-provider/src/server.rs | 81 ++++++++++++++----- 7 files changed, 140 insertions(+), 73 deletions(-) diff --git a/conformance/mock-provider/src/methods.rs b/conformance/mock-provider/src/methods.rs index c4d9702..4b01ddf 100644 --- a/conformance/mock-provider/src/methods.rs +++ b/conformance/mock-provider/src/methods.rs @@ -248,7 +248,7 @@ impl Server { let edits: Vec = sorted_entries(¶ms.documents) .into_iter() .map(|(key, doc)| { - let Some(puzzle) = parse_document(&doc.text).into_puzzle() else { + let Some(puzzle) = parse_cleanly(doc) else { return Err(HandlerError::lpp( "invalidDocument", json!({ "uri": doc.uri, "reason": "documentHasErrors" }), @@ -450,6 +450,10 @@ fn parsed_set( .collect() } +fn parse_cleanly(doc: &Document) -> Option { + parse_document(&doc.text).into_puzzle() +} + fn check_document(doc: &Document) -> Result<(), HandlerError> { if doc.language_id != LANGUAGE_ID { return Err(HandlerError::lpp( diff --git a/conformance/mock-provider/src/puzzle/artifact.rs b/conformance/mock-provider/src/puzzle/artifact.rs index 21469c0..983d55d 100644 --- a/conformance/mock-provider/src/puzzle/artifact.rs +++ b/conformance/mock-provider/src/puzzle/artifact.rs @@ -3,6 +3,7 @@ use serde_json::{Value, json}; use std::fmt::Write; use super::Puzzle; +use super::symbols::is_valid_identifier; pub(crate) const ARTIFACT_FORMAT: &str = "x-demo/puzzle-eval-v1"; pub(crate) const SUMMARY_ARTIFACT_FORMAT: &str = "x-demo/puzzle-summary-v1"; @@ -43,17 +44,11 @@ struct SheetOp { arg: i64, } -fn is_ident(name: &str) -> bool { - let mut chars = name.chars(); - matches!(chars.next(), Some(c) if c.is_ascii_alphabetic() || c == '_') - && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') -} - pub(crate) fn reconstruct_source(content: &str) -> Option { let sheet: Sheet = serde_json::from_str(content).ok()?; - if !is_ident(&sheet.name) - || sheet.ops.iter().any(|op| !is_ident(&op.name)) - || sheet.solution.iter().any(|name| !is_ident(name)) + if !is_valid_identifier(&sheet.name) + || sheet.ops.iter().any(|op| !is_valid_identifier(&op.name)) + || sheet.solution.iter().any(|name| !is_valid_identifier(name)) { return None; } diff --git a/conformance/mock-provider/src/puzzle/edits.rs b/conformance/mock-provider/src/puzzle/edits.rs index f2b6a9d..97a6e53 100644 --- a/conformance/mock-provider/src/puzzle/edits.rs +++ b/conformance/mock-provider/src/puzzle/edits.rs @@ -19,31 +19,40 @@ impl EditValidation { } } +struct ResolvedEdit { + request_index: usize, + start: usize, + end: usize, +} + pub(crate) fn validate_edits(text: &str, edits: &[(Range, String)]) -> EditValidation { let src = SourceText::new(text); let mut resolved = Vec::with_capacity(edits.len()); - for (i, (range, _)) in edits.iter().enumerate() { + for (request_index, (range, _)) in edits.iter().enumerate() { match (src.byte_of(range.start), src.byte_of(range.end)) { - (Some(start), Some(end)) if start <= end => resolved.push((start, end)), - _ => return EditValidation::invalid("rangeOutOfBounds", Some(i)), + (Some(start), Some(end)) if start <= end => resolved.push(ResolvedEdit { + request_index, + start, + end, + }), + _ => return EditValidation::invalid("rangeOutOfBounds", Some(request_index)), } } - let mut order: Vec = (0..edits.len()).collect(); - order.sort_by_key(|&i| resolved[i].0); - for pair in order.windows(2) { - if resolved[pair[0]].1 > resolved[pair[1]].0 { - return EditValidation::invalid("overlappingEdits", Some(pair[1])); + resolved.sort_by_key(|edit| edit.start); + for pair in resolved.windows(2) { + if pair[0].end > pair[1].start { + return EditValidation::invalid("overlappingEdits", Some(pair[1].request_index)); } } let mut result = String::with_capacity(text.len()); let mut cursor = 0; - for &i in &order { - result.push_str(&text[cursor..resolved[i].0]); - result.push_str(&edits[i].1); - cursor = resolved[i].1; + for edit in &resolved { + result.push_str(&text[cursor..edit.start]); + result.push_str(&edits[edit.request_index].1); + cursor = edit.end; } result.push_str(&text[cursor..]); diff --git a/conformance/mock-provider/src/puzzle/parse.rs b/conformance/mock-provider/src/puzzle/parse.rs index fc749d9..7c8a131 100644 --- a/conformance/mock-provider/src/puzzle/parse.rs +++ b/conformance/mock-provider/src/puzzle/parse.rs @@ -12,15 +12,17 @@ pub(crate) fn parse_document(text: &str) -> ParseOutput { diagnostics: Vec::new(), }; let puzzle = parse_top(&mut parser); - parser - .diagnostics - .sort_by_key(|d| (d.range.start.line, d.range.start.character)); + sort_diagnostics_by_range_start(&mut parser.diagnostics); ParseOutput { puzzle, diagnostics: parser.diagnostics, } } +fn sort_diagnostics_by_range_start(diagnostics: &mut [Diagnostic]) { + diagnostics.sort_by_key(|d| (d.range.start.line, d.range.start.character)); +} + struct Parser { tokens: Vec, pos: usize, diff --git a/conformance/mock-provider/src/puzzle/symbols.rs b/conformance/mock-provider/src/puzzle/symbols.rs index f8a58cf..ff12cc1 100644 --- a/conformance/mock-provider/src/puzzle/symbols.rs +++ b/conformance/mock-provider/src/puzzle/symbols.rs @@ -59,19 +59,21 @@ impl Puzzle { } } - pub(crate) fn occurrences(&self, name: &str, include_declaration: bool) -> Vec { - let mut ranges = Vec::new(); - if include_declaration { - if let Some(op) = self.op(name) { - ranges.push(op.name_range); - } - } - ranges.extend( - self.solution - .iter() - .filter(|entry| entry.name == name) - .map(|entry| entry.range), - ); + fn references<'a>(&'a self, name: &'a str) -> impl Iterator + 'a { + self.solution + .iter() + .filter(move |entry| entry.name == name) + .map(|entry| entry.range) + } + + pub(crate) fn occurrences_in_document_order(&self, name: &str) -> Vec { + let mut ranges: Vec = self + .op(name) + .map(|op| op.name_range) + .into_iter() + .chain(self.references(name)) + .collect(); + ranges.sort_by_key(|r| (r.start.line, r.start.character)); ranges } @@ -85,7 +87,12 @@ impl Puzzle { .then_some(self.name_range) .into_iter() .collect(), - Symbol::Op { name } => self.occurrences(name, include_declaration), + Symbol::Op { name } => self + .declaration_range(symbol) + .filter(|_| include_declaration) + .into_iter() + .chain(self.references(name)) + .collect(), } } @@ -105,11 +112,7 @@ impl Puzzle { Vec::new() } } - Symbol::Op { name } => { - let mut ranges = self.occurrences(name, true); - ranges.sort_by_key(|r| (r.start.line, r.start.character)); - ranges - } + Symbol::Op { name } => self.occurrences_in_document_order(name), } } } diff --git a/conformance/mock-provider/src/puzzle/tests.rs b/conformance/mock-provider/src/puzzle/tests.rs index e244031..ce23c68 100644 --- a/conformance/mock-provider/src/puzzle/tests.rs +++ b/conformance/mock-provider/src/puzzle/tests.rs @@ -169,15 +169,17 @@ fn utf16_positions_are_exact() { } #[test] -fn trailing_newline_phantom_line_is_a_valid_position() { +fn trailing_newline_phantom_line_has_an_empty_span() { let src = SourceText::new("a\n"); assert_eq!(src.line_text(0), "a"); assert_eq!(src.line_text(1), ""); assert_eq!(src.byte_of(pos(1, 0)), Some(2)); + assert_eq!(src.position_of(2), pos(1, 0)); let src = SourceText::new("a\r\n"); assert_eq!(src.line_text(1), ""); assert_eq!(src.byte_of(pos(1, 0)), Some(3)); + assert_eq!(src.position_of(3), pos(1, 0)); let src = SourceText::new("a\n\nb"); assert_eq!(src.line_text(1), ""); @@ -383,18 +385,29 @@ fn declaration_range_points_at_the_declaration() { } #[test] -fn occurrences_list_declaration_first_then_references() { - let (_, puzzle) = puzzle_of(CLEAN); +fn occurrences_are_in_document_order() { + const SRC: &str = "puzzle p {\n target = 2\n start = 1\n solution = [ d, d ]\n ops {\n d: x => x + 1\n }\n}"; + let (_, puzzle) = puzzle_of(SRC); + assert_eq!( + puzzle.occurrences_in_document_order("d"), + vec![rng(3, 15, 3, 16), rng(3, 18, 3, 19), rng(5, 4, 5, 5)] + ); + assert!(puzzle.occurrences_in_document_order("triple").is_empty()); +} + +#[test] +fn symbol_occurrences_list_the_declaration_first() { + const SRC: &str = "puzzle p {\n target = 2\n start = 1\n solution = [ d, d ]\n ops {\n d: x => x + 1\n }\n}"; + let (src, puzzle) = puzzle_of(SRC); + let symbol = symbol_at(&src, &puzzle, 3, 15).expect("op d"); assert_eq!( - puzzle.occurrences("double", true), - vec![rng(4, 4, 4, 10), rng(7, 15, 7, 21), rng(7, 23, 7, 29)] + puzzle.symbol_occurrences(&symbol, true), + vec![rng(5, 4, 5, 5), rng(3, 15, 3, 16), rng(3, 18, 3, 19)] ); assert_eq!( - puzzle.occurrences("double", false), - vec![rng(7, 15, 7, 21), rng(7, 23, 7, 29)] + puzzle.symbol_occurrences(&symbol, false), + vec![rng(3, 15, 3, 16), rng(3, 18, 3, 19)] ); - assert_eq!(puzzle.occurrences("plus1", true), vec![rng(5, 4, 5, 9)]); - assert!(puzzle.occurrences("triple", true).is_empty()); } #[test] diff --git a/conformance/mock-provider/src/server.rs b/conformance/mock-provider/src/server.rs index 9a115af..27ca6ca 100644 --- a/conformance/mock-provider/src/server.rs +++ b/conformance/mock-provider/src/server.rs @@ -13,19 +13,60 @@ pub(crate) const LANGUAGE_ID: &str = "x-demo-lang"; const SERVER_NAME: &str = "lpp-mock-provider"; const LANGUAGE_EXTENSIONS: [&str; 1] = ["xdl"]; -const CAPABILITIES: &[(&str, u32)] = &[ - ("check", 0), - ("compile", 0), - ("reconstruct", 0), - ("symbols", 0), - ("definition", 0), - ("references", 0), - ("rename", 0), - ("editValidation", 0), - ("projectLoading", 1), - ("sourceIdentity", 3), - ("lookup", 5), - ("resolveIds", 6), +struct Capability { + name: &'static str, + min_minor: u32, +} + +const CAPABILITIES: &[Capability] = &[ + Capability { + name: "check", + min_minor: 0, + }, + Capability { + name: "compile", + min_minor: 0, + }, + Capability { + name: "reconstruct", + min_minor: 0, + }, + Capability { + name: "symbols", + min_minor: 0, + }, + Capability { + name: "definition", + min_minor: 0, + }, + Capability { + name: "references", + min_minor: 0, + }, + Capability { + name: "rename", + min_minor: 0, + }, + Capability { + name: "editValidation", + min_minor: 0, + }, + Capability { + name: "projectLoading", + min_minor: 1, + }, + Capability { + name: "sourceIdentity", + min_minor: 3, + }, + Capability { + name: "lookup", + min_minor: 5, + }, + Capability { + name: "resolveIds", + min_minor: 6, + }, ]; fn minor_of(version: &str) -> Option { @@ -46,9 +87,9 @@ fn not_initialized(id: &Value) -> Value { } pub(crate) fn disable_capability(disabled: &mut HashSet<&'static str>, name: &str) -> bool { - match CAPABILITIES.iter().find(|(n, _)| *n == name) { - Some(&(n, _)) => { - disabled.insert(n); + match CAPABILITIES.iter().find(|cap| cap.name == name) { + Some(cap) => { + disabled.insert(cap.name); true } None => false, @@ -79,18 +120,18 @@ impl Server { } pub(crate) fn capability(&self, name: &str) -> bool { - let Some(&(_, min)) = CAPABILITIES.iter().find(|(n, _)| *n == name) else { + let Some(cap) = CAPABILITIES.iter().find(|cap| cap.name == name) else { return false; }; - self.since(min) && !self.disabled.contains(name) + self.since(cap.min_minor) && !self.disabled.contains(name) } fn capabilities_json(&self) -> Value { let minor = self.version_minor.unwrap_or(0); CAPABILITIES .iter() - .filter(|(_, min)| minor >= *min) - .map(|(name, _)| ((*name).to_string(), json!(self.capability(name)))) + .filter(|cap| minor >= cap.min_minor) + .map(|cap| (cap.name.to_string(), json!(self.capability(cap.name)))) .collect::>() .into() }