Skip to content

Commit

Permalink
More precise completions and namespace member completions (#1947)
Browse files Browse the repository at this point in the history
## Feature overview

This PR enables:

1. Much more precise keyword completions where we only show the relevant
syntax at a particular cursor location. For example:

```qsharp
within { } | 
```

At the cursor location indicated by `|`, we should only get the keyword
`apply` .

2. More precise completions for names where we take the syntactic
context into account. For example:

```qsharp
operation Foo(x : | )
```

Here, we should only get type names.

3. Namespace member completions. For example:
```qsharp
import Std.Diagnostics.|
```

The completions here should return only items that exist under
`Std.Diagnostics`.

## Code changes

### Parser completions module

Introduce the `completion` module in the parser.

- `possible_words_at_offset_in_source` - a new API to return completions
at a particular offset . Uses the parser directly as the best source of
knowledge about what tokens to expect.
- Introduce the `WordKinds` flags to define the different kinds of word
tokens that the parser can expect during parsing. (See
`compiler/qsc_parse/src/completion/word_kinds.rs` for longer
description)
- See `compiler/qsc_parse/src/completion/collector.rs` for a description
of how this all works.
- Sprinkle `expect()`s at all possible locations in the parser where we
expect a word.

### Error recovery - AST changes

- Introduce `PathKind::Ok` and `PathKind::Err` .
- This new approach allows for a valid AST node to exist even when path
parsing fails for a partial path. This is good for syntax such as "`open
Std.`" where we need:
- The identifier nodes preceding `.` to be valid, so we can use them to
look up possible completions.
- The `open` statement to also be a valid node, so we can tell that the
context requires a namespace (and not, say, a type name).
    - Most existing code will continue to only process valid paths. 
- Remove `Idents` struct and replace it with a trait implemented by
various types such as `Path` and `Ident` slices. This should make the
convenience methods easier to use without cloning in/out of the
different types we need.

### Error recovery - parser changes

- Recovering struct parsing, so that e.g. `new Foo.` is still parsed as
a valid struct expression node, allowing us to access the `Foo` ident
from the AST
- Recovering `open` statement parsing so that `open Foo.` can be
recognized as an open item in the AST.

### Language service

In `language_service` is essentially a complete rewrite of
`completions.rs`, so I suggest reviewing the new code only and ditching
the diff.

### Other parser changes

- Move implicit namespace parsing a bit so that it works better with the
completion mechanism.
- This also fixes the spurious "expected item, found eof" error when
document only contains one token
- Delete the dead path  `path_field_op` 
- Add some missing keywords to item parsing recovery (`export`,
`import`)

### Other notes

- Locals completions exclude `import`s from locals (they conflict with
globals already added to completion) (fix in `resolve.rs`)
- `language-configuration.json` : and removed wonky word pattern in
playground and VS Code because we don't need it anymore now that we have
proper `.` completions! Also update trigger characters.
- Only include the samples snippets in empty documents, because now with
the more concise completion list, it gets quite intrusive for them to
show up at every word when typing code.
- Update integration test to account for the fact that completions are
more accurate now.

### Not in this PR (coming next)

- Struct and UDT field access completions.
  • Loading branch information
minestarks authored Oct 17, 2024
1 parent 2856c76 commit 74c488b
Show file tree
Hide file tree
Showing 56 changed files with 5,943 additions and 1,918 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions compiler/qsc/src/interpret/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,9 @@ mod given_interpreter {
use expect_test::expect;
use indoc::indoc;

use qsc_ast::ast::{Expr, ExprKind, NodeId, Package, Path, Stmt, StmtKind, TopLevelNode};
use qsc_ast::ast::{
Expr, ExprKind, NodeId, Package, Path, PathKind, Stmt, StmtKind, TopLevelNode,
};
use qsc_data_structures::span::Span;
use qsc_frontend::compile::SourceMap;
use qsc_passes::PackageType;
Expand Down Expand Up @@ -1865,7 +1867,7 @@ mod given_interpreter {
let path_expr = Expr {
id: NodeId::default(),
span: Span::default(),
kind: Box::new(ExprKind::Path(Box::new(path))),
kind: Box::new(ExprKind::Path(PathKind::Ok(Box::new(path)))),
};
let expr = Expr {
id: NodeId::default(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/qsc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub mod circuit {
}

pub mod parse {
pub use qsc_parse::top_level_nodes;
pub use qsc_parse::{completion, top_level_nodes};
}

pub mod partial_eval {
Expand Down
24 changes: 0 additions & 24 deletions compiler/qsc/src/packages/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,30 +237,6 @@ fn dependency_error() {
),
),
},
WithSource {
sources: [
Source {
name: "librarymain",
contents: "broken_syntax",
offset: 0,
},
],
error: Frontend(
Error(
Parse(
Error(
ExpectedItem(
Eof,
Span {
lo: 0,
hi: 13,
},
),
),
),
),
),
},
]
"#]]
.assert_debug_eq(&buildable_program.dependency_errors);
Expand Down
Loading

0 comments on commit 74c488b

Please sign in to comment.