`Codegen::with_source_text` gives us enough information to use `Codegen::with_capacity`. This change makes the API cleaner as users have to provide less redundant information.
Micro-optimization. Rather than `inside_arrow_function_stack` sometimes being run down to empty, initialize it with a single entry which is never popped off. This means that `self.inside_arrow_function_stack.last()` *always* returns `Some` - so the branch is completely predictable.
Push/pop to stacks in matching `enter_*` / `exit_*` visitors.
The reason why there was a bug here is a little bit subtle.
Between `enter_expression` and `exit_expression`, another transform can replace the `Expression` with something else. Ditto `enter_declaration` + `exit_declaration`.
So pushing+popping from stacks in `enter_expression` + `exit_expression` can make the stack get out of sync. e.g.:
```rs
impl<'a> Traverse for TransformerWithStack {
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: TraverseCtx<'a>) {
if let Expression::FunctionExpression(_) = expr {
self.stack.push(true);
}
}
fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: TraverseCtx<'a>) {
if let Expression::FunctionExpression(_) = expr {
self.stack.pop();
}
}
}
// Transformer that replaces `null` with a function expression (!)
impl<'a> Traverse for SomeOtherTransformer {
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: TraverseCtx<'a>) {
if let Expression::NullLiteral(_) = expr {
*expr = ctx.ast.expression_function( /* ... */ );
}
}
}
```
`TransformerWithStack` is assuming in `exit_expression` that it previously saw a `FunctionExpression` in `enter_expression`. But `SomeOtherTransformer` has created a *new* `FunctionExpression` after `TransformerWithStack::enter_expression` ran. So in `TransformerWithStack`, `exit_expression` is called 1 more time than `enter_expression`. When `exit_expression` calls `self.stack.pop()`, `self.stack` is empty, and it panics.
This example is silly, but real cases of this do exist. e.g. TS transformer creates a new functions when transforming `enum`s.
`enter_function` + `exit_function` / `enter_arrow_function_expression` + `exit_arrow_function_expression` not have this problem. As we cannot mutate upwards, there are always the same number of calls to `enter_*` and `exit_*` (same for all `enter_*` / `exit_*` pairs which operate on a struct, *not an enum*).
Use `unwrap()` on `scope_id` field of `ArrowFunctionExpression`. Previously we had errors in other transforms which did not set `scope_id` correctly, but this appears to now be fixed.
Wrapping a function expression in parentheses is unnecessary. Codegen already adds parentheses when printing a function expression as a statement expression. i.e. `(function() {});`.
Cleans diagnostic messages and help texts for (almost) all remaining rules.
There were two rules I couldn' find good replacements for.
I also made some logical improvements to a few rules, and added `pending` fixers
to others.
This rule is enabled by default in typescript-eslint's recommended config, so we
should enable it by default too. I checked this change with `oxlint-ecosystem-ci` locally and saw no problems.
This PR also improves diagnostic messages for this rule.
## What This PR Does
Each time a lint rule is run on a file, it will now store diagnostics in a
shared vec instead of having its own list. This is done by moving `diagnostics`
from `LintContext` to `ContextHost`.
The motivating line of code can be found in `Linter::run` in
`oxc_linter/src/lib.rs#145`
```rust
rules.into_iter().flat_map(|(_, ctx)| ctx.into_message()).collect::<Vec<_>>()
```
Each `LintContext` allocates a new vec, and pushes diagnostics into it. After
all run-related methods have been run, a new vec is created and those
diagnostics are copied into it. This is `O(n+1)` allocations and `O(n)` copies,
not to mention that allocations for the original vecs cannot be re-used since
those vecs aren't dropped until after the new one is created.
- Part of https://github.com/oxc-project/oxc/issues/1022
This implements the [recommended rule
`no-danger-with-children`](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-danger-with-children.md)
from the `eslint-plugin-react` package.
This rule proved to be a little bit trickier to implement than I
anticipated, as it required searching up through all of the scopes
recursively in order to resolve object properties. This would be easier
with something like a type-check system, but this will work for now.
(Sidenote: this is my first real attempt at Rust programming in ~5
years, so any feedback on making this more idiomatic is welcome.)
`UniquePromise::new_for_tests` is not used in tests, so produces a dead code warning when running tests. Prevent that.
Also, rename it to `new_for_benchmarks`, since that's where it's used.
> Related to #5770
## What This PR Does
Moves state that is constant over a linted file out of `LintContext` and into a shared `ContextHost` struct, turning `LintContext` into a [flyweight](https://en.wikipedia.org/wiki/Flyweight_pattern).
This brings `LintContext` from 144 bytes down to 96. `Linter::run` iterates over `(rule, ctx)` pairs in a very tight loop, and each rule instance gets its own clone of `ctx`. A smaller `LintContext` means better cache locality and a smaller heap allocation for this vec. I'm hoping to eventually get it small enough to fit in a single cache line.
I made a PR a while ago that did something similar to this one, but instead of using an `Rc`, each `LintContext` stored a read-only reference. This added an extra lifetime parameter, making it slightly unwieldy, but I saw up to 15%/25% perf improvements on local benchmarks. I'll dig around for it and add a link shortly.
### Architecture
----
This is to fix the cases mentioned in the comment section of #5365.
In short, it will report these as PASS test cases:
```js
let inner;
function foo1() {
inner = function() {}
}
function foo2() {
inner = function() {}
}
```
```js
let inner;
function outer() {
inner = function() {}
}
```
And report these below as FAIL test cases:
```js
let inner;
function foo1() {
inner = function smth() {}
}
function foo2() {
inner = function bar() {}
}
```
```js
let inner;
function outer() {
inner = function inner() {}
}
```
The case below was already done in #5312 but mentioned in #5365. It is a
FAIL case as well:
```js
function outer() {
const inner = function inner() {}
}
```
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Don Isaac <donald.isaac@gmail.com>
Pure refactor. Split implementation of `#[ast]` macro into multiple files. This means each file works with a single version of `TokenStream`, rather than having `proc_macro::TokenStream` and `proc_macro2::TokenStream` both in use in a single file.
The last failed test is only because of the comments. the oxc prettier
output is:
```
enum Direction {
Up = 1,
Down,
Left,
Right,
}
enum FileAccess {
None,
Read = // constant members
1 << 1,
Write = 1 << 2,
ReadWrite = Read | Write,
G = // computed member
"123".length,
}
enum Empty {}
const enum Enum {
A = 1,
B = A * 2,
}
```
Expected output:
aa3853b776/tests/format/typescript/enum/__snapshots__/format.test.js.snap (L91-L99)
Hope you can tell more why this happens :)
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
