From 96d6dcecb99a00262c41e28f301fb0c786b827b8 Mon Sep 17 00:00:00 2001 From: Mine Starks <16928427+minestarks@users.noreply.github.com> Date: Fri, 11 Oct 2024 21:44:11 +0000 Subject: [PATCH] Handle when path ends with a keyword, and multi-character op tokens --- compiler/qsc_ast/src/ast.rs | 6 +- compiler/qsc_parse/src/completion/tests.rs | 12 ++ compiler/qsc_parse/src/item.rs | 4 +- compiler/qsc_parse/src/item/tests.rs | 58 ++++++++-- compiler/qsc_parse/src/prim.rs | 8 ++ compiler/qsc_parse/src/prim/tests.rs | 27 +++++ language_service/src/completion.rs | 4 +- .../src/completion/path_context.rs | 60 ++++++---- language_service/src/completion/tests.rs | 106 ++++++++++++++++++ 9 files changed, 247 insertions(+), 38 deletions(-) diff --git a/compiler/qsc_ast/src/ast.rs b/compiler/qsc_ast/src/ast.rs index 737429810b..616bba164d 100644 --- a/compiler/qsc_ast/src/ast.rs +++ b/compiler/qsc_ast/src/ast.rs @@ -1423,10 +1423,14 @@ impl Default for PathKind { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct IncompletePath { /// The whole span of the incomplete path, - /// including the final `.` and any whitespace that follows it. + /// including the final `.` and any whitespace or keyword + /// that follows it. pub span: Span, /// Any segments that were successfully parsed before the final `.`. pub segments: Box<[Ident]>, + /// Whether a keyword exists after the final `.`. + /// This keyword can be presumed to be a partially typed identifier. + pub keyword: bool, } impl Display for PathKind { diff --git a/compiler/qsc_parse/src/completion/tests.rs b/compiler/qsc_parse/src/completion/tests.rs index eb1ef3ba56..b4a387e0f5 100644 --- a/compiler/qsc_parse/src/completion/tests.rs +++ b/compiler/qsc_parse/src/completion/tests.rs @@ -365,3 +365,15 @@ fn base_type_tuple() { "]], ); } + +#[test] +fn keyword_after_incomplete_path() { + check_valid_words( + "import Foo.in|", + &expect![[r" + WordKinds( + PathSegment, + ) + "]], + ); +} diff --git a/compiler/qsc_parse/src/item.rs b/compiler/qsc_parse/src/item.rs index 2dfb7a5351..0ebe54865b 100644 --- a/compiler/qsc_parse/src/item.rs +++ b/compiler/qsc_parse/src/item.rs @@ -647,7 +647,9 @@ fn path_import(s: &mut ParserContext) -> Result<(PathKind, bool)> { match path(s, WordKinds::PathImport) { Ok(path) => Ok((PathKind::Ok(path), false)), Err((error, Some(incomplete_path))) => { - if token(s, TokenKind::ClosedBinOp(ClosedBinOp::Star)).is_ok() { + if !incomplete_path.keyword + && token(s, TokenKind::ClosedBinOp(ClosedBinOp::Star)).is_ok() + { let (name, namespace) = incomplete_path .segments .split_last() diff --git a/compiler/qsc_parse/src/item/tests.rs b/compiler/qsc_parse/src/item/tests.rs index 3e60684deb..fcf2e2fb37 100644 --- a/compiler/qsc_parse/src/item/tests.rs +++ b/compiler/qsc_parse/src/item/tests.rs @@ -2076,6 +2076,44 @@ fn invalid_glob_syntax_with_following_ident() { ); } +#[test] +fn invalid_glob_syntax_follows_keyword() { + check( + parse_import_or_export, + "import Foo.in*;", + &expect![[r#" + ImportOrExportDecl [0-13]: [Err IncompletePath [7-13]: + Ident _id_ [7-10] "Foo"] + + [ + Error( + Rule( + "identifier", + Keyword( + In, + ), + Span { + lo: 11, + hi: 13, + }, + ), + ), + Error( + Token( + Semi, + ClosedBinOp( + Star, + ), + Span { + lo: 13, + hi: 14, + }, + ), + ), + ]"#]], + ); +} + #[test] fn disallow_top_level_recursive_glob() { check( @@ -2158,11 +2196,9 @@ fn missing_semi_between_items() { Namespace _id_ [0-45] (Ident _id_ [10-13] "Foo"): Item _id_ [16-24]: Open (Path _id_ [21-24] (Ident _id_ [21-24] "Foo")) - Item _id_ [25-35]: - Open (Err IncompletePath [30-35]: + Item _id_ [25-39]: + Open (Err IncompletePath [30-39]: Ident _id_ [30-33] "Bar") - Item _id_ [35-43]: - Open (Path _id_ [40-43] (Ident _id_ [40-43] "Baz")) [ Error( @@ -2192,24 +2228,22 @@ fn missing_semi_between_items() { Error( Token( Semi, - Keyword( - Open, - ), + Ident, Span { - lo: 35, - hi: 39, + lo: 40, + hi: 43, }, ), ), Error( Token( - Semi, Close( Brace, ), + Ident, Span { - lo: 44, - hi: 45, + lo: 40, + hi: 43, }, ), ), diff --git a/compiler/qsc_parse/src/prim.rs b/compiler/qsc_parse/src/prim.rs index d684a6ed1d..89ca09a8d2 100644 --- a/compiler/qsc_parse/src/prim.rs +++ b/compiler/qsc_parse/src/prim.rs @@ -114,12 +114,20 @@ pub(super) fn path( Ok(ident) => parts.push(*ident), Err(error) => { let _ = s.skip_trivia(); + let peek = s.peek(); + let keyword = matches!(peek.kind, TokenKind::Keyword(_)); + if keyword { + // Consume any keyword that comes after the final + // dot, assuming it was intended to be part of the path. + s.advance(); + } return Err(( error, Some(Box::new(IncompletePath { span: s.span(lo), segments: parts.into(), + keyword, })), )); } diff --git a/compiler/qsc_parse/src/prim/tests.rs b/compiler/qsc_parse/src/prim/tests.rs index efef4e139a..504007f599 100644 --- a/compiler/qsc_parse/src/prim/tests.rs +++ b/compiler/qsc_parse/src/prim/tests.rs @@ -143,6 +143,33 @@ fn path_trailing_dot() { ); } +#[test] +fn path_followed_by_keyword() { + check( + path, + "Foo.Bar.in", + &expect![[r#" + Err IncompletePath [0-10]: + Ident _id_ [0-3] "Foo" + Ident _id_ [4-7] "Bar" + + [ + Error( + Rule( + "identifier", + Keyword( + In, + ), + Span { + lo: 8, + hi: 10, + }, + ), + ), + ]"#]], + ); +} + #[test] fn pat_bind() { check( diff --git a/language_service/src/completion.rs b/language_service/src/completion.rs index cfcda832ab..ec61ad7cac 100644 --- a/language_service/src/completion.rs +++ b/language_service/src/completion.rs @@ -257,7 +257,9 @@ fn collect_paths( /// `let x : Microsoft.Quantum.Math.↘` should include `Complex` (a type) while /// `let x = Microsoft.Quantum.Math.↘` should include `PI` (a callable). fn collect_path_segments(globals: &Globals, path_context: &IncompletePath) -> Vec> { - let (path_kind, qualifier) = path_context.context(); + let Some((path_kind, qualifier)) = path_context.context() else { + return Vec::new(); + }; match path_kind { PathKind::Namespace => globals.namespaces_in(&qualifier), diff --git a/language_service/src/completion/path_context.rs b/language_service/src/completion/path_context.rs index a7f3bfc26a..41928e66ae 100644 --- a/language_service/src/completion/path_context.rs +++ b/language_service/src/completion/path_context.rs @@ -4,9 +4,9 @@ use qsc::{ ast::{ visit::{self, Visitor}, - Attr, Block, CallableDecl, Expr, ExprKind, FieldAssign, FieldDef, FunctorExpr, Ident, Item, - ItemKind, Namespace, Package, Pat, Path, QubitInit, SpecDecl, Stmt, StructDecl, Ty, TyDef, - TyKind, + Attr, Block, CallableDecl, Expr, ExprKind, FieldAssign, FieldDef, FunctorExpr, Ident, + Idents, Item, ItemKind, Namespace, Package, Pat, Path, QubitInit, SpecDecl, Stmt, + StructDecl, Ty, TyDef, TyKind, }, parse::completion::PathKind, }; @@ -18,7 +18,7 @@ use std::rc::Rc; /// Methods may panic if the offset does not fall within an incomplete path. #[derive(Debug)] pub(super) struct IncompletePath<'a> { - qualifier: Option<&'a [Ident]>, + qualifier: Option>, context: Option, offset: u32, } @@ -41,10 +41,28 @@ impl<'a> IncompletePath<'a> { } impl<'a> Visitor<'a> for IncompletePath<'a> { - fn visit_item(&mut self, item: &Item) { - match *item.kind { + fn visit_item(&mut self, item: &'a Item) { + match &*item.kind { ItemKind::Open(..) => self.context = Some(PathKind::Namespace), - ItemKind::ImportOrExport(..) => self.context = Some(PathKind::Import), + ItemKind::ImportOrExport(decl) => { + self.context = Some(PathKind::Import); + for item in &decl.items { + if item.is_glob + && item.span.touches(self.offset) + && item + .alias + .as_ref() + .map_or(true, |a| !a.span.touches(self.offset)) + { + // Special case when the cursor falls *between* the + // `Path` and the glob asterisk, + // e.g. `foo.bar.|*` . In that case, the visitor + // will not visit the path since the cursor technically + // is not within the path. + self.visit_path_kind(&item.path); + } + } + } _ => {} } } @@ -65,35 +83,31 @@ impl<'a> Visitor<'a> for IncompletePath<'a> { fn visit_path_kind(&mut self, path: &'a qsc::ast::PathKind) { self.qualifier = match path { - qsc::ast::PathKind::Ok(path) => path.segments.as_ref().map(AsRef::as_ref), - qsc::ast::PathKind::Err(Some(incomplete_path)) => Some(&incomplete_path.segments), + qsc::ast::PathKind::Ok(path) => Some(path.iter().collect()), + qsc::ast::PathKind::Err(Some(incomplete_path)) => { + Some(incomplete_path.segments.iter().collect()) + } qsc::ast::PathKind::Err(None) => None, }; } } impl IncompletePath<'_> { - pub fn context(&self) -> (PathKind, Vec>) { + pub fn context(&self) -> Option<(PathKind, Vec>)> { + let context = self.context?; let qualifier = self.segments_before_offset(); - // WARNING: this assumption appears to hold true today, but it's subtle - // enough that parser and AST changes can easily violate it in the future. - assert!( - !qualifier.is_empty(), - "path segment completion should only be invoked for a partially parsed path" - ); - - let context = self - .context - .expect("context must exist for path segment completion"); + if qualifier.is_empty() { + return None; + } - (context, qualifier) + Some((context, qualifier)) } fn segments_before_offset(&self) -> Vec> { self.qualifier - .into_iter() - .flat_map(AsRef::as_ref) + .iter() + .flatten() .take_while(|i| i.span.hi < self.offset) .map(|i| i.name.clone()) .collect::>() diff --git a/language_service/src/completion/tests.rs b/language_service/src/completion/tests.rs index 75644c58ae..d0f7ceda7b 100644 --- a/language_service/src/completion/tests.rs +++ b/language_service/src/completion/tests.rs @@ -3404,3 +3404,109 @@ fn array_size() { "#]], ); } + +#[test] +fn path_segment_partial_ident_is_keyword() { + check( + "namespace Test { import FakeStdLib.struct↘ }", + &["StructFn"], + &expect![[r#" + [ + Some( + CompletionItem { + label: "StructFn", + kind: Interface, + sort_text: Some( + "0300StructFn", + ), + detail: Some( + "struct StructFn { inner : (Int -> Int) }", + ), + additional_text_edits: None, + }, + ), + ] + "#]], + ); +} + +#[test] +fn path_segment_followed_by_wslash() { + // `w/` is a single token, so it gets tricky + // to separate out the `w` and treat it as an identifier. + // We're just not going to worry about doing anything clever here. + check( + "namespace Test { import FakeStdLib.w↘/ }", + &["StructFn"], + &expect![[r#" + [ + None, + ] + "#]], + ); +} + +#[test] +fn path_segment_followed_by_op_token() { + // Invoking in the middle of a multi-character op token + // shouldn't break anything. + check( + "namespace Test { import FakeStdLib.<↘<< }", + &["StructFn"], + &expect![[r#" + [ + None, + ] + "#]], + ); +} + +#[test] +fn path_segment_before_glob() { + check( + "namespace Test { import FakeStdLib.↘* }", + &["StructFn"], + &expect![[r#" + [ + Some( + CompletionItem { + label: "StructFn", + kind: Interface, + sort_text: Some( + "0300StructFn", + ), + detail: Some( + "struct StructFn { inner : (Int -> Int) }", + ), + additional_text_edits: None, + }, + ), + ] + "#]], + ); +} + +#[test] +fn path_segment_before_glob_with_alias() { + check( + "namespace Test { import FakeStdLib.↘* as Alias }", + &["StructFn"], + &expect![[r#" + [ + Some( + CompletionItem { + label: "StructFn", + kind: Interface, + sort_text: Some( + "0300StructFn", + ), + detail: Some( + "struct StructFn { inner : (Int -> Int) }", + ), + additional_text_edits: None, + }, + ), + ] + "#]], + ); +}