refactor(mock-provider): make comment-only behavior explicit

conformance/mock-provider/src/methods.rs

@@ -248,7 +248,7 @@ impl Server {
         let edits: Vec<Value> = sorted_entries(&params.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<Puzzle> {
+    parse_document(&doc.text).into_puzzle()
+}
+
 fn check_document(doc: &Document) -> Result<(), HandlerError> {
     if doc.language_id != LANGUAGE_ID {
         return Err(HandlerError::lpp(

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<String> {
     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;
     }

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<usize> = (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..]);
 

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<Token>,
     pos: usize,

conformance/mock-provider/src/puzzle/symbols.rs

@@ -59,19 +59,21 @@ impl Puzzle {
         }
     }
 
-    pub(crate) fn occurrences(&self, name: &str, include_declaration: bool) -> Vec<Range> {
-        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<Item = Range> + '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<Range> {
+        let mut ranges: Vec<Range> = 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),
         }
     }
 }

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]

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<u32> {
@@ -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::<serde_json::Map<String, Value>>()
             .into()
     }