Beginning of #6347. Instead of using serde-derive, we generate
`Serialize` impls manually.
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
`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.
As of now if we remove the implementation of a trait for a type and implement the method on that type directly it wouldn't break while it isn't the original trait anymore so that method might do something entirely different.
This change is more explicit on trait calls so we hit compile errors on these kinds of changes.
So far, the `ReferenceFlags::TSTypeQuery` only used indicates it is referenced by `TSTypeQuery` that we can confirm the reference should be regarded as a type reference, namely `ReferenceFlags::Type`.
This PR adds a `ReferenceFlags::ValueAsType` instead of `ReferenceFlags::TSTypeQuery`. The new flag has the same behavior as the previous one. But it looks more general and is not only used in `TSTypeQuery`. But now it is a temporary flag. We use it to resolve the symbol correctly and replace `ReferenceFlags::ValueAsTyoe` with `ReferenceFlags::Type` after resolved.
Also, this change eliminates the inconsistency in behavior between the `Reference::is_type` and `ReferenceFlags::is_type` methods.
Follow-on after #5425. Further optimize `oxc_syntax::identifier::is_identifier_name` by processing string in blocks of 8 bytes, and checking if all bytes in a block are ASCII in one go, rather than testing each byte individually.
Optimize `oxc_syntax::identifier::is_identifier_name`. Add a fast path for ASCII, which will be the common case. Only fall back to iterating over `char`s and using the more expensive test functions e.g. `is_identifier_start_unicode` if non-ASCII chars are found.
Removes the temporary `CloneIn` derive macro used for "foreign" types - as in types outside of the `oxc_ast` crate - since now we have support for multiple derive outputs - one per each crate - that allows us to use `generate_derive` across multiple crates.
There are many cases in lint rules where we want to see if a symbol is a
function by checking its SymbolFlags. This is currently not fully possible,
since variables assigned to arrow functions are not distinguished from any other
kind of variable. This PR adds `SymbolFlags::ArrowFunction` for variables that
are initialized to arrow functions. Symbols that are re-assigned to arrow
functions will not have this flag, but this is acceptable for lint rules.
Add `#[inline]` to trivial bitflags methods. Very likely this makes no difference within Oxc, as we compile with LTO enabled, but for external consumers of Oxc who don't use LTO, this will enable cross-crate inlining.
Similar to #4375. #4694 added `#[ast]` attribute to types in `oxc_syntax`, so these types can use `#[serde]` attrs without the `#[cfg_attr(feature = "serialize", ...)]` guard.
Panic if try to create an `AstNodeId`, `ReferenceId`, `ScopeId` or `SymbolId` from a `usize` which can't be stored as a `u32`.
Previously we checked for `u32::MAX`, but didn't check for numbers larger than that.
> Re-creation of #4427 due to rebasing issues. Original attempt: #642
-----
Third time's the charm?
Each time I attempt this rule, I find a bunch of bugs in `Semantic`, and I expect this attempt to be no different. Expect sidecar issues+PRs stemming from this PR here.
## Not Supported
These are cases supported in the original eslint rule, but that I'm intentionally deciding not to support
- export comments in scripts
```js
/* exported a */ var a;
```
- global comments
```js
/* global a */ var a;
```
## Behavior Changes
These are intentional deviations from the original rule's behavior:
- logical re-assignments are not considered usages
```js
// passes in eslint/no-unused-vars, fails in this implementation
let a = 0; a ||= 1;
let b = 0; b &&= 2;
let c = undefined; c ??= []
```
## Known Limitations
- Lint rules do not have babel or tsconfig information, meaning we can't determine if `React` imports are being used or not. The relevant tsconfig settings here are `jsx`, `jsxPragma`, and `jsxFragmentName`. To accommodate this, all imports to symbols named `React` or `h` are ignored in JSX files.
- References to symbols used in JSDoc `{@link}` tags are not created, so symbols that are only used in doc comments will be reported as unused. See: #4443
- `.vue` files are skipped completely, since variables can be used in templates in ways we cannot detect
> note: `.d.ts` files are skipped as well.
## Todo
- [x] Skip unused TS enum members on used enums
- [x] Skip unused parameters followed by used variables in object/array spreads
- [x] Re-assignments to array/object spreads do not respect `destructuredArrayIgnorePattern` (related to: https://github.com/oxc-project/oxc/issues/4435)
- [x] #4493
- [x] References inside a nested scope are not considered usages (#4447)
- [x] Port over typescript-eslint test cases _(wip, they've been copied and I'm slowly enabling them)_
- [x] Handle constructor properties
```ts
class Foo {
constructor(public a) {} // `a` should be allowed
}
```
- [x] Read references in sequence expressions (that are not in the last position) should not count as a usage
```js
let a = 0; let b = (a++, 0); console.log(b)
```
> Honestly, is anyone even writing code like this?
- [x] function overload signatures should not be reported
- [x] Named functions returned from other functions get incorrectly reported as unused (found by @camc314)
```js
function foo() {
return function bar() { }
}
Foo()()
```
- [x] false positive for TS modules within ambient modules
```ts
declare global {
// incorrectly marked as unused
namespace jest { }
}
```
## Blockers
- https://github.com/oxc-project/oxc/issues/4436
- https://github.com/oxc-project/oxc/issues/4437
- #4446
- #4447
- #4494
- #4495
## Non-Blocking Issues
- #4443
- #4475 (prevents checks on exported symbols from namespaces)
It may be a problem, but doesn't matter previously.
Formerly, the `is_reserved_keyword_or_global_object` is
`is_reserved_keyword(s) || is_reserved_keyword(s)`. I think it should be
`is_reserved_keyword(s) || is_global_object(s)` according to its name.
Also, the `.idea` may be because I am using RustRover, which may
automatically create `.idea` folder. So I ignore it in `.gitignore`.
I think I can contribute to `oxc` more when I am free.
Make `AstNodeId` a type with a niche, using `NonMaxU32` as its internal storage. This makes `Option<AstNodeId>` 4 bytes instead of 8. That halves the size of the `Vec` for parent IDs in `AstNodes` (which gets pretty big).
Make `ScopeId` a type with a niche, like `SymbolId` and `ReferenceId`. This makes `Option<ScopeId>` 4 bytes instead of 8, and shrinks various AST types e.g. `ArrowFunctionExpression` by 8 bytes, and halves the size of the `Vec` in `ScopeTree::parent_ids`.
The snapshot change on `prefer-hooks-in-order` lint rule appears incidental - it doesn't alter what errors are reported, only the order they're reported in. This appears to be because it changes the order of keys in a hashmap keyed by `ScopeId` that [the rule uses](a49f4915de/crates/oxc_linter/src/rules/jest/prefer_hooks_in_order.rs (L143)).
`SymbolId` and `ReferenceId` are stored as `NonZeroU32`, but with a wrapper to make `u32::MAX` the illegal value, instead of `0`.
Use the existing `nonmax` crate for this. Our current implementation uses `idx + 1` to avoid the zero value, whereas `nonmax` crate uses XOR `idx ^ u32::MAX`, which is a cheaper operation.
Initially I made this change manually instead of pulling in a dependency, but it's a pain because it requires implementing `Debug` and `PartialOrd` by hand to handle the difference between the "actual" value and its stored representation. So I thought better to use a crate which does this for us.
`NonZeroU32::new_unchecked(idx as u32 + 1)` is unsound because if `idx == u32::MAX`, `idx + 1` wraps around back to zero. So unfortunately we need to use the checked version `NonZeroU32::new(idx as u32 + 1).unwrap()` to avoid UB in this edge case.
Most symbols don't have redeclarations.
So instead of storing `Vec<Span>` directly in `redeclare_variables` (24 bytes per symbol), store `Option<RedeclarationId>` (4 bytes).
`RedeclarationId` indexes into `redeclarations` where the actual `Vec<Span>` is stored. But for symbols with no redeclarations (the vast majority), it takes 4 bytes per symbol only.
```ts
type A = any;
const B = 0;
export { A, B }
^^^^^^^^ ExportSpecifiers
export { A }
^^^^^ type-only ExportSpecifiers
```
non-type-only `ExportSpecifier` can reference value and type symbols. but currently, `IdentifierReference` in ExportSpecifier only has a `ReferenceFlags::Read`
`AstNodeId` was a `usize`. This seems excessive.
Parser has a limit on size of a JS file of 4 GiB. While it is *possible* for a JS file of that size to create an AST with more than `1 << 32` (~4 billion) AST nodes, that would be insanely large.
So make `AstNodeId` `u32` instead.
