8221: Prefer adding `mod` declaration to lib.rs over file.rs in UnlinkedFile fix r=Veykril a=Veykril
When there is a `lib.rs` and `main.rs` in one crate, one usually wants the `lib.rs` file to declare the modules.
bors r+
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8201: Fix recursive macro statements expansion r=edwin0cheng a=edwin0cheng
This PR attempts to properly handle macro statement expansion by implementing the following:
1. Merge macro expanded statements to parent scope statements.
2. Add a new hir `Expr::MacroStmts` for handle tail expression infer.
PS : The scope of macro expanded statements are so strange that it took more time than I thought to understand and implement it :(
Fixes #8171
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7907: Autoderef with visibility r=cynecx a=cynecx
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/7841.
I am not sure about the general approach here. Right now this simply tries to check whether the autoderef candidate is reachable from the current module. ~~However this doesn't exactly work with traits (see the `tests::macros::infer_derive_clone_in_core` test, which fails right now).~~ see comment below
Refs:
- `rustc_typeck` checking fields: 66ec64ccf3/compiler/rustc_typeck/src/check/expr.rs (L1610)
r? @flodiebold
Co-authored-by: cynecx <me@cynecx.net>
8190: Fix chalk_ir assertion r=flodiebold a=flodiebold
Fixes#8150.
I implemented a validator that catches this in the tests, but it'd need to get merged in Chalk first.
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
8183: Fix missing command error with macros r=Veykril a=brandondong
**Reproduction:**
1. Define a struct through a macro (can be via `macro_rules`, proc macro, or `include!()`).
2. !!MISSING: command!! annotation appears. Clicking on it results in an error message. No matter where the macro is called/defined, the annotation is always at the start of the file.
**Cause:**
- For struct `A`, a `HasImpls` annotation is added just like for struct `B`. Unlike `B`, the file id for `A` is not the file we are adding annotations to but a macro file.
- The resolving step of the code lens does not succeed.
**Fix:**
- Check that the files match before computing offsets and adding `HasImpls`/`HasReferences` annotations.
Co-authored-by: Brandon <brandondong604@hotmail.com>
8162: Compute more mathematically well-rounded notion of transitive deps r=Veykril a=matklad
By including the crate itself, we make the resulting set closed with
respect to `transitve_reveres_dependencies` operation, as it becomes a
proper transitive closure. This just feels more proper and mathy.
And, indeed, this actually allows us to simplify call sites somewhat.
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
By including the crate itself, we make the resulting set closed with
respect to `transitve_reveres_dependencies` operation, as it becomes a
proper transitive closure. This just feels more proper and mathy.
And, indeed, this actually allows us to simplify call sites somewhat.
8142: temp disable broken ref match completions for struct fields/methods r=matklad a=JoshMcguigan
This PR implements a temporary workaround for #8058 by disabling ref match completions for struct fields and methods. Disabling this doesn't break any existing functionality (that I am aware of) since these completions were broken.
I plan to keep working on a real fix for the underlying issue here, but I think a proper fix could take some time, so I'd prefer to quickly fix the bug to buy some more time to implement a better solution (which would ultimately allow re-enabling ref matches for struct fields and methods).
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
8154: rewrite merge use trees assist to use muatable syntax trees r=matklad a=matklad
bors r+
🤖
8155: Fix confusion between parameters and the function r=jonas-schievink a=jonas-schievink
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8152
bors r+
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
8136: Introduce QuantifiedWhereClause and DynTy analogous to Chalk r=flodiebold a=flodiebold
This introduces a bunch of new binders in lots of places, which we have to be careful about, but we had to add them at some point. There's a lot of skipping of the binders; once we're done with the Chalk move, we should review the remaining ones.
8146: Document patch policy r=matklad a=matklad
bors r+
🤖
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This in particular means storing a chalk_ir::Environment, not our
TraitEnvironment. This makes InEnvironment not usable for Type, where we
need to keep the full TraitEnvironment.
8134: Correct the paths of submodules from the include! macro r=edwin0cheng a=sticnarf
This PR should fix#7846. It mostly follows the instructions from @edwin0cheng in that issue.
Co-authored-by: Yilin Chen <sticnarf@gmail.com>
8133: Ignore type bindings in generic_predicates_for_param (fix panic on ena and crates depending on it) r=flodiebold a=flodiebold
This allows us to handle more cases without a query cycle, which includes certain cases that rustc accepted. That in turn means we avoid triggering salsa-rs/salsa#257 on valid code (it will still happen if the user writes an actual cycle).
We actually accept more definitions than rustc now; that's because rustc only ignores bindings when looking up super traits, whereas we now also ignore them when looking for predicates to disambiguate associated type shorthand. We could introduce a separate query for super traits if necessary, but for now I think this should be fine.
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
8132: Add `'` to trigger_characters, allowing more direct lifetime completions r=Veykril a=Veykril
Fixes having to type a character after `'` to complete lifetimes and labels
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This allows us to handle more cases without a query cycle, which
includes certain cases that rustc accepted. That in turn means we avoid
triggering salsa-rs/salsa#257 on valid code (it will still happen if the
user writes an actual cycle).
We actually accept more definitions than rustc now; that's because rustc
only ignores bindings when looking up super traits, whereas we now also
ignore them when looking for predicates to disambiguate associated type
shorthand. We could introduce a separate query for super traits if
necessary, but for now I think this should be fine.
8122: Make bare underscore token an Ident rather than Punct in proc-macro r=edwin0cheng a=kevinmehall
In rustc and proc-macro2, a bare `_` token is parsed for procedural macro purposes as `Ident` rather than `Punct` (see https://github.com/rust-lang/rust/pull/48842). This changes rust-analyzer to match rustc's behavior and implementation by handling `_` as an Ident in token trees, but explicitly preventing `$x:ident` from matching it in MBE.
proc macro crate:
```rust
#[proc_macro]
pub fn input(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
dbg!(input)
}
```
test crate:
```rust
test_proc_macro::input!(_);
```
output (rustc):
```rust
[test-proc-macro/src/lib.rs:10] input = TokenStream [
Ident {
ident: "_",
span: #0 bytes(173..174),
},
]
```
output (rust-analyzer before this change):
```rust
[test-proc-macro/src/lib.rs:10] input = TokenStream [
Punct {
ch: '_',
spacing: Joint,
span: 4294967295,
},
]
```
output (rust-analyzer after this change):
```rust
[test-proc-macro/src/lib.rs:10] input = TokenStream [
Ident {
ident: "_",
span: 4294967295,
},
]
```
Co-authored-by: Kevin Mehall <km@kevinmehall.net>
8124: Add basic lifetime completion r=Veykril a=Veykril
This adds basic lifetime completion, basic in the sense that the completions for lifetimes are only shown when the user enters `'` followed by a char. Showing them when nothing is entered is kind of a pain, as we would want them to only show up where they are useful which in turn requires a lot of tree traversal and cursor position checking to verify whether the position is valid for a lifetime. This in itself doesn't seem too bad as usually when you know you want to write a lifetime putting `'` to ask for lifetime completions seems fine.
~~I'll take a look at whether its possible to lift the restriction of having to put a char after `'`.~~ This actually already works so I guess this is the clients responsibility, in which case VSCode doesn't like it.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8083: Track source file IDs in source mapping of Attrs r=jonas-schievink a=Veykril
Fixes the panics/incorrect injection highlighting of outline module declarations until we figure out a nicer source mapping strategy for attributes.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8065: Better handling of block doc comments r=Veykril a=Veykril
Moves doc string processing to `Attrs::docs`, as we need the indent info from all comments before being able to know how much to strip
Closes#7774
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8063: couple clippy::complexity fixes r=matklad a=matthiaskrgr
avoid redundant `.into()` calls to convert T into identical T (`let x: String = String::from("hello").into();`)
use `if let Some(x)` instead of `.is_some()` + `.unwrap()`
don't clone Copy types
remove redundant wrapped ?s: `Some(Some(3)?)` can just be `Some(3)`
use `.map(|x| y)` instead of `and_then(|x| Some(y)` on `Option`s
Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
8052: minor style fixes per feedback on #8036 r=JoshMcguigan a=JoshMcguigan
cc @matklad - this PR addresses your comments in #8036.
changelog fixup #8036
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
7900: show function params in completion detail r=matklad a=JoshMcguigan
This resolves#7842 by updating the detail for function completions from `-> T` to `fn(T, U) -> V`. I added an expicit unit test for this, `ide_completion::render::fn_detail_includes_args_and_return_type`, which passes.
Lots of other unit tests fail (~60 of them) due to this change, although I believe the failures are purely cosmetic (they were testing the exact format of this output). I'm happy to go update those tests, but before I do that I'd like to make sure this is in fact the format we want for the detail?
edit - I realized `UPDATE_EXPECT=1 cargo test` automatically updates `expect!` tests. Big 👍 to whoever worked on that! So I'll go ahead and update all these tests soon. But I still would like to confirm `fn(T, U) -> V` is the desired content in the `detail` field.
8000: Use hir formatter for hover text r=matklad a=oxalica
Fix#2765 , (should) fix#4665
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
Co-authored-by: oxalica <oxalicc@pm.me>
8036: completions: provide relevance bonus for enum types, and suggest ref matches for fn and enum r=matklad a=JoshMcguigan
This PR makes several improvements to completions and completion sorting:
1. Provide exact match type relevance score bonus for enum variants
2. Suggest `&Foo` (ref_match) for enums if that is an exact type match
3. Suggest `&foo()` (ref_match) if `foo` returns a type which would be an exact match either with the reference or due to a `Deref` impl
### Before
### After
### Caveats
I think generic types will require some kind of fancier logic when testing for `exact_type_match`, so for now `Option`/`Result`/etc unfortunately still don't have great completions.
### Implementation
I implemented this in a way that I think makes it most likely for each completion type to be handled consistently. Just replace `CompletionItem::new` with `CompletionItem::new_with_type_info` and `exact_type_match`/`exact_name_match`/`ref_match` are all handled for you, in a way which is sure to be consistent across completion types.
This approach does introduce some coupling/plumbing that didn't exist before. Notice for example `set_is_local` on the builder, because `set_relevance` was removed from the builder to enforce that the relevance was built "properly" with `CompletionItem::new_with_type_info`. But I think there are benefits to this approach, like `CompletionRelevance` should probably consider deprecation status, and we already tell the builder about that, so in the (likely near term) future we can just pass that information along to `CompletionRelevance` when the user calls `set_deprecated` rather than the user having to manually set it in two places. This basically just hides `CompletionRelevance` from the individual completions, so they only worry about the `CompletionItem` interface. At the moment this seems like a cleaner approach to me, but I'm open to feedback here.
edit - I've reimplemented this in a simpler way, per feedback below.
8046: Prefer match to if let else r=matklad a=matklad
bors r+
🤖
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
7970: Fix incorrect diagnostics for failing built in macros r=jonas-schievink a=brandondong
**Reproduction:**
1. Use a built in macro in such a way that rust-analyzer fails to expand it. For example:
**lib.rs**
```
include!("<valid file but without a .rs extension so it is not indexed by rust-analyzer>");
```
2. rust-analyzer highlights the macro call and says the macro itself cannot be resolved even though include! is in the standard library (unresolved-macro-call diagnostic).
3. No macro-error diagnostic is raised.
**Root cause for incorrect unresolved-macro-call diagnostic:**
1. collector:collect_macro_call is able to resolve include! in legacy scope but the expansion fails. Therefore, it's pushed into unexpanded_macros to be retried with module scope.
2. include! fails at the resolution step in collector:resolve_macros now that it's using module scope. Therefore, it's retained in unexpanded_macros.
3. Finally, collector:finish tries resolving the remaining unexpanded macros but only with module scope. include! again fails at the resolution step so a diagnostic is created.
**Root cause for missing macro-error diagnostic:**
1. In collector:resolve_macros, directive.legacy is None since eager expansion failed in collector:collect_macro_call. The macro_call_as_call_id fails to resolve since we're retrying in module scope. Therefore, collect_macro_expansion is not called for the macro and no macro-error diagnostic is generated.
**Fix:**
- In collector:collect_macro_call, do not add failing built-in macros to the unexpanded_macros list and immediately raise the macro-error diagnostic. This is in contrast to lazy macros which are resolved in collector::resolve_macros and later expanded in collect_macro_expansion where a macro-error diagnostic may be raised.
Co-authored-by: Brandon <brandondong604@hotmail.com>
Co-authored-by: brandondong <brandondong604@hotmail.com>
8037: Assist is empty 7709 r=Veykril a=chetankhilosiya
Updated the implementation to get the function from implementation
Co-authored-by: Chetan Khilosiya <chetan.khilosiya@gmail.com>
8020: Power up goto_implementation r=matklad a=Veykril
by allowing it to be invoked on references of names, now showing all (trait)
implementations of the given type in all crates instead of just the defining
crate as well as including support for builtin types
Example screenshot of `impl`s of Box in `log`, `alloc`, `std` and the current crate. Before you had to invoke it on the definition where it would only show the `impls` in `alloc`.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8027: Completion context remove exact match method in favor of fields r=JoshMcguigan a=JoshMcguigan
This is a minor cleanup PR following #8008. It removes the `expected_name_and_type` method on completion context in favor of using the fields.
I thought this method was used in more places, or else it may have just made sense to make this change directly in #8008🤷
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
8008: Completion context expected type r=matklad a=JoshMcguigan
Currently there are two ways completions use to determine the expected type. There is the `expected_type` field on the `CompletionContext`, as well as the `expected_name_and_type` method on the `RenderContext`. These two things returned slightly different results, and their results were only valid if you had pre-checked some (undocumented) invariants. A simple combination of the two approaches doesn't work because they are both too willing to go far up the syntax tree to find something that fits what they are looking for.
This PR makes the following changes:
1. Updates the algorithm that sets `expected_type` on `CompletionContext`
2. Adds `expected_name` field to `CompletionContext`
3. Re-writes the `expected_name_and_type` method to simply return the underlying fields from `CompletionContext` (I'd like to save actually removing this method for a follow up PR just to keep the scope of the changes down)
4. Adds unit tests for the `expected_type`/`expected_name` fields
All the existing unit tests still pass (unmodified), but this new algorithm certainly has some gaps (although I believe all the `FIXME` introduced in this PR are also flaws in the current code). I wanted to stop here and get some feedback though - is this approach fundamentally sound?
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
8018: Make Ty wrap TyKind in an Arc r=flodiebold a=flodiebold
... to further move towards Chalk.
This is a bit of a slowdown (218ginstr vs 213ginstr for inference on RA), even though it allows us to unwrap the Substs in `TyKind::Ref` etc..
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
What happens here is that we lower `: ` to a missing expression, and
then correctly record that the corresponding field expression resolves
to a specific field. Where we fail is in the mapping of syntax to this
missing expression. Doing it via `ast_field.expr()` fails, as that
expression is `None`. Instead, we go in the opposite direcition and ask
each lowered field about its source.
This works, but has wrong complexity `O(N)` and, really, the
implementation is just too complex. We need some better management of
data here.
8021: Enable searching for builtin types r=matklad a=Veykril
Not too sure how useful this is for reference search overall, but for completeness sake it should be there
Also enables document highlighting for them.
8022: some clippy::performance fixes r=matklad a=matthiaskrgr
use vec![] instead of Vec::new() + push()
avoid redundant clones
use chars instead of &str for single char patterns in ends_with() and starts_with()
allocate some Vecs with capacity to avoid unnecessary resizing
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
use vec![] instead of Vec::new() + push()
avoid redundant clones
use chars instead of &str for single char patterns in ends_with() and starts_with()
allocate some Vecs with capacity to avoid unneccessary resizing
7966: Diagnose files that aren't in the module tree r=jonas-schievink a=jonas-schievink
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/6377
I'm not sure if this is the best way to do this. It will cause false positives for all `include!`d files (though I'm not sure how much IDE functionality we have for these).
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
... like it will be in Chalk. We still keep `interned_mut` and
`into_inner` methods that will probably not exist with Chalk.
This worsens performance slightly (5ginstr inference on RA), but doesn't
include other simplifications we can do yet.
8014: increase completion relevance for items in local scope r=matklad a=JoshMcguigan
This PR provides a small completion relevance score bonus for items in local scope. The changes here are relatively minimal, since `coc` by default pre-sorts by position in file. But as we move toward fully server side sorting #7935 I think we'll want some relevance score bump for items in local scope.
### Before
Note `let~` and `syntax` are both ahead of locals. Ultimately we may decide that `let~` is a high relevance completion given my cursor position here, but that should be done with some explicit scoring on the server side, rather than being caused by (I think) `coc` preferring shorter completions.
### After
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
8011: Add no-sysroot flag for analysis-stats r=edwin0cheng a=edwin0cheng
Add `no-sysroot` flag for `rust-analyzer analysis-stats`. It is very useful for debugging propose.
bors r+
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7984: Improve version display r=matklad a=lnicola
Maybe closes#7854
The version string for unreleased builds looks like this now:
```
$ rust-analyzer --version
rust-analyzer 2021-03-08-159-gc0459c535
```
Release builds should only have the tag name (`2021-03-15`).
Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
7994: Speed up mbe matching in heavy recursive cases r=edwin0cheng a=edwin0cheng
In some cases (e.g. #4186), mbe matching is very slow due to a lot of copy and allocation for bindings, this PR try to solve this problem by introduce a semi "link-list" approach for bindings building.
I used this [test case](https://github.com/weiznich/minimal_example_for_rust_81262) (for `features(32-column-tables)`) to run following command to benchmark:
```
time rust-analyzer analysis-stats --load-output-dirs ./
```
Before this PR : 2 mins
After this PR: 3 seconds.
However, for 64-column-tables cases, we still need 4 mins to complete.
I will try to investigate in the following weeks.
bors r+
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7904: Improved completion sorting r=JoshMcguigan a=JoshMcguigan
I was working on extending #3954 to apply completion scores in more places (I'll have another PR open for that soon) when I discovered that actually completion sorting was not working for me at all in `coc.nvim`. This led me down a bit of a rabbit hole of how coc and vs code each sort completion items.
Before this PR, rust-analyzer was setting the `sortText` field on completion items to `None` if we hadn't applied any completion score for that item, or to the label of the item with a leading whitespace character if we had applied any completion score. Completion score is defined in rust-analyzer as an enum with two variants, `TypeMatch` and `TypeAndNameMatch`.
In vs code the above strategy works, because if `sortText` isn't set [they default it to the label](b4ead4ed66). However, coc [does not do this](e211e36147/src/completion/complete.ts (L245)).
I was going to file a bug report against coc, but I read the [LSP spec for the `sortText` field](https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_completion) and I feel like it is ambiguous and coc could claim what they do is a valid interpretation of the spec.
Further, the existing rust-analyzer behavior of prepending a leading whitespace character for completion items with any completion score does not handle sorting `TypeAndNameMatch` completions above `TypeMatch` completions. They were both being treated the same.
The first change this PR makes is to set the `sortText` field to either "1" for `TypeAndNameMatch` completions, "2" for `TypeMatch` completions, or "3" for completions which are neither of those. This change works around the potential ambiguity in the LSP spec and fixes completion sorting for users of coc. It also allows `TypeAndNameMatch` items to be sorted above just `TypeMatch` items (of course both of these will be sorted above completion items without a score).
The second change this PR makes is to use the actual completion scores for ref matches. The existing code ignored the actual score and always assumed these would be a high priority completion item.
#### Before
Here coc just sorts based on how close the items are in the file.
#### After
Here we correctly get `zzz` first, since that is both a type and name match. Then we get `ccc` which is just a type match.
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
7961: add user docs for ssr assist r=JoshMcguigan a=JoshMcguigan
@matklad
This is a small follow up on #7874, adding user docs for the SSR assist functionality. Since most other assists aren't handled this way I wasn't sure exactly how we wanted to document this, so feel free to suggest alternatives.
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
6822: Read version of rustc that compiled proc macro r=edwin0cheng a=jsomedon
Signed-off-by: Jay Somedon <jay.somedon@outlook.com>
This PR is to fix#6174.
I basically
* added two methods, `read_version` and `read_section`(used by `read_version`)
* two new crates `snap` and `object` to be used by those two methods
I just noticed that some part of code were auto-reformatted by rust-analyzer on file save. Does it matter?
Co-authored-by: Jay Somedon <jay.somedon@outlook.com>
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7878: Remove `item_scope` field from `Body` r=jonas-schievink a=jonas-schievink
Closes https://github.com/rust-analyzer/rust-analyzer/issues/7632
Instead of storing an `ItemScope` filled with inner items, we store the list of `BlockId`s for all block expressions that are part of a `Body`. Code can then query the `block_def_map` for those.
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
7873: Consider unresolved qualifiers during flyimport r=matklad a=SomeoneToIgnore
Closes https://github.com/rust-analyzer/rust-analyzer/issues/7679
Takes unresolved qualifiers into account, providing better completions (or none, if the path is resolved or do not match).
Does not handle cases when both path qualifier and some trait has to be imported: there are many extra issues with those (such as overlapping imports, for instance) that will require large diffs to address.
Also does not do a fuzzy search on qualifier, that requires some adjustments in `import_map` for better queries and changes to the default replace range which also seems relatively big to include here.
7933: Improve compilation speed r=matklad a=matklad
bors r+
🤖
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
7898: generate_function assist: infer return type r=JoshMcguigan a=JoshMcguigan
This PR makes two changes to the generate function assist:
1. Attempt to infer an appropriate return type for the generated function
2. If a return type is inferred, and that return type is not unit, don't render the snippet
```rust
fn main() {
let x: u32 = foo$0();
// ^^^ trigger the assist to generate this function
}
// BEFORE
fn foo() ${0:-> ()} {
todo!()
}
// AFTER (only change 1)
fn foo() ${0:-> u32} {
todo!()
}
// AFTER (change 1 and 2, note the lack of snippet around the return type)
fn foo() -> u32 {
todo!()
}
```
These changes are made as two commits, in case we want to omit change 2. I personally feel like it is a nice change, but I could understand there being some opposition.
#### Pros of change 2
If we are able to infer a return type, and especially if that return type is not the unit type, the return type is almost as likely to be correct as the argument names/types. I think this becomes even more true as people learn how this feature works.
#### Cons of change 2
We could never be as confident about the return type as we are about the function argument types, so it is more likely a user will want to change that. Plus it is a confusing UX to sometimes have the cursor highlight the return type after triggering this assist and sometimes not have that happen.
#### Why omit unit type?
The assumption is that if we infer the return type as unit, it is likely just because of the current structure of the code rather than that actually being the desired return type. However, this is obviously just a heuristic and will sometimes be wrong. But being wrong here just means falling back to the exact behavior that existed before this PR.
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
7891: Improve handling of rustc_private r=matklad a=DJMcNab
This PR changes how `rust-analyzer` handles `rustc_private`. In particular, packages now must opt-in to using `rustc_private` in `Cargo.toml`, by adding:
```toml
[package.metadata.rust-analyzer]
rustc_private=true
```
This means that depending on crates which also use `rustc_private` will be significantly improved, since their dependencies on the `rustc_private` crates will be resolved properly.
A similar approach could be used in #6714 to allow annotating that your package uses the `test` crate, although I have not yet handled that in this PR.
Additionally, we now only index the crates which are transitive dependencies of `rustc_driver` in the `rustcSource` directory. This should not cause any change in behaviour when using `rustcSource: "discover"`, as the source used then will only be a partial clone. However, if `rustcSource` pointing at a local checkout of rustc, this should significantly improve the memory usage and lower indexing time. This is because we avoids indexing all crates in `src/tools/`, which includes `rust-analyzer` itself.
Furthermore, we also prefer named dependencies over dependencies from `rustcSource`. This ensures that feature resolution for crates which are depended on by both `rustc` and your crate uses the correct set for analysing your crate.
See also [introductory zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Fixed.20crate.20graphs.20and.20optional.20builtin.20crates/near/229086673)
I have tested this in [priroda](https://github.com/oli-obk/priroda/), and it provides a significant improvement to the development experience (once I give `miri` the required data in `Cargo.toml`)
Todo:
- [ ] Documentation
This is ready to review, and I will add documentation if this would be accepted (or if I get time to do so anyway)
Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
7892: Fix TokenStream::from_str for input consisting of a single group with delimiter r=edwin0cheng a=kevinmehall
TokenStream holds a `tt::Subtree` but assumes its `delimiter` is always `None`. In particular, the iterator implementation iterates over the inner `token_trees` and ignores the `delimiter`.
However, `TokenStream::from_str` violated this assumption when the input consists of a single group by producing a Subtree with an outer delimiter, which was ignored as seen by a procedural macro.
`tt::Subtree` is just `pub delimiter: Option<Delimiter>, pub token_trees: Vec<TokenTree>`, so a Subtree that is statically guaranteed not to have a delimiter is just `Vec<TokenTree>`.
Fixes#7810Fixes#7875
Co-authored-by: Kevin Mehall <km@kevinmehall.net>
7865: preserve escape sequences when replacing string with char r=Veykril a=jDomantas
Currently it replaces escape sequence with the actual value, which is very wrong for `"\n"`.
Co-authored-by: Domantas Jadenkus <djadenkus@gmail.com>
`TokenStream` assumes that its subtree's delimeter is `None`, and this
should be encoded in the type system instead of having a delimiter field
that is mostly ignored.
`tt::Subtree` is just `pub delimiter: Option<Delimiter>, pub
token_trees: Vec<TokenTree>`, so a Subtree that is statically guaranteed
not to have a delimiter is just Vec<TokenTree>.
TokenStream holds a `tt::Subtree` but assumes its `delimiter` is always
`None`. In particular, the iterator implementation iterates over the
inner `token_trees` and ignores the `delimiter`.
However, `TokenStream::from_str` violated this assumption when the input
consists of a single Group by producing a Subtree with an outer
delimiter, which was ignored as seen by a procedural macro.
In this case, wrap an extra level of Subtree around it.
Fixes#7810Fixes#7875
This is a hack to work around miri being included in
our analysis of rustc-dev
Really, we should probably use an include set of the actual root libraries
I'm not sure how those are determined however
7880: Honor snippet capability when using the extract function assist r=lnicola a=Arthamys
This fixes issue #7793
Co-authored-by: san <san@alien.parts>
7870: Use chalk_ir::AdtId r=Veykril a=Veykril
It's a bit unfortunate that we got two AdtId's now(technically 3 with the alias in the chalk module but that one won't allow pattern matching), one from hir_def and one from chalk_ir(hir_ty). But the hir_ty/chalk one doesn't leave hir so it shouldn't be that bad I suppose. Though if I see this right this will happen for almost all IDs.
I imagine most of the intermediate changes to using chalk ids will turn out not too nice until the refactor is over.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7795: Show docs on hover for keywords and primitives r=matklad a=Veykril
It's a bit annoying that this requires the `SyntaxNode` and `Semantics` to be pulled through `hover_for_definition` just so we can get the `std` crate but I couldn't think of a better way.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7335: added region folding r=matklad a=LucianoBestia
Regions of code that you'd like to be folded can be wrapped with `// #region` and `// #endregion` line comments.
This is called "Region Folding". It is originally available for many languages in VSCode. But Rust-analyzer has its own folding function and this is missing.
With this Pull Request I am suggesting a simple solution.
The regions are a special kind of comments, so I added a bit of code in the comment folding function.
The regex to match are: `^\s*//\s*#?region\b` and `^\s*//\s*#?endregion\b`.
The number of space characters is not important. There is an optional # character. The line can end with a name of the region.
Example:
```rust
// 1. some normal comment
// region: test
// 2. some normal comment
calling_function(x,y);
// endregion: test
```
I added a test for this new functionality in `folding_ranges.rs`.
Please, take a look and comment.
I found that these exact regexes are already present in the file `language-configuration.json`, but I don't find a way to read this configuration. So my regex is hardcoded in the code.
7691: Suggest name in extract variable r=matklad a=cpud36
Generate better default name in extract variable assist as was mentioned in issue #1587
# Currently supported
(in order of declining precedence)
1. Expr is argument to a function; use corresponding parameter name
2. Expr is result of a function or method call; use this function/method's name
3. Use expr type name (if possible)
4. Fallback to `var_name` otherwise
# Showcase
# Questions
* Should we more aggressively strip known types? E.g. we already strip `&T -> T`; should we strip `Option<T> -> T`, `Result<T, E> -> T`, and others?
* Integers and floats use `var_name` by default. Should we introduce a name, like `i`, `f` etc?
* Can we return a list and suggest a name when renaming(like IntelliJ does)?
* Should we add counters to remove duplicate variables? E.g. `type`, `type1`, type2`, etc.
Co-authored-by: Luciano Bestia <LucianoBestia@gmail.com>
Co-authored-by: Luciano <31509965+LucianoBestia@users.noreply.github.com>
Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
7827: Fix proc macro TokenStream::from_str token ids r=vlad20012 a=vlad20012
To be honest, I don't know what it changes from a user perspective.
Internally, this fixes spans (token ids) of a `TokenStream` parsed from a string:
```rust
#[proc_macro_derive(FooDerive)]
pub fn foo_derive(item: TokenStream) -> TokenStream {
"fn foo() {}".parse().unwrap()
}
```
Previously, `TokenStream` was constructed from tokens with incremental ids (that conflicted with call-site tokens). Now they are `-1`.
Co-authored-by: vlad20012 <beskvlad@gmail.com>
7829: Bump deps r=matklad a=lnicola
Unfortunately, this brings a bunch of proc macros dep because `cargo-metadata` went full-in on `derive-builder`. I'm not sure what we can do here..
7833: Use chalk_ir::Mutability r=Veykril a=Veykril
Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7778: Fix lowering trailing self paths in UseTrees r=Veykril a=Veykril
Noticed that hovering over `self` in a use tree like `use foo::bar::{self}` showing documentation and such for the current module instead of `bar`.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7819: Speedup heavy tests r=matklad a=matklad
bors r+
🤖
7820: Update vscode README with a small features list r=matklad a=Veykril
Nothing grande but I figured this is a bit better than showing almost nothing
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7813: Inline TypeCtor into Ty r=flodiebold a=Veykril
This removes the `ApplicationTy` variant from `Ty` bringing the representation a lot closer to chalk's `TyKind`.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7804: Introduce TypeCtor::Scalar r=lnicola a=Veykril
`TypeCtor::Int(..) | TypeCtor::Float(..) | TypeCtor::Char | TypeCtor::Bool` => `TypeCtor::Scalar(..)`, in this case we can actually just straight up use `chalk_ir::Scalar` already since its just a POD without any IDs or anything.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7797: Format generated lints and features manually r=matklad a=lnicola
As `quote` and `rustfmt` leave them on a single line, which makes running `grep` in the repository quite annoying.
Also removes a dead `gen_features.rs` file (`gen_lint_completions.rs` does the same thing).
Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
7566: Add benchmark tests for mbe r=matklad a=edwin0cheng
This PR add more real world tests dumped from `rust-analyzer analysis-stats .` to benchmark its performance.
cc #7513
r? @matklad
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
7741: Add convert_for_to_iter_for_each assist r=mattyhall a=mattyhall
Implements one direction of #7681
I wonder if this tries to guess too much at the right thing here. A common pattern is:
```rust
let col = vec![1, 2, 3];
for v in &mut col {
*v *= 2;
}
// equivalent to:
col.iter_mut().for_each(|v| *v *= 2);
```
I've tried to detect this case by checking if the expression after the `in` is a (mutable) reference and if not inserting iter()/iter_mut(). This is just a convention used in the stdlib however, so could sometimes be wrong. I'd be happy to make an improvement for this, but not sure what would be best. A few options spring to mind:
1. Only allow this for types that are known to have iter/iter_mut (ie stdlib types)
2. Try to check if iter/iter_mut exists and they return the right iterator type
3. Don't try to do this and just add `.into_iter()` to whatever is after `in`
Co-authored-by: Matt Hall <matthew@quickbeam.me.uk>
7732: Don't lower TypeBound::Lifetime as GenericPredicate::Error r=flodiebold a=Veykril
Basically we just discard the typebound for now instead when lowering to `GenericPredicate`. I think this shouldn't have any other side effects?
Fixes #7683(hopefully for real this time)
I also played around with introducing `GenericPredicate::LifetimeOutlives` and `GenericPredicate::TypeOutlives`(see b9d6904845) but that won't fix this issue(at least not for now) due to lifetime predicate mismatches when resolving methods so I figure this is a good way to fix it for now.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7699: Implement ast::AstNode for NameLike and move it to node_ext r=matklad a=Veykril
With this `search`(and 2 other modules) don't necessarily go through 3 calls of `find_node_at_offset_with_descend` to find the correct node. Also makes the code that searches for NameLikes a bit easier on the eyes imo, though that can be fixed with just a helper function as well so its not that relevant.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
7705: Show hover info of the definition of ConstReference patterns instead of its type r=Veykril a=Veykril
Closes#7671
bors r+
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Reading through the code for diagnostics and observing debug logs, I noticed
that diagnostics are transmitted after every change for every opened file,
even if they haven't changed (especially visible for files with no diagnostics).
This change avoids marking files as "changed" if diagnostics are the same to what
was already sent before. This will only work if diagnostics are always produced in
the same order, but from my limited testing it seems this is the case.
7703: Allow comments between newlines in chaining hints r=Veykril a=unratito
Currently, chaining hints are not generated if there are comments between newlines, which is a very common pattern:
```rust
let vec = (0..10)
// Multiply by 2
.map(|x| x * 2)
// Add 3
.map(|x| x + 3)
.collect::<Vec<i32>>();
```
Besides, it seems a bit weird that this piece of code generates a chaining hint:
```rust
let vec = (0..10)
.collect::<Vec<i32>>();
```
But this one doesn't:
```rust
let vec = (0..10)
// This is a comment
.collect::<Vec<i32>>();
```
Co-authored-by: Paco Soberón <unratito@gmail.com>
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.
