Skip to content

Commit

Permalink
Disable completions in attribute arguments (#1986)
Browse files Browse the repository at this point in the history
Disable completions inside attribute arguments, despite what the parser
says, since we support either hardcoded values or nothing at all.

examples:

```qsharp
@config( | ) 
operation Foo() : Unit {}
```

```qsharp
@entrypoint( | ) 
operation Foo() : Unit {}
```

no completions at this locations.
  • Loading branch information
minestarks authored Oct 31, 2024
1 parent e6b8682 commit d591b52
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 35 deletions.
34 changes: 23 additions & 11 deletions language_service/src/completion.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

mod ast_context;
mod fields;
mod global_items;
mod locals;
mod path_context;
#[cfg(test)]
mod tests;
mod text_edits;
Expand All @@ -13,11 +13,11 @@ use crate::{
compilation::{Compilation, CompilationKind},
protocol::{CompletionItem, CompletionItemKind, CompletionList, TextEdit},
};
use ast_context::AstContext;
use fields::Fields;
use global_items::Globals;
use locals::Locals;
use log::{log_enabled, trace, Level::Trace};
use path_context::PathOrFieldAccess;
use qsc::{
line_column::{Encoding, Position},
parse::completion::{
Expand Down Expand Up @@ -61,6 +61,14 @@ pub(crate) fn get_completions(
trace!("the character before the cursor is: {last_char:?}");
}

// Special case: no completions in attribute arguments, even when the
// parser expects an expression.
let ast_context = AstContext::init(source_offset, &compilation.user_unit().ast.package);
if ast_context.is_in_attr_arg() {
// No completions in attribute expressions, they're misleading.
return CompletionList::default();
}

// What kinds of words are expected at the cursor location?
let expected_words_at_cursor = expected_word_kinds(
compilation,
Expand Down Expand Up @@ -131,6 +139,10 @@ fn collect_hardcoded_words(expected: WordKinds) -> Vec<Completion> {
completions.extend([
Completion::new("EntryPoint".to_string(), CompletionItemKind::Interface),
Completion::new("Config".to_string(), CompletionItemKind::Interface),
Completion::new(
"SimulatableIntrinsic".to_string(),
CompletionItemKind::Interface,
),
Completion::new("Measurement".to_string(), CompletionItemKind::Interface),
Completion::new("Reset".to_string(), CompletionItemKind::Interface),
]);
Expand Down Expand Up @@ -175,20 +187,20 @@ fn collect_names(
}
NameKind::PathSegment => {
let globals = Globals::init(cursor_offset, compilation);
let path =
PathOrFieldAccess::init(cursor_offset, &compilation.user_unit().ast.package);
let fields = Fields::new(compilation, &path);
let ast_context =
AstContext::init(cursor_offset, &compilation.user_unit().ast.package);
let fields = Fields::new(compilation, &ast_context);

groups.extend(collect_path_segments(&path, &globals, &fields));
groups.extend(collect_path_segments(&ast_context, &globals, &fields));
}
NameKind::TyParam => {
let locals = Locals::new(cursor_offset, compilation);
groups.push(locals.type_names());
}
NameKind::Field => {
let path =
PathOrFieldAccess::init(cursor_offset, &compilation.user_unit().ast.package);
let fields = Fields::new(compilation, &path);
let ast_context =
AstContext::init(cursor_offset, &compilation.user_unit().ast.package);
let fields = Fields::new(compilation, &ast_context);

groups.push(fields.fields());
}
Expand Down Expand Up @@ -260,11 +272,11 @@ 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(
path_context: &PathOrFieldAccess,
ast_context: &AstContext,
globals: &Globals,
fields: &Fields,
) -> Vec<Vec<Completion>> {
let Some((path_kind, qualifier)) = path_context.path_segment_context() else {
let Some((path_kind, qualifier)) = ast_context.path_segment_context() else {
return Vec::new();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ use qsc::{
};
use std::rc::Rc;

/// Provides context for a path or field access expression at the cursor offset.
///
/// Undefined behavior if the given offset does not fall within a path,
/// field access or field assigment.
/// Provides additional syntactic context for the cursor offset,
/// for various special cases where it's needed.
#[derive(Debug)]
pub(super) struct PathOrFieldAccess<'a> {
pub(super) struct AstContext<'a> {
path_qualifier: Option<Vec<&'a Ident>>,
context: Option<Context<'a>>,
offset: u32,
Expand All @@ -33,13 +31,15 @@ enum Context<'a> {
/// The type of this expression will be the record used for the field access.
record: &'a Expr,
},
/// The cursor is on an attribute.
AttrArg,
}

impl<'a> PathOrFieldAccess<'a> {
impl<'a> AstContext<'a> {
pub fn init(offset: u32, package: &'a Package) -> Self {
let mut offset_visitor = OffsetVisitor {
offset,
visitor: PathOrFieldAccess {
visitor: AstContext {
offset,
context: None,
path_qualifier: None,
Expand All @@ -50,14 +50,21 @@ impl<'a> PathOrFieldAccess<'a> {

offset_visitor.visitor
}

fn set_context(&mut self, context: Context<'a>) {
if matches!(self.context, Some(Context::AttrArg)) {
return;
}
self.context = Some(context);
}
}

impl<'a> Visitor<'a> for PathOrFieldAccess<'a> {
impl<'a> Visitor<'a> for AstContext<'a> {
fn visit_item(&mut self, item: &'a Item) {
match &*item.kind {
ItemKind::Open(..) => self.context = Some(Context::Path(PathKind::Namespace)),
ItemKind::Open(..) => self.set_context(Context::Path(PathKind::Namespace)),
ItemKind::ImportOrExport(decl) => {
self.context = Some(Context::Path(PathKind::Import));
self.set_context(Context::Path(PathKind::Import));
for item in &decl.items {
if item.is_glob
&& item.span.touches(self.offset)
Expand All @@ -81,23 +88,23 @@ impl<'a> Visitor<'a> for PathOrFieldAccess<'a> {

fn visit_ty(&mut self, ty: &Ty) {
if let TyKind::Path(..) = *ty.kind {
self.context = Some(Context::Path(PathKind::Ty));
self.set_context(Context::Path(PathKind::Ty));
}
}

fn visit_expr(&mut self, expr: &'a Expr) {
if let ExprKind::Path(..) = *expr.kind {
self.context = Some(Context::Path(PathKind::Expr));
self.set_context(Context::Path(PathKind::Expr));
} else if let ExprKind::Struct(..) = expr.kind.as_ref() {
self.context = Some(Context::Field { record: expr });
self.set_context(Context::Field { record: expr });
} else if let ExprKind::Field(record, _) = expr.kind.as_ref() {
self.context = Some(Context::Field { record });
self.set_context(Context::Field { record });
}
}

fn visit_path_kind(&mut self, path: &'a ast::PathKind) {
if let Some(Context::Field { .. }) = self.context {
self.context = Some(Context::Path(PathKind::Struct));
self.set_context(Context::Path(PathKind::Struct));
}
self.path_qualifier = match path {
ast::PathKind::Ok(path) => Some(path.iter().collect()),
Expand All @@ -107,16 +114,21 @@ impl<'a> Visitor<'a> for PathOrFieldAccess<'a> {
ast::PathKind::Err(None) => None,
};
}

fn visit_attr(&mut self, attr: &'a Attr) {
if attr.arg.span.touches(self.offset) {
self.set_context(Context::AttrArg);
}
}
}

impl PathOrFieldAccess<'_> {
impl AstContext<'_> {
/// Returns the path kind and the path qualifier before the cursor offset.
///
/// Returns `None` if the cursor is not on a path.
pub fn path_segment_context(&self) -> Option<(PathKind, Vec<Rc<str>>)> {
let path_kind = match self.context? {
Context::Path(path_kind) => path_kind,
Context::Field { .. } => return None,
let Some(Context::Path(path_kind)) = self.context else {
return None;
};

let qualifier = self.segments_before_offset();
Expand Down Expand Up @@ -145,6 +157,11 @@ impl PathOrFieldAccess<'_> {
}
}

/// Returns whether the cursor is in an attribute argument.
pub fn is_in_attr_arg(&self) -> bool {
matches!(self.context, Some(Context::AttrArg))
}

fn idents_before_cursor(&self) -> impl Iterator<Item = &Ident> {
self.path_qualifier
.iter()
Expand Down
10 changes: 5 additions & 5 deletions language_service/src/completion/fields.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use super::{path_context::PathOrFieldAccess, Completion};
use super::{ast_context::AstContext, Completion};
use crate::{compilation::Compilation, protocol::CompletionItemKind};
use qsc::{
display::Lookup,
Expand All @@ -15,19 +15,19 @@ use qsc::{
/// provides the possible field names.
pub(super) struct Fields<'a> {
compilation: &'a Compilation,
path_context: &'a PathOrFieldAccess<'a>,
ast_context: &'a AstContext<'a>,
}

impl<'a> Fields<'a> {
pub(crate) fn new(compilation: &'a Compilation, path_context: &'a PathOrFieldAccess) -> Self {
pub(crate) fn new(compilation: &'a Compilation, ast_context: &'a AstContext) -> Self {
Self {
compilation,
path_context,
ast_context,
}
}

pub(crate) fn fields(&self) -> Vec<Completion> {
let Some(id) = self.path_context.field_access_context() else {
let Some(id) = self.ast_context.field_access_context() else {
return vec![];
};

Expand Down
42 changes: 42 additions & 0 deletions language_service/src/completion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4072,3 +4072,45 @@ fn incomplete_return_type_in_partial_callable_signature() {
"#]],
);
}

#[test]
fn no_path_segment_completion_inside_attr() {
check(
"namespace Test {
@Config(FakeStdLib.↘)
function Main() : Unit {
}
}",
&["Fake", "not", "Test"],
&expect![[r#"
[
None,
None,
None,
]
"#]],
);
}

#[test]
fn no_completion_inside_attr() {
check(
"namespace Test {
@Config(↘)
function Main() : Unit {
let FakeStdLib = new Foo { bar = 3 };
FakeStdLib.↘
}
}",
&["Fake", "not", "Test"],
&expect![[r#"
[
None,
None,
None,
]
"#]],
);
}

0 comments on commit d591b52

Please sign in to comment.