Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable "liftShorter" completion classifier on 221 platform #8516

Merged
merged 2 commits into from Feb 7, 2022

Conversation

vlad20012
Copy link
Member

@vlad20012 vlad20012 commented Feb 5, 2022

LiftShorterItemsClassifier aka "liftShorter" is a completion variants sorter ("classifier") that does a text-based sorting that is hard to explain in words. Let's look at the picture:

image

Here grayed variants should have been at the end of the list, but they were upped by liftShorter. Why? Well, because they are prefixes of their previous variants! For example, is_equal is a prefix of is_equal_private, so is_equal variant is placed after is_equal_private. Why? Well, I heard that it helps in Java: "liftShorter" places interfaces near their implementations, e.g. places an interface Foo after FooImpl if FooImpl is prioritized for some reason. Why does liftShorter work in all languages by default? I have no idea. We decided to disable liftShorter in Rust because we can't find any Rust naming pattern where it is useful.

How to see "liftShorter" in action:

  1. Disable Machine Learning-Assisted completion in File | Settings | Editor | General | Code Completion
struct S;
impl S {
    fn foobar(&self) -> String { todo!() }
    fn foo(&self) -> i32 { todo!() }
    fn zzz(&self) -> String { todo!() }
}

fn main() {
    let a: String = S./*caret*/
}

Here, foobar and zzz should be prioritized over foo because their return types match with the expected type "String". But if "liftShorter" works, it will place foo after foobar:

image

This PR disables liftShorter in 2022.1 IDEs. I'd like to disable it in all versions, but this can break the ML completion model.

@vlad20012 vlad20012 self-assigned this Feb 5, 2022
@vlad20012 vlad20012 changed the title Disable lift shorter completion classifier Disable "liftShorter" completion classifier on 221 platform Feb 5, 2022
@vlad20012 vlad20012 added this to the v165 milestone Feb 7, 2022
@vlad20012 vlad20012 force-pushed the disable-lift-shorter-completion-classifier branch from c42fd5d to 37b34cc Compare February 7, 2022 14:35
@vlad20012 vlad20012 added this to In Progress in To test via automation Feb 7, 2022
@vlad20012 vlad20012 added the fix Pull requests that fix some bug(s) label Feb 7, 2022
@vlad20012 vlad20012 force-pushed the disable-lift-shorter-completion-classifier branch from 37b34cc to aa5fb1e Compare February 7, 2022 17:44
@vlad20012
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Feb 7, 2022
8516: Disable "liftShorter" completion classifier on 221 platform r=vlad20012 a=vlad20012

`LiftShorterItemsClassifier` aka "liftShorter" is a completion variants sorter ("classifier") that does a text-based sorting that is hard to explain in words. Let's look at the picture:  

![image](https://user-images.githubusercontent.com/3221931/152832037-0e4b3566-201e-46c7-83d3-5334974e527d.png)

Here grayed variants should have been at the end of the list, but they were upped by liftShorter. Why? Well, because they are prefixes of their previous variants! For example, `is_equal` is a prefix of `is_equal_private`, so `is_equal` variant is placed after `is_equal_private`. Why? Well, I heard that it helps in Java: "liftShorter" places interfaces near their implementations, e.g. places an interface `Foo` after `FooImpl` if `FooImpl` is prioritized for some reason. Why does liftShorter work in all languages by default? I have no idea. We decided to disable liftShorter in Rust because we can't find any Rust naming pattern where it is useful. 

How to see "liftShorter" in action:

1. Disable Machine Learning-Assisted completion in `File | Settings | Editor | General | Code Completion`
2. 

```rust
struct S;
impl S {
    fn foobar(&self) -> String { todo!() }
    fn foo(&self) -> i32 { todo!() }
    fn zzz(&self) -> String { todo!() }
}

fn main() {
    let a: String = S./*caret*/
}
```

Here, `foobar` and `zzz` should be prioritized over `foo` because their return types match with the expected type "String". But if "liftShorter" works, it will place `foo` after `foobar`:

![image](https://user-images.githubusercontent.com/3221931/152836115-a4ab7476-e03a-4089-995a-c36434dc57db.png)

This PR disables `liftShorter` in 2022.1 IDEs. I'd like to disable it in all versions, but this can break the ML completion model.

Co-authored-by: vlad20012 <beskvlad@gmail.com>
@bors
Copy link
Contributor

bors bot commented Feb 7, 2022

Build failed:

@vlad20012
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 7, 2022

Build succeeded:

@bors bors bot merged commit 031052d into master Feb 7, 2022
To test automation moved this from In Progress to Test Feb 7, 2022
@bors bors bot deleted the disable-lift-shorter-completion-classifier branch February 7, 2022 20:52
@neonaot neonaot self-assigned this Feb 9, 2022
@neonaot neonaot moved this from Test to Done in To test Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests that fix some bug(s)
Projects
To test
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants