11190: fix(completions): improve fn_param r=dbofmmbt a=dbofmmbt
- insert commas around when necessary
- only suggest `self` completions when param list is empty
- stop suggesting completions for identifiers which are already on the param list
Closes#11085
Co-authored-by: Eduardo Canellas <eduardocanellas98@gmail.com>
- insert commas around when necessary
- only suggest `self` completions when param list is empty
- stop suggesting completions for identifiers which are already on the param list
11184: Correctly pass through mutable parameter references when extracting a function r=Veykril a=Vannevelj
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10277
I have based this investigation based on my understanding of [the Borrowing chapter](https://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html) but I wasn't able to debug the test runs or see it in action in an IDE. I'll try to figure out how to do that for future PRs but for now, the tests seem to confirm my understanding. I'll lay out my hypothesis below.
Here we define the parameters for the to-be-generated function:
7409880a07/crates/ide_assists/src/handlers/extract_function.rs (L882)
Three values in particular are important here: `requires_mut`, `is_copy` and `move_local`. These will in turn be used here to determine the kind of parameter:
7409880a07/crates/ide_assists/src/handlers/extract_function.rs (L374-L381)
and then here to determine what transformation is needed for the calling argument:
7409880a07/crates/ide_assists/src/handlers/extract_function.rs (L383-L390)
which then gets transformed here:
7409880a07/crates/syntax/src/ast/make.rs (L381-L383)
What I believe is happening is that
* `requires_mut` is `false` (it already is marked as mutable),
* `is_copy` is `false` (`Foo` does not implement `Copy`), and
* `move_local` is `false` (it has further usages)
According to the pattern matching in `fn kind()`, that would lead to `ParamKind::SharedRef` which in turn applies a transformation that prepends `&`.
However if I look at the chapter on borrowing then we only need to mark an argument as a reference if we actually own it. In this case the value is passed through as a reference parameter into the current function which means we never had ownership in the first place. By including the additional check for a reference parameter, `move_local` now becomes `true` and the resulting parameter is now `ParamKind::Value` which will avoid applying any transformations. This was further obscured by the fact that you need further usages of the variable or `move_local` would be considered `true` after all.
I didn't follow it in depth but it appears this idea applies for both the generated argument and the generated parameter.
There are existing tests that account for `&mut` values but they refer to local variables for which we do have ownership and as such they didn't expose this issue.
Co-authored-by: Jeroen Vannevel <jer_vannevel@outlook.com>
11061: Support "move if to guard" for if else chains r=weirane a=weirane
The idea is to first parse the if else chain into a vector of `(Condition, BlockExpr)`s until we reach an iflet branch, an else branch, or the end (the tail). Then add the match arms with guard for the vector, and add the tail with no if guard.
Because the whole original match arm is replaced and the generated code doesn't have redundent commas, I removed redundent commas in some test cases.
Closes#11033.
Co-authored-by: Wang Ruochen <wrc@ruo-chen.wang>
11088: closes#10446 hide type inlay hints r=Veykril a=Heinenen
Passes tests as described in #10446
Works for all happy cases, there may be some cases that I forgot as I am not that familiar with Rust and r-a (yet).
Co-authored-by: Heinenen <th.m.heinen@gmail.com>
11173: Allow adding partially resolved types r=Veykril a=SomeoneToIgnore
Sometimes when writing something like `let foo = Arc::new(Mutex::new(CrazyGenerics::new(HashMap::new())))`, I want/have to specify an explicit type for the expression.
Using turbofish isn't very readable and not always appreciated by guidelines, so `let foo: T` has to be filled.
To ease that, the PR enables the `add_explicit_type` assist on types that contain unknown types and some generics.
Fully unresolved types, arrays with unknown types and other known cases behave the same.
`_` placeholder was chosen to replace an unknown type:
```rust
let foo = HashMap::new();
// after assist usage, turns into
let foo: HashMap<_, _> = HashMap::new();
```
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
11169: internal: Handle macro calls better in highlighting r=Veykril a=Veykril
Introduces a new semantic highlighting tag for the `!` character of macro calls.
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10962
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
11136: Turbo fish assist supports multiple type arguments r=matklad a=Vannevelj
This fixes#11135 (changelog: bug).
I've only started using Rust a few days ago but saw this issue on the top of the list when I looked at this repo. I based myself on [this blog post](https://techblog.tonsser.com/posts/what-is-rusts-turbofish) to understand what a "turbo fish" is so let me know if I missed anything.
Co-authored-by: Jeroen Vannevel <jer_vannevel@outlook.com>
11151: feat: correctly fallback to notify if the clinet-side file watching is not supported r=matklad a=matklad
bors r+
🤖
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
The direct reason for this is to fix CI on windows, which seems to fail
for some reason after we fixed the watcher-selection logic which (I
think) changed the tests behavior to use notify rather than client.
But this patch seems to make sense in general -- file watching is
notoriously finicky, so controlling it explicitly leads to less fragile
tests.
This has to re-introduce the `sink` pattern, because doing this purely
with iterators is awkward :( Maaaybe the event vector was a false start?
But, anyway, I like the current factoring more -- it sort-of obvious
that we do want to keep ws-attachment business in the parser, and that
we also don't want that to depend on the particular tree structure. I
think `shortcuts` module achieves that.
The general theme of this is to make parser a better independent
library.
The specific thing we do here is replacing callback based TreeSink with
a data structure. That is, rather than calling user-provided tree
construction methods, the parser now spits out a very bare-bones tree,
effectively a log of a DFS traversal.
This makes the parser usable without any *specifc* tree sink, and allows
us to, eg, move tests into this crate.
Now, it's also true that this is a distinction without a difference, as
the old and the new interface are equivalent in expressiveness. Still,
this new thing seems somewhat simpler. But yeah, I admit I don't have a
suuper strong motivation here, just a hunch that this is better.
