7690: Extract `fn load_workspace(…)` from `fn load_cargo(…)` r=matklad a=regexident
Unfortunately in https://github.com/rust-analyzer/rust-analyzer/pull/7595 I forgot to `pub use` (rather than just `use`) the newly introduced `LoadCargoConfig`.
So this PR fixes this now.
It also:
- splits up `fn load_cargo` into a "workspace loading" and a "project loading" phase
- adds a `progress: &dyn Fn(String)` to allow third-parties to provide CLI progress updates, too
The motivation behind both of these is the fact that rust-analyzer currently does not support caching.
As such any third-party making use of `ra_ap_…` needs to providing a caching layer itself.
Unlike for rust-analyzer itself however a common use-pattern of third-parties is to analyze a specific target (`--lib`/`--bin <BIN>`/…) from a specific package (`--package`). The targets/packages of a crate can be obtained via `ProjectWorkspace::load(…)`, which currently is performed inside of `fn load_cargo`, effectively making the returned `ProjectWorkspace` inaccessible to the outer caller. With this information one can then provide early error handling via CLI (in case of ambiguities or invalid arguments, etc), instead of `fn load_cargo` failing with a possibly obscure error message. It also allows for annotating the persisted caches with its specific associated package/target selector and short-circuit quickly if a matching cache is found on disk, significantly cutting load times.
Before:
```rust
pub struct LoadCargoConfig {
pub cargo_config: &CargoConfig,
pub load_out_dirs_from_check: bool,
pub with_proc_macro: bool,
}
pub fn load_cargo(
root: &Path,
config: &LoadCargoConfig
) -> Result<(AnalysisHost, vfs::Vfs)> {
// ...
}
```
After:
```rust
pub fn load_workspace(
root: &Path,
config: &CargoConfig,
progress: &dyn Fn(String),
) -> Result<ProjectWorkspace> {
// ...
}
pub struct LoadCargoConfig {
pub load_out_dirs_from_check: bool,
pub with_proc_macro: bool,
}
pub fn load_cargo(
ws: ProjectWorkspace,
config: &LoadCargoConfig,
progress: &dyn Fn(String),
) -> Result<(AnalysisHost, vfs::Vfs)> {
// ...
}
```
Co-authored-by: Vincent Esche <regexident@gmail.com>
7657: utf8 r=matklad a=matklad
- Prepare for utf-8 offsets
- reduce code duplication in tests
- Make utf8 default, implement utf16 in terms of it
- Make it easy to add additional context for offset conversion
- Implement utf8 offsets
closes#7453
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
7687: Specialization for async traits r=matklad a=arnaudgolfouse
Fixes#7669.
Adapting the parser seemed to be all that was needed, but I am not very experienced with the codebase. Is this enough ?
Co-authored-by: Arnaud <arnaud.golfouse@laposte.net>
7620: Support control flow in `extract_function` assist r=matklad a=cpud36
Support `return`ing from outer function, `break`ing and `continue`ing outer loops when extracting function.
# Example
Transforms
```rust
fn foo() -> i32 {
let items = [1,2,3];
let mut sum = 0;
for &item in items {
<|>if item == 42 {
break;
}<|>
sum += item;
}
sum
}
```
Into
```rust
fn foo() -> i32 {
let items = [1,2,3];
let mut sum = 0;
for &item in items {
if fun_name(item) {
break;
}
sum += item;
}
sum
}
fn fun_name(item: i32) -> bool {
if item == 42 {
return true;
}
false
}
```
# Features
Supported variants
- break and function does not return => uses `bool` and plain if
- break and function does return => uses `Option<T>` and matches on it
- break with value and function does not return => uses `Option<T>` and if let
- break with value and function does return => uses `Result<T, U>` and matches on t
- same for `return` and `continue`(but we can't continue with value)
Assist does handle nested loops and nested items(like functions, modules, impls)
Try `expr?` operator is allowed together with `return Err(_)` and `return None`.
`return expr` is not allowed.
# Not supported
## Mixing `return` with `break` or `continue`
If we have e.g. a `return` and a `break` in the selected code, it is unclear what the produced code should look like.
We can try `Result<T, Option<U>>` or something like that, but it isn't idiomatic, nor it is established. Otherwise, implementation
is relatively simple.
## `break` with label
Not sure how to handle different labels for multiple `break`s.
[edit] implemented try `expr?`
Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
7661: Start LSP 3.17 support r=kjeremy a=kjeremy
Companion to https://github.com/gluon-lang/lsp-types/pull/199 which <strike>has not been merged yet</strike> has been merged.
This doesn't opt into any 3.17 functionality yet.
Co-authored-by: Jeremy Kolb <kjeremy@gmail.com>
7656: Implement constructor usage search for almost all items r=matklad a=Veykril
This PR moves the filering for enum constructors to the HIR, with this unprefixed variants as well as when the enum has been renamed via use will then still show up properly.
We now walk the ast of the `NameRef` up until we find a `PathExpr`(which also handles `CallExpr` for tuple-type structs and variants already) or a `RecordExpr`. For enum search we then take the `path` out of that expression and do a resolution on it to compare it with the definition enum.
With this PR we now support searching for all constructor literals, Unit-, Tuple- and Record-Structs, Unit-, Tuple- and Record-Variants as well as Unions.
There is one shortcoming due to how the search is triggered. Unit Variants constructors can't be searched as we have no position for it to kick off the search(since a comma doesn't have to exist for the last variant).
Closes#2549 though it doesn't implement it as outlined in the issue since the reference kind was removed recently, though I believe the approach taken here is better personally.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7643: Automatically detect the rustc-src directory (fixes#3517) r=matklad a=bnjbvr
If the configured rustcSource was not set, then try to automatically
detect a source for the sysroot rustc directory.
I wasn't sure how to do it in the case of the project.json file, though.
7663: Tolerate spaces in nix binary patching r=matklad a=CertainLach
If path to original file contains space (I.e on code insiders, where
default data directory is ~/Code - Insiders/), then there is syntax
error evaluating src arg.
Instead pass path as str, and coerce to path back in nix expression
Co-authored-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Yaroslav Bolyukin <iam@lach.pw>
7668: Finalize rename infra rewrite r=matklad a=Veykril
This should be the final PR in regards to rewriting rename stuff, #4290.
It addresses 3 things:
- Currently renaming import aliases causes some undesired behavior(see #5198) which is why this PR causes us to just return an error if an attempt at renaming an alias is made for the time being. Though this only prevents it from happening when the alias import is renamed, so its not too helpful.
- Fixes#6898
- If we are inside a macro file simply rename the input name node as there isn't really a way to do any of the fancy shorthand renames and similar things as for that we would have to exactly know what the macro generates and what not.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7664: refactor impl generation in assists r=Veykril a=jDomantas
Follow-up to #7659: all impl generation in assists (at least what I found) is now done through `utils::{generate_impl_text, generate_trait_impl_text}`.
Co-authored-by: Domantas Jadenkus <djadenkus@gmail.com>
7665: Don't classify attribute macros as their path unless it's a function with the proc_macro_attribute attribute r=Veykril a=Veykril
bors r+
Closes#6389
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
For all struct kinds, unions and enums, as well as for record- and
tuple-variants but not for unit-variants, as these have no trailing
character we can anchor the search to. Functionality wise it is
implemented though.
In some situations we reloaded the workspace in the tests after having reported
to be ready. There's two fixes here:
1. Add a version to the VFS config and include that version in progress reports,
so that we don't think we're done prematurely;
2. Delay status transitions until after changes are applied. Otherwise the last
change during loading can potentially trigger a workspace reload, if it contains
interesting changes.
7617: Add getter/setter assists r=Veykril a=yoshuawuyts
This patch makes progress towards the design outlined in https://github.com/rust-analyzer/rust-analyzer/issues/5943, and includes a small refactor which closes https://github.com/rust-analyzer/rust-analyzer/issues/7607. All together this patch does 4 things:
- Adds a `generate_getter` assist.
- Adds a `generate_getter_mut` assist.
- Adds a `generate_setter` assist.
- Moves the `generate_impl_text` function from `generate_new` into `utils` (which closes#7607).
## Design Notes
I've chosen to follow the [Rust API guidelines on getters](https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter) as closely as possible. This deliberately leaves "builder pattern"-style setters out of scope.
Also, similar to https://github.com/rust-analyzer/rust-analyzer/pull/7570 this assist generates doc comments. I think this should work well in most cases, and for the few where it doesn't it's probably easily edited. This makes it slightly less correct than the #7570 implementation, but I think this is still useful enough to include for many of the same reasons.
The reason why this PR contains 3 assists, rather than 1, is because each of them is so similar to the others that it felt more noisy to do them separately than all at once. The amount of code added does not necessarily reflect that, but hope that still makes sense.
## Examples
**Input**
```rust
struct Person {
name: String, // <- cursor on "name"
}
```
**generate getter**
```rust
struct Person {
name: String,
}
impl Person {
/// Get a reference to the person's name.
fn name(&self) -> &String {
&self.name
}
}
```
**generate mut getter**
```rust
struct Person {
name: String,
}
impl Person {
/// Get a mutable reference to the person's name.
fn name_mut(&mut self) -> &mut String {
&mut self.name
}
}
```
**generate setter**
```rust
struct Person {
name: String,
}
impl Person {
/// Set the person's name.
fn set_name(&mut self, name: String) {
self.name = name;
}
}
```
Co-authored-by: Yoshua Wuyts <yoshuawuyts+github@gmail.com>
7572: Add `find_or_create_impl_block` to assist utils r=matklad a=yoshuawuyts
This is another continuation of https://github.com/rust-analyzer/rust-analyzer/pull/7562, introducing a small util to either find an `impl` block, or create a new one if none exists. I copied this code from the `generate_new` assist into https://github.com/rust-analyzer/rust-analyzer/pull/7562, and this unifies both into a helper.
It doesn't feel super polished in its current state, but my hope is that this is enough of a starting point that it can be expanded on later. For example something that would be useful would be a flag which either returns the index of the start of the block, or the end of the block.
Anyway, I hope this is useful. Thanks!
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
7574: Remove various redundant clones r=kjeremy a=yoshuawuyts
I noticed when running clippy through RA that there are a few instances where `clone` is called where it's not actually needed. I figured a small patch to remove these might be welcome here.
Thanks!
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
7505: Widen Highlights root range to covering element r=Veykril a=Veykril
There have been a few issues about/containing spurious syntax highlighting panics, which all seem to come from the `rust_analyzer::handlers::handle_semantic_tokens_range` request, which I believe this to be the cause of as the text range we want to highlight here is currently potentially smaller than that of the covering element, so we might highlight something that is inside the covering element, but outside of the text range we wish to highlight causing the assert to fail.
Unfortunately this isn't really easy to test since I have yet to find a reproducible cause(#7504 doesn't work for me cause I can't seem to checkout the given commit).
See #7504, #7298, #7299 and #7416, all of those contain an assertion failure in syntax highlighting, but only in the range request.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7570: Add doc gen to the `generate_enum_match_method` assist r=yoshuawuyts a=yoshuawuyts
Implements a small extension to https://github.com/rust-analyzer/rust-analyzer/pull/7562, generating default comments. I wasn't sure if this would fit the goals of Rust-Analyzer, so I chose to split it into a separate PR. This is especially useful when writing code in a codebase which uses `#![warn(missing_docs)]` lint, as many production-grade libraries do.
The comments we're generating here are similar to the ones found on [`Option::is_some`](https://doc.rust-lang.org/std/option/enum.Option.html#method.is_some) and [`Result::is_err`](https://doc.rust-lang.org/std/result/enum.Result.html#method.is_err). I briefly considered only generating these for `pub` types, but they seem small and unobtrusive enough that they're probably useful in the general case. Thanks!
## Example
__input__
```rust
pub(crate) enum Variant {
Undefined,
Minor, // cursor here
Major,
}
```
__output__
```rust
pub(crate) enum Variant {
Undefined,
Minor,
Major,
}
impl Variant {
/// Returns `true` if the variant is [`Minor`].
pub(crate) fn is_minor(&self) -> bool {
matches!(self, Self::Minor)
}
}
```
## Future Directions
This opens up the path to adding an assist for generating these comments on existing `is_` methods. This would make it both easy to document new code, and update existing code with documentation.
7571: Cleanup decl_check r=Veykril a=Veykril
bors r+
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7562: add `generate_enum_match` assist r=matklad a=yoshuawuyts
This adds a `generate_enum_match` assist, which generates `is_` variants for enums (e.g. `Option::{is_none,is_some}` in std). This is my first attempt at contributing to Rust-Analyzer, so I'm not sure if I've gotten everything right. Thanks!
## Example
**Input**
```rust
pub(crate) enum Variant {
Undefined,
Minor, // cursor here
Major,
}
```
**Output**
```rust
pub(crate) enum Variant {
Undefined,
Minor,
Major,
}
impl Variant {
pub(crate) fn is_minor(&self) -> bool {
matches!(self, Self::Minor)
}
}
```
## Future Directions
I made this as a stepping stone for some of the more involved refactors (e.g. #5944). I'm not sure yet how to create, use, and test `window.showQuickPick`-based asssists in RA. But once that's possible, it'd probably be nice to be able to generate match methods in bulk through the quickpick UI rather than one-by-one:
```
[x] Select enum members to generate methods for. (3 selected) [ OK ]
---------------------------------------------------------------------------
[x] Undefined
[x] Minor
[x] Major
```
Co-authored-by: Yoshua Wuyts <yoshuawuyts+github@gmail.com>
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
7568: Fix merging of `segment_index` in path resolution r=jonas-schievink a=jonas-schievink
This caused associated item lookup to fail when modifying `resolver.rs` to handle block expressions with inner items.
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7535: Extract function assist r=cpud36 a=cpud36
This PR adds `extract function/method` assist. closes#5409.
# Supported features
Assist should support extracting from expressions(`1`, `2 + 2`, `loop { }`) and from a series of statements, e.g.:
```rust
foo();
$0bar();
baz();$0
quix();
```
Assist also supports extracting parameters, like:
```rust
fn foo() -> i32 {
let n = 1;
$0n + 1$0
}
// -
fn foo() -> i32 {
let n = 1;
fun_name(n)
}
fn fun_name(n: i32) -> i32 {
n + 1
}
```
Extracting methods also generally works.
Assist allows referencing outer variables, both mutably and immutably, and handles handles access to variables local to extracted function:
```rust
fn foo() {
let mut n = 1;
let mut m = 2;
let mut moved_v = Vec::new();
let mut ref_mut_v = Vec::new();
$0
n += 1;
let k = 1;
moved_v.push(n);
let r = &mut m;
ref_mut_v.push(*r);
let h = 3;
$0
n = ref_mut_v.len() + k;
n -= h + m;
}
// -
fn foo() {
let mut n = 1;
let mut m = 2;
let mut moved_v = Vec::new();
let mut ref_mut_v = Vec::new();
let (k, h) = fun_name(&mut n, moved_v, &mut m, &mut ref_mut_v);
n = ref_mut_v.len() + k;
n -= h + m;
}
fn fun_name(n: &mut i32, mut moved_v: Vec<i32>, m: &mut i32, ref_mut_v: &mut Vec<i32>) -> (i32, i32) {
*n += 1;
let k = 1;
moved_v.push(*n);
let r = m;
ref_mut_v.push(*r);
let h = 3;
(k, h)
}
```
So we handle both input and output paramters
# Showcase
# Working with non-`Copy` types
Consider the following example:
```rust
fn foo() {
let v = Vec::new();
$0
let n = v.len();
$0
let is_empty = v.is_empty();
}
```
`v` must be a parameter to extracted function.
The question is, what type should it have.
It could be `v: Vec<i32>`, or `v: &Vec<i32>`.
The former is incorrect for `Vec<i32>`, but the later is silly for `i32`.
To resolve this we need to know if the type implements `Copy` trait.
I didn't find any api available from assists to query this.
`hir_ty::method_resolution::implements` seems relevant, but is isn't publicly re-exported from `hir`.
# Star(`*`) token and pointer dereference
If I understand correctly, in order to create expression like `*p`, one should use `ast::make::expr_prefix(T![*], ...)`, which
in turn calls `token(T![*])`.
`token` does not have star in `tokens::SOURCE_FILE`, so this panics.
I had to add `*` to `SOURCE_FILE` to make it work.
Correct me if this is not intended way to do this.
# Lowering access `value -> mut ref -> shared ref`
Consider the following example:
```rust
fn foo() {
let v = Vec::new();
$0 let n = v.len(); $0
}
```
`v` is not used after extracted function body, so both `v: &Vec<i32>` and `v: Vec<i32>` would work.
Currently the later would be chosen.
We can however check the body of extracted function and conclude that `v: &Vec<i32>` is sufficient.
Using `v: &Vec<i32>`(that is a minimal required access level) might be a better default.
I am unsure.
# Cleanup
The assist seems to be reasonably handling most of common cases.
If there are no concerns with code it produces(i.e. with test cases), I will start cleaning up
[edit]
added showcase
Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
7541: Use block_def_map in body lowering (third time's the charm) r=jonas-schievink a=jonas-schievink
After https://github.com/rust-analyzer/rust-analyzer/pull/7380 and https://github.com/rust-analyzer/rust-analyzer/pull/7506 both had to be reverted, this should have finally resolved all remaining bugs.
Most importantly, the optimization to skip `block_def_map` computation when the block contains no inner items was fixed (which fortunately was simpler than expected).
I've ran `analysis-stats` on libstd locally, which works fine, and also ran this PR locally for a short while without issues.
Note that this *still* has no (or almost no) user-facing impact, because the rest of r-a still relies on some local item support hacks.
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
there are a few currently limitations:
* no modifications of function body
* does not handle mutability and references
* no method support
* may produce incorrect results
I've noticed that there are various suggestions that rust-analyzer seems
to filter out, even if they make sense.
Here's an example of where it seems like there should be a suggestion,
but there isn't:
It turns out that this specific suggestion is not considered
`MachineApplicable`, which are the only suggestions that rust-analyzer
accepts. However if you read the documentation for `MachineApplicable`,
b3897e3d13/compiler/rustc_lint_defs/src/lib.rs (L27-L29)
then you realize that these are specifically only those suggestions that
rust-analyzer could even automatically apply (in some distant future,
behind some setting or so). Other suggestions that may have some
semantic impact do not use `MachineApplicable`. So all other suggestions
are still intended to be suggested to the user, just not automatically
applied without the user being consulted.
b3897e3d13/compiler/rustc_lint_defs/src/lib.rs (L22-L24)
So with that in mind, rust-analyzer should almost definitely not filter
out `MaybeIncorrect` (which honestly is named horribly, it just means
that it's a semantic change, not just a syntactical one).
Then there's `HasPlaceholders` which basically is just another semantic
one, but with placeholders. The user will have to make some adjustments,
but the suggestion still is perfectly valid. rust-analyzer could
probably detect those placeholders and put proper "tab through" markers
there for the IDE, but that's not necessary for now.
Then the last one is `Unspecified` which is so unknown that I don't even
know how to judge it, meaning that the suggestion should probably also
just be suggested to the user and then they can decide.
So with all that in mind, I'm proposing to get rid of the check for
Applicability entirely.
