`SymbolFlags::ArrowFunction` is an oddity, as whether a symbol is an arrow function is not statically knowable. In the following cases, `f` symbol did not have `ArrowFunction` flag set:
```js
const {f} = {f: () => {}};
```
```js
let f = 123;
f = () => {};
```
`SymbolFlags::ArrowFunction` is therefore not particularly useful, and possibly misleading. Having it complicates the transformer, and it's not used anywhere in Oxc.
This PR removes it.
I am unable to print all comments correctly. Comments have way too much semantic meaning in JavaScript.
This PR reduces the scope to only print jsdoc comments that are attached to statements and class elements, in order to get isolated declarations shipped.
Previously it included a newline in the value
```
"hashbang": {
"type": "Hashbang",
"start": 0,
"end": 16,
"value": "/usr/bin/node\n"
},
```
This change will also make the lexer emit a `\n` token, which will make comment position detection correct.
`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.
