From 1fa90aff0e480b7078a9c72afc30c23083f55dfd Mon Sep 17 00:00:00 2001 From: Sienna Meridian Satterwhite Date: Mon, 23 Feb 2026 21:09:36 +0000 Subject: [PATCH] fix(lsp): fix remaining completion scoping issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - is_typing_declaration_name(): had the same Token::Ident bug as determine_context() — matched string keywords against Token::Ident but the lexer produces dedicated variants. Extracted shared is_declaration_keyword() helper that matches actual token variants. - determine_context(): add Token::Schedule so schedule blocks get InFieldBlock context instead of falling through to Unknown. - Tests: add negative assertions to test_top_level_completions, test_field_block_completions, and test_relationship_completions. Add test_no_completions_after_declaration_keyword, test_no_completions_while_typing_name, and test_schedule_completions_no_toplevel. --- src/lsp/completion.rs | 65 +++++++++----------- src/lsp/completion_tests.rs | 118 ++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 37 deletions(-) diff --git a/src/lsp/completion.rs b/src/lsp/completion.rs index 22a2c5d..1700f57 100644 --- a/src/lsp/completion.rs +++ b/src/lsp/completion.rs @@ -135,6 +135,24 @@ fn position_to_offset(doc: &Document, line: usize, character: usize) -> Option bool { + use crate::syntax::lexer::Token; + matches!( + token, + Token::Character | + Token::Template | + Token::Species | + Token::Behavior | + Token::LifeArc | + Token::Relationship | + Token::Institution | + Token::Location | + Token::Schedule + ) +} + /// Check if we're typing a new identifier name after a declaration keyword fn is_typing_declaration_name(text: &str, offset: usize) -> bool { use crate::syntax::lexer::{ @@ -155,47 +173,19 @@ fn is_typing_declaration_name(text: &str, offset: usize) -> bool { if !tokens.is_empty() { let last_idx = tokens.len() - 1; - // Check last token - if let (_offset, Token::Ident(keyword), _end) = &tokens[last_idx] { - if matches!( - keyword.as_str(), - "character" | - "template" | - "species" | - "behavior" | - "life_arc" | - "relationship" | - "institution" | - "location" | - "enum" | - "schedule" - ) { - return true; - } + // Check last token — cursor is right after a declaration keyword + if is_declaration_keyword(&tokens[last_idx].1) { + return true; } // Check second-to-last token (in case we're in the middle of typing an - // identifier) + // identifier after the keyword) if tokens.len() >= 2 { let second_last_idx = tokens.len() - 2; - if let (_offset, Token::Ident(keyword), _end) = &tokens[second_last_idx] { - if matches!( - keyword.as_str(), - "character" | - "template" | - "species" | - "behavior" | - "life_arc" | - "relationship" | - "institution" | - "location" | - "enum" | - "schedule" - ) { - // Make sure the last token is an identifier (not a colon or brace) - if let (_offset, Token::Ident(_), _end) = &tokens[last_idx] { - return true; - } + if is_declaration_keyword(&tokens[second_last_idx].1) { + // Make sure the last token is an identifier (not a colon or brace) + if let (_offset, Token::Ident(_), _end) = &tokens[last_idx] { + return true; } } } @@ -511,7 +501,8 @@ fn determine_context(text: &str, offset: usize) -> CompletionContext { Token::Template | Token::Species | Token::Institution | - Token::Location => { + Token::Location | + Token::Schedule => { last_context = Some(CompletionContext::InFieldBlock); seen_colon_without_brace = false; }, diff --git a/src/lsp/completion_tests.rs b/src/lsp/completion_tests.rs index c1b5d9f..e75b127 100644 --- a/src/lsp/completion_tests.rs +++ b/src/lsp/completion_tests.rs @@ -44,6 +44,28 @@ mod tests { assert!(items.iter().any(|item| item.label == "character")); assert!(items.iter().any(|item| item.label == "template")); assert!(items.iter().any(|item| item.label == "behavior")); + // Should NOT have behavior-only keywords + assert!( + !items.iter().any(|item| item.label == "?"), + "? should not appear at top level" + ); + assert!( + !items.iter().any(|item| item.label == "choose"), + "choose should not appear at top level" + ); + assert!( + !items.iter().any(|item| item.label == "then"), + "then should not appear at top level" + ); + // Should NOT have field-only keywords + assert!( + !items.iter().any(|item| item.label == "age"), + "age should not appear at top level" + ); + assert!( + !items.iter().any(|item| item.label == "bond"), + "bond should not appear at top level" + ); }, | _ => panic!("Expected array response"), } @@ -67,6 +89,24 @@ mod tests { assert!(items.iter().any(|item| item.label == "from")); assert!(items.iter().any(|item| item.label == "age")); assert!(items.iter().any(|item| item.label == "bond")); + // Should NOT have top-level declaration keywords + assert!( + !items.iter().any(|item| item.label == "character"), + "character should not appear inside character block" + ); + assert!( + !items.iter().any(|item| item.label == "behavior"), + "behavior should not appear inside character block" + ); + // Should NOT have behavior-only keywords + assert!( + !items.iter().any(|item| item.label == "?"), + "? should not appear inside character block" + ); + assert!( + !items.iter().any(|item| item.label == "choose"), + "choose should not appear inside character block" + ); }, | _ => panic!("Expected array response"), } @@ -237,6 +277,24 @@ mod tests { assert!(items.iter().any(|item| item.label == "as")); assert!(items.iter().any(|item| item.label == "self")); assert!(items.iter().any(|item| item.label == "other")); + // Should NOT have top-level declaration keywords + assert!( + !items.iter().any(|item| item.label == "institution"), + "institution should not appear inside relationship block" + ); + assert!( + !items.iter().any(|item| item.label == "behavior"), + "behavior should not appear inside relationship block" + ); + assert!( + !items.iter().any(|item| item.label == "character"), + "character keyword should not appear inside relationship block" + ); + // Should NOT have behavior-only keywords + assert!( + !items.iter().any(|item| item.label == "?"), + "? should not appear inside relationship block" + ); }, | _ => panic!("Expected array response"), } @@ -377,6 +435,66 @@ character Bob {}"#; } } + #[test] + fn test_no_completions_after_declaration_keyword() { + // After typing "behavior " the user is naming a new entity — no completions + let source = "behavior "; + let doc = Document::new(source.to_string()); + let params = make_params(0, 9); // right after "behavior " + + let result = completion::get_completions(&doc, ¶ms); + assert!( + result.is_none(), + "Should suppress completions when typing a name after a declaration keyword" + ); + } + + #[test] + fn test_no_completions_while_typing_name() { + // User is typing "behavior Fo" — in the middle of naming + let source = "behavior Fo"; + let doc = Document::new(source.to_string()); + let params = make_params(0, 11); // right after "Fo" + + let result = completion::get_completions(&doc, ¶ms); + assert!( + result.is_none(), + "Should suppress completions when in the middle of typing a declaration name" + ); + } + + #[test] + fn test_schedule_completions_no_toplevel() { + let source = "schedule Daily {\n \n}"; + let doc = Document::new(source.to_string()); + let params = make_params(1, 4); + + let result = completion::get_completions(&doc, ¶ms); + assert!(result.is_some()); + + if let Some(response) = result { + match response { + | tower_lsp::lsp_types::CompletionResponse::Array(items) => { + // Should NOT have top-level declaration keywords + assert!( + !items.iter().any(|item| item.label == "institution"), + "institution should not appear inside schedule block" + ); + assert!( + !items.iter().any(|item| item.label == "behavior"), + "behavior should not appear inside schedule block" + ); + // Should NOT have behavior-only keywords + assert!( + !items.iter().any(|item| item.label == "?"), + "? should not appear inside schedule block" + ); + }, + | _ => panic!("Expected array response"), + } + } + } + #[test] fn test_no_duplicate_completions() { let source = "character Alice {}\ncharacter Alice {}"; // Duplicate name