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.
> 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.
