[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 https://github.com/astral-sh/ty/issues/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? -->
This commit is contained in:
RasmusNygren 2025-11-21 18:06:46 +01:00 committed by GitHub
parent e3c78d8203
commit 03dfbf21eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 133 additions and 6 deletions

View File

@ -8,7 +8,7 @@ use ruff_python_ast as ast;
use ruff_python_ast::name::Name; use ruff_python_ast::name::Name;
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_python_parser::{Token, TokenAt, TokenKind, Tokens}; use ruff_python_parser::{Token, TokenAt, TokenKind, Tokens};
use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use ty_python_semantic::{ use ty_python_semantic::{
Completion as SemanticCompletion, ModuleName, NameKind, SemanticModel, Completion as SemanticCompletion, ModuleName, NameKind, SemanticModel,
types::{CycleDetector, Type}, types::{CycleDetector, Type},
@ -329,7 +329,7 @@ pub fn completion<'db>(
let tokens = tokens_start_before(parsed.tokens(), offset); let tokens = tokens_start_before(parsed.tokens(), offset);
let typed = find_typed_text(db, file, &parsed, offset); let typed = find_typed_text(db, file, &parsed, offset);
if is_in_no_completions_place(db, file, tokens, typed.as_deref()) { if is_in_no_completions_place(db, file, &parsed, offset, tokens, typed.as_deref()) {
return vec![]; return vec![];
} }
@ -1270,10 +1270,14 @@ fn find_typed_text(
fn is_in_no_completions_place( fn is_in_no_completions_place(
db: &dyn Db, db: &dyn Db,
file: File, file: File,
parsed: &ParsedModuleRef,
offset: TextSize,
tokens: &[Token], tokens: &[Token],
typed: Option<&str>, typed: Option<&str>,
) -> bool { ) -> bool {
is_in_comment(tokens) || is_in_string(tokens) || is_in_definition_place(db, file, tokens, typed) is_in_comment(tokens)
|| is_in_string(tokens)
|| is_in_definition_place(db, file, parsed, offset, tokens, typed)
} }
/// Whether the last token is within a comment or not. /// Whether the last token is within a comment or not.
@ -1296,11 +1300,18 @@ fn is_in_string(tokens: &[Token]) -> bool {
/// Returns true when the tokens indicate that the definition of a new /// Returns true when the tokens indicate that the definition of a new
/// name is being introduced at the end. /// name is being introduced at the end.
fn is_in_definition_place(db: &dyn Db, file: File, tokens: &[Token], typed: Option<&str>) -> bool { fn is_in_definition_place(
db: &dyn Db,
file: File,
parsed: &ParsedModuleRef,
offset: TextSize,
tokens: &[Token],
typed: Option<&str>,
) -> bool {
fn is_definition_token(token: &Token) -> bool { fn is_definition_token(token: &Token) -> bool {
matches!( matches!(
token.kind(), token.kind(),
TokenKind::Def | TokenKind::Class | TokenKind::Type | TokenKind::As TokenKind::Def | TokenKind::Class | TokenKind::Type | TokenKind::As | TokenKind::For
) )
} }
@ -1314,11 +1325,37 @@ fn is_in_definition_place(db: &dyn Db, file: File, tokens: &[Token], typed: Opti
false false
} }
}; };
match tokens { if match tokens {
[.., penultimate, _] if typed.is_some() => is_definition_keyword(penultimate), [.., penultimate, _] if typed.is_some() => is_definition_keyword(penultimate),
[.., last] if typed.is_none() => is_definition_keyword(last), [.., last] if typed.is_none() => is_definition_keyword(last),
_ => false, _ => false,
} {
return true;
} }
// Analyze the AST if token matching is insufficient
// to determine if we're inside a name definition.
is_in_variable_binding(parsed, offset, typed)
}
/// Returns true when the cursor sits on a binding statement.
/// E.g. naming a parameter, type parameter, or `for` <name>).
fn is_in_variable_binding(parsed: &ParsedModuleRef, offset: TextSize, typed: Option<&str>) -> bool {
let range = if let Some(typed) = typed {
let start = offset - typed.text_len();
TextRange::new(start, offset)
} else {
TextRange::empty(offset)
};
let covering = covering_node(parsed.syntax().into(), range);
covering.ancestors().any(|node| match node {
ast::AnyNodeRef::Parameter(param) => param.name.range.contains_range(range),
ast::AnyNodeRef::TypeParamTypeVar(type_param) => {
type_param.name.range.contains_range(range)
}
ast::AnyNodeRef::StmtFor(stmt_for) => stmt_for.target.range().contains_range(range),
_ => false,
})
} }
/// Order completions according to the following rules: /// Order completions according to the following rules:
@ -5174,6 +5211,96 @@ match status:
); );
} }
#[test]
fn no_completions_in_empty_for_variable_binding() {
let builder = completion_test_builder(
"\
for <CURSOR>
",
);
assert_snapshot!(
builder.build().snapshot(),
@"<No completions found>",
);
}
#[test]
fn no_completions_in_for_variable_binding() {
let builder = completion_test_builder(
"\
for foo<CURSOR>
",
);
assert_snapshot!(
builder.build().snapshot(),
@"<No completions found>",
);
}
#[test]
fn no_completions_in_for_tuple_variable_binding() {
let builder = completion_test_builder(
"\
for foo, bar<CURSOR>
",
);
assert_snapshot!(
builder.build().snapshot(),
@"<No completions found>",
);
}
#[test]
fn no_completions_in_function_param() {
let builder = completion_test_builder(
"\
def foo(p<CURSOR>
",
);
assert_snapshot!(
builder.build().snapshot(),
@"<No completions found>",
);
}
#[test]
fn no_completions_in_function_type_param() {
let builder = completion_test_builder(
"\
def foo[T<CURSOR>]
",
);
assert_snapshot!(
builder.build().snapshot(),
@"<No completions found>",
);
}
#[test]
fn completions_in_function_type_param_bound() {
completion_test_builder(
"\
def foo[T: s<CURSOR>]
",
)
.build()
.contains("str");
}
#[test]
fn completions_in_function_param_type_annotation() {
// Ensure that completions are no longer
// suppressed when have left the name
// definition block.
completion_test_builder(
"\
def foo(param: s<CURSOR>)
",
)
.build()
.contains("str");
}
#[test] #[test]
fn favour_symbols_currently_imported() { fn favour_symbols_currently_imported() {
let snapshot = CursorTest::builder() let snapshot = CursorTest::builder()