[ty] Don't provide completions when in class or function definition by MatthewMckee4 · Pull Request #21146 · astral-sh/ruff

MatthewMckee4 · 2025-10-30T16:50:17Z

Summary

We now check if we are in a situation where we are in the name of class or function definition like def f or class f. We check if the second last token is def or class.

Test Plan

Added test shown in that issue.

Also add test for situation where we have not started typing the name like def and class . My changes do not change any behavior there, but i think it is good to show we still dont provide completions there

github-actions · 2025-10-30T16:52:26Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

github-actions · 2025-10-30T16:53:51Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

MichaReiser

Thank you

MichaReiser · 2025-10-30T19:43:05Z

crates/ty_ide/src/completion.rs

+    if is_in_comment(&parsed, offset)
+        || is_in_string(&parsed, offset)
+        || is_in_definition_place(&parsed, offset)


Should we pull out the tokens.before()? It seems every one of those functions now searches the tokens before offset.

MichaReiser · 2025-10-30T19:44:07Z

crates/ty_ide/src/completion.rs

+        .len()
+        .checked_sub(2)
+        .and_then(|i| tokens.get(i))
+        .is_some_and(|t| matches!(t.kind(), TokenKind::Def | TokenKind::Class))


I forgot to mention this in my PR but we should probably do this too in `type .

cool, will cover that, though i think there is a case where we can't catch it:

type here is just Name in the ast.

What we can catch is this:

type f<CURSOR> = int

(unless i look directly at the string, but I'm not sure if this is right or not.

Oh right, because it is a contextual keyword. I think inspecting the source would be fine here because I don't see any case where type <name> is valid.

MatthewMckee4 · 2025-10-30T22:07:18Z

crates/ty_ide/src/completion.rs

+            ) || file.read_to_string(db).is_ok_and(|source| {
+                source[t.range().start().to_usize()..t.range().end().to_usize()] == *"type"


This does not seem like the right approach, but i can't seem to find any other apis to use to do this.

You want something like:

if matches!(t.kind(), TokenKind::Def | TokenKind::Class | TokenKind::Type) { true } else if t.kind() == TokenKind::Name { let source = source_text(db, file); &source[t.range()] == "type" } else { false }

cool, thanks

MichaReiser

Sweet

BurntSushi

I realize this PR was already merged, but I wanted to leave feedback anyway.

I can address these in a follow-up PR if you don't want though. :-)

BurntSushi · 2025-10-31T13:00:30Z

crates/ty_ide/src/completion.rs

@@ -839,8 +841,7 @@ fn is_in_comment(parsed: &ParsedModuleRef, offset: TextSize) -> bool {
 ///
 /// Note that this will return `false` when positioned within an
 /// interpolation block in an f-string or a t-string.


The doc comments for this function (and the one above) should be updated.

BurntSushi · 2025-10-31T13:03:16Z

crates/ty_ide/src/completion.rs

+        );
+
+        builder.auto_import().build().not_contains("fabs");
+    }


I don't think these tests need to be enabling auto_import(). That's a bit of a big hammer. You can instead add a local item in the source file and test completions on that.

BurntSushi · 2025-10-31T13:06:04Z

crates/ty_ide/src/completion.rs


+/// If the tokens end with `class f` or `def f` we return true.
+/// If the tokens end with `class` or `def`, we return false.
+/// This is fine because we don't provide completions anyway.


I think it would be better to not repeat the implementation in the doc comment where possible. e.g., Your doc comment doesn't include type. Instead, something like, "Returns true when the tokens indicate that the definition of a new name is being introduced at the end."

* origin/main: [ty] Fix generic inference for non-dataclass inheriting from generic dataclass (#21159) Update etcetera to 0.11.0 (#21160) Fix missing diagnostics for notebooks (#21156) [ty] Fix tests for definition completions (#21153) Bump v0.14.3 (#21152) [ty] Don't provide completions when in class or function definition (#21146) [ty] Use the top materialization of classes for narrowing in class-patterns for `match` statements (#21150)

MatthewMckee4 · 2025-11-01T14:54:34Z

Thanks for the feedback, ill address this in a PR

@BurntSushi

## Summary @BurntSushi provided some feedback in #21146 so i address it here.

MatthewMckee4 added 2 commits October 30, 2025 16:49

Dont provide completions when cursor in a definition

bd69c89

Update docs

f085488

MatthewMckee4 requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners October 30, 2025 16:50

AlexWaygood added bug Something isn't working server Related to the LSP server ty Multi-file analysis & type inference labels Oct 30, 2025

MatthewMckee4 changed the title ~~No completions in definitions~~ [ty] Don't provide completions when in class or function definition Oct 30, 2025

Un indent test file contents

92b4b62

MichaReiser requested review from BurntSushi and removed request for AlexWaygood, carljm, dcreager and sharkdp October 30, 2025 19:41

MichaReiser approved these changes Oct 30, 2025

View reviewed changes

MatthewMckee4 added 2 commits October 30, 2025 20:27

Address review

9e5aa99

Add support for type def

f738453

MatthewMckee4 commented Oct 30, 2025

View reviewed changes

Refactor

358815e

MichaReiser approved these changes Oct 30, 2025

View reviewed changes

MichaReiser merged commit 3be3a10 into astral-sh:main Oct 30, 2025
41 checks passed

MatthewMckee4 added a commit to MatthewMckee4/ruff that referenced this pull request Oct 30, 2025

Fix tests for astral-sh#21146

046ef87

BurntSushi reviewed Oct 31, 2025

View reviewed changes

MatthewMckee4 mentioned this pull request Nov 1, 2025

Clean up definition completions docs and tests #21183

Merged

MatthewMckee4 deleted the no-completions-in-definitions branch November 1, 2025 15:07

AlexWaygood mentioned this pull request Nov 11, 2025

All completions should be suppressed after import <NAME> as t<CURSOR> astral-sh/ty#1523

Closed

		) \|\| file.read_to_string(db).is_ok_and(\|source\| {
		source[t.range().start().to_usize()..t.range().end().to_usize()] == *"type"

Conversation

MatthewMckee4 commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthewMckee4 Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BurntSushi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthewMckee4 commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MatthewMckee4 commented Oct 30, 2025 •

edited

Loading

github-actions bot commented Oct 30, 2025 •

edited

Loading

github-actions bot commented Oct 30, 2025 •

edited

Loading

`mypy_primer` results

MatthewMckee4 Oct 30, 2025 •

edited

Loading