I incorrectly included these as deps, we were only using these to verify
codegen. It seems fine to leave in but when I imported it internally vscode
would error, so just remove it
- Made most static methods on CompilerError take a single options object as an
argument. With the exception of invariant which takes a condition and an options
object. - Added a new `suggestions` field on CompilerErrorDetail, which we'll
use to provide eslint suggestions - Updated eslint-plugin-react-forget to handle
suggestions
Most of these errors were incorrectly using InvalidInput as a catchall for
rejecting code. I went through each one and manually updated them to be more
accurate
Found this while running Forget on the React tests.
This isn't a high priority because the ESLint plugin would've caught this. But
it'd be nice if either our validation rules caught this or if our compiler did
correctly eliminate the dep array.
This allows us to mimic the `invariant` api, which means you can just assert
that something holds true after execution proceeds to the next line without
throwing
These were effectively unused since we almost always passed in null when
creating errors, so remove them and instead pass in a `loc` explicitly. The
downside is that we no longer will see Babel codeframes in our test fixtures,
which doesn't seem like a huge loss
Fixes the mutable range validation in PrintHIR: when we checked the identifier
scope's range we failed to allow end=start=0 cases which are also valid.
While I was here, I created a separate assertion pass to check all ranges, that
way we aren't relying on the printer to validate them. The pass is on for
playground and tests, but disabled by default so it can't break internal apps.
Distinguishes between two types of "validation" passes:
- Passes which assert the validity of the HIR. These remain in HIR/ but are
renamed "assertFoo".
- Passes which validate that the user code is correct. These move to
Validation/.
Fixes#1751. Function ids can be plain strings, and we can refer to them as
globals rather than via a local identifier. In addition to the bug from $1751
this also cleans up an existing todo.
This is a fix specifically for the `enableOptimizeFunctionExpression` feature
(disabled by default). I tried running all of our fixtures with that flag on
everywhere, and this was the only issue. It's actually extracted from another
fixture which is more complicated, this is a distilled version. There were two
bugs:
* DCE was running after LeaveSSA when it needs to run before. Fixing the order
means code inside the function expression stops getting removed.
* In the feature flag, context variables share Identifier instances with the
surrounding code, whereas they are distinct instances without the feature. This
meant that mutable ranges from inside the function propagated outside the
function, throwing off our inference. The fix was to reset the ranges of context
variables after inferring the function expression's effects.
inference
Integrates the new inference to detect definitive mutations of context
variables. This allows us to more precisely infer mutable function expressions
and validate that they aren't passed where frozen values are expected.
New pass to infer context variables which are definitively mutated,
distinguishing from context variables which _may_ have been mutated. See the
code comment on the new pass for more details.
Small tweak necessary for the subsequent PRs, refs and ref values take
precedence over other mutations when computing the effect type of context
variables. We always want to record ref access within a function as capture
(since we have later validation) rather than a mutation. For now this has no
impact, either order records a Capture. But it allows later diffs to make
non-ref cases have other effects.
The original version of the code wasn't checking return values. I missed this
since my examples were passing functions _into_ the return value, as opposed to
return the functions directly. This revealed some existing test fixtures that
were technically invalid, but easy to fix by changing the return value.
In InferReferenceEffects, locations that are ConditionallyMutate are either
recorded as a Mutate or Read, which means we lose the distinction btw
conditional/unconditional mutation in later passes. This PR changes to remember
that these places were conditionally mutable, used in later analysis.
Defaults to false, ie it runs the codegen pass. When enabled it will simply run
all passes up to codegen and then skip over it. Naming of this option is copied
from [TypeScript](https://www.typescriptlang.org/tsconfig#noEmit) which has the
same named option that makes the compiler only perform typechecking.
I'm adding this option primarily to get around some issues running the eslint
plugin on Meta code. The plugin would error because Forget would report
duplicate Babel AST nodes, which I presume would only occur during codegen. It
should also make it a tiny bit faster to not run codegen, which is a nice plus.
This was causing some issues in the eslint plugin where the babel `hub` wasn't
defined. afaik the hub is only setup when running the plugin as part of a babel
pipeline, instead of a manual parse/traversal. We're using some of that infra
for printing codeframes
The existing errors thrown were marked as InvalidInput, which is now considered
critical. This was causing an error in the sync since we had 2 occurrences of
the errors being thrown. These are really todos and not invalid code.
Turning off this flag makes only critical errors throw, so TODO errors will no
longer be surfaced by the plugin. The previously failing test for unsupported
syntax is now valid.
Lauren Tan
Sathya Gunasekaran
Joe Savona