In `AstBuilder`'s `alloc_*` methods, use `Box::new_in` instead of `.into_in`. This is more explicit, so I feel easier to understand. It may also make life easier for compiler by not requiring it to perform type coercion.
Mark everything mentioned in https://github.com/oxc-project/oxc/pull/3815#issuecomment-2186736258 as AST.
We now error on the occurrence of non-ast items in the source of truth. It doesn't make sure that all fields and variants are `#[ast]` and therefore `repr_stable` but there are only a handful of non-AST types used here(mainly Atom and Span). Since we don't have access to the external types we can't make sure of it unless we find a way to const assert it.
The best we can do until then is to check all field/variant types to be either `#[ast]` or in a white list. I can add this check to the codegen in an upcoming PR.
> This PR is (unfortunately) quite large, but all changes are needed in tandem for this to work properly.
## What This PR Does
Updates the linter to populate diagnostics reported by rules with error codes statically derived from `RuleMeta` + `RuleEnum`.
Doing so required changing how we handle vitest rules. I know @mysterven was hoping to refactor that part of the code, and I think this approach is an improvement (but could probably be cleaned up further).
## Changes
### 1. Auto-Populate Error Codes
`LintContext` now sets an error code scope + error code number for diagnostics reported by lint rules. `LintContext` will not clobber existing codes set by rules, allowing for rule-specific override behavior (e.g. to use `eslint-plugin-react-hooks` as an error scope).
In order to accomplish this, I had to update every diagnostic factory for every rule. While doing this I found some incorrect error messages, or messages that could be easily improved. This is where a large majority of the snapshot diffs come from. Additionally, I was able to reduce string allocations from `format!` usages in diagnostic factories, especially within jest rules.
### 2. Framework and Library Detection
This PR adds `FrameworkFlags`, which specify what (if any) set of libraries and frameworks are being used by a project and/or file. They are passed in two ways:
1. `LintOptions` can specify a set of `framework_hints` that apply to the entire target codebase. Right now these are always empty, but I'm thinking in the future we could sniff `package.json`. It may be helpful for enabling/disabling default rules.
2. When `Linter` gets run on a file, framework information is sniffed from the `LintContext`. Right now, we are only checking for `vitest` imports in `ModuleRecord` and test path prefixes from `source_path`. It may be useful to do something similar for React/NextJS rules in the future. I know that [next/no-html-link-for-pages](https://nextjs.org/docs/messages/no-html-link-for-pages) could benefit greatly from this.
Add `#[inline]` to empty default implementations of `enter_node` etc. Hopefully compiler will inline them automatically within Oxc even cross-crate because we compile with LTO, but maybe not for external consumers who don't use LTO.
close: #3969close: #2023
We need to add scope according to [this](d8086f14b6/src/compiler/binder.ts (L3883-L3962)). There are still some ASTs that need to be added to the scope.
---
Context from @Boshen:
Before this whole journey of fixing symbols and scopes I asked @Dunqing to debug through binder.ts via a debugger to fully understand how it does resolution.
We then agreed to align the implementation so that when a problem occurs, we can debug through both implementations and find where our problem is.
tsc doesn't have a specification, so we need to align with the reference implementation instead.
We have a strange workaround for `visit_function` where we pass in `ScopeFlags`, to support creating the scope inside `Function`, but setting different flags for `MethodDefinition`s.
Previously `visit_function` took `Option<ScopeFlags>` and then did `flags.unwrap_or(ScopeFlags::empty()) | ScopeFlags::Function` to it. Personally, I found this confusing. When I was looking at `MethodDefinition`, I was wondering "It's a function, why doesn't it set Function flag too?"
This changes makes it more explicit and clear what `ScopeFlags` everything has.
related: https://github.com/oxc-project/oxc/issues/4142#issuecomment-2219125356
Although we called `enter_node(Class)`, that doesn't mean we're in the `class` scope. It causes our must to visit decorators before `enter_node`.
Let's look at this case. It causes a syntax error if we don't visit decorators before `enter_node`
```js
// This file was procedurally generated from the following sources:
// - src/decorator/decorator-call-expr-identifier-reference-yield.case
// - src/decorator/syntax/valid/cls-expr-decorators-valid-syntax.template
/*---
description: Decorator @ DecoratorCallExpression (Valid syntax for decorator on class expression)
esid: prod-ClassExpression
features: [class, decorators]
flags: [generated, noStrict]
info: |
ClassExpression[Yield, Await] :
DecoratorList[?Yield, ?Await]opt class BindingIdentifier[?Yield, ?Await]opt ClassTail[?Yield, ?Await]
DecoratorList[Yield, Await] :
DecoratorList[?Yield, ?Await]opt Decorator[?Yield, ?Await]
Decorator[Yield, Await] :
@ DecoratorMemberExpression[?Yield, ?Await]
@ DecoratorParenthesizedExpression[?Yield, ?Await]
@ DecoratorCallExpression[?Yield, ?Await]
...
DecoratorCallExpression[Yield, Await] :
DecoratorMemberExpression[?Yield, ?Await] Arguments[?Yield, ?Await]
DecoratorMemberExpression[Yield, Await] :
IdentifierReference[?Yield, ?Await]
DecoratorMemberExpression[?Yield, ?Await] . IdentifierName
DecoratorMemberExpression[?Yield, ?Await] . PrivateIdentifier
IdentifierReference[Yield, Await] :
[~Yield] yield
...
---*/
function decorator() {
return () => {};
}
var yield = decorator;
var C = @yield() class {};
```
Errors:
```shell
× The keyword 'yield' is reserved
╭─[language/statements/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js:45:2]
44 │
45 │ @yield() class C {}
· ─────
╰────
```
The changed code makes more sense. Only if we call `enter_scope` for class, the flags will contain `StrictMode`. Also, we can get the exact `flags` of the `scope` in the `class` at the transformer
For example:
```
class A {
B() {
// Before: flags is `Function`
// After: flags is `Function | StrictMode`
}
}
```
The regression tests will be fixed in follow-up PRs
```ts
class Klass <T> extends Root <R> {}
// ^^^^^ ^^^ ^^^^ ^^^ ^^
// id type_paramter super_class super_type_parameters body
```
I reorder fields according to the order above
The class scope is not defined in the spec. But we need to create a scope for `class` to store `TypeParamters`
I'm going to be AFK today(till about 9 PM UTC). Meanwhile, I Didn't want to be a blocker so here we go.
It would fix the #4200 merge if you guys find it in the correct order otherwise feel free to close it.
close: #4186
CatchClause has two scopes. The first one is `CatchClause`, which will add a `CatchParameter` to it. The second one is `Block`, which will add binding that declares in the current block scope.
The spec has a syntax error about `CatchParameter`
- It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the LexicallyDeclaredNames of Block.
