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
It changes behaviour and can cause collisions. E.g. for the following snippet
```rs
mod foo {
pub mod bar {}
pub const bar: i32 = 8;
}
// tranforming 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
minor : Fix duplicate snippets showing up on hover.
With each `config::apply_change` duplicate configs were being added.
Now we first drain the vec that holds these and then start adding. This fixes#17485
internal: Some more small memory optimizations
Not a big impact on metrics, though there are some more savings in queries mainly used by the IDE layer from this
fix: Improve hover text in unlinked file diagnostics
Use full sentences, and mention how to disable the diagnostic if users are intentionally working on unowned files.
![Screenshot 2024-06-12 at 5 55 48 PM](https://github.com/rust-lang/rust-analyzer/assets/70800/c91ee1ed-1c72-495a-9ee3-9e360a5c6977)
(Full disclosure: I've tested a rust-analyzer build in VS Code, but the pop-up logic is currently disabled due to #17062, so I haven't tested that.)
fix: pattern completions in let-stmt
fix#17480.
We can write `let S { a, b } = s;` or `let Some(x) = a else {}`, so it is reasonable to allow pattern completions in `LetStmt`.
Simplify some term search tactics
Working on the paper `@phijor` found that "Data constructor" tactic could be simplified quite a bit by running it only in the backwards direction. With n+1 rounds it has same coverage as previous implementation in n rounds, however the tactic it self is more simple and also potentially faster as there is less to do.
In a nutshell the idea is to only work with types in the wish-list rather than with any types.
Turns out it is quite a bit faster:
Before:
```
ripgrep:
Tail Expr syntactic hits: 238/1692 (14%)
Tail Exprs found: 1223/1692 (72%)
Term search avg time: 15ms
nalgebra:
Tail Expr syntactic hits: 125/3001 (4%)
Tail Exprs found: 2143/3001 (71%)
Term search avg time: 849ms
```
After
````
ripgrep:
Tail Expr syntactic hits: 246/1692 (14%)
Tail Exprs found: 1209/1692 (71%)
Term search avg time: 8ms
nalgebra:
Tail Expr syntactic hits: 125/3001 (4%)
Tail Exprs found: 2028/3001 (67%)
Term search avg time: 305ms
````
_Also removed niche optimization of removing scope defs from the search space as this wasn't helping much anyway and made code a bit more complex._
fix: use ItemInNs::Macros to convert ModuleItem to ItemInNs
fix#17425.
When converting `PathResolution` to `ItemInNs`, we should convert `ModuleDef::Macro` to `ItemInNs::Macros` to ensure that it can be found in `DefMap`.
Filter builtin macro expansion
This PR adds a filter on the types of built in macros that are allowed to be expanded.
Currently, This list of allowed macros contains, `stringify, cfg, core_panic, std_panic, concat, concat_bytes, include, include_str, include_bytes, env` and `option_env`.
Fixes#14177
fix: ensure there are no cycles in the source_root_parent_map
See #17409
We can view the connections between roots as a graph. The problem is that this graph may contain cycles, so when adding edges, it is necessary to check whether it will lead to a cycle.
Since we ensure that each node has at most one outgoing edge (because each SourceRoot can have only one parent), we can use a disjoint-set to maintain the connectivity between nodes. If an edge’s two nodes belong to the same set, they are already connected.
Additionally, this PR includes the following three changes:
1. Removed the workaround from #17409.
2. Added an optimization: If `map.contains_key(&SourceRootId(*root_id as u32))`, we can skip the current loop iteration since we have already found its parent.
3. Modified the inner loop to iterate in reverse order with `roots[..idx].iter().rev()` at line 319. This ensures that if we are looking for the parent of `a/b/c`, and both `a` and `a/b` meet the criteria, we will choose the longer match (`a/b`).
Term search: new tactic for associated item constants
New tactic to cover some more exotic cases that started bothering me.
Associated constants seem to be common in [axum](806bc26e62/examples/readme/src/main.rs (L53)).
feat: add space after specific keywords in completion
fix#17428.
When completing some specific keywords, it would be convenient if r-a could automatically add a space afterwards.
This PR implements this feature for the following keywords:
- Visibility: `pub`, `pub(crate)`, `pub(super)`, `pub(in xxx)`
- Pattern: `ref` / `mut`
- Others: `unsafe` / `for` / `where`
fix: handle character boundaries for wide chars in extend_selection
fix#17420.
When calling 'extend_selection' within a string, r-a attempts to locate the current word at the cursor. This is done by finding the first char before the cursor which is not a letter, digit, or underscore.
The position of this character is referred to as `start_idx`, and the word is considered to start from `start_idx + 1`. However, for wide characters, `start_idx + 1` is not character boundaries, which leading to panic. We should use `ceil_char_boundary` to ensure that the idx is always on character boundaries.
fix: Only show unlinked-file diagnostic on first line during startup
This partially reverts #17350, based on the feedback in #17397.
If we don't have an autofix, it's more annoying to highlight the whole file. This autofix heuristic fixes the diagnostic being overwhelming during startup.
docs: document omission heuristics for parameter inlay hints
These are not currently documented and could cause users to think that their rust-analyzer configuration is broken.
Partially addresses #17433.
refactor: Prefer plain trait definitions over macros for impl_intern_value_trivial
`impl_intern_value_trivial` can be defined with a trait directly, so prefer that over a macro definition.
This partially reverts #17350, based on the feedback in #17397.
If we don't have an autofix, it's more annoying to highlight the whole line.
This heuristic fixes the diagnostic overwhelming the user during startup.
Allow rust-project.json to include arbitrary shell commands for runnables
This is a follow-up on #16135, resolving the feedback raised :)
Allow rust-project.json to include shell runnables, of the form:
```
{
"build_info": {
"label": "//project/foo:my-crate",
"target_kind": "bin",
"shell_runnables": [
{
"kind": "run",
"program": "buck2",
"args": ["run", "//project/foo:my-crate"]
},
{
"kind": "test_one",
"program": "test_runner",
"args": ["--name=$$TEST_NAME$$"]
}
]
}
}
```
If these runnable configs are present for the current crate in rust-project.json, offer them as runnables in VS Code.
This PR required some boring changes to APIs that previously only handled cargo situations. I've split out these changes as commits labelled 'refactor', so it's easy to see the interesting changes.
feat: show type bounds from containers when hovering on functions
fix#12917.
### Changes
1. Added Support for displaying the container and type bounds from it when hovering on functions with generic types.
2. Added a user config to determine whether to display container bounds (enabled by default).
3. Added regression tests.
4. Simplified and refactored `hir/display.rs` to improve readability.
fix: ensure that the parent of a SourceRoot cannot be itself
fix#17378.
In `FileSetConfig.map`, different roots might be mapped to the same `root_id` due to deduplication in `ProjectFolders::new`:
```rust
// Example from rustup
/Users/roife/code/rustup/target/debug/build/rustup-863a063426b56c51/out
/Users/roife/code/rustup
```
In `source_root_parent_map`, r-a might encounter paths where their SourceRootId (i.e. `root_id`) is identical, yet one the them is the parent of the another. This situation can cause the `root_id` to be its own parent, potentially leading to an infinite loop.
This PR resolves such cases by adding a check.
feat: TOML based config for rust-analyzer
> Important
>
> We don't promise _**any**_ stability with this feature yet, any configs exposed may be removed again, the grouping may change etc.
# TOML Based Config for RA
This PR ( addresses #13529 and this is a follow-up PR on #16639 ) makes rust-analyzer configurable by configuration files called `rust-analyzer.toml`. Files **must** be named `rust-analyzer.toml`. There is not a strict rule regarding where the files should be placed, but it is recommended to put them near a file that triggers server to start (i.e., `Cargo.{toml,lock}`, `rust-project.json`).
## Configuration Types
Previous configuration keys are now split into three different classes.
1. Client keys: These keys only make sense when set by the client (e.g., by setting them in `settings.json` in VSCode). They are but a small portion of this list. One such example is `rust_analyzer.files_watcher`, based on which either the client or the server will be responsible for watching for changes made to project files.
2. Global keys: These keys apply to the entire workspace and can only be set on the very top layers of the hierarchy. The next section gives instructions on which layers these are.
3. Local keys: Keys that can be changed for each crate if desired.
### How Am I Supposed To Know If A Config Is Gl/Loc/Cl ?
#17101
## Configuration Hierarchy
There are 5 levels in the configuration hierarchy. When a key is searched for, it is searched in a bottom-up depth-first fashion.
### Default Configuration
**Scope**: Global, Local, and Client
This is a hard-coded set of configurations. When a configuration key could not be found, then its default value applies.
### User configuration
**Scope**: Global, Local
If you want your configurations to apply to **every** project you have, you can do so by setting them in your `$CONFIG_DIR/rust-analyzer/rust-analyzer.toml` file, where `$CONFIG_DIR` is :
| Platform | Value | Example |
| ------- | ------------------------------------- | ---------------------------------------- |
| Linux | `$XDG_CONFIG_HOME` or `$HOME`/.config | /home/alice/.config |
| macOS | `$HOME`/Library/Application Support | /Users/Alice/Library/Application Support |
| Windows | `{FOLDERID_RoamingAppData}` | C:\Users\Alice\AppData\Roaming |
### Client configuration
**Scope**: Global, Local, and Client
Previously, the only way to configure rust-analyzer was to configure it from the settings of the Client you are using. This level corresponds to that.
> With this PR, you don't need to port anything to benefit from new features. You can continue to use your old settings as they are.
### Workspace Root Configuration
**Scope**: Global, Local
Rust-analyzer already used the path of the workspace you opened in your Client. We used this information to create a configuration file that won't affect your other projects and define global level configurations at the same time.
### Local Configuration
**Scope**: Local
You can also configure rust-analyzer on a crate level. Although it is not an error to define global ( or client ) level keys in such files, they won't be taken into consideration by the server. Defined local keys will affect the crate in which they are defined and crate's descendants. Internally, a Rust project is split into what we call `SourceRoot`s. This, although with exceptions, is equal to splitting a project into crates.
> You may choose to have more than one `rust-analyzer.toml` files within a `SourceRoot`, but among them, the one closer to the project root will be
Add preference modifier for workspace-local crates when using auto import.
`@joshka` pointed out some odd behavior of auto import ordering. It doesn't seem that the current heuristics were applying any sort of precedence to imports from the workspace. I've went ahead and added that.
I hope to get some feedback on the modifier numbers here. I just went with something that felt like it balanced giving more power to workspace crates without completely ignoring relative path distance.
closes https://github.com/rust-lang/rust-analyzer/issues/17303
fix: do not resolve prelude within block modules
fix#17338 (continuing from #17251).
In #17251, we injected preludes into non-top-level modules, which leading to r-a to directly resolve names in preludes in block modules. This PR fix it by checking whether the module is a pseudo-module introduced by blocks. (similar to what we do for extern preludes)
Changed package.json so vscode extension settings have submenus
There are a lot of options that are a part of rust-analyzer, sometimes it can be hard to find an option that you are looking for. To fix this I have put all configurations into categories based on their names. I have also changed the schema in `crates/rust-analyzer/src/config.rs` to reflect this.
Currently for each generated entry the title is redeclared, this does function but I am prepared to change this if it is a problem.
Currently, rust-analyzer highlights the entire region when a `cfg` is
inactive (e.g. `#[cfg(windows)]` on a Linux machine). However,
unlinked files only highlight the first three characters of the file.
This was introduced in #8444, but users have repeatedly found
themselves with no rust-analyzer support for a file and unsure
why (see e.g. #13226 and the intentionally prominent pop-up added in
PR #14366).
(Anecdotally, we see this issue bite our users regularly, particularly
people new to Rust.)
Instead, highlight the entire inactive file, but mark it as all as
unused. This allows users to hover and run the quickfix from any line.
Whilst this is marginally more prominent, it's less invasive than a
pop-up, and users do want to know why they're getting no rust-analyzer
support in certain files.
Feat: hide double underscored symbols from symbol search
Fixes#17272 by changing the default behavior of query to skip results that start with `__` (two underscores).
Not sure if this has any far reaching implications - a review would help to understand if this is the right place to do the filtering, and if it's fine to do it by default on the query.
If you type `__` as your search, then we'll show the matching double unders, just in case you actually need the symbol.
Don't mark `#[rustc_deprecated_safe_2024]` functions as unsafe
`std::env::set_var` will be unsafe in edition 2024, but not before it. I couldn't quite figure out how to check for the span properly, so for now we just turn the false positives into false negatives, which are less bad.
Add `Function::fn_ptr_type(…)` for obtaining name-erased function type
The use case of this function if being able to group functions by their function ptr type.
cc `@flodiebold`
fix: Only generate snippets for `extract_expressions_from_format_string` if snippets are supported
Part of #17332
Fixes `extract_expressions_from_format_string` so that it doesn't generate snippets if the client doesn't support it.
`std::env::set_var` will be unsafe in edition 2024, but not before it.
I couldn't quite figure out how to check for the span properly, so for now
we just turn the false positives into false negatives, which are less bad.
fix diagnostics clearing when flychecks run per-workspace
This might be causing #17300 or it's a different bug with the same functionality.
I wonder if the decision to clear diagnostics should stay in the main loop or maybe the flycheck itself should track it and tell the mainloop?
I have used a hash map but we could just as well use a vector since the IDs are `usizes` in some given range starting at 0. It would be probably faster but this just felt a bit cleaner and it allows us to change the ID to newtype later and we can just use a hasher that returns the underlying integer.
Add `toggle_async_sugar` assist code action
Implement code action for sugaring and de-sugaring asynchronous functions.
This code action does not import `Future` trait when de-sugaring and does not touch function boby, I guess this can be implemented later if needed. This action also does not take into consideration other bounds because IMO it's usually "let me try to use sugared version here".
Feel free to request changes, that's my first code action implementation 😄Closes#17010
Relates to #16195
Implement assist to switch between doc and normal comments
Hey first PR to rust-analyzer to get my feet wet with the code base. It's an assist to switch a normal comment to a doc comment and back, something I've found myself doing by hand a couple of times.
I shamelessly stole `relevant_line_comments` from `convert_comment_block`, because I didn't see any inter-assist imports happening in the files I peeked at so I thought this would be preferable.
Fix `data_constructor` ignoring generics for struct
Previously didn't work for structs with generics due to `field.ty()` having placeholders in type.
_Enums were handeled correctly already._
Also renamed `type_constructor -> data_constructor` as this is more correct name for it
Allow sysroots to only consist of the source root dir
Fixes https://github.com/rust-lang/rust-analyzer/issues/17159
This PR encodes the `None` case of an optional sysroot into `Sysroot` itself. This simplifies a lot of things and allows us to have sysroots that consist of nothing, only standard library sources, everything but the standard library sources or everything. This makes things a lot more flexible. Additionally, this removes the workspace status bar info again, as it turns out that that can be too much information for the status bar to handle (this is better rendered somewhere else, like in the status view).
Fix: infer type of async block with tail return expr
Fixes#17106
The `infer_async_block` method calls the `infer_block` method internally, which returns the never type without coercion when `tail_expr` is `None` and `ctx.diverges` is `Diverges::Always`.This is the reason for the bug in this issue.
cfce2bb46d/crates/hir-ty/src/infer/expr.rs (L1411-L1413)
This PR solves the bug by adding a process to coerce after calling `infer_block` method.
This code passes all the tests, including tests I added for this isuue, however, I am not sure if this solution is right. I think that this solution is an ad hoc solution. So, I would appreciate to have your review.
I apologize if I'm off the mark, but `infer_async_block` method should be rewritten to share code with the process of infering type of `expr::Closure` instead of the `infer_block` method. That way it will be closer to the infer process of rustc.
handle {self} when removing unused imports
Fixes#17139
On master
```rs
mod inner {
pub struct X();
pub struct Y();
}
mod z {
use super::inner::{self, X}$0;
fn f() {
let y = inner::Y();
}
}
```
becomes
```rs
mod inner {
pub struct X();
pub struct Y();
}
mod z {
use super::inner:self;
fn f() {
let y = inner::Y();
}
}
```
with this fix it instead becomes
```
```rs
mod inner {
pub struct X();
pub struct Y();
}
mod z {
use super::inner;
fn f() {
let y = inner::Y();
}
}
```
fix: ensure implied bounds from associated types are considered in autocomplete
closes: #16989
rust-analyzer needs to consider implied bounds from associated types in order to get all methods suggestions people expect. A pretty easy way to do that is to keep the `candidate_trait_id`'s receiver if it matches `TyFingerprint::Unnameable`. When benchmarking this change, I didn't notice a meaningful difference in autocomplete latency.
(`TyFingerprint::Unnameable` corresponds to `TyKind::AssociatedType`, `TyKind::OpaqueType`, `TyKind::FnDef`, `TyKind::Closure`, `TyKind::Coroutine`, and `TyKind::CoroutineWitness`.)
Clear diagnostics only after new ones were received
Closes#15934
This adds a flag inside the global state which controls when old diagnostics are cleared. Now, old diagnostics should be cleared only after at least one new diagnostic is available.
feat: More callable info
With this PR we retain more info about callables other than functions, allowing for closure parameter type inlay hints to be linkable as well as better signature help around closures and `Fn*` implementors.
Fix OOM caused by term search
The issue came from multi Cartesian product for exprs with many (25+) arguments, each having multiple options.
The solution is two fold:
### Avoid blowing up in Cartesian product
**Before the logic was:**
1. Find expressions for each argument/param - there may be many
2. Take the Cartesian product (which blows up in some cases)
4. If there are more than 2 options throw them away by squashing them to `Many`
**Now the logic is:**
1. Find expressions for each argument/param and squash them to `Many` if there are more than 2 as otherwise we are guaranteed to also have more than 2 after taking the product which means squashing them anyway.
2. Take the Cartesian product on iterator
3. Start consuming it one by one
4. If there are more than 2 options throw them away by squashing them to `Many` (same as before)
This is also why I had to update some tests as the expressions get squashed to many more eagerly.
### Use fuel to avoid long search times and high memory usage
Now all the tactics use `should_continue: Fn() -> bool` to chech if they should keep iterating _(Similarly to chalk)_.
This reduces the search times by a magnitude, for example from ~139ms/hole to ~14ms/hole for `ripgrep` crate.
There are slightly less expressions found, but I think speed gain worth it for usability.
Also note that syntactic hits decreases more because of squashing so you simple need to run search multiple times to get full terms.
Also the worst case time (For example `nalgebra` crate cus it has tons of generics) has search times mostly under 200ms.
Benchmarks on `ripgrep` crate
Before:
```
Tail Expr syntactic hits: 291/1692 (17%)
Tail Exprs found: 1253/1692 (74%)
Term search avg time: 139ms
````
After:
```
Tail Expr syntactic hits: 239/1692 (14%)
Tail Exprs found: 1226/1692 (72%)
Term search avg time: 14ms
```
fix: keep parentheses when the precedence of inner expr is lower than the outer one
fix#17185
Additionally, this PR simplifies some code in `apply_demorgan`.