Fixes a bug in #3229.
The logic to prevent a duplicate scope being created for a `Function` which is a `MethodDefinition` would also stop a scope being created for inner function in:
```rs
class X {
foo() {
function bar() {}
}
}
```
or
```rs
class X {
foo( bar = function() {} ) {}
}
```
This PR fixes that. This change also allows removing `ScopeFlags::Method` which #3229 added.
part of #3213
We should only have one diagnostic struct instead 353 copies of them, so we don't end up choking LLVM with 50k lines of the same code due to monomorphization.
If the proposed approach is good, then I'll start writing a codemod to turn all the existing structs to plain functions.
---
Background:
Using `--timings`, we see `oxc_linter` is slow on codegen (the purple part).
The crate currently contains 353 miette errors. [cargo-llvm-lines](https://github.com/dtolnay/cargo-llvm-lines) displays
```
cargo llvm-lines -p oxc_linter --lib --release
Lines Copies Function name
----- ------ -------------
830350 33438 (TOTAL)
29252 (3.5%, 3.5%) 808 (2.4%, 2.4%) <alloc::boxed::Box<T,A> as core::ops::drop::Drop>::drop
23298 (2.8%, 6.3%) 353 (1.1%, 3.5%) miette::eyreish::error::object_downcast
19062 (2.3%, 8.6%) 706 (2.1%, 5.6%) core::error::Error::type_id
12610 (1.5%, 10.1%) 65 (0.2%, 5.8%) alloc::raw_vec::RawVec<T,A>::grow_amortized
12002 (1.4%, 11.6%) 706 (2.1%, 7.9%) miette::eyreish::ptr::Own<T>::boxed
9215 (1.1%, 12.7%) 115 (0.3%, 8.2%) core::iter::traits::iterator::Iterator::try_fold
9150 (1.1%, 13.8%) 1 (0.0%, 8.2%) oxc_linter::rules::RuleEnum::read_json
8825 (1.1%, 14.9%) 353 (1.1%, 9.3%) <miette::eyreish::error::ErrorImpl<E> as core::error::Error>::source
8822 (1.1%, 15.9%) 353 (1.1%, 10.3%) miette::eyreish::error::<impl miette::eyreish::Report>::construct
8119 (1.0%, 16.9%) 353 (1.1%, 11.4%) miette::eyreish::error::object_ref
8119 (1.0%, 17.9%) 353 (1.1%, 12.5%) miette::eyreish::error::object_ref_stderr
7413 (0.9%, 18.8%) 353 (1.1%, 13.5%) <miette::eyreish::error::ErrorImpl<E> as core::fmt::Display>::fmt
7413 (0.9%, 19.7%) 353 (1.1%, 14.6%) miette::eyreish::ptr::Own<T>::new
6669 (0.8%, 20.5%) 39 (0.1%, 14.7%) alloc::raw_vec::RawVec<T,A>::try_allocate_in
6173 (0.7%, 21.2%) 353 (1.1%, 15.7%) miette::eyreish::error::<impl miette::eyreish::Report>::from_std
6027 (0.7%, 21.9%) 70 (0.2%, 16.0%) <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
6001 (0.7%, 22.7%) 353 (1.1%, 17.0%) miette::eyreish::error::object_drop
6001 (0.7%, 23.4%) 353 (1.1%, 18.1%) miette::eyreish::error::object_drop_front
5648 (0.7%, 24.1%) 353 (1.1%, 19.1%) <miette::eyreish::error::ErrorImpl<E> as core::fmt::Debug>::fmt
```
It's totalling more than 50k llvm lines, and is putting pressure on rustc codegen (the purple part on `oxc_linter` in the image above.
---
It's pretty obvious by looking at https://github.com/zkat/miette/blob/main/src/eyreish/error.rs, the generics can expand out to lots of code.
Add scope flags to `TraverseCtx`.
Closes#3189.
`walk_*` functions build a stack of `ScopeFlags` as AST is traversed, and they can be queried from within visitors with `ctx.scope()`, `ctx.ancestor_scope()` and `ctx.find_scope()`.
The codegen which generates `walk_*` functions gets the info about which AST types have scopes, and how to check for strict mode from the `#[visited_node]` attrs on AST type definitions in `oxc_ast`.
A few notes:
Each scope inherits the strict mode flag from the level before it in the stack, so if you need to know "am I in strict mode context here?", `ctx.scope().is_strict_mode()` will tell you - no need to travel back up the stack to find out.
Scopes do *not* inherit any other flags from level before it. So `ctx.scope()` in a block nested in a function will return `ScopeFlags::empty()` not `ScopeFlags::Function`.
I had to add an extra flag `ScopeFlags::Method`. The reason for this is to deal with when a `Function` is actually a `MethodDefinition`, and to avoid creating 2 scopes in this case. The principle I'm trying to follow is to encode as little logic in the codegen as possible, as it's rather hidden away. Instead the codegen follows a standard logic for every node, guided by attributes which are visible next to the types in `oxc_ast`. This hopefully makes how `Traverse`'s visitors are generated less mysterious, and easier to change.
The case of `Function` within `MethodDefinition` is a weird one and would not be possible to implement without encoding a magic "special case" within the codegen without this extra `ScopeFlags::Method` variant. Its existence does not alter the operation of any other code in Oxc which uses `ScopeFlags`.
In my view `ScopeFlags` might benefit from a little bit of an overhaul anyway. I believe we could pack more information into the bits and make it more useful.
Reorder the fields in a couple of AST types.
`walk_*` functions in `oxc_traverse` visit the fields in the order they appear in AST. So this change means that decorators are visited before classes and methods that they decorate.
Sliced off from #3152.
Re-implement `transform-react-display-name` using bottom-up lookup
provided by `Traverse` trait.
This fixes the 1 remaining failing test case for this plugin (see
#2937).
`Traverse` is not complete yet (see #3182), so this is also not ready to
merge yet.
Sliced off from #3152.
This switches the transformer over to use `Traverse` instead of
`VisitMut`.
This is incomplete - scopes are not implemented yet. At present, no
transforms use scopes anyway, so all tests pass, but regardless I don't
think should be merged until the implementation is complete.
Make the code for `retag_stack` in `walk_*` functions more
comprehensible, by replacing hard-coded enum discriminants as integers
with an `AncestorType` type.
Alternative solution to the problem raised in #3170.
close: #3170
basically:
`^#/i.test(hex)` is the same as `/^#/.test(hex)`
so, in this case the `i` flag does nothing and we can safely ignore it
inspired by https://x.com/the_moisrex/status/1787444601571221892
---
This could potentially lead a new `oxc` rule called
`no_useless_case_insensitive_regex_flag` that reports if you have a
regex with the `i` flag with no ascii alphabetic chars.
First part of #3152.
This adds a crate `oxc_traverse`, but doesn't connect it up to the
transformer or anything else yet.
I think we could merge this now - as it doesn't affect any code that's
in use - and then iterate on it to add scopes before using it in
transformer. Please see
https://github.com/oxc-project/oxc/pull/3152#issuecomment-2094965406 for
the broader picture.
### Correctness
- named
- export
- default
- namespace
- no-unresolved
The above rules will prevent bugs or broken, so I changed these to
correctness
### Suspicious
- no-duplicates
- no-named-as-default-member
- no-named-as-default
I'm still not sure whether the `no-unresolved` rule should be changed to
`correctness`. Because I found that the rule reported a lot in the
AFFiNE. But in fact, they are all correct reports
---------
Co-authored-by: Boshen <boshenc@gmail.com>
