Skip to content

fix: Add type_match score for struct_pat#22452

Merged
ChayimFriedman2 merged 1 commit into
rust-lang:masterfrom
A4-Tacks:pattern-score
May 25, 2026
Merged

fix: Add type_match score for struct_pat#22452
ChayimFriedman2 merged 1 commit into
rust-lang:masterfrom
A4-Tacks:pattern-score

Conversation

@A4-Tacks

Copy link
Copy Markdown
Member

Example

struct Foo(Bar);
struct Bar { field: i32 }
fn go(Foo($0): Foo) {}

Before this PR

st Bar  []
bn Bar {…} Bar { field$1 }$0 []
st Foo  []
bn Foo(…) Foo($1)$0 []

After this PR

bn Bar {…} Bar { field$1 }$0 [type]
st Bar  []
st Foo  []
bn Foo(…) Foo($1)$0 []

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2026
@ChayimFriedman2

Copy link
Copy Markdown
Contributor

I've already said that I am not a fan of extending completion analysis until we find a more principled approach, like speculative execution.

@A4-Tacks

Copy link
Copy Markdown
Member Author

extending completion analysis

This is basically an improvement on pure rendering

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

This is basically an improvement on pure rendering

What do you mean?

My stance is that completion analysis right now is unmaintainable and implemented the wrong way and extending it is amplifying that. Therefore I'm opposed to any extension to it.

@A4-Tacks

Copy link
Copy Markdown
Member Author

This PR is just to supplement the missing type match score

  1. Add type_match for struct pat (pure render/pattern.rs)
  2. Add type_match for nested enum variant (bit of context/analysis.rs)

About 2., it shouldn't worsen the situation

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

It does worsen the situation. Just a bit, sure, but a bit here and a bit there and suddenly the situation is a lot more worse than it's already is. As I said, I object to any change to analysis. Analysis is pretty fine currently, and nothing will happen if some bugs will stay unfixed until we figure out a better way (and of course, I invite you to be part of this process).

Example
---
```rust
struct Foo(Bar);
struct Bar { field: i32 }
fn go(Foo($0): Foo) {}
```

**Before this PR**

```text
st Bar  []
bn Bar {…} Bar { field$1 }$0 []
st Foo  []
bn Foo(…) Foo($1)$0 []
```

**After this PR**

```text
bn Bar {…} Bar { field$1 }$0 [type]
st Bar  []
st Foo  []
bn Foo(…) Foo($1)$0 []
```
@A4-Tacks

Copy link
Copy Markdown
Member Author

Okay, this is no longer related to the context/analysis

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

This is worse now because this should be done via analysis. Adding workarounds around analysis is worse than changing it.

As I said, I have absolutely no problem that this bug will remain unfixed until we figure out a better way to handle analysis. Open an issue and that's it.

@A4-Tacks

A4-Tacks commented May 25, 2026

Copy link
Copy Markdown
Member Author

This is worse now because this should be done via analysis.

Why? IMO, this is the most correct approach rather than analysis

IMO, the old behavior is the workarounds, this should have always applied type match

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

It's the job of analysis to determine the expected type for what the user will type, which is exactly what we need here.

Framing it differently: if we would use inference's analysis for that, which is what I want us to do eventually, we would get this for free.

@A4-Tacks

Copy link
Copy Markdown
Member Author

It's the job of analysis to determine the expected type for what the user will type, which is exactly what we need here.

This PR fix: The type that has already been analyzed, but the old solution uses selective addition of type matches to the completion item to change the score

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

Okay, I see now I misunderstood the current change, and you were right (although I can't understand what the intent of the original code was, why only compute type match for missing variants?).

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue May 25, 2026
@A4-Tacks

Copy link
Copy Markdown
Member Author

although I can't understand what the intent of the original code was, why only compute type match for missing variants?

As I said, this is a workaround that can be used to improve scores when type matching occurs

The correct approach is this PR, type matching always occurs, and missing variants is an additional score

Merged via the queue into rust-lang:master with commit 462d95c May 25, 2026
18 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2026
@A4-Tacks A4-Tacks deleted the pattern-score branch May 25, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants