Skip to content

Commit b700869

Browse files
authored
No completions in comments (#1999)
Suppress completions inside comments like ```qsharp import Foo; // Hello there | import Bar; ```
1 parent 424ea6d commit b700869

File tree

2 files changed

+60
-20
lines changed

2 files changed

+60
-20
lines changed

language_service/src/completion.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,16 @@ fn expected_word_kinds(
102102
source_contents: &str,
103103
cursor_offset: u32,
104104
) -> WordKinds {
105+
// We should not retun any completions in comments.
106+
// This compensates for a bug in [`possible_words_at_offset_in_source`] .
107+
// Ideally, that function would be aware of the comment context and not
108+
// return any completions, however this is difficult to do today because
109+
// of the parser's unawareness of comment tokens.
110+
// So we do a simple check here where we have access to the full source contents.
111+
if in_comment(source_contents, cursor_offset) {
112+
return WordKinds::empty();
113+
}
114+
105115
match &compilation.kind {
106116
CompilationKind::OpenProject {
107117
package_graph_sources,
@@ -121,6 +131,16 @@ fn expected_word_kinds(
121131
}
122132
}
123133

134+
fn in_comment(source_contents: &str, cursor_offset: u32) -> bool {
135+
// find the last newline before the cursor
136+
let last_line_start = source_contents[..cursor_offset as usize]
137+
.rfind('\n')
138+
.unwrap_or(0);
139+
// find the last comment start before the cursor
140+
let last_comment_start = source_contents[last_line_start..cursor_offset as usize].rfind("//");
141+
last_comment_start.is_some()
142+
}
143+
124144
/// Collects hardcoded completions from the given set of parser predictions.
125145
///
126146
/// Hardcoded words are actual keywords (`let`, etc) as well as other words that are

language_service/src/completion/tests.rs

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ fn check_with_dependency(
121121
assert_no_duplicates(actual_completions);
122122
}
123123

124+
fn check_no_completions(source_with_cursor: &str) {
125+
let (compilation, cursor_position, _) = compile_with_markers(source_with_cursor, true);
126+
let actual_completions =
127+
get_completions(&compilation, "<source>", cursor_position, Encoding::Utf8);
128+
assert_eq!(actual_completions.items, Vec::default());
129+
}
130+
124131
fn assert_no_duplicates(mut actual_completions: CompletionList) {
125132
actual_completions
126133
.items
@@ -4075,42 +4082,55 @@ fn incomplete_return_type_in_partial_callable_signature() {
40754082

40764083
#[test]
40774084
fn no_path_segment_completion_inside_attr() {
4078-
check(
4085+
check_no_completions(
40794086
"namespace Test {
40804087
40814088
@Config(FakeStdLib.↘)
40824089
function Main() : Unit {
40834090
}
40844091
}",
4085-
&["Fake", "not", "Test"],
4086-
&expect![[r#"
4087-
[
4088-
None,
4089-
None,
4090-
None,
4091-
]
4092-
"#]],
40934092
);
40944093
}
40954094

40964095
#[test]
40974096
fn no_completion_inside_attr() {
4098-
check(
4097+
check_no_completions(
40994098
"namespace Test {
41004099
41014100
@Config(↘)
41024101
function Main() : Unit {
4103-
let FakeStdLib = new Foo { bar = 3 };
4104-
FakeStdLib.↘
41054102
}
41064103
}",
4107-
&["Fake", "not", "Test"],
4108-
&expect![[r#"
4109-
[
4110-
None,
4111-
None,
4112-
None,
4113-
]
4114-
"#]],
4104+
);
4105+
}
4106+
4107+
#[test]
4108+
fn in_comment() {
4109+
check_no_completions(
4110+
"namespace Test {
4111+
import Foo;
4112+
// Hello there ↘
4113+
import Bar;
4114+
}",
4115+
);
4116+
}
4117+
4118+
#[test]
4119+
fn in_doc_comment() {
4120+
check_no_completions(
4121+
"namespace Test {
4122+
import Foo;
4123+
/// Hello there ↘
4124+
import Bar;
4125+
}",
4126+
);
4127+
}
4128+
4129+
#[test]
4130+
fn in_trailing_comment() {
4131+
check_no_completions(
4132+
"namespace Test {
4133+
import Foo; // Hello there ↘
4134+
}",
41154135
);
41164136
}

0 commit comments

Comments
 (0)