fix: Get rid of `$crate` in expansions shown to the user
Be it "Expand Macro Recursively", "Inline macro" or few other things.
We replace it with the crate name, as should've always been.
Probably fixes some issues, but I don't know what they are.
fix: Always cache macro expansions' root node in Semantics
Previously some expansions were not cached, but were cached in the expansion cache, which caused panics when later queries tried to lookup the node from the expansion cache.
Fixes#18089.
Previously some expansions were not cached, but were cached in the expansion cache, which caused panics when later queries tried to lookup the node from the expansion cache.
The previous analysis was top-down, and worked on a single file (expanding macros). The new analysis is bottom-up, starting from the diagnostics and climbing up the syntax and module tree.
While this is more efficient (and in fact, efficiency was the motivating reason to work on this), unfortunately the code was already fast enough. But luckily, it also fixes a correctness problem: outline parent modules' attributes were not respected for the previous analysis. Case lints specifically did their own analysis to accommodate that, but it was limited to only them. The new analysis works on all kinds of lints, present and future.
It was basically impossible to fix the old analysis without rewriting it because navigating the module hierarchy must come bottom-up, and if we already have a bottom-up analysis (including syntax analysis because modules can be nested in other syntax elements, including macros), it makes sense to use only this kind of analysis.
Few other bugs (not fundamental ti the previous analysis) are also fixed, e.g. overwriting of lint levels (i.e. `#[allow(lint)] mod foo { #[warn(lint)] mod bar; }`.
feat: render patterns in params for hovering
Fix#17858
This PR introduces an option to [hir-def/src/body/pretty.rs](08c7bbc2db/crates/hir-def/src/body/pretty.rs) to render the result as a single line, which is then reused for rendering patterns in parameters for hovering.
fix: Fix `inline_const_as_literal` error when the number >= 10
## Description
### The Bug
This PR fixes a small bug in the IDE assistence (`inline_const_as_literal`). When the being-inlined constant is a number and it is greater than or equal to 10, the assistence inserts unexpected string `(0x...)` after the number itself. A simple example is followed:
Current `inline_const_as_literal` changes
```rs
const A: usize = 16;
fn f() -> usize {
A // inline the constant
}
```
into
```rs
const A: usize = 16;
fn f() -> usize {
16 (0x10)
}
```
The bug originates from #14925 & #15306 . #14925 added some unittests, but it just tested the number-inlining behavior when the number is `0`.
50882fbfa2/crates/ide-assists/src/handlers/inline_const_as_literal.rs (L124-L138)
And #15306 modified the behavior of `Const::render_eval` and added the `(0x...)` part after the number (if the number >= `10`). Because of insufficient unittests in #14925, changes about `Const::render_eval` in #15306 introduced this bug with no CI failure.
### The Fix
I think `Const::render_eval` is intended for user-facing value displaying (e.g. hover) and not designed for `inline_const_as_literal`. To fix the bug, I defined a new function named `Const::eval`, which evaluates the value itself faithfully and simply and does nothing else.
## Thanks
Thanks `@roife` for your kind help. Your guidance helped me better understand the code.
fix: use Result type aliases in "Wrap return type in Result" assist
This commit makes the "Wrap return type in Result" assist prefer type aliases of standard library type when the are in scope, use at least one generic parameter, and have the name `Result`.
The last restriction was made in an attempt to avoid false assumptions about which type the user is referring to, but that might be overly strict. We could also do something like this, in order of priority:
* Use the alias named "Result".
* Use any alias if only a single one is in scope, otherwise:
* Use the standard library type.
This is easy to add if others feel differently that is appropriate, just let me know.
Fixes#17796
This commit makes the "Wrap return type in Result" assist prefer type aliases of standard library
type when the are in scope, use at least one generic parameter, and have the name "Result".
The last restriction was made in an attempt to avoid false assumptions about which type the
user is referring to, but that might be overly strict. We could also do something like this, in
order of priority:
* Use the alias named "Result".
* Use any alias if only a single one is in scope, otherwise:
* Use the standard library type.
This is easy to add if others feel differently that is appropriate, just let me know.
internal: Lay basic ground work for standalone mbe tests
Most of our mbe hir-def tests don't actually do anything name res relevant, we can (and should) move those down the stack into `mbe/hir-expand`.
feat: Create an assist to convert closure to freestanding fn
The assist converts all captures to parameters.
Closes#17920.
This was more work than I though, since it has to handle a bunch of edge cases...
Based on #17941. Needs to merge it first.
This avoids the need to analyze the file when we are not inside a macro call.
This is especially important for the optimization in the next commit(s), as there the common case will be to descent into macros but then not analyze.
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`.
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 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.
