Skip to content

Commit 03dfbf2

Browse files
authored
[ty] suppress autocomplete suggestions during variable binding (#21549)
Statements such as `def foo(p<CURSOR>`, `def foo[T<CURSOR>` and `for foo<CURSOR>` should not generate any suggestions as these cases are introducing new names. If it's not possible to determine that suggestions should be omitted using token matching in an easy way, we turn to traversing the AST to determine the context. <!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary Fixes astral-sh/ty#1563 It keeps using the existing token matching pattern for the easy cases (nothing typed and most recent token is a definition token) and fallbacks to AST traveral for the slightly more difficult cases where token matching becomes difficult and error prone. <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan New test cases and sanity-checking in the ty playground <!-- How was it tested? -->
1 parent e3c78d8 commit 03dfbf2

File tree

1 file changed

+133
-6
lines changed

1 file changed

+133
-6
lines changed

crates/ty_ide/src/completion.rs

Lines changed: 133 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use ruff_python_ast as ast;
88
use ruff_python_ast::name::Name;
99
use ruff_python_codegen::Stylist;
1010
use ruff_python_parser::{Token, TokenAt, TokenKind, Tokens};
11-
use ruff_text_size::{Ranged, TextRange, TextSize};
11+
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
1212
use ty_python_semantic::{
1313
Completion as SemanticCompletion, ModuleName, NameKind, SemanticModel,
1414
types::{CycleDetector, Type},
@@ -329,7 +329,7 @@ pub fn completion<'db>(
329329
let tokens = tokens_start_before(parsed.tokens(), offset);
330330
let typed = find_typed_text(db, file, &parsed, offset);
331331

332-
if is_in_no_completions_place(db, file, tokens, typed.as_deref()) {
332+
if is_in_no_completions_place(db, file, &parsed, offset, tokens, typed.as_deref()) {
333333
return vec![];
334334
}
335335

@@ -1270,10 +1270,14 @@ fn find_typed_text(
12701270
fn is_in_no_completions_place(
12711271
db: &dyn Db,
12721272
file: File,
1273+
parsed: &ParsedModuleRef,
1274+
offset: TextSize,
12731275
tokens: &[Token],
12741276
typed: Option<&str>,
12751277
) -> bool {
1276-
is_in_comment(tokens) || is_in_string(tokens) || is_in_definition_place(db, file, tokens, typed)
1278+
is_in_comment(tokens)
1279+
|| is_in_string(tokens)
1280+
|| is_in_definition_place(db, file, parsed, offset, tokens, typed)
12771281
}
12781282

12791283
/// Whether the last token is within a comment or not.
@@ -1296,11 +1300,18 @@ fn is_in_string(tokens: &[Token]) -> bool {
12961300

12971301
/// Returns true when the tokens indicate that the definition of a new
12981302
/// name is being introduced at the end.
1299-
fn is_in_definition_place(db: &dyn Db, file: File, tokens: &[Token], typed: Option<&str>) -> bool {
1303+
fn is_in_definition_place(
1304+
db: &dyn Db,
1305+
file: File,
1306+
parsed: &ParsedModuleRef,
1307+
offset: TextSize,
1308+
tokens: &[Token],
1309+
typed: Option<&str>,
1310+
) -> bool {
13001311
fn is_definition_token(token: &Token) -> bool {
13011312
matches!(
13021313
token.kind(),
1303-
TokenKind::Def | TokenKind::Class | TokenKind::Type | TokenKind::As
1314+
TokenKind::Def | TokenKind::Class | TokenKind::Type | TokenKind::As | TokenKind::For
13041315
)
13051316
}
13061317

@@ -1314,11 +1325,37 @@ fn is_in_definition_place(db: &dyn Db, file: File, tokens: &[Token], typed: Opti
13141325
false
13151326
}
13161327
};
1317-
match tokens {
1328+
if match tokens {
13181329
[.., penultimate, _] if typed.is_some() => is_definition_keyword(penultimate),
13191330
[.., last] if typed.is_none() => is_definition_keyword(last),
13201331
_ => false,
1332+
} {
1333+
return true;
13211334
}
1335+
// Analyze the AST if token matching is insufficient
1336+
// to determine if we're inside a name definition.
1337+
is_in_variable_binding(parsed, offset, typed)
1338+
}
1339+
1340+
/// Returns true when the cursor sits on a binding statement.
1341+
/// E.g. naming a parameter, type parameter, or `for` <name>).
1342+
fn is_in_variable_binding(parsed: &ParsedModuleRef, offset: TextSize, typed: Option<&str>) -> bool {
1343+
let range = if let Some(typed) = typed {
1344+
let start = offset - typed.text_len();
1345+
TextRange::new(start, offset)
1346+
} else {
1347+
TextRange::empty(offset)
1348+
};
1349+
1350+
let covering = covering_node(parsed.syntax().into(), range);
1351+
covering.ancestors().any(|node| match node {
1352+
ast::AnyNodeRef::Parameter(param) => param.name.range.contains_range(range),
1353+
ast::AnyNodeRef::TypeParamTypeVar(type_param) => {
1354+
type_param.name.range.contains_range(range)
1355+
}
1356+
ast::AnyNodeRef::StmtFor(stmt_for) => stmt_for.target.range().contains_range(range),
1357+
_ => false,
1358+
})
13221359
}
13231360

13241361
/// Order completions according to the following rules:
@@ -5174,6 +5211,96 @@ match status:
51745211
);
51755212
}
51765213

5214+
#[test]
5215+
fn no_completions_in_empty_for_variable_binding() {
5216+
let builder = completion_test_builder(
5217+
"\
5218+
for <CURSOR>
5219+
",
5220+
);
5221+
assert_snapshot!(
5222+
builder.build().snapshot(),
5223+
@"<No completions found>",
5224+
);
5225+
}
5226+
5227+
#[test]
5228+
fn no_completions_in_for_variable_binding() {
5229+
let builder = completion_test_builder(
5230+
"\
5231+
for foo<CURSOR>
5232+
",
5233+
);
5234+
assert_snapshot!(
5235+
builder.build().snapshot(),
5236+
@"<No completions found>",
5237+
);
5238+
}
5239+
5240+
#[test]
5241+
fn no_completions_in_for_tuple_variable_binding() {
5242+
let builder = completion_test_builder(
5243+
"\
5244+
for foo, bar<CURSOR>
5245+
",
5246+
);
5247+
assert_snapshot!(
5248+
builder.build().snapshot(),
5249+
@"<No completions found>",
5250+
);
5251+
}
5252+
5253+
#[test]
5254+
fn no_completions_in_function_param() {
5255+
let builder = completion_test_builder(
5256+
"\
5257+
def foo(p<CURSOR>
5258+
",
5259+
);
5260+
assert_snapshot!(
5261+
builder.build().snapshot(),
5262+
@"<No completions found>",
5263+
);
5264+
}
5265+
5266+
#[test]
5267+
fn no_completions_in_function_type_param() {
5268+
let builder = completion_test_builder(
5269+
"\
5270+
def foo[T<CURSOR>]
5271+
",
5272+
);
5273+
assert_snapshot!(
5274+
builder.build().snapshot(),
5275+
@"<No completions found>",
5276+
);
5277+
}
5278+
5279+
#[test]
5280+
fn completions_in_function_type_param_bound() {
5281+
completion_test_builder(
5282+
"\
5283+
def foo[T: s<CURSOR>]
5284+
",
5285+
)
5286+
.build()
5287+
.contains("str");
5288+
}
5289+
5290+
#[test]
5291+
fn completions_in_function_param_type_annotation() {
5292+
// Ensure that completions are no longer
5293+
// suppressed when have left the name
5294+
// definition block.
5295+
completion_test_builder(
5296+
"\
5297+
def foo(param: s<CURSOR>)
5298+
",
5299+
)
5300+
.build()
5301+
.contains("str");
5302+
}
5303+
51775304
#[test]
51785305
fn favour_symbols_currently_imported() {
51795306
let snapshot = CursorTest::builder()

0 commit comments

Comments
 (0)