Upgrading to Rust 1.80.0 (#4474) regressed some benchmarks (#4478). We
didn't notice because benchmarks didn't run on the PR. Make benchmarks
run on changes to `rust-toolchain.toml` so we'll be alerted if this
happens again on any future updates.
`eslint(no_shadow_restricted_names)` lint rule emits a diagnostic for every declaration of a symbol with a restricted name.
Currently for a var which has redeclarations, the var name is looked up in hash map of restricted names repeatedly for each redeclaration. This PR changes that to only do a single hashmap lookup.
Also, if the var name is `undefined`, skip looking it up in hash map, because we already know it's a restricted name.
`Span` is `Copy` and 8 bytes. So better to pass `Span` to functions, rather than `&Span` (which is also 8 bytes, but involves the indirection of a reference).
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.
In `nextjs:no_duplicate_head` rule, avoid allocating a `Vec` unless more than one `<Head>` is found (uncommon case).
Also, #4464 made getting span of a `Reference` a little more expensive. Avoid this work unless it's needed (which it generally won't be).
Remove `span` field from `Reference`. Span can instead be got via looking up span on `AstKind` via `reference.node_id`.
This shrinks `Reference` from 20 bytes to 12 bytes.
It does make getting span of a `Reference` a bit slower, as there's an extra table lookup involved, but the only places this is needed is in linter, and almost always when generating a diagnostic (i.e. cold path).
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.
`declarations` field of `SymbolTable` is part of SoA group, where all `Vec`s in the group are indexed by `SymbolId`. Populate `declarations` field along with the rest in `create_symbol`, to prevent them getting out of sync. This will prevent bugs like the one fixed in #4460.
`declarations` field of `SymbolTable` is part of the SoA group field. For it to be valid to index into with `SymbolId`, an entry must be added for every new symbol which is created.
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.
# What This PR Does
Massively improves all `react-perf` rules
- feat: handle new objects/etc assigned to variables
```tsx
const Foo = () => {
const x = { foo: 'bar' } // <- now reports this new object
return <Bar x={x} />
}
```
- feat: handle new objects/etc in binding patterns
```tsx
const Foo = ({ x = [] }) => {
// ^^^^^^ now reports this new array
return <Bar x={x} />
}
```
-feat: nice and descriptive labels for new objects/etc assigned to intermediate variables
```
⚠ eslint-plugin-react-perf(jsx-no-new-object-as-prop): JSX attribute values should not contain objects created in the same scope.
╭─[jsx_no_new_object_as_prop.tsx:1:27]
1 │ const Foo = () => { const x = {}; return <Bar x={x} /> }
· ┬ ─┬ ┬
· │ │ ╰── And used here
· │ ╰── And assigned a new value here
· ╰── The prop was declared here
╰────
help: simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).
```
- feat: consider `Object.assign()` and `Object.create()` as a new object
- feat: consider `arr.[map, filter, concat]` as a new array
- refactor: move shared implementation code to `ReactPerfRule` in `oxc_linter::utils::react_perf`
Consider the following code:
```tsx
import { FC } from 'react'
import { SvgIcon } from '@mui/material'
const StyledIcon = <SvgIcon sx={{ padding: 1, color: '#ff0000' }} />
// reported violation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
export const MyComponent: FC = () => {
return (
<div>
{StyledIcon}
{/* ... */}
</div>
)
}
```
This should not be a violation since the JSX is pre-computed and re-used, which
does not break React's `Object.is()` checks.
