From fab95167a6836376c6dd27fb2fa81cee7e58cc45 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 7 Oct 2026 01:07:03 +0800 Subject: [PATCH] refactor(conformance): move x-demo-lang symbol semantics behind the puzzle seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit methods.rs re-derived declaration/reference/rename rules from Puzzle's public fields in three private derivations (puzzle_symbols, rename_edits, and inline matches). Those semantics now live as typed queries on Puzzle in a new puzzle::symbols module — symbol_at, document_symbols, declaration_range, occurrences (single-sourcing the declaration+references rule), rename_collides, and rename_ranges — so the wire adapter only shapes params, envelopes, and method-namespaced refusal codes. Op, OpKind, SolutionEntry, and Puzzle's fields are now module-private, making further AST-derivation a compile error. Behavior-preserving: the full fixture suite passes unchanged, and the moved rules gain unit coverage in puzzle/tests.rs. Fixes #48 --- conformance/mock-provider/src/methods.rs | 132 ++++------------- conformance/mock-provider/src/puzzle.rs | 80 +++------- .../mock-provider/src/puzzle/symbols.rs | 140 ++++++++++++++++++ conformance/mock-provider/src/puzzle/tests.rs | 133 +++++++++++++++++ 4 files changed, 317 insertions(+), 168 deletions(-) create mode 100644 conformance/mock-provider/src/puzzle/symbols.rs diff --git a/conformance/mock-provider/src/methods.rs b/conformance/mock-provider/src/methods.rs index cf6f6a9..78aa539 100644 --- a/conformance/mock-provider/src/methods.rs +++ b/conformance/mock-provider/src/methods.rs @@ -13,10 +13,9 @@ use sha2::{Digest, Sha256}; use crate::project::documents_for_request; use crate::puzzle::{ - self, ARTIFACT_FORMAT, COMPILE_ARTIFACT_FORMATS, KIND_OP, KIND_PUZZLE, LookupScope, - ParseOutput, Position, Puzzle, Range, SUMMARY_ARTIFACT_FORMAT, SourceText, Symbol, - compile_artifact, is_valid_identifier, parse_document, reconstruct_source, summary_artifact, - symbol_at, + self, ARTIFACT_FORMAT, COMPILE_ARTIFACT_FORMATS, LookupScope, ParseOutput, Position, Puzzle, + Range, SUMMARY_ARTIFACT_FORMAT, SourceText, Symbol, compile_artifact, is_valid_identifier, + parse_document, reconstruct_source, summary_artifact, }; use crate::rpc::{HandlerError, parse_params}; use crate::server::{LANGUAGE_ID, Server}; @@ -172,7 +171,7 @@ impl Server { let symbols = parsed .puzzle .as_ref() - .map(puzzle_symbols) + .map(Puzzle::document_symbols) .unwrap_or_default(); document_view(doc, "symbols", json!(symbols)) }) @@ -188,15 +187,9 @@ impl Server { "definition.noSymbolAtPosition", ¶ms.document.uri, )?; - let range = match symbol { - Symbol::Puzzle { range, .. } => range, - Symbol::Op { name } => { - puzzle - .op(&name) - .expect("resolved puzzles reference declared ops") - .name_range - } - }; + let range = puzzle + .declaration_range(&symbol) + .expect("resolved puzzles reference declared ops"); Ok(json!({ "locations": [{ "uri": params.document.uri, "range": range }] })) @@ -210,20 +203,11 @@ impl Server { "references.noSymbolAtPosition", ¶ms.document.uri, )?; - let name = symbol.name(); - let mut locations = Vec::new(); - if params.include_declaration { - if let Some(op) = puzzle.op(name) { - locations.push(location_json(¶ms.document.uri, op.name_range)); - } - } - locations.extend( - puzzle - .solution - .iter() - .filter(|entry| entry.name == name) - .map(|entry| location_json(¶ms.document.uri, entry.range)), - ); + let locations: Vec = puzzle + .occurrences(symbol.name(), params.include_declaration) + .into_iter() + .map(|range| location_json(¶ms.document.uri, range)) + .collect(); Ok(json!({ "locations": locations })) } @@ -256,34 +240,23 @@ impl Server { ¶ms.position_document_uri, )?; - let name = symbol.name().to_string(); - let target = match symbol { - Symbol::Puzzle { .. } => RenameTarget::Puzzle(name), - Symbol::Op { .. } => { - if puzzle.op(&name).is_some() - && name != params.new_name - && puzzle.op(¶ms.new_name).is_some() - { - return Err(HandlerError::refusal( - "rename.nameCollision", - json!({ "newName": params.new_name }), - "new name collides with an existing symbol", - )); - } - RenameTarget::Op(name) - } - }; + if puzzle.rename_collides(&symbol, ¶ms.new_name) { + return Err(HandlerError::refusal( + "rename.nameCollision", + json!({ "newName": params.new_name }), + "new name collides with an existing symbol", + )); + } let edits: Vec = sorted_entries(¶ms.documents) .into_iter() .filter_map(|(key, doc)| { let puzzle = parse_document(&doc.text).puzzle?; - let edits = rename_edits( - &puzzle, - &target, - ¶ms.new_name, - key == ¶ms.position_document_uri, - ); + let edits: Vec = puzzle + .rename_ranges(&symbol, key == ¶ms.position_document_uri) + .into_iter() + .map(|range| json!({ "range": range, "newText": params.new_name })) + .collect(); (!edits.is_empty()).then(|| { json!({ "documentUri": doc.uri, @@ -376,12 +349,6 @@ impl Server { /// The provider-side bound applied when a request omits `limit`. const DEFAULT_LOOKUP_LIMIT: u32 = 20; -#[derive(Debug)] -enum RenameTarget { - Puzzle(String), - Op(String), -} - fn select_artifact_format(accepted: Option>) -> Result<&'static str, HandlerError> { match accepted { None => Ok(ARTIFACT_FORMAT), @@ -471,22 +438,6 @@ fn location_json(uri: &str, range: Range) -> Value { json!({ "uri": uri, "range": range }) } -fn puzzle_symbols(puzzle: &Puzzle) -> Vec { - let mut symbols = vec![json!({ - "name": puzzle.name, - "kind": KIND_PUZZLE, - "range": puzzle.name_range, - })]; - symbols.extend(puzzle.ops.iter().map(|op| { - json!({ - "name": op.name, - "kind": KIND_OP, - "range": op.name_range, - }) - })); - symbols -} - /// The shared definition/references/rename preamble: document checks, then /// the symbol under `position`, or the method's `noSymbolAtPosition` refusal. fn symbol_at_position( @@ -509,39 +460,6 @@ fn symbol_at_position( let puzzle = parse_document(&doc.text) .into_puzzle() .ok_or_else(no_symbol)?; - let symbol = symbol_at(&src, &puzzle, byte).ok_or_else(no_symbol)?; + let symbol = puzzle.symbol_at(&src, byte).ok_or_else(no_symbol)?; Ok((puzzle, symbol)) } - -fn rename_edits( - puzzle: &Puzzle, - target: &RenameTarget, - new_name: &str, - is_position_doc: bool, -) -> Vec { - let edit = |range: Range| json!({ "range": range, "newText": new_name }); - match target { - RenameTarget::Puzzle(name) => { - if is_position_doc && puzzle.name == *name { - vec![edit(puzzle.name_range)] - } else { - Vec::new() - } - } - RenameTarget::Op(name) => { - let mut edits: Vec = puzzle - .op(name) - .map(|op| edit(op.name_range)) - .into_iter() - .collect(); - edits.extend( - puzzle - .solution - .iter() - .filter(|entry| entry.name == *name) - .map(|entry| edit(entry.range)), - ); - edits - } - } -} diff --git a/conformance/mock-provider/src/puzzle.rs b/conformance/mock-provider/src/puzzle.rs index 48d8a6e..b4049be 100644 --- a/conformance/mock-provider/src/puzzle.rs +++ b/conformance/mock-provider/src/puzzle.rs @@ -10,6 +10,7 @@ mod catalog; mod edits; mod lex; mod parse; +mod symbols; mod text; pub(crate) use artifact::{ @@ -19,6 +20,7 @@ pub(crate) use artifact::{ pub(crate) use catalog::{LookupScope, lookup}; pub(crate) use edits::{EditValidation, validate_edits}; pub(crate) use parse::parse_document; +pub(crate) use symbols::{Symbol, is_valid_identifier}; pub(crate) use text::{Position, Range, SourceText}; #[cfg(test)] @@ -49,7 +51,7 @@ pub(crate) fn diagnostic( } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum OpKind { +enum OpKind { Add, Sub, Mul, @@ -77,32 +79,32 @@ impl OpKind { } #[derive(Debug, Clone)] -pub(crate) struct Op { - pub name: String, - pub name_range: Range, - pub kind: OpKind, - pub arg: i64, +struct Op { + name: String, + name_range: Range, + kind: OpKind, + arg: i64, } #[derive(Debug, Clone)] -pub(crate) struct SolutionEntry { - pub name: String, - pub range: Range, +struct SolutionEntry { + name: String, + range: Range, } #[derive(Debug, Clone)] pub(crate) struct Puzzle { - pub name: String, - pub name_range: Range, - pub target: i64, - pub start: i64, - pub ops: Vec, - pub solution: Vec, - pub solution_range: Range, + name: String, + name_range: Range, + target: i64, + start: i64, + ops: Vec, + solution: Vec, + solution_range: Range, } impl Puzzle { - pub(crate) fn op(&self, name: &str) -> Option<&Op> { + fn op(&self, name: &str) -> Option<&Op> { self.ops.iter().find(|op| op.name == name) } @@ -135,47 +137,3 @@ impl ParseOutput { self.diagnostics.iter().any(|d| d.severity == "error") } } - -pub(crate) const KIND_PUZZLE: &str = "puzzle"; -pub(crate) const KIND_OP: &str = "op"; - -#[derive(Debug, Clone)] -pub(crate) enum Symbol { - Puzzle { name: String, range: Range }, - Op { name: String }, -} - -impl Symbol { - pub(crate) fn name(&self) -> &str { - match self { - Symbol::Puzzle { name, .. } | Symbol::Op { name } => name, - } - } -} - -/// The symbol under `byte`: the puzzle name, an op declaration, or a solution -/// reference, in that order. -pub(crate) fn symbol_at(src: &SourceText<'_>, puzzle: &Puzzle, byte: usize) -> Option { - if src.contains_byte(puzzle.name_range, byte) { - return Some(Symbol::Puzzle { - name: puzzle.name.clone(), - range: puzzle.name_range, - }); - } - puzzle - .ops - .iter() - .map(|op| (op.name_range, &op.name)) - .chain(puzzle.solution.iter().map(|e| (e.range, &e.name))) - .find(|(range, _)| src.contains_byte(*range, byte)) - .map(|(_, name)| Symbol::Op { name: name.clone() }) -} - -pub(crate) fn is_valid_identifier(name: &str) -> bool { - let mut chars = name.chars(); - let Some(first) = chars.next() else { - return false; - }; - (first.is_ascii_alphabetic() || first == '_') - && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') -} diff --git a/conformance/mock-provider/src/puzzle/symbols.rs b/conformance/mock-provider/src/puzzle/symbols.rs new file mode 100644 index 0000000..37fb4c5 --- /dev/null +++ b/conformance/mock-provider/src/puzzle/symbols.rs @@ -0,0 +1,140 @@ +//! Symbol semantics for `x-demo-lang`: the queries behind `lpp/symbols`, +//! `lpp/definition`, `lpp/references`, and `lpp/rename`. +//! +//! Each query answers a language question with a typed result; the handlers +//! in `methods.rs` shape those results into wire envelopes and refusals. + +use serde::Serialize; + +use super::Puzzle; +use super::text::{Range, SourceText}; + +/// `kind` values for `lpp/symbols` entries: the language's declaration +/// vocabulary. +#[derive(Debug, Clone, Copy, Serialize)] +#[serde(rename_all = "camelCase")] +enum SymbolKind { + Puzzle, + Op, +} + +/// A symbol declared in a document, as listed by `lpp/symbols`. +#[derive(Debug, Serialize)] +pub(crate) struct DocumentSymbol { + name: String, + kind: SymbolKind, + range: Range, +} + +/// The symbol a position resolves to: the puzzle's name, or an op via its +/// declaration or a solution reference. +#[derive(Debug, Clone)] +pub(crate) enum Symbol { + Puzzle { name: String }, + Op { name: String }, +} + +impl Symbol { + /// The name the symbol binds to. + pub(crate) fn name(&self) -> &str { + match self { + Symbol::Puzzle { name } | Symbol::Op { name } => name, + } + } +} + +impl Puzzle { + /// The symbol under `byte`: the puzzle name, an op declaration, or a + /// solution reference, in that order. + pub(crate) fn symbol_at(&self, src: &SourceText<'_>, byte: usize) -> Option { + if src.contains_byte(self.name_range, byte) { + return Some(Symbol::Puzzle { + name: self.name.clone(), + }); + } + self.ops + .iter() + .map(|op| (op.name_range, &op.name)) + .chain(self.solution.iter().map(|e| (e.range, &e.name))) + .find(|(range, _)| src.contains_byte(*range, byte)) + .map(|(_, name)| Symbol::Op { name: name.clone() }) + } + + /// The symbols this puzzle declares, in declaration order: the puzzle + /// name, then each op. + pub(crate) fn document_symbols(&self) -> Vec { + let mut symbols = vec![DocumentSymbol { + name: self.name.clone(), + kind: SymbolKind::Puzzle, + range: self.name_range, + }]; + symbols.extend(self.ops.iter().map(|op| DocumentSymbol { + name: op.name.clone(), + kind: SymbolKind::Op, + range: op.name_range, + })); + symbols + } + + /// The declaration range `symbol` resolves to: its own range for the + /// puzzle name, the op's declaration range for an op. + pub(crate) fn declaration_range(&self, symbol: &Symbol) -> Option { + match symbol { + Symbol::Puzzle { .. } => Some(self.name_range), + Symbol::Op { name } => self.op(name).map(|op| op.name_range), + } + } + + /// Ranges where `name` occurs as an op: its declaration (first, when + /// `include_declaration`), then solution references in document order. + 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), + ); + ranges + } + + /// Whether renaming `symbol` to `new_name` would collide with an + /// existing op. Only op renames can collide. + pub(crate) fn rename_collides(&self, symbol: &Symbol, new_name: &str) -> bool { + let Symbol::Op { name } = symbol else { + return false; + }; + self.op(name).is_some() && name.as_str() != new_name && self.op(new_name).is_some() + } + + /// The ranges a rename of `symbol` edits in this puzzle. A puzzle-name + /// rename only edits the document holding the position; an op rename + /// edits the op's declaration and every solution reference to it. + pub(crate) fn rename_ranges(&self, symbol: &Symbol, is_position_doc: bool) -> Vec { + match symbol { + Symbol::Puzzle { name } => { + if is_position_doc && self.name == *name { + vec![self.name_range] + } else { + Vec::new() + } + } + Symbol::Op { name } => self.occurrences(name, true), + } + } +} + +/// `x-demo-lang` identifier: `[A-Za-z_][A-Za-z0-9_]*`. +pub(crate) fn is_valid_identifier(name: &str) -> bool { + let mut chars = name.chars(); + let Some(first) = chars.next() else { + return false; + }; + (first.is_ascii_alphabetic() || first == '_') + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} diff --git a/conformance/mock-provider/src/puzzle/tests.rs b/conformance/mock-provider/src/puzzle/tests.rs index da6c06a..19e5ec8 100644 --- a/conformance/mock-provider/src/puzzle/tests.rs +++ b/conformance/mock-provider/src/puzzle/tests.rs @@ -1,7 +1,9 @@ +use super::Puzzle; use super::artifact::{compile_artifact, reconstruct_source}; use super::catalog::{LookupScope, lookup}; use super::edits::{EditValidation, validate_edits}; use super::parse::parse_document; +use super::symbols::{Symbol, is_valid_identifier}; use super::text::{Position, Range, SourceText}; const CLEAN: &str = "puzzle clean {\n target = 40\n start = 10\n ops {\n double: x => x * 2\n plus1: x => x + 1\n }\n solution = [ double, double ]\n}"; @@ -201,3 +203,134 @@ fn lookup_unknown_within_is_none() { assert!(lookup("", None, &scope, 20).is_none()); } } + +// -- Symbol queries (lpp/symbols, definition, references, rename) ---------- + +/// Parses `text` into a puzzle and returns it with its source text. +fn puzzle_of(text: &str) -> (SourceText<'_>, Puzzle) { + let src = SourceText::new(text); + let puzzle = parse_document(text).puzzle.expect("parsed"); + (src, puzzle) +} + +/// The symbol under `line:character`, resolved the way the wire handlers do. +fn symbol_at(src: &SourceText<'_>, puzzle: &Puzzle, line: u32, character: u32) -> Option { + puzzle.symbol_at(src, src.byte_of(pos(line, character)).expect("in document")) +} + +#[test] +fn document_symbols_lists_puzzle_then_ops_in_declaration_order() { + let (_, puzzle) = puzzle_of(CLEAN); + let symbols = serde_json::to_value(puzzle.document_symbols()).expect("serializes"); + assert_eq!( + symbols, + serde_json::json!([ + { "name": "clean", "kind": "puzzle", "range": rng(0, 7, 0, 12) }, + { "name": "double", "kind": "op", "range": rng(4, 4, 4, 10) }, + { "name": "plus1", "kind": "op", "range": rng(5, 4, 5, 9) }, + ]) + ); +} + +#[test] +fn symbol_at_resolves_puzzle_op_declaration_and_reference() { + let (src, puzzle) = puzzle_of(CLEAN); + + match symbol_at(&src, &puzzle, 0, 9).expect("puzzle name") { + Symbol::Puzzle { name } => assert_eq!(name, "clean"), + Symbol::Op { .. } => panic!("expected puzzle symbol"), + } + // An op declaration and each solution reference resolve to the same op. + for (line, character) in [(4, 6), (7, 16), (7, 25)] { + match symbol_at(&src, &puzzle, line, character).expect("op") { + Symbol::Op { name } => assert_eq!(name, "double"), + Symbol::Puzzle { .. } => panic!("expected op symbol at {line}:{character}"), + } + } + match symbol_at(&src, &puzzle, 5, 5).expect("op") { + Symbol::Op { name } => assert_eq!(name, "plus1"), + Symbol::Puzzle { .. } => panic!("expected op symbol"), + } + + // Keywords, settings, and punctuation are not symbols. + for (line, character) in [(0, 3), (1, 4), (7, 3), (7, 11)] { + assert!(symbol_at(&src, &puzzle, line, character).is_none()); + } +} + +#[test] +fn declaration_range_points_at_the_declaration() { + let (src, puzzle) = puzzle_of(CLEAN); + let at = |line, character| symbol_at(&src, &puzzle, line, character).expect("symbol"); + + // The puzzle name is its own declaration. + assert_eq!(puzzle.declaration_range(&at(0, 9)), Some(rng(0, 7, 0, 12))); + // An op declaration and its solution references share one declaration. + for (line, character) in [(4, 6), (7, 16), (7, 25)] { + assert_eq!( + puzzle.declaration_range(&at(line, character)), + Some(rng(4, 4, 4, 10)) + ); + } +} + +#[test] +fn occurrences_list_declaration_first_then_references() { + let (_, puzzle) = puzzle_of(CLEAN); + assert_eq!( + puzzle.occurrences("double", true), + vec![rng(4, 4, 4, 10), rng(7, 15, 7, 21), rng(7, 23, 7, 29)] + ); + assert_eq!( + puzzle.occurrences("double", false), + vec![rng(7, 15, 7, 21), rng(7, 23, 7, 29)] + ); + // A declared-but-unreferenced op lists only its declaration. + assert_eq!(puzzle.occurrences("plus1", true), vec![rng(5, 4, 5, 9)]); + // Unknown names have no occurrences. + assert!(puzzle.occurrences("triple", true).is_empty()); +} + +#[test] +fn rename_ranges_for_op_cover_declaration_and_references() { + let (src, puzzle) = puzzle_of(CLEAN); + let symbol = symbol_at(&src, &puzzle, 7, 16).expect("symbol"); + // Op renames edit every document that mentions the op, positioned or not. + for is_position_doc in [true, false] { + assert_eq!( + puzzle.rename_ranges(&symbol, is_position_doc), + vec![rng(4, 4, 4, 10), rng(7, 15, 7, 21), rng(7, 23, 7, 29)] + ); + } +} + +#[test] +fn rename_ranges_for_puzzle_name_only_edit_the_position_document() { + let (src, puzzle) = puzzle_of(CLEAN); + let symbol = symbol_at(&src, &puzzle, 0, 9).expect("symbol"); + assert_eq!(puzzle.rename_ranges(&symbol, true), vec![rng(0, 7, 0, 12)]); + assert!(puzzle.rename_ranges(&symbol, false).is_empty()); +} + +#[test] +fn rename_collides_only_with_another_op() { + let (src, puzzle) = puzzle_of(CLEAN); + let op = symbol_at(&src, &puzzle, 7, 16).expect("op"); + let puzzle_name = symbol_at(&src, &puzzle, 0, 9).expect("puzzle name"); + + assert!(puzzle.rename_collides(&op, "plus1")); + // Renaming to the current name is not a collision. + assert!(!puzzle.rename_collides(&op, "double")); + assert!(!puzzle.rename_collides(&op, "triple")); + // Puzzle-name renames never collide with ops. + assert!(!puzzle.rename_collides(&puzzle_name, "plus1")); +} + +#[test] +fn identifier_rules_match_the_lexer() { + assert!(is_valid_identifier("double")); + assert!(is_valid_identifier("_x9")); + assert!(!is_valid_identifier("9x")); + assert!(!is_valid_identifier("")); + assert!(!is_valid_identifier("a-b")); +}