Sorry about the thrash in advance! This removes the top level `forget` directory
which adds unnecessary nesting to our repo
Hopefully everything still works
ESTree expects the `range` value to be an array of `[start, end]`, we support
deserializing from that format but serialized as an object of `{start,end}`, now
we serialize to the array form.
We’ve had this feature turned on internally for a while with only a couple minor
bugs and no fundamental issues. It’s a necessary change for simplifying function
expression dependencies and context variables, so let’s remove the feature flag
and fix forward for any issues.
Adds SAFETY comments for the two instances of unsafe blocks that are *not* just
FFI. It seems like overkill to annotate all the FFI calls so i'm skipping that,
let me know if anyone has strong preferences otherwise.
This is mostly to test that the new configuration skips the Rust CI step if
there are no rust changes - yay, that worked! But also worth mentioning in the
readme so lets land.
Adds name resolution support for class declarations and expressions. Mostly this
involves _not_ visiting some `Identifier` nodes that don't actually represent
variables. For example, method names don't introduce new variables (but if they
are computed, they may _refer_ to variables).
Adds an option to pass a list of known globals into the semantic analyzer so
that references to globals can be checked. As a follow-up we'll need to
distinguish between different types of semantic analysis errors, so that callers
which don't want to validate globals can ignore "unknown variable" reference
errors while still handling definite errors such as duplicate declarations.
JavaScript has ~sane~ fun rules around where variables can be redeclared or not,
and which kinds of variables this applies to. Actually the rules are pretty
straightforward:
* `var` can be redeclared any number of times.
* In strict mode, other declarations cannot be redeclared within the same scope.
This implies that a `var` declaration cannot conflict with these other forms,
which must take into account hoisting. So you can't have a `var a; let a` in the
same scope, but you also can't have a `let a` at a scope and then a `var a`
which will hoist to that same scope.
The [temporal dead
zone](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#temporal_dead_zone_tdz),
often abbreviated TDZ, is the period between the start of its declaring block
and the line that contains the let/const/class declaration. Not all instances of
TDZ can be detected statically, because whether or not a TDZ error will occur at
runtime is a property of which control flow path is taken and whether some other
code has initialized the value. However, a subset of cases can be detected, and
that's what we implement here.
When we encounter a variable reference we record the next declaration id at that
point in time. Then when resolving references after visiting the program, we can
check: did the reference end up referring to a let/const/class binding whose id
is equal or greater to that "next declaration"? If so, it means the reference
refers to a variable that is provably declared later and is a known TDZ
violation. The catch is that when resolving references, we reset the "next
declaration" limit value when we bubble up out of a function scope. That's
because references to let/const within a function may occur after the
declaration, and we can't statically validate them.
Previously we attempted to resolve each reference at the close of its defining
scope, and if it couldn't be resolved yet we bubbled the unresolved reference up
to the parent scope. That approach isn't ideal for two reasons:
* First, it's inefficient since we may have to make multiple attempts to resolve
the same reference.
* Second, it's incorrect. There can be cases where we think we can resolve a
reference to a value defined in an outer scope, but there is a hoisted
declaration from an intermediate scope that we haven't seen yet.
The safest and most optimal thing is to just queue all references and resolve
them at the end.
Rather than always create a Global scope and then immediately add a Module child
node, we now set the kind of the root scope to global (for scripts) or module
(for modules) based on the program node.
Teaches the semantic analyzer about import statements. We now treat imports as
declarations, so that subsequent references to the imported value can be
resolved.
Teaches the hermes->estree conversion to convert source ranges. This means our
diagnostics now point to the actual source of the error:
<img width="903" alt="Screenshot 2023-08-14 at 5 39 50 PM"
src="https://github.com/facebook/react-forget/assets/6425824/2960d114-0cc6-4531-9e7d-00ff3bfb1eb3">
Of course I still need to teach our semantic analysis about imports, but that's
a separate issue!
---
I added ~20 more tests to Sprout to get more of a feel for what the test
framework would need to support all fixtures. I'm relatively confident that the
approach outlined in [the original workplace
post](https://fburl.com/workplace/ftu8woch) works to migrate almost all existing
fixture tests to sprout (TLDR: use shared functions when possible, otherwise
write helper functions in-file).
Add `shared-runtime` file to reduce the amount of overhead needed to set up a
fixture test.
- All generalizable functions and values should be added here (e.g.
`shallowCopy`, `deepCopy`, `sum`).
- Editor integration is set up through a custom tsconfig, so importing from
`shared-runtime` should just work (with hover annotations, click-to-definition,
etc).
<img width="682" alt="Screenshot 2023-08-17 at 11 09 47 AM"
src="https://github.com/facebook/react-forget/assets/34200447/78c3dff9-ba10-4057-b3f6-2fa842d19b1d">
---
Tested new test fixtures added to sprout by adding them to `testfilter.txt` and
running with `--verbose --filter`.
Note that there are no unexpected exceptions, and all logs + returned values
match.
```
feifei0@feifei0-mbp babel-plugin-react-forget % yarn sprout:build && yarn sprout
--verbose --filter
yarn run v1.22.19
$ yarn workspace sprout run build
$ rimraf dist && tsc --build
✨ Done in 2.07s.
yarn run v1.22.19
$ node ../sprout/dist/main.js --verbose --filter
PASS console-readonly
ok {"a":1,"b":2} [
"{ a: 1, b: 2 }",
"{ a: 1, b: 2 }",
"{ a: 1, b: 2 }",
"{ a: 1, b: 2 }",
"{ a: 1, b: 2 }"
]
PASS constant-propagate-global-phis-constant
ok <div>global string 0</div>
PASS constant-propagate-global-phis
ok <div>global string 1</div>
PASS dce-loop
ok 10
PASS destructure-capture-global
ok {"a":"value 1","someGlobal":{}}
PASS destructuring-mixed-scope-and-local-variables-with-default
ok {"media":null,"allUrls":["url1","url2","url3"],"onClick":"[[ function
params=1 ]]"}
PASS holey-array-expr
ok [null,"global string 0",{"a":1,"b":2}]
PASS infer-global-object
ok {"primitiveVal1":2,"primitiveVal2":null,"primitiveVal3":null}
PASS infer-phi-primitive
ok 1
PASS infer-types-through-type-cast.flow
ok 4
PASS issue933-disjoint-set-infinite-loop
ok [2]
PASS jsx-tag-evaluation-order-non-global
ok <div>StaticText1<div>StaticText2</div></div>
PASS jsx-tag-evaluation-order
ok <div>StaticText1string value 1<div>StaticText2</div></div>
PASS method-call
ok 4
PASS mutable-lifetime-loops
ok {"a":{"value":6},"b":{"value":5},"c":{"value":4},"d":{"value":6}}
PASS mutable-lifetime-with-aliasing
ok {"b":[{}],"value":[{"c":{},"value":"[[ cyclic ref *0 ]]"},null]}
PASS update-expression-in-sequence
ok [4,2,3,4]
PASS update-expression-on-function-parameter
ok [4,1,4,2,4,3,1,4,4]
PASS update-expression
ok {"x":1,"y":1,"z":2}
PASS useMemo-multiple-if-else
ok 2
20 Tests, 20 Passed, 0 Failed
✨ Done in 4.11s.
```
---
This PR is an example of how to update fixture to add sprout annotations.
Running with `yarn sprout --verbose --filter` shows the fixture outputs.
```
PASS array-access-assignment
ok [[2],[[2],[],[3]]]
PASS array-expression-spread
ok [0,1,2,3,null,4,5,6,"z"]
PASS array-map-frozen-array
ok [[],[]]
PASS array-map-mutable-array-mutating-lambda
ok [[],[]]
PASS array-pattern-params
ok [{"a":"val1"},{"b":"val2"}]
PASS array-properties
ok {"a":[[1,2],2,"hello"],"x":3,"y":"[[ function params=1 ]]","z":"[[ function
params=1 ]]"}
PASS array-property-call
ok {"a":[1,2,"hello",42],"x":4,"y":1}
PASS assignment-expression-computed
ok [7]
PASS assignment-expression-nested-path
ok {"b":{"c":6}}
PASS capturing-func-mutate-2
ok {"a":2}
PASS capturing-function-alias-computed-load-2
ok "val2"
PASS capturing-function-alias-computed-load-4
ok "val2"
```
---
Rename `SproutOnlyFilterTodoRemove.ts` to `SproutTodoFilter.ts`. I've tried to
group the skipped fixtures by difficulty and add comments about what need to be
done, also happy to change the structure of this.
From this point, sprout will run on all new fixtures by default. If the fixture
forgets to export a `FIXTURE_ENTRYPOINT`, sprout will fail with this error.
<img width="853" alt="Screenshot 2023-08-15 at 5 59 32 PM"
src="https://github.com/facebook/react-forget/assets/34200447/0f80d650-1dc1-4df4-9710-e49acbb424b0">
See #1961 for an example of how to annotate existing skipped fixtures or new
ones. The `README.md` is also updated with a guide.
---
```
yarn sprout --verbose
```
Verbose mode prints out all outputs of tests. The test output is a status (`ok`
or `exception`), a returned or thrown value, and a set of console logs.
Currently as of #1960 , this is the output:
```sh
$ yarn workspace babel-plugin-react-forget run build && node
../sprout/dist/main.js --verbose
$ rimraf dist && tsc
PASS alias-nested-member-path
ok {"y":{"z":[]}}
PASS assignment-variations-complex-lvalue
ok {"y":{"z":4}}
PASS assignment-variations
ok 1
PASS chained-assignment-expressions
ok {"z":null}
PASS computed-call-evaluation-order
ok {"f":"[[ function params=0 ]]"} [
"A",
"B",
"arg",
"original"
]
PASS const-propagation-into-function-expression-primitive
ok 42 [
"42"
]
PASS constant-propagation-for
ok 0
PASS constant-propagation-while
ok 0
PASS constant-propagation
ok -6 [
"foo"
]
PASS controlled-input
ok <input value="0">
PASS do-while-continue
ok [1.5,1,0.5]
PASS do-while-simple
ok [6,4,2]
PASS expression-with-assignment
ok 5
PASS for-of-break
ok []
PASS for-of-conditional-break
ok []
PASS for-of-continue
ok [0.5,1,1.5]
PASS for-of-destructure
ok [0,2,4]
PASS for-of-simple
ok [0,2,4]
PASS function-declaration-reassign
ok {}
PASS function-declaration-redeclare
ok "[[ function params=0 ]]"
PASS lambda-reassign-primitive
ok 41
PASS lambda-reassign-shadowed-primitive
ok {}
PASS property-call-evaluation-order
ok {"f":"[[ function params=0 ]]"} [
"A",
"arg",
"original"
]
PASS reactive-scope-grouping
ok {"y":[{}]}
PASS sequentially-constant-progagatable-if-test-conditions
ok "ok"
PASS simple-function-1
ok "[[ function params=1 ]]"
PASS ssa-complex-multiple-if
ok
PASS ssa-complex-single-if
ok
PASS ssa-for
ok 11
PASS ssa-if-else
ok
PASS ssa-objectexpression-phi
ok {"x":1,"y":3}
PASS ssa-property-call
ok {"x":[[]]}
PASS ssa-property
ok {"x":[]}
PASS ssa-return
ok 2
PASS ssa-simple-phi
ok
PASS ssa-simple
ok
PASS ssa-single-if
ok
PASS ssa-switch
ok
PASS ssa-throw
exception undefined
PASS ssa-while
ok 10
PASS type-field-load
ok 1
PASS type-test-field-store
ok {}
PASS type-test-primitive
ok 2
PASS update-expression-constant-propagation
ok {"a":0,"b":0,"c":2,"d":2,"e":0}
44 Tests, 44 Passed, 0 Failed
✨ Done in 9.27s.
```
---
Not sure why, but snap kept silently crashing when I used fs/promises to write
to many file handles. I tried a `.catch(...)`, but couldn't figure it out. This
diff changes snap to use sync fs apis to avoid crashes, but I'd love to get
feedback if someone knows how to debug this.
This PR moves the phi evaluation to a separate function.
Most importantly, it inverts the default case to _not_ constant propagate unless
we have explicit validation of the phi operands.
---
## Sprout 🌱 Overview
**(Overview copied from
[sprout/README.md](https://github.com/facebook/react-forget/blob/0468ddf8bbee54a9394317a667ea4ed96bb3bef3/forget/packages/sprout/README.md))**
React Forget test framework that executes compiler fixtures.
Currently, Sprout runs each fixture with a known set of inputs and annotations.
We hope to add fuzzing capabilities to Sprout, synthesizing sets of program
inputs based on type and/or effect annotations.
Sprout is currently WIP and only executes files listed in
`src/SproutOnlyFilterTodoRemove.ts`.
### Milestones:
- [✅] Render fixtures with React runtime / `testing-library/react`.
- [ ] Make Sprout CLI -runnable and report results in process exit code.
After this point:
- Sprout can be enabled by default and added to the Github Actions pipeline.
- `SproutOnlyFilterTodoRemove` can be renamed to `SproutSkipFilter`.
- All new tests should provide a `FIXTURE_ENTRYPOINT`.
- [ ] Annotate `FIXTURE_ENTRYPOINT` (fn entrypoint and params) for rest of
fixtures.
- [ ] Edit rest of fixtures to use shared functions or define their own helpers.
- [ ] *(optional)* Store Sprout output as snapshot files. i.e. each fixture
could have a `fixture.js`, `fixture.snap.md`, and `fixture.sprout.md`.
### Constraints
Each fixture test executed by Sprout needs to export a `FIXTURE_ENTRYPOINT`, a
single function and parameter set with the following type signature.
```js
type FIXTURE_ENTRYPOINT<T> = {
// function to be invoked
fn: ((...params: Array<T>) => any),
// params to pass to fn
params: Array<T>,
// true if fn should be rendered as a React Component
// i.e. returns jsx or primitives
isComponent?: boolean,
}
```
Example:
```js
// test.js
function MyComponent(props) {
return <div>{props.a + props.b}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: MyComponent,
params: [{a: "hello ", b: "world"}],
isComponent: true,
};
```
---
## Implementation Details
- jest-worker test orchestrator (similar to Snap 🫰).
I chose to write a test runner instead of directly using Jest for flexibility
and speed.
- Sprout 🌱 currently runs much more code per fixture (2 babel transform
pipelines + 2 `exec(...)` than snap, so all scaling concerns apply.
- Sprout may need more customization in the future (e.g. fuzzing component
inputs, caching artifacts)
- We probably want to add snapshot files for Sprout, which is much easier with a
custom runner.
This is also one of the main reasons we wrote Snap. Jest consolidates all
external snapshots (i.e. non-inline snapshots) from a test into [a single
file](https://github.com/facebook/react-forget/blob/d0a006ffa9842ee9db6b23ada94622af901eb07f/forget/src/__tests__/__snapshots__/compiler-test.ts.snap).
This was painful mainly for rebasing changes, but also for small papercuts (e.g.
needing to run `yarn test -u` twice when deleting a fixture)
- Currently does not save output to snapshot file, but we can easily add this
later (i.e. each test would have a `.snap.md` and `.sprout.md` file)
- Supports filter mode (same testfilter.txt file as snap)
- Currently does not support watch mode. I expect that sprout's primary use will
be catching bugs in the PR / Github Actions phase. Can be changed if we need to
iterate on sprout output while developing.
- All tests are run with `react-test-renderer` to access the React Runtime,
which is needed for calls to `useMemoCache` (and other potential hooks).
- react-test-renderer required a mocked DOM, so I ~~used js-dom~~ did a terrible
js-dom hack to add document, window, and other browser globals to the
jest-worker globals, then exec the test code in the jest worker global.
I can clean this up later by bundling library code (e.g. react,
react-test-renderer) to not use `require(...)`, then calling `exec` on all "test
client code" in the js-dom mock global (instead of the real jest worker one)
- Tests marked `isComponent` need to return valid jsx or primitives. Values
returned by all other tests are converted to a string primitive via
JSON.stringify.
```sh
# install new dependencies to node_modules
$ yarn
$ cd forget/packages/babel-plugin-react-forget
$ yarn sprout:build && yarn sprout
PASS alias-nested-member-path
PASS assignment-variations-complex-lvalue
PASS assignment-variations
PASS chained-assignment-expressions
PASS computed-call-evaluation-order
PASS const-propagation-into-function-expression-primitive
PASS constant-propagation-for
PASS constant-propagation-while
PASS constant-propagation
...
✨ Done in 3.91s.
```
Nice find from @mofeiz, we weren't adding declarations for function params in
LeaveSSA. This caused us to hit an invariant for update expressions that updated
params which assumed that all named variables had a declaration. This is why
it's helpful to add invariants, it helps you find places where you violate them
:-)
Adds basic support for hoisting semantics: * Resolution of variable references
is _always_ deferred in case the correct binding hasn't been seet yet due to
hoisting. * Var and function declarations bubble to the appropriate scope
There are lots of subtleties that aren't implemented yet but these rules cover a
lot.
This is a precursor to adding support for hoisting in semantic analysis.
Previously when we encountered an unknown reference we immediately reported an
error. But hoisted variables may be referenced before they're defined, so we
don't know for sure when we see an unknown variable if its actually unbound or
not.
This PR adds the first part of hoistingn support: rather than immediately report
an error when encountering an unbound variable we store it in a list of
unresolved references on the current scope. As we close each scope we recheck
and see if the variable can now be resolved. If yes we record that, otherwise we
bubble up the unresolved reference to the parent scope (and try again there).
The next PR(s) will handle hoisting of `var` and other syntax to the apropriate
nearest scope boundary (function/module).
lauren
Joe Savona
Mofei Zhang
Sathya Gunasekaran