Skip to content

Commit

Permalink
feat(vscode): completions for bring and hiding more irrelevant comp…
Browse files Browse the repository at this point in the history
…letions (#4433)

#### Stops completions on the left side of variable declarations
##### Fixes #3991
<img width="331" alt="image" src="https://github.com/winglang/wing/assets/1237390/5c5c7fed-c416-41ec-ba66-b2b3628d5597">

#### Stops completions in the `in` part of `for in` (prevent the common annoying `inflight` completions)
##### Fixes #3997
<img width="377" alt="image" src="https://github.com/winglang/wing/assets/1237390/ab776893-6cea-4aed-a4d5-bcee27025964">


#### Adds completions for `bring` (and prevent irrelevant completions)
##### Fixes #3975
<img width="599" alt="image" src="https://github.com/winglang/wing/assets/1237390/8118cce8-c2f6-49fb-8f72-0564062dfa82">

_Didn't add documentation for them yet, wasn't sure how to go about it without some duplicated hardcoding._

---

Misc:
- `bring` followed by a single character previously caused a panic (e.g. `bring c`). Most people probably didn't notice cause they were typing `bring cloud` so fast
- No completions in comments or strings
- Tried to centralize many of the cases where it's obvious there will be no completions available. `completions.rs` is still begging for a rewrite.

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
MarkMcCulloh authored Oct 6, 2023
1 parent a496713 commit fc47625
Show file tree
Hide file tree
Showing 13 changed files with 282 additions and 14 deletions.
6 changes: 5 additions & 1 deletion examples/tests/invalid/bring.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ bring cloud;
bring cloud;
// ^^^^^ "cloud" is already defined
bring fs;
// ^^^^^ "fs" is not a built-in module
// ^^^^^ "fs" is not a built-in module
bring ;
//^^^^^ Expected module specification (see https://www.winglang.io/docs/libraries)
bring c;
//^^^^^^ "c" is not a built-in module
2 changes: 1 addition & 1 deletion libs/tree-sitter-wing/grammar.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ module.exports = grammar({
import_statement: ($) =>
seq(
"bring",
field("module_name", choice($.identifier, $.string)),
optional(field("module_name", choice($.identifier, $.string))),
optional(seq("as", field("alias", $.identifier))),
$._semicolon
),
Expand Down
170 changes: 161 additions & 9 deletions libs/wingc/src/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::type_check::{
};
use crate::visit::{visit_expr, visit_type_annotation, Visit};
use crate::wasm_util::{ptr_to_string, string_to_combined_ptr, WASM_RETURN_ERROR};
use crate::{UTIL_CLASS_NAME, WINGSDK_ASSEMBLY_NAME, WINGSDK_STD_MODULE};
use crate::{UTIL_CLASS_NAME, WINGSDK_ASSEMBLY_NAME, WINGSDK_BRINGABLE_MODULES, WINGSDK_STD_MODULE};

use super::sync::check_utf8;

Expand All @@ -38,6 +38,72 @@ pub unsafe extern "C" fn wingc_on_completion(ptr: u32, len: u32) -> u64 {
}
}

/// Using tree-sitter data only, check if there should be no valid completions at this position.
/// This allows for quick short-circuits to avoid visiting the AST for unambiguous cases.
/// Returns true if there should be no completions, false otherwise
fn check_ts_to_completions(interesting_node: &Node) -> bool {
let interesting_node_kind = interesting_node.kind();

let mut no_completions = match interesting_node_kind {
"comment" => true,

// Loose backslash? Get outta here
"\\" => true,

// Strings are just strings
// TODO Remove this for https://github.com/winglang/wing/issues/4420
"string" | "\"" => true,

// After "for", we are expecting a variable name
"for" => true,

// we are at the end of an import statement and/or expecting an alias
"import_statement" | "as" => true,

// Starting a definition
"let" | "reassignable" => true,

// Following the keyword of a block-style declaration
"class" | "struct" | "interface" | "test" => true,

// Starting an inflight closure
"inflight_specifier" => true,

// No completions are valid immediately following a closing brace
")" | "}" | "]" => true,

// There will always be options following a colon
":" => return false,

_ => false,
};

if no_completions {
return no_completions;
}

if let Some(parent) = interesting_node.parent() {
let parent_kind = parent.kind();

no_completions = match parent_kind {
"ERROR" => {
if let Some(first_child) = parent.child(0) {
match first_child.kind() {
"for" => matches!(interesting_node_kind, "in" | "identifier"),
"let" => !matches!(interesting_node_kind, "="),
_ => false,
}
} else {
false
}
}
_ => false,
};
}

no_completions
}

pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse {
WING_TYPES.with(|types| {
let mut types = types.borrow_mut();
Expand Down Expand Up @@ -86,6 +152,30 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse
);
let node_to_complete_kind = node_to_complete.kind();

if check_ts_to_completions(&node_to_complete) {
return vec![];
}

if matches!(node_to_complete.parent(), Some(p) if p.kind() == "import_statement")
&& matches!(node_to_complete_kind, "identifier" | "bring")
{
let mut modules = WINGSDK_BRINGABLE_MODULES
.map(|module| CompletionItem {
label: module.to_string(),
kind: Some(CompletionItemKind::MODULE),
..Default::default()
})
.to_vec();
modules.push(CompletionItem {
label: "\"module\"".to_string(),
insert_text: Some("\"$1\" as $2".to_string()),
kind: Some(CompletionItemKind::SNIPPET),
insert_text_format: Some(InsertTextFormat::SNIPPET),
..Default::default()
});
return modules;
}

let mut scope_visitor = ScopeVisitor::new(
node_to_complete.parent().map(|parent| WingSpan {
start: parent.start_position().into(),
Expand Down Expand Up @@ -395,9 +485,6 @@ fn get_current_scope_completions(
let mut in_type = preceding_text.ends_with(':');

match node_to_complete.kind() {
// there are no possible completions that could follow ")"
")" => return vec![],

"set_literal" | "struct_literal" => {
in_type = false;
}
Expand Down Expand Up @@ -439,11 +526,6 @@ fn get_current_scope_completions(
});
}

// in block-style statements, there are no possible completions that can follow the keyword
"interface" | "struct" | "class" | "test" => {
return vec![];
}

_ => {}
}

Expand Down Expand Up @@ -1485,4 +1567,74 @@ let x: Outer = { bThing: { } }
//^
"#
);

test_completion_list!(
bring_suggestions,
r#"
bring
//^
"#
);

test_completion_list!(
bring_suggestions_partial,
r#"
bring c
//^
"#
);

test_completion_list!(
bring_alias,
r#"
bring "" as
//^
"#,
assert!(bring_alias.is_empty())
);

test_completion_list!(
definition_identifier_partial,
r#"
let a
//^
"#,
assert!(definition_identifier_partial.is_empty())
);

test_completion_list!(
definition_identifier,
r#"
let
//^
"#,
assert!(definition_identifier.is_empty())
);

test_completion_list!(
string_inner,
r#"
let x = ""
//^
"#,
assert!(string_inner.is_empty())
);

test_completion_list!(
for_in_inner,
r#"
for x i
//^
"#,
assert!(for_in_inner.is_empty())
);

test_completion_list!(
comment,
r#"
let x = // hi
//^
"#,
assert!(comment.is_empty())
);
}
5 changes: 5 additions & 0 deletions libs/wingc/src/lsp/snapshots/completions/bring_alias.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

30 changes: 30 additions & 0 deletions libs/wingc/src/lsp/snapshots/completions/bring_suggestions.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: libs/wingc/src/lsp/completions.rs
---
- label: aws
kind: 9
sortText: kk|aws
- label: cloud
kind: 9
sortText: kk|cloud
- label: ex
kind: 9
sortText: kk|ex
- label: http
kind: 9
sortText: kk|http
- label: math
kind: 9
sortText: kk|math
- label: regex
kind: 9
sortText: kk|regex
- label: util
kind: 9
sortText: kk|util
- label: "\"module\""
kind: 15
sortText: "ll|\"module\""
insertText: "\"$1\" as $2"
insertTextFormat: 2

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: libs/wingc/src/lsp/completions.rs
---
- label: aws
kind: 9
sortText: kk|aws
- label: cloud
kind: 9
sortText: kk|cloud
- label: ex
kind: 9
sortText: kk|ex
- label: http
kind: 9
sortText: kk|http
- label: math
kind: 9
sortText: kk|math
- label: regex
kind: 9
sortText: kk|regex
- label: util
kind: 9
sortText: kk|util
- label: "\"module\""
kind: 15
sortText: "ll|\"module\""
insertText: "\"$1\" as $2"
insertTextFormat: 2

5 changes: 5 additions & 0 deletions libs/wingc/src/lsp/snapshots/completions/comment.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

5 changes: 5 additions & 0 deletions libs/wingc/src/lsp/snapshots/completions/for_in_inner.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

5 changes: 5 additions & 0 deletions libs/wingc/src/lsp/snapshots/completions/string_inner.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: libs/wingc/src/lsp/completions.rs
---
[]

12 changes: 10 additions & 2 deletions libs/wingc/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,15 +791,23 @@ impl<'s> Parser<'s> {
}

fn build_bring_statement(&self, statement_node: &Node) -> DiagnosticResult<StmtKind> {
let module_name_node = self.get_child_field(&statement_node, "module_name")?;
let Some(module_name_node) = statement_node.child_by_field_name("module_name") else {
return self.with_error("Expected module specification (see https://www.winglang.io/docs/libraries)", &statement_node.child(statement_node.child_count() - 1).unwrap_or(*statement_node));
};

let module_name = self.node_symbol(&module_name_node)?;
let alias = if let Some(identifier) = statement_node.child_by_field_name("alias") {
Some(self.check_reserved_symbol(&identifier)?)
} else {
None
};

let module_path = Utf8Path::new(&module_name.name[1..module_name.name.len() - 1]);
let module_path = if module_name.name.len() > 1 {
Utf8Path::new(&module_name.name[1..module_name.name.len() - 1])
} else {
Utf8Path::new(&module_name.name)
};

if is_absolute_path(&module_path) {
return self.with_error(
format!("Cannot bring \"{}\" since it is not a relative path", module_path),
Expand Down
16 changes: 15 additions & 1 deletion tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,14 @@ Duration <DURATION>"
`;
exports[`bring.test.w 1`] = `
"error: Redundant bring of \\"std\\"
"error: Expected module specification (see https://www.winglang.io/docs/libraries)
--> ../../../examples/tests/invalid/bring.test.w:8:7
|
8 | bring ;
| ^ Expected module specification (see https://www.winglang.io/docs/libraries)
error: Redundant bring of \\"std\\"
--> ../../../examples/tests/invalid/bring.test.w:1:1
|
1 | bring std;
Expand All @@ -294,6 +301,13 @@ error: \\"fs\\" is not a built-in module
| ^^^^^^^^^ \\"fs\\" is not a built-in module
error: \\"c\\" is not a built-in module
--> ../../../examples/tests/invalid/bring.test.w:10:1
|
10 | bring c;
| ^^^^^^^^ \\"c\\" is not a built-in module
Tests 1 failed (1)
Expand Down

0 comments on commit fc47625

Please sign in to comment.