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.
7465: Only hide parameter hints for path, field and methodcall expressions r=SomeoneToIgnore a=Veykril
Doing this check for other expressions makes little sense to me.
Fixes#7458
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7457: Add no-buffering file logging and wait for a debugger option. r=vsrs a=vsrs
Adds two command line flags: `--no-buffering` and `--wait-dbg`.
Not sure if someone else needs this, but personally I found both flags extremely useful trying to figure out why RA does not work with Visual Studio. Or better to say why Visual Studio does not work with RA.
Co-authored-by: vsrs <vit@conrlab.com>
7433: Support Macro v2 in mbe r=jonas-schievink a=edwin0cheng
Added `mbe::MacroDef` for Macro v2.
cc @jonas-schievink
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7426: Create all `ModuleId`s through a `DefMap` method r=jonas-schievink a=jonas-schievink
`ModuleId` needs to be able to represent blocks, and only the
associated `DefMap` will know how to construct that `ModuleId`
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7399: Make `ModuleId`'s `krate` field private and audit uses r=jonas-schievink a=jonas-schievink
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
Detail should be rendered as shtort one line, just dumping fn header
there is not useful, despite the fact that TS does this.
The fact that this is a function should be indicated by the icon, the
same goes for pub/const/async etc qualitfiers
name is already present in the lable (and arg list should be a part of
that, as in idea)
But the return type is the small genuinerlly useful bit of info we can show here
7310: Add assist: add lifetime to type r=matklad a=bnjjj
close#7200
7395: Obtain `ModuleId`'s `DefMap` through a method r=jonas-schievink a=jonas-schievink
Once `ModuleId` can represent modules inside block expressions, this will be required in order to obtain the correct `DefMap`. Otherwise existing code might use a `LocalModuleId` from the wrong `DefMap` and cause a panic.
bors r+
Co-authored-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7353: Add LifetimeParam and ConstParam to CompletionItemKind r=matklad a=Veykril
Adds `LifetimeParam` and `ConstParam` to `CompletionItemKind` and maps them both to `TypeParam` in the protocol conversion as there are no equivalents, so nothing really changes there.
`ConstParam` could be mapped to `Const` I guess but I'm split on whether that would be better?
Additions were solely inspired by (the single) test output for const params.
Also sorts the variants of `CompletionItemKind` and its to_proto match.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7386: Handle box with allocator r=flodiebold a=tweksteen
In 1.49.0, the definition of Box was modified to support an optional
Allocator[1]. Adapt the parsing of the `box` keyword to supply the
expected number of parameters to the constructor.
[1] f288cd2e17Fixes: #6956
Co-authored-by: Thiébaud Weksteen <tweek@google.com>
In 1.49.0, the definition of Box was modified to support an optional
Allocator[1]. Adapt the parsing of the `box` keyword to supply the
expected number of parameters to the constructor.
[1] f288cd2e17
7348: Fix broken link in intra-doc r=edwin0cheng a=sasurau4
Fix#5546
The intra-doc link doesn't work fine for trait associated method.
Co-authored-by: Daiki Ihara <sasurau4@gmail.com>
7387: Refactor build script specific data r=edwin0cheng a=edwin0cheng
It refactors for separating logic for build script specific data.
bors r+
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7375: Add support for running name resolution in block expressions r=jonas-schievink a=jonas-schievink
This adds a `block_def_map` query that runs the name resolution algorithm on a block expression, and returns a `DefMap` that stores links to the parent `DefMap` (either the containing block or the crate-level `DefMap`). Blocks with no inner items return the parent's `DefMap` as-is, to avoid creating unnecessarily long `DefMap` chains.
Path resolution is updated to recurse into the parent `DefMap` after looking up a path in the original `DefMap`.
I've added a few new tests for this, but outside of those this isn't used yet.
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7359: ItemTree: store a mapping from blocks to inner items r=jonas-schievink a=jonas-schievink
To do name resolution within block expressions, we need to know which inner items are located inside each block expression. This adds such a mapping to `ItemTree`, replacing the previous one, which was seemingly unused other than to access all the inner items.
This also assigns `AstId`s to block expressions, which is needed to store the mapping in salsa.
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7341: Fix warnings when running `cargo doc --document-private-items` r=Veykril a=jyn514
These were the warnings previously:
<details>
```
warning: could not parse code block as Rust code
--> crates/stdx/src/lib.rs:137:9
|
137 | /// ∀ x in slice[..idx]: pred(x)
| _________^
138 | | /// && ∀ x in slice[idx..]: !pred(x)
| |____^
|
= note: error from rustc: unknown start of token: \u{2200}
warning: 1 warning emitted
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= note: `#[warn(broken_intra_doc_links)]` on by default
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: 2 warnings emitted
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= note: `#[warn(broken_intra_doc_links)]` on by default
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: 2 warnings emitted
```
</details>
This does *not* fix the following warning, because it is actually rust
code and rustdoc is being over eager:
```
warning: Rust code block is empty
--> crates/parser/src/grammar.rs:16:5
|
16 | //! ```
| _____^
17 | | //! // test function_with_zero_parameters
18 | | //! // fn foo() {}
19 | | //! ```
| |_______^
|
help: mark blocks that do not contain Rust code as text
|
16 | //! ```text
| ^^^^^^^
```
https://github.com/rust-lang/rust/pull/79816 should make this
configurable so the warning can be `allow`ed.
Co-authored-by: Joshua Nelson <jyn514@gmail.com>
These were the warnings previously:
```
warning: could not parse code block as Rust code
--> crates/stdx/src/lib.rs:137:9
|
137 | /// ∀ x in slice[..idx]: pred(x)
| _________^
138 | | /// && ∀ x in slice[idx..]: !pred(x)
| |____^
|
= note: error from rustc: unknown start of token: \u{2200}
warning: 1 warning emitted
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= note: `#[warn(broken_intra_doc_links)]` on by default
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: 2 warnings emitted
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= note: `#[warn(broken_intra_doc_links)]` on by default
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: unresolved link to `package`
--> crates/base_db/src/input.rs:181:15
|
181 | /// it's [package].name, can be different for other project types or even
| ^^^^^^^ no item named `package` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
warning: 2 warnings emitted
```
This does *not* fix the following warning, because it is actually rust
code and rustdoc is being over eager:
```
warning: Rust code block is empty
--> crates/parser/src/grammar.rs:16:5
|
16 | //! ```
| _____^
17 | | //! // test function_with_zero_parameters
18 | | //! // fn foo() {}
19 | | //! ```
| |_______^
|
help: mark blocks that do not contain Rust code as text
|
16 | //! ```text
| ^^^^^^^
```
https://github.com/rust-lang/rust/pull/79816 should make this
configurable so the warning can be `allow`ed.
7340: Remove obsolete RangeInfo usage in references r=Veykril a=Veykril
Didn't even realize these were only here for renaming as well!
bors r+
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7336: Rename `CrateDefMap` to `DefMap` r=matklad a=jonas-schievink
I propose handling local items by computing a `DefMap` for every block expression, using the regular (early) name resolution algorithm. The result of that will be a `DefMap` that has a reference to the parent `DefMap`, which is either the one computed for the containing block expression, or the crate's root `DefMap`. Name resolution will fall back to a name in the parent `DefMap` if it cannot be resolved in the inner block.
The `DefMap`s computed for block expressions will go through a separate query that can be garbage-collected much more aggressively, since these `DefMap`s should be cheap to compute and are never part of a crate's public API.
The first step towards that is to make `CrateDefMap` not specific to crates anymore, hence this rename (if this plans sounds reasonable).
cc https://github.com/rust-analyzer/rust-analyzer/issues/7325 and https://github.com/rust-analyzer/rust-analyzer/issues/1165
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
7339: Delete optional arg for inline attr and add doc alias attr completion r=lnicola a=bnjjj
close#7167
Co-authored-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
