fix: Wrong BoundVar index when lowering impl trait parameter of parent generics
Fixes#17711
From the following test code;
```rust
//- minicore: deref
use core::ops::Deref;
struct Struct<'a, T>(&'a T);
trait Trait {}
impl<'a, T: Deref<Target = impl Trait>> Struct<'a, T> {
fn foo(&self) -> &Self { self }
fn bar(&self) {
let _ = self.foo();
}
}
```
when we call `register_obligations_for_call` for `let _ = self.foo();`,
07659783fd/crates/hir-ty/src/infer/expr.rs (L1939-L1952)
we are querying `generic_predicates` and it has `T: Deref<Target = impl Trait>` predicate from the parent `impl Struct`;
07659783fd/crates/hir-ty/src/lower.rs (L375-L399)
but as we can see above, lowering `TypeRef = impl Trait` doesn't take into account the parent generic parameters, so the `BoundVar` index here is `0`, as `fn foo` has no generic args other than parent's,
But this `BoundVar` is pointing at `'a` in `<'a, T: Deref<Target = impl Trait>>`.
So, in the first code reference `register_obligations_for_call`'s L:1948 - `.substitute(Interner, parameters)`, we are substituting `'a` with `Ty`, not `Lifetime` and this makes panic inside the chalk.
This PR fixes this wrong `BoundVar` index in such cases
Add scip/lsif flag to exclude vendored libaries
#17809 changed StaticIndex to include vendored libraries. This PR adds a flag to disable that behavior.
At work, our monorepo has too many rust targets to index all at once, so we split them up into several shards. Since all of our libraries are vendored, if rust-analyzer includes them, sharding no longer has much benefit, because every shard will have to index the entire transitive dependency graphs of all of its targets. We get around the issue presented in #17809 because some other shard will index the libraries directly.
fix: Properly account for editions in names
This PR touches a lot of parts. But the main changes are changing `hir_expand::Name` to be raw edition-dependently and only when necessary (unrelated to how the user originally wrote the identifier), and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware (this was done in #17896, but the FIXMEs were fixed here).
It is possible that I missed some cases, but most IDE parts should properly escape (or not escape) identifiers now.
The rules of thumb are:
- If we show the identifier to the user, its rawness should be determined by the edition of the edited crate. This is nice for IDE features, but really important for changes we insert to the source code.
- For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update tests when an edition becomes stable, to avoid churn).
- For debugging tools (helper methods and logs), I used `Edition::LATEST`.
Reviewing notes:
This is a really big PR but most of it is mechanical translation. I changed `Name` displayers to require an edition, and followed the compiler errors. Most methods just propagate the edition requirement. The interesting cases are mostly in `ide-assists`, as sometimes the correct crate to fetch the edition from requires awareness (there may be two). `ide-completions` and `ide-diagnostics` were solved pretty easily by introducing an edition field to their context. `ide` contains many features, for most of them it was propagated to the top level function and there the edition was fetched based on the file.
I also fixed all FIXMEs from #17896. Some required introducing an edition parameter (usually not for many methods after the changes to `Name`), some were changed to a new method `is_any_identifier()` because they really want any possible keyword.
Fixes#17895.
Fixes#17774.
This PR touches a lot of parts. But the main changes are changing
`hir_expand::Name` to be raw edition-dependently and only when necessary
(unrelated to how the user originally wrote the identifier),
and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware
(this was done in #17896, but the FIXMEs were fixed here).
It is possible that I missed some cases, but most IDE parts should properly
escape (or not escape) identifiers now.
The rules of thumb are:
- If we show the identifier to the user, its rawness should be determined
by the edition of the edited crate. This is nice for IDE features,
but really important for changes we insert to the source code.
- For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update
tests when an edition becomes stable, to avoid churn).
- For debugging tools (helper methods and logs), I used `Edition::LATEST`.
Allow flycheck process to exit gracefully
Assuming it isn't cancelled. Closes#17902.
The only place CommandHandle::join() is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.
The only reason I can see for the existing behavior is if the command is somehow holding onto a build lock longer than it should, this would force it to be released. But it would be a pretty heavy-handed way to solve that issue. I'm not aware of this occurring in practice.
Test for word boundary in `FindUsages`
This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).
Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).
Feel free to close this if you consider this a non-issue, as most short identifiers are local.
internal: Replace once_cell with std's recently stabilized OnceCell/Lock and LazyCell/Lock
This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
This speeds up short identifiers search significantly, while unlikely to have an effect on long identifiers (the analysis takes much longer than some character comparison).
Tested by finding all references to `eq()` (from `PartialEq`) in the rust-analyzer repo. Total time went down from 100s to 10s (a 10x reduction!).
This doesn't get rid of the once_cell dependency, unfortunately, since we have dependencies that use it, but it's a nice to do cleanup. And when our deps will eventually get rid of once_cell we will get rid of it for free.
Assuming it isn't cancelled. Closes#17902.
The only place CommandHandle::join is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.
fix: Panic while hovering associated function with type annotation on generic param that not inherited from its container type
Fixes#17871
We call `generic_args_sans_defaults` here;
64a140527b/crates/hir-ty/src/display.rs (L1021-L1034)
but the following substitution inside that function panic in #17871;
64a140527b/crates/hir-ty/src/display.rs (L1468)
it's because the `Binders.binder` inside `default_parameters` has a same length with the generics of the function we are hovering on, but the generics of it is split into two, `fn_params` and `parent_params`.
Because of this, it may panic if the function has one or more default parameters and both `fn_params` and `parent_params` are non-empty, like the case in the title of this PR.
So, we must call `generic_args_sans_default` first and then split it into `fn_params` and `parent_params`
fix: Panic while canonicalizing erroneous projection type
Fixes#17866
The root cause of #17866 is quite horrifyng 😨
```rust
trait T {
type A;
}
type Foo = <S as T>::A; // note that S isn't defined
fn main() {
Foo {}
}
```
While inferencing alias type `Foo = <S as T>::A`;
78c2bdce86/crates/hir-ty/src/infer.rs (L1388-L1398)
the error type `S` in it is substituted by inference var in L1396 above as below;
78c2bdce86/crates/hir-ty/src/infer/unify.rs (L866-L869)
This new inference var's index is `1`, as the type inferecing procedure here previously inserted another inference var into same `InferenceTable`.
But after that, the projection type made from the above then passed to the following function;
78c2bdce86/crates/hir-ty/src/traits.rs (L88-L96)
here, a whole new `InferenceTable` is made, without any inference var and in the L94, this table calls;
78c2bdce86/crates/hir-ty/src/infer/unify.rs (L364-L370)
And while registering `AliasEq` `obligation`, this obligation contains inference var `?1` made from the previous table, but this table has only one inference var `?0` made at L365.
So, the chalk panics when we try to canonicalize that obligation to register it, because the obligation contains an inference var `?1` that the canonicalizing table doesn't have.
Currently, we are calling `InferenceTable::new()` to do some normalizing, unifying or coercing things to some targets that might contain inference var that the new table doesn't have.
I think that this is quite dangerous footgun because the inference var is just an index that does not contain the information which table does it made from, so sometimes this "foreign" index might cause panic like this case, or point at the wrong variable.
This PR mitigates such behaviour simply by inserting sufficient number of inference vars to new table to avoid such problem.
This strategy doesn't harm current r-a's intention because the inference vars that passed into new tables are just "unresolved" variables in current r-a, so this is just making sure that such "unresolved" variables exist in the new table
internal: Be more resilient to bad language item definitions in binop inference
Fixes#16287Fixes#16286
There's one more in `write_fn_trait_method_resolution`, but I'm not sure if it won't cause further problems in `infer_closures`.
fix: Resolve included files to their calling modules in IDE layer
Fixes https://github.com/rust-lang/rust-analyzer/issues/17390 at the expense of reporting duplicate diagnostics for modules that have includes in them when both the calling and called file are included.
internal: Reply to requests with defaults when vfs is still loading
There is no reason for us to hit the database with queries when we certainly haven't reached a stable state yet. Instead we just reply with default request results until we are in a state where we can do meaningful work. This should save us from wasting resources while starting up at worst, and at best save us from creating query and interning entries that are non-meaningful which ultimately just end up wasting memory.
internal: Optimize the usage of channel senders
Used `Sender` directly instead of a boxed closure. There is no need to use the boxed closure. This also allows the caller to decide to do something other than `unwrap` (not a fan of it BTW).
Reuse recursion limit as expansion limit
A configurable recursion limit was introduced by looking at the recursion_limit crate attribute. Instead of relying on a global constant we will reuse this value for expansion limit as well.
Addresses: https://github.com/rust-lang/rust-analyzer/issues/8640#issuecomment-2271740272
feat: Implement TAIT and fix ATPIT a bit
Closes#16296 (Commented on the issue)
In #16852, I implemented ATPIT, but as I didn't discern ATPIT and other non-assoc TAIT, I guess that it has been working for some TAITs.
As the definining usage of TAIT requires it should be appear in the Def body's type(const blocks' type annotations or functions' signatures), this can be done in simlilar way with ATPIT
And this PR also corrects some defining-usage resolution for ATPIT
A configurable recursion limit was introduced by looking at the
recursion_limit crate attribute. Instead of relying on a global constant
we will reuse this value for expansion limit as well.
Include vendored crates in StaticIndex
`StaticIndex::compute` filters out modules from libraries. This makes an exceptions for vendored libraries, ie libraries actually defined inside the workspace being indexed.
This aims to solve https://bugzilla.mozilla.org/show_bug.cgi?id=1846041 In general StaticIndex is meant for code browsers, which likely want to index all visible source files.
fix: tyck for non-ADT types when searching refs for `Self` kw
See e0276dc5dd (r1389848845)
For ADTs, to handle `{error}` in generic args, we should to convert them to ADT for comparisons; for others, we can directly compare the types.
StaticIndex::compute filters out modules from libraries. This makes an
exceptions for vendored libraries, ie libraries actually defined inside
the workspace being indexed.
This aims to solve https://bugzilla.mozilla.org/show_bug.cgi?id=1846041
In general StaticIndex is meant for code browsers, which likely want to
index all visible source files.
With the lack of a README on the individually published library crates and the somewhat cryptic `ra_ap_` prefix it is hard to figure out where those crates belong to, so mentioning "rust-analyzer" feels like auseful hint there.
internal: Load VFS config changes in parallel
Simple attempt to make some progress f or https://github.com/rust-lang/rust-analyzer/issues/17373
No clue if those atomic orderings are right, though I don't think they are really too relevant either.
A more complete fix would probably need to replace our `ProjectFolders` handling a bit.
fix: remove AbsPath requirement from linkedProjects
Should (fingers crossed!) fix https://github.com/rust-lang/rust-analyzer/issues/17664. I opened the `rustc` workspace with the [suggested configuration](e552c168c7/src/etc/rust_analyzer_settings.json) and I was able to successfully open some rustc crates (`rustc_incremental`) and have IDE functionality.
`@Veykril:` can you try these changes and let me know if it fixed rustc?
feat: Introduce workspace `rust-analyzer.toml`s
In order to globally configure a project it was, prior to this PR, possible to have a `ratoml` at the root path of a project. This is not the case anymore. Instead we now let ratoml files that are placed at the root of any workspace have a new scope called `workspace`. Although there is not a difference between a `workspace` scope and and a `global` scope, future PRs will change that.
feat: Use spans for builtin and declarative macro expansion errors
This should generally improve some error reporting for macro expansion errors. Especially for `compile_error!` within proc-macros
feat(ide-completion): explictly show `async` keyword on `impl trait` methods
OLD:
<img width="676" alt="image" src="https://github.com/user-attachments/assets/f6fa626f-6b6d-4c22-af27-b0755e7a6bf8">
Now:
<img width="684" alt="image" src="https://github.com/user-attachments/assets/efbaac0e-c805-4dd2-859d-3e44b2886dbb">
---
This is an preparation for https://github.com/rust-lang/rust-analyzer/issues/17719.
```rust
use std::future::Future;
trait DesugaredAsyncTrait {
fn foo(&self) -> impl Future<Output = usize> + Send;
fn bar(&self) -> impl Future<Output = usize> + Send;
}
struct Foo;
impl DesugaredAsyncTrait for Foo {
fn foo(&self) -> impl Future<Output = usize> + Send {
async { 1 }
}
//
async fn bar(&self) -> usize {
1
}
}
fn main() {
let fut = Foo.bar();
fn _assert_send<T: Send>(_: T) {}
_assert_send(fut);
}
```
If we don't distinguish `async` or not. It would be confusing to generate sugared version `async fn foo ....` and original form `fn foo` for `async fn in trait` that is defined in desugar form.
fix: let glob imports override other globs' visibility
Follow up to #14930Fixes#11858Fixes#14902Fixes#17704
I haven't reworked the code here at all - I don't feel confident in the codebase to do so - just rebased it onto the current main branch and fixed conflicts.
I'm not _entirely_ sure I understand the structure of the `check` function in `crates/hir-def/src/nameres` tests. I think the change to the test expectation from #14930 is correct, marking the `crate::reexport::inner` imports with `i`, and I understand it to mean there's a specific token in the import that we can match it to (in this case, `Trait`, `function` and `makro` of `pub use crate::defs::{Trait, function, makro};` respectively), but I had some trouble understanding the meaning of the different parts of `PerNs` to be sure.
Does this make sense?
I tested building and using RA locally with `cargo xtask install` and after this change the documentation for `arrow_array::ArrowPrimitiveType` seems to be picked up correctly!
And that is due to a case where we have two ratomls in a source root, one of which
is a `workspace_ratoml` and the other one is simple old ratoml. Since we are not checking to see if
the source root is already populated with workspace ratoml, this test fails. Due to principles of clear
code I believe it is reasonable to not have two HashMaps that are almost for the exact same thing.
So next commit should remove `workspace_ratoml` and merge it with `krate_ratoml`s.
The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to resolve the Deref::Target type, and instead uses a TyKind::BoundVar (i.e. an unknown type). This "autoderefed" type then incorrectly acts as if it implements all traits in scope.
The fix is to re-apply the same sanity-check that is done in iterate_method_candidates_with_autoref(), that is: don't try to resolve methods on unknown types. This same sanity-check is now done on each autoderefed type for which trait methods are about to be checked. If the autoderefed type is unknown, then the iterating of the trait methods for that type is skipped.
Includes a unit test that only passes after applying the fixes in this commit.
Includes a change to the assertion count in test syntax_highlighting::tests::benchmark_syntax_highlighting_parser as suggested by Lukas Wirth during review.
Includes a change to the sanity-check code as suggested by Florian Diebold during review.
feat: add preliminary support for `+ use<..>` `precise_capturing` syntax
## Summary
This PR adds basic support for the following syntax.
```rs
fn captures<'a: 'a, 'b: 'b, T>() -> impl Sized + use<'b, T> {}
// ~~~~~~~~~~~~~~~~~~~~~~~
// This opaque type does not capture `'a`.
fn outlives<'o, T: 'o>(_: T) {}
fn caller<'o, 'a, 'b: 'o, T: 'o>() {
// ~~
// ^ Note that we don't need `'a: 'o`.
outlives::<'o>(captures::<'a, 'b, T>());
}
```
Related to #17598
internal: add more output to the status command
Bit of a lazy change, but this is was pretty handy. I think I should clean up the configuration into something a bit more legible (maybe serialize as JSON?), but I think this is a good enough starting point that we might as well start asking people for it in issue reports.
Add method `Impl::all_in_module(…)` for allowing more localized querying
This PR is motivated by an [outside use](https://github.com/regexident/cargo-modules) of the `ra_ap_hir` crate that would benefit from being able to more efficiently query for all impls of a given module (instead of having to query for its parent crate's impls and then filtering on `impl_hir.module(db) == module_hir`).
I have the suspicion that the code as is won't quite work for file-level modules, since those don't have a block, afaict, but with all the crate-rename and version shenanigans around the `ra_ap_` release process I haven't yet been able to figure out how to patch a `ra_ap_` dependency from a third-party crate (let alone how to test this from inside of `rust-analyzer`), so haven't been able to actually run and test this. 😔
Any hints on how to make this (i.e. the code itself, as well as the testing) work are more than welcome!
chore: remove `UnindexinedProject` notification
This PR is split out from https://github.com/rust-lang/rust-analyzer/pull/17246/ (and contains its changes, which is a little annoying from a review perspective...). I'd like to land this change a week or so after #17246 lands in order to give any users of the unindexed project notification time to adopt migrate.
minor : fixes for ratoml module
This is a follow-up PR to #17058.
- Parse errors are reflected as such by defining a new variant called `ConfigError::ParseError`
- New error collection has been added to store config level agnostic errors.
EDIT : Some things that this PR promised to solve are removed and will be addressed by other PRs
LRU `body_with_source_map` query
This query is being invalidated all the time anyways (we have an extra query on top of it for the body incrementality that is not source dependent), so there is little reason to keep these around all the time when only some IDE features are interested in them.
feat: go-to-def and find-references on control-flow keywords
fix#17517.
This PR implements **go-to-definition** and **find-references** functionalities for control flow keywords, which is similar to the behaviors in the `highlight-related` module. Besides, this PR also fixes some incorrect behaviors in `highlight-related`.
## Changes
1. **Support for go-to-definition on control flow keywords**:
This PR introduces functionality allowing users to navigate on the definition of control flow keywords (`return`, `break`, `continue`).
Commit: 2a3244ee147f898dd828c06352645ae1713c260f..7391e7a608634709db002a4cb09229de4d12c056.
2. **Bug fixes and refactoring in highlight-related**:
- **Handling return/break/continue within try_blocks**:
This PR adjusted the behavior of these keywords when they occur within `try_blocks`. When encounter these keywords, the program should exit the outer function or loop which containing the `try_blocks`, rather than the `try_blocks` itself; while the `?` will cause the program to exit `try_blocks`.
Commit: 59d697e807f0197f59814b37dca1563959da4aa1.
- **Support highlighting keywords in macro expansion for highlight-related**:
Commit: 88df24f01727c23a667a763ee3ee0cec22d5ad52.
- Detailed description for the bug fixes
+ The previous implementation of `preorder_expr` incorrectly treated `try_blocks` as new contexts, thereby r-a will not continue to traverse inner `return` and `break/continue` statements. To resolve this, a new function `preorder_expr_with_ctx_checker` has been added, allowing users to specify which expressions to skip.
* For example, when searching for the `?` in the context, r-a should skip `try_blocks` where the `?` insides just works for `try_blocks`. But when search for the `return` keyword, r-a should collect both the `return` keywords inside and outside the `try_blocks`
+ Thus, this PR added `WalkExpandedExprCtx` (builder pattern). It offers the following improvements: customizable context skipping, maintenance of loop depth (for `break`/`continue`), and handling macro expansion during traversal.
3. **Support for find-references on control flow keywords**:
This PR enables users to find all references to control flow keywords.
Commit: 9202a33f81218fb9c2edb5d42e6b4de85b0323a8.
Fix more path resolution for included submodules
Now with more comprehensive testing! This adds tests for includes within modules. Previous testing was not comprehensive enough since submodules that use `include!` didn't actually work either! The `ModDir` used for resolving mods relative to included files has to be `ModDir::root()`. The original test just so happened to put the submodules in the root which made this work, but if you put the `include!` inside a `mod` block it didn't work.
With this change, when collecting a macro expansion, if the macro call is an `include!`, we use the `ModDir::root()` instead of the current module we're in.
- Parse errors are reflected as such by defining a new variant called `ConfigError::ParseError`
- New error collection has been added to store config level agnostic errors.
Prefer standard library paths over shorter extern deps re-exports
This should generally speed up path finding for std items as we no longer bother looking through all external dependencies. It also makes more sense to prefer importing std items from the std dependencies directly.
Fixes https://github.com/rust-lang/rust-analyzer/issues/17540
Fix path resolution for child mods of those expanded by `include!`
Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass.
Having no knowledge of how any of this works, I believe this fixes#17645. Using another test that writes the included mod directly into `lib.rs` instead, I found the difference can be traced to the candidate files we use to look up mods. A separate branch for if the file comes from an `include!` macro doesn't take into account the original mod we're contained within:
```rust
None if file_id.macro_file().map_or(false, |it| it.is_include_macro(db.upcast())) => {
candidate_files.push(format!("{}.rs", name.display(db.upcast())));
candidate_files.push(format!("{}/mod.rs", name.display(db.upcast())));
}
```
I'm not sure why this branch exists. Tracing the branch back takes us to 3bb9efb but it doesn't say *why* the branch was added. The test case that was added in this commit passes with the branch removed, so I think it's just superfluous at this point.
Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass.
fix: Panic in debug profile for tuple deconstruct with arity mismatch
Fixes#17585, which doesn't affect daily use cases but quite annoying in development of r-a itself like writing tests.
This PR applies similar approach as in #17534, skipping match usefulness check for patterns containing errors
The commands `editor.action.triggerParameterHints` and
`editor.action.rename` are now renamed to
`rust-analyzer.triggerParameterHints` and `rust-analyzer.rename`
This change helps make it clear that these commands are specific to
rust-analyzer and not part of the default set of commands provided by
VSCode.
Fixes: https://github.com/rust-lang/rust-analyzer/issues/17644
Avoid ref when using format! in compiler
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing). Inlining format args prevents accidental `&` misuse.
See https://github.com/rust-lang/rust-clippy/issues/10851
Some more small salsa memory improvements
This does limit our lru limits to 2^16 but if you want to set them higher than that you might as well not set them at all. Also makes `LRU` opt-in per query now, allowing us to drop all the unnecessary LRU stuff for most queries
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing). Inlining format args prevents accidental `&` misuse.
feature: teach rust-analyzer to discover `linked_projects`
This PR's been a long-time coming, but like the title says, it introduces server-side project discovery and removes the extension hooks I previously introduced. I don't think this PR is ready to land, but here are the things I'm feeling squishy about:
- I don't think I like the idea of introducing the `cargo-metadata` command-but-for-everything-else in the `flycheck` module, but the progress reporting infrastructure was too convenient to pass up. Happy to move it elsewhere.
Here are the things I _know_ I need to change:
- For progress reporting, I'm extracting from a `serde_json::Value` that corresponds to `tracing_subsciber::fmt::Layer`'s JSON output. I'd like to make this a bit more structured/documented than the current nonsense I wrote.
- The progress reporting currently hardcodes "Buck"; it should be deriving that from the previously mentioned more-structured-output.
- This doesn't handle *reloading* when a corresponding buildfile is changed. It should be doing that.
<details>
<summary>Anyway, here's a video of rust-analyzer discovering a Buck target.</summary>
https://github.com/rust-lang/rust-analyzer/assets/2067774/be6cd9b9-2c9a-402d-847f-05f860a91df1
</details>
feat: Add incorrect case diagnostics for enum variant fields and all variables/params
Updates the incorrect case diagnostic to check:
1. Fields of enum variants. Example:
```rust
enum Foo {
Variant { nonSnake: u8 }
}
```
2. All variable bindings, instead of just let bindings and certain match arm patters. Examples:
```rust
match 1 { nonSnake => () }
match 1 { nonSnake @ 1 => () }
match 1 { nonSnake1 @ nonSnake2 => () } // slightly cursed, but these both introduce new
// bindings that are bound to the same value.
const ONE: i32 = 1;
match 1 { nonSnake @ ONE } // ONE is ignored since it is not a binding
match Some(1) { Some(nonSnake) => () }
struct Foo { field: u8 }
match (Foo { field: 1 } ) {
Foo { field: nonSnake } => ();
}
struct Foo { nonSnake: u8 } // diagnostic here, at definition
match (Foo { nonSnake: 1 } ) { // no diagnostic here...
Foo { nonSnake } => (); // ...or here, since these are not where the name is introduced
}
for nonSnake in [] {}
struct Foo(u8);
for Foo(nonSnake) in [] {}
```
3. All parameter bindings, instead of just top-level binding identifiers. Examples:
```rust
fn func(nonSnake: u8) {} // worked before
struct Foo { field: u8 }
fn func(Foo { field: nonSnake }: Foo) {} // now get diagnostic for nonSnake
```
This is accomplished by changing the way binding identifier patterns are filtered:
- Previously, all binding idents were skipped, except a few classes of "good" binding locations that were checked.
- Now, all binding idents are checked, except field shorthands which are skipped.
Moving from a whitelist to a blacklist potentially makes the analysis more brittle:
If new pattern types are added in the future where ident pats don't introduce new names, then they may incorrectly create diagnostics.
But the benefit of the blacklist approach is simplicity: I think a whitelist approach would need to recursively visit patterns to collect renaming candidates?
Trigger VSCode to rename after extract variable assist is applied
When the user applies the "Extract Variable" assist, the cursor is
positioned at the newly inserted variable. This commit adds a command
to the assist that triggers the rename action in VSCode. This way, the
user can quickly rename the variable after applying the assist.
Fixes part of: #17579https://github.com/user-attachments/assets/4cf38740-ab22-4b94-b0f1-eddd51c26c29
I haven't yet looked at the module or function extraction assists yet.
use "bootstrap" instead of "rustbuild" in comments and docs
Let's stick with the single name "bootstrap" to refer to the bootstrap project to avoid confusion. This should make it clearer, especially for new contributors.
When the user applies the "Extract Variable" assist, the cursor is
positioned at the newly inserted variable. This commit adds a command
to the assist that triggers the rename action in VSCode. This way, the
user can quickly rename the variable after applying the assist.
Fixes part of: #17579
feat: do not add new enum if it already exists
## Summary
This PR introduces a check for the existence of another enum within the current scope, and if it exist, we skip `add_enum_def`.
## Why?
Currently, when using the `bool_to_enum` assist more than once, it is possible to add multiple enum definitions. For example, the following snippet,
```rs
#[derive(PartialEq, Eq)]
enum Bool {
True,
False,
}
fn main() {
let a = Bool::True;
let b = true;
println!("Hello, world!");
}
```
will be transformed into,
```rs
#[derive(PartialEq, Eq)]
enum Bool {
True,
False,
}
#[derive(PartialEq, Eq)]
enum Bool {
True,
False,
}
fn main() {
let a = Bool::True;
let b = Bool::True;
println!("Hello, world!");
}
```
This can be annoying for users to clean up.
Add `f16` and `f128` support
Adds `f16` and `f128` support, using the `rustc_apfloat` library (also used by `rustc`) for parsing/arithmetic/displaying since the types aren't stable yet so can't be used by rust-analyzer itself.
Issue: #17451
feat: add inlay hints for generic parameters
fixes#11091
By default, only hints for const generic parameters are shown, and this can be configured through `rust-analyzer.inlayHints.genericParameterHints.enable`.
Probably needs more testing.
Make casts of pointers to trait objects stricter
This is an attempt to `fix` https://github.com/rust-lang/rust/issues/120222 and https://github.com/rust-lang/rust/issues/120217.
This is done by adding restrictions on casting pointers to trait objects.
Before this PR the rules were as follows:
> When casting `*const X<dyn A>` -> `*const Y<dyn B>`, principal traits in `A` and `B` must refer to the same trait definition (or no trait).
With this PR the rules are changed to
> When casting `*const X<dyn Src>` -> `*const Y<dyn Dst>`
> - if `Dst` has a principal trait `DstP`,
> - `Src` must have a principal trait `SrcP`
> - `dyn SrcP` and `dyn DstP` must be the same type (modulo the trait object lifetime, `dyn T+'a` -> `dyn T+'b` is allowed)
> - Auto traits in `Dst` must be a subset of auto traits in `Src`
> - Not adhering to this is currently a FCW (warn-by-default + `FutureReleaseErrorReportInDeps`), instead of an error
> - if `Src` has a principal trait `Dst` must as well
> - this restriction will be removed in a follow up PR
This ensures that
1. Principal trait's generic arguments match (no `*const dyn Tr<A>` -> `*const dyn Tr<B>` casts, which are a problem for [#120222](https://github.com/rust-lang/rust/issues/120222))
2. Principal trait's lifetime arguments match (no `*const dyn Tr<'a>` -> `*const dyn Tr<'b>` casts, which are a problem for [#120217](https://github.com/rust-lang/rust/issues/120217))
3. No auto traits can be _added_ (this is a problem for arbitrary self types, see [this comment](https://github.com/rust-lang/rust/pull/120248#discussion_r1463835350))
Some notes:
- We only care about the metadata/last field, so you can still cast `*const dyn T` to `*const WithHeader<dyn T>`, etc
- The lifetime of the trait object itself (`dyn A + 'lt`) is not checked, so you can still cast `*mut FnOnce() + '_` to `*mut FnOnce() + 'static`, etc
- This feels fishy, but I couldn't come up with a reason it must be checked
The diagnostics are currently not great, to say the least, but as far as I can tell this correctly fixes the issues.
cc `@oli-obk` `@compiler-errors` `@lcnr`
Add an option to use "::" for the external crate prefix.
Fixes#11823 .
Hi I'm very new to rust-analyzer and not sure how the review process are. Can somebody take a look at this PR? thanks!
internal: Clean up runnable lsp extension
This feels like a natural addition to me, and also allows us to drop the expect-test hardcoding from the extension. Additionally, `cargoExtraArgs` is pointless, all the client will do is merge it with `cargoArgs` so the server can do that instead of delegating that to the client.
Improve dead code analysis
Fixes#120770
1. check impl items later if self ty is private although the trait method is public, cause we must use the ty firstly if it's private
2. mark the adt live if it appears in pattern, like generic argument, this implies the use of the adt
3. based on the above, we can handle the case that private adts impl Default, so that we don't need adding rustc_trivial_field_reads on Default, and the logic in should_ignore_item
r? ``@pnkfelix``
fix: Don't emit semantic diagnostics in files with a lot of syntax errors
These will only add to the noise when something very unexpected breaks or where parser recovery fails to kick in.
do not normalize `use foo::{self}` to `use foo`
It changes behaviour and can cause collisions. E.g. for the following snippet
```rs
mod foo {
pub mod bar {}
pub const bar: i32 = 8;
}
// transforming the below to `use foo::bar;` causes the error:
//
// the name `bar` is defined multiple times
use foo::bar::{self};
const bar: u32 = 99;
fn main() {
let local_bar = bar;
}
```
we still normalize
```rs
use foo::bar;
use foo::bar::{self};
```
to `use foo::bar;` because this cannot cause collisions.
See: https://github.com/rust-lang/rust-analyzer/pull/17140#issuecomment-2079189725
Quality of life improvements to term search
Basically two things:
- Allow optionally disabling "borrow checking" restrictions on term search code assists. Sometimes it is better to get invalid suggestions and fix borrow checking issues later...
- Remove explicit generics in generated expressions. I find it quite rare that one writes `None::<T>` instead of `None`.
feat: add bool_to_enum assist for parameters
## Summary
This PR adds parameter support for `bool_to_enum` assists. Essentially, the assist can now transform this:
```rs
fn function($0foo: bool) {
if foo {
println!("foo");
}
}
```
To this,
```rs
#[derive(PartialEq, Eq)]
enum Bool { True, False }
fn function(foo: Bool) {
if foo == Bool::True {
println!("foo");
}
}
```
Thanks to `@/davidbarsky` for the test skeleton (:
Closes#17400
