-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: Correctly support SelfType when searching for usages #8773
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
Conversation
| FileId(0) 28..32 | ||
| FileId(0) 50..54 | ||
| "#]], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s somewhat inconsistent indentation :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one downside of expect-testing is rustfmt not touching it while also continuing to pass with a different indentation 😄
crates/ide_db/src/search.rs
Outdated
| let module = sema.to_module_def(root_file)?; | ||
| Some(it.ty(sema.db, module)) | ||
| } | ||
| ModuleDef::Trait(_it) => None, // FIXME turn trait into its self-type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s tricky, but this operation doesn’t make sense. Self has surprisingly different semantics in traits and impls.
In traits, it is a type parameter which implements the trait itself (but might as well implement other traits).
in impls, it is a type alias referring to the impl’s type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ye I didn't realize that, should definitely not do this for Self in trait defs then, changed the tests.
|
bors r+ |
Fixes #7443