From 4c4a57c4f9f7f7d44e4cbe868c066e3691cd4038 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 2 May 2025 15:04:45 -0400 Subject: [PATCH 01/12] [eslint-plugin-react-hooks] updates for component syntax (#33089) Adds support for Flow's component and hook syntax. [docs](https://flow.org/en/docs/react/component-syntax/) --- .../__tests__/ESLintRulesOfHooks-test.js | 89 +- .../src/code-path-analysis/LICENSE | 19 + .../src/code-path-analysis/README.md | 6 + .../src/code-path-analysis/assert.js | 9 + .../code-path-analysis/code-path-analyzer.js | 802 +++++++++ .../code-path-analysis/code-path-segment.js | 225 +++ .../src/code-path-analysis/code-path-state.js | 1441 +++++++++++++++++ .../src/code-path-analysis/code-path.js | 239 +++ .../src/code-path-analysis/fork-context.js | 252 +++ .../src/code-path-analysis/id-generator.js | 37 + .../src/rules/RulesOfHooks.ts | 44 +- 11 files changed, 3152 insertions(+), 11 deletions(-) create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/LICENSE create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/README.md create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/assert.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-segment.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-state.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/fork-context.js create mode 100644 packages/eslint-plugin-react-hooks/src/code-path-analysis/id-generator.js diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index b8ec97678aa16..22d2427de69d3 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -34,7 +34,7 @@ function normalizeIndent(strings) { // } // *************************************************** -const tests = { +const allTests = { valid: [ { code: normalizeIndent` @@ -44,6 +44,25 @@ const tests = { } `, }, + { + syntax: 'flow', + code: normalizeIndent` + // Component syntax + component Button() { + useHook(); + return
Button!
; + } + `, + }, + { + syntax: 'flow', + code: normalizeIndent` + // Hook syntax + hook useSampleHook() { + useHook(); + } + `, + }, { code: normalizeIndent` // Valid because components can use hooks. @@ -563,6 +582,28 @@ const tests = { }, ], invalid: [ + { + syntax: 'flow', + code: normalizeIndent` + component Button(cond: boolean) { + if (cond) { + useConditionalHook(); + } + } + `, + errors: [conditionalError('useConditionalHook')], + }, + { + syntax: 'flow', + code: normalizeIndent` + hook useTest(cond: boolean) { + if (cond) { + useConditionalHook(); + } + } + `, + errors: [conditionalError('useConditionalHook')], + }, { code: normalizeIndent` // Invalid because it's dangerous and might not warn otherwise. @@ -1287,8 +1328,8 @@ const tests = { }; if (__EXPERIMENTAL__) { - tests.valid = [ - ...tests.valid, + allTests.valid = [ + ...allTests.valid, { code: normalizeIndent` // Valid because functions created with useEffectEvent can be called in a useEffect. @@ -1385,8 +1426,8 @@ if (__EXPERIMENTAL__) { `, }, ]; - tests.invalid = [ - ...tests.invalid, + allTests.invalid = [ + ...allTests.invalid, { code: normalizeIndent` function MyComponent({ theme }) { @@ -1536,7 +1577,7 @@ function asyncComponentHookError(fn) { if (!process.env.CI) { let only = []; let skipped = []; - [...tests.valid, ...tests.invalid].forEach(t => { + [...allTests.valid, ...allTests.invalid].forEach(t => { if (t.skip) { delete t.skip; skipped.push(t); @@ -1555,10 +1596,23 @@ if (!process.env.CI) { } return true; }; - tests.valid = tests.valid.filter(predicate); - tests.invalid = tests.invalid.filter(predicate); + allTests.valid = allTests.valid.filter(predicate); + allTests.invalid = allTests.invalid.filter(predicate); +} + +function filteredTests(predicate) { + return { + valid: allTests.valid.filter(predicate), + invalid: allTests.invalid.filter(predicate), + }; } +const flowTests = filteredTests(t => t.syntax == null || t.syntax === 'flow'); +const tests = filteredTests(t => t.syntax !== 'flow'); + +allTests.valid.forEach(t => delete t.syntax); +allTests.invalid.forEach(t => delete t.syntax); + describe('rules-of-hooks/rules-of-hooks', () => { const parserOptionsV7 = { ecmaFeatures: { @@ -1594,6 +1648,25 @@ describe('rules-of-hooks/rules-of-hooks', () => { tests ); + new ESLintTesterV7({ + parser: require.resolve('hermes-eslint'), + parserOptions: { + sourceType: 'module', + enableExperimentalComponentSyntax: true, + }, + }).run('eslint: v7, parser: hermes-eslint', ReactHooksESLintRule, flowTests); + + new ESLintTesterV9({ + languageOptions: { + ...languageOptionsV9, + parser: require('hermes-eslint'), + parserOptions: { + sourceType: 'module', + enableExperimentalComponentSyntax: true, + }, + }, + }).run('eslint: v9, parser: hermes-eslint', ReactHooksESLintRule, flowTests); + new ESLintTesterV7({ parser: require.resolve('@typescript-eslint/parser-v2'), parserOptions: parserOptionsV7, diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/LICENSE b/packages/eslint-plugin-react-hooks/src/code-path-analysis/LICENSE new file mode 100644 index 0000000000000..b607bb36e96c3 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/LICENSE @@ -0,0 +1,19 @@ +Copyright OpenJS Foundation and other contributors, + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/README.md b/packages/eslint-plugin-react-hooks/src/code-path-analysis/README.md new file mode 100644 index 0000000000000..1109cb2f387eb --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/README.md @@ -0,0 +1,6 @@ +# Code Path Analyzer + +This code is a forked version of ESLints Code Path Analyzer which includes +support for Component Syntax. + +Forked from: https://github.com/eslint/eslint/tree/main/lib/linter/code-path-analysis diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/assert.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/assert.js new file mode 100644 index 0000000000000..d6dc3539f3ad8 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/assert.js @@ -0,0 +1,9 @@ +'use strict'; + +function assert(cond) { + if (!cond) { + throw new Error('Assertion violated.'); + } +} + +module.exports = assert; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js new file mode 100644 index 0000000000000..8dd76e57129b5 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-analyzer.js @@ -0,0 +1,802 @@ +'use strict'; + +/* eslint-disable react-internal/no-primitive-constructors */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +// eslint-disable-next-line +const assert = require('./assert'); +// eslint-disable-next-line +const CodePath = require('./code-path'); +// eslint-disable-next-line +const CodePathSegment = require('./code-path-segment'); +// eslint-disable-next-line +const IdGenerator = require('./id-generator'); + +const breakableTypePattern = + /^(?:(?:Do)?While|For(?:In|Of)?|Switch)Statement$/u; + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Checks whether or not a given node is a `case` node (not `default` node). + * @param {ASTNode} node A `SwitchCase` node to check. + * @returns {boolean} `true` if the node is a `case` node (not `default` node). + */ +function isCaseNode(node) { + return Boolean(node.test); +} + +/** + * Checks if a given node appears as the value of a PropertyDefinition node. + * @param {ASTNode} node THe node to check. + * @returns {boolean} `true` if the node is a PropertyDefinition value, + * false if not. + */ +function isPropertyDefinitionValue(node) { + const parent = node.parent; + + return ( + parent && parent.type === 'PropertyDefinition' && parent.value === node + ); +} + +/** + * Checks whether the given logical operator is taken into account for the code + * path analysis. + * @param {string} operator The operator found in the LogicalExpression node + * @returns {boolean} `true` if the operator is "&&" or "||" or "??" + */ +function isHandledLogicalOperator(operator) { + return operator === '&&' || operator === '||' || operator === '??'; +} + +/** + * Checks whether the given assignment operator is a logical assignment operator. + * Logical assignments are taken into account for the code path analysis + * because of their short-circuiting semantics. + * @param {string} operator The operator found in the AssignmentExpression node + * @returns {boolean} `true` if the operator is "&&=" or "||=" or "??=" + */ +function isLogicalAssignmentOperator(operator) { + return operator === '&&=' || operator === '||=' || operator === '??='; +} + +/** + * Gets the label if the parent node of a given node is a LabeledStatement. + * @param {ASTNode} node A node to get. + * @returns {string|null} The label or `null`. + */ +function getLabel(node) { + if (node.parent.type === 'LabeledStatement') { + return node.parent.label.name; + } + return null; +} + +/** + * Checks whether or not a given logical expression node goes different path + * between the `true` case and the `false` case. + * @param {ASTNode} node A node to check. + * @returns {boolean} `true` if the node is a test of a choice statement. + */ +function isForkingByTrueOrFalse(node) { + const parent = node.parent; + + switch (parent.type) { + case 'ConditionalExpression': + case 'IfStatement': + case 'WhileStatement': + case 'DoWhileStatement': + case 'ForStatement': + return parent.test === node; + + case 'LogicalExpression': + return isHandledLogicalOperator(parent.operator); + + case 'AssignmentExpression': + return isLogicalAssignmentOperator(parent.operator); + + default: + return false; + } +} + +/** + * Gets the boolean value of a given literal node. + * + * This is used to detect infinity loops (e.g. `while (true) {}`). + * Statements preceded by an infinity loop are unreachable if the loop didn't + * have any `break` statement. + * @param {ASTNode} node A node to get. + * @returns {boolean|undefined} a boolean value if the node is a Literal node, + * otherwise `undefined`. + */ +function getBooleanValueIfSimpleConstant(node) { + if (node.type === 'Literal') { + return Boolean(node.value); + } + return void 0; +} + +/** + * Checks that a given identifier node is a reference or not. + * + * This is used to detect the first throwable node in a `try` block. + * @param {ASTNode} node An Identifier node to check. + * @returns {boolean} `true` if the node is a reference. + */ +function isIdentifierReference(node) { + const parent = node.parent; + + switch (parent.type) { + case 'LabeledStatement': + case 'BreakStatement': + case 'ContinueStatement': + case 'ArrayPattern': + case 'RestElement': + case 'ImportSpecifier': + case 'ImportDefaultSpecifier': + case 'ImportNamespaceSpecifier': + case 'CatchClause': + return false; + + case 'FunctionDeclaration': + case 'ComponentDeclaration': + case 'HookDeclaration': + case 'FunctionExpression': + case 'ArrowFunctionExpression': + case 'ClassDeclaration': + case 'ClassExpression': + case 'VariableDeclarator': + return parent.id !== node; + + case 'Property': + case 'PropertyDefinition': + case 'MethodDefinition': + return parent.key !== node || parent.computed || parent.shorthand; + + case 'AssignmentPattern': + return parent.key !== node; + + default: + return true; + } +} + +/** + * Updates the current segment with the head segment. + * This is similar to local branches and tracking branches of git. + * + * To separate the current and the head is in order to not make useless segments. + * + * In this process, both "onCodePathSegmentStart" and "onCodePathSegmentEnd" + * events are fired. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function forwardCurrentToHead(analyzer, node) { + const codePath = analyzer.codePath; + const state = CodePath.getState(codePath); + const currentSegments = state.currentSegments; + const headSegments = state.headSegments; + const end = Math.max(currentSegments.length, headSegments.length); + let i, currentSegment, headSegment; + + // Fires leaving events. + for (i = 0; i < end; ++i) { + currentSegment = currentSegments[i]; + headSegment = headSegments[i]; + + if (currentSegment !== headSegment && currentSegment) { + if (currentSegment.reachable) { + analyzer.emitter.emit('onCodePathSegmentEnd', currentSegment, node); + } + } + } + + // Update state. + state.currentSegments = headSegments; + + // Fires entering events. + for (i = 0; i < end; ++i) { + currentSegment = currentSegments[i]; + headSegment = headSegments[i]; + + if (currentSegment !== headSegment && headSegment) { + CodePathSegment.markUsed(headSegment); + if (headSegment.reachable) { + analyzer.emitter.emit('onCodePathSegmentStart', headSegment, node); + } + } + } +} + +/** + * Updates the current segment with empty. + * This is called at the last of functions or the program. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function leaveFromCurrentSegment(analyzer, node) { + const state = CodePath.getState(analyzer.codePath); + const currentSegments = state.currentSegments; + + for (let i = 0; i < currentSegments.length; ++i) { + const currentSegment = currentSegments[i]; + if (currentSegment.reachable) { + analyzer.emitter.emit('onCodePathSegmentEnd', currentSegment, node); + } + } + + state.currentSegments = []; +} + +/** + * Updates the code path due to the position of a given node in the parent node + * thereof. + * + * For example, if the node is `parent.consequent`, this creates a fork from the + * current path. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function preprocess(analyzer, node) { + const codePath = analyzer.codePath; + const state = CodePath.getState(codePath); + const parent = node.parent; + + switch (parent.type) { + // The `arguments.length == 0` case is in `postprocess` function. + case 'CallExpression': + if ( + parent.optional === true && + parent.arguments.length >= 1 && + parent.arguments[0] === node + ) { + state.makeOptionalRight(); + } + break; + case 'MemberExpression': + if (parent.optional === true && parent.property === node) { + state.makeOptionalRight(); + } + break; + + case 'LogicalExpression': + if (parent.right === node && isHandledLogicalOperator(parent.operator)) { + state.makeLogicalRight(); + } + break; + + case 'AssignmentExpression': + if ( + parent.right === node && + isLogicalAssignmentOperator(parent.operator) + ) { + state.makeLogicalRight(); + } + break; + + case 'ConditionalExpression': + case 'IfStatement': + /* + * Fork if this node is at `consequent`/`alternate`. + * `popForkContext()` exists at `IfStatement:exit` and + * `ConditionalExpression:exit`. + */ + if (parent.consequent === node) { + state.makeIfConsequent(); + } else if (parent.alternate === node) { + state.makeIfAlternate(); + } + break; + + case 'SwitchCase': + if (parent.consequent[0] === node) { + state.makeSwitchCaseBody(false, !parent.test); + } + break; + + case 'TryStatement': + if (parent.handler === node) { + state.makeCatchBlock(); + } else if (parent.finalizer === node) { + state.makeFinallyBlock(); + } + break; + + case 'WhileStatement': + if (parent.test === node) { + state.makeWhileTest(getBooleanValueIfSimpleConstant(node)); + } else { + assert(parent.body === node); + state.makeWhileBody(); + } + break; + + case 'DoWhileStatement': + if (parent.body === node) { + state.makeDoWhileBody(); + } else { + assert(parent.test === node); + state.makeDoWhileTest(getBooleanValueIfSimpleConstant(node)); + } + break; + + case 'ForStatement': + if (parent.test === node) { + state.makeForTest(getBooleanValueIfSimpleConstant(node)); + } else if (parent.update === node) { + state.makeForUpdate(); + } else if (parent.body === node) { + state.makeForBody(); + } + break; + + case 'ForInStatement': + case 'ForOfStatement': + if (parent.left === node) { + state.makeForInOfLeft(); + } else if (parent.right === node) { + state.makeForInOfRight(); + } else { + assert(parent.body === node); + state.makeForInOfBody(); + } + break; + + case 'AssignmentPattern': + /* + * Fork if this node is at `right`. + * `left` is executed always, so it uses the current path. + * `popForkContext()` exists at `AssignmentPattern:exit`. + */ + if (parent.right === node) { + state.pushForkContext(); + state.forkBypassPath(); + state.forkPath(); + } + break; + + default: + break; + } +} + +/** + * Updates the code path due to the type of a given node in entering. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function processCodePathToEnter(analyzer, node) { + let codePath = analyzer.codePath; + let state = codePath && CodePath.getState(codePath); + const parent = node.parent; + + /** + * Creates a new code path and trigger the onCodePathStart event + * based on the currently selected node. + * @param {string} origin The reason the code path was started. + * @returns {void} + */ + function startCodePath(origin) { + if (codePath) { + // Emits onCodePathSegmentStart events if updated. + forwardCurrentToHead(analyzer, node); + } + + // Create the code path of this scope. + codePath = analyzer.codePath = new CodePath({ + id: analyzer.idGenerator.next(), + origin, + upper: codePath, + onLooped: analyzer.onLooped, + }); + state = CodePath.getState(codePath); + + // Emits onCodePathStart events. + analyzer.emitter.emit('onCodePathStart', codePath, node); + } + + /* + * Special case: The right side of class field initializer is considered + * to be its own function, so we need to start a new code path in this + * case. + */ + if (isPropertyDefinitionValue(node)) { + startCodePath('class-field-initializer'); + + /* + * Intentional fall through because `node` needs to also be + * processed by the code below. For example, if we have: + * + * class Foo { + * a = () => {} + * } + * + * In this case, we also need start a second code path. + */ + } + + switch (node.type) { + case 'Program': + startCodePath('program'); + break; + + case 'FunctionDeclaration': + case 'ComponentDeclaration': + case 'HookDeclaration': + case 'FunctionExpression': + case 'ArrowFunctionExpression': + startCodePath('function'); + break; + + case 'StaticBlock': + startCodePath('class-static-block'); + break; + + case 'ChainExpression': + state.pushChainContext(); + break; + case 'CallExpression': + if (node.optional === true) { + state.makeOptionalNode(); + } + break; + case 'MemberExpression': + if (node.optional === true) { + state.makeOptionalNode(); + } + break; + + case 'LogicalExpression': + if (isHandledLogicalOperator(node.operator)) { + state.pushChoiceContext(node.operator, isForkingByTrueOrFalse(node)); + } + break; + + case 'AssignmentExpression': + if (isLogicalAssignmentOperator(node.operator)) { + state.pushChoiceContext( + node.operator.slice(0, -1), // removes `=` from the end + isForkingByTrueOrFalse(node), + ); + } + break; + + case 'ConditionalExpression': + case 'IfStatement': + state.pushChoiceContext('test', false); + break; + + case 'SwitchStatement': + state.pushSwitchContext(node.cases.some(isCaseNode), getLabel(node)); + break; + + case 'TryStatement': + state.pushTryContext(Boolean(node.finalizer)); + break; + + case 'SwitchCase': + /* + * Fork if this node is after the 2st node in `cases`. + * It's similar to `else` blocks. + * The next `test` node is processed in this path. + */ + if (parent.discriminant !== node && parent.cases[0] !== node) { + state.forkPath(); + } + break; + + case 'WhileStatement': + case 'DoWhileStatement': + case 'ForStatement': + case 'ForInStatement': + case 'ForOfStatement': + state.pushLoopContext(node.type, getLabel(node)); + break; + + case 'LabeledStatement': + if (!breakableTypePattern.test(node.body.type)) { + state.pushBreakContext(false, node.label.name); + } + break; + + default: + break; + } + + // Emits onCodePathSegmentStart events if updated. + forwardCurrentToHead(analyzer, node); +} + +/** + * Updates the code path due to the type of a given node in leaving. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function processCodePathToExit(analyzer, node) { + const codePath = analyzer.codePath; + const state = CodePath.getState(codePath); + let dontForward = false; + + switch (node.type) { + case 'ChainExpression': + state.popChainContext(); + break; + + case 'IfStatement': + case 'ConditionalExpression': + state.popChoiceContext(); + break; + + case 'LogicalExpression': + if (isHandledLogicalOperator(node.operator)) { + state.popChoiceContext(); + } + break; + + case 'AssignmentExpression': + if (isLogicalAssignmentOperator(node.operator)) { + state.popChoiceContext(); + } + break; + + case 'SwitchStatement': + state.popSwitchContext(); + break; + + case 'SwitchCase': + /* + * This is the same as the process at the 1st `consequent` node in + * `preprocess` function. + * Must do if this `consequent` is empty. + */ + if (node.consequent.length === 0) { + state.makeSwitchCaseBody(true, !node.test); + } + if (state.forkContext.reachable) { + dontForward = true; + } + break; + + case 'TryStatement': + state.popTryContext(); + break; + + case 'BreakStatement': + forwardCurrentToHead(analyzer, node); + state.makeBreak(node.label && node.label.name); + dontForward = true; + break; + + case 'ContinueStatement': + forwardCurrentToHead(analyzer, node); + state.makeContinue(node.label && node.label.name); + dontForward = true; + break; + + case 'ReturnStatement': + forwardCurrentToHead(analyzer, node); + state.makeReturn(); + dontForward = true; + break; + + case 'ThrowStatement': + forwardCurrentToHead(analyzer, node); + state.makeThrow(); + dontForward = true; + break; + + case 'Identifier': + if (isIdentifierReference(node)) { + state.makeFirstThrowablePathInTryBlock(); + dontForward = true; + } + break; + + case 'CallExpression': + case 'ImportExpression': + case 'MemberExpression': + case 'NewExpression': + case 'YieldExpression': + state.makeFirstThrowablePathInTryBlock(); + break; + + case 'WhileStatement': + case 'DoWhileStatement': + case 'ForStatement': + case 'ForInStatement': + case 'ForOfStatement': + state.popLoopContext(); + break; + + case 'AssignmentPattern': + state.popForkContext(); + break; + + case 'LabeledStatement': + if (!breakableTypePattern.test(node.body.type)) { + state.popBreakContext(); + } + break; + + default: + break; + } + + // Emits onCodePathSegmentStart events if updated. + if (!dontForward) { + forwardCurrentToHead(analyzer, node); + } +} + +/** + * Updates the code path to finalize the current code path. + * @param {CodePathAnalyzer} analyzer The instance. + * @param {ASTNode} node The current AST node. + * @returns {void} + */ +function postprocess(analyzer, node) { + /** + * Ends the code path for the current node. + * @returns {void} + */ + function endCodePath() { + let codePath = analyzer.codePath; + + // Mark the current path as the final node. + CodePath.getState(codePath).makeFinal(); + + // Emits onCodePathSegmentEnd event of the current segments. + leaveFromCurrentSegment(analyzer, node); + + // Emits onCodePathEnd event of this code path. + analyzer.emitter.emit('onCodePathEnd', codePath, node); + + codePath = analyzer.codePath = analyzer.codePath.upper; + } + + switch (node.type) { + case 'Program': + case 'FunctionDeclaration': + case 'ComponentDeclaration': + case 'HookDeclaration': + case 'FunctionExpression': + case 'ArrowFunctionExpression': + case 'StaticBlock': { + endCodePath(); + break; + } + + // The `arguments.length >= 1` case is in `preprocess` function. + case 'CallExpression': + if (node.optional === true && node.arguments.length === 0) { + CodePath.getState(analyzer.codePath).makeOptionalRight(); + } + break; + + default: + break; + } + + /* + * Special case: The right side of class field initializer is considered + * to be its own function, so we need to end a code path in this + * case. + * + * We need to check after the other checks in order to close the + * code paths in the correct order for code like this: + * + * + * class Foo { + * a = () => {} + * } + * + * In this case, The ArrowFunctionExpression code path is closed first + * and then we need to close the code path for the PropertyDefinition + * value. + */ + if (isPropertyDefinitionValue(node)) { + endCodePath(); + } +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * The class to analyze code paths. + * This class implements the EventGenerator interface. + */ +class CodePathAnalyzer { + /** + * @param {EventGenerator} eventGenerator An event generator to wrap. + */ + constructor(emitters) { + this.emitter = { + emit(event, ...args) { + emitters[event]?.(...args); + }, + }; + this.codePath = null; + this.idGenerator = new IdGenerator('s'); + this.currentNode = null; + this.onLooped = this.onLooped.bind(this); + } + + /** + * Does the process to enter a given AST node. + * This updates state of analysis and calls `enterNode` of the wrapped. + * @param {ASTNode} node A node which is entering. + * @returns {void} + */ + enterNode(node) { + this.currentNode = node; + + // Updates the code path due to node's position in its parent node. + if (node.parent) { + preprocess(this, node); + } + + /* + * Updates the code path. + * And emits onCodePathStart/onCodePathSegmentStart events. + */ + processCodePathToEnter(this, node); + + this.currentNode = null; + } + + /** + * Does the process to leave a given AST node. + * This updates state of analysis and calls `leaveNode` of the wrapped. + * @param {ASTNode} node A node which is leaving. + * @returns {void} + */ + leaveNode(node) { + this.currentNode = node; + + /* + * Updates the code path. + * And emits onCodePathStart/onCodePathSegmentStart events. + */ + processCodePathToExit(this, node); + + // Emits the last onCodePathStart/onCodePathSegmentStart events. + postprocess(this, node); + + this.currentNode = null; + } + + /** + * This is called on a code path looped. + * Then this raises a looped event. + * @param {CodePathSegment} fromSegment A segment of prev. + * @param {CodePathSegment} toSegment A segment of next. + * @returns {void} + */ + onLooped(fromSegment, toSegment) { + if (fromSegment.reachable && toSegment.reachable) { + this.emitter.emit( + 'onCodePathSegmentLoop', + fromSegment, + toSegment, + this.currentNode, + ); + } + } +} + +module.exports = CodePathAnalyzer; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-segment.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-segment.js new file mode 100644 index 0000000000000..fb015296a74fb --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-segment.js @@ -0,0 +1,225 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Checks whether or not a given segment is reachable. + * @param {CodePathSegment} segment A segment to check. + * @returns {boolean} `true` if the segment is reachable. + */ +function isReachable(segment) { + return segment.reachable; +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * A code path segment. + */ +class CodePathSegment { + /** + * @param {string} id An identifier. + * @param {CodePathSegment[]} allPrevSegments An array of the previous segments. + * This array includes unreachable segments. + * @param {boolean} reachable A flag which shows this is reachable. + */ + constructor(id, allPrevSegments, reachable) { + /** + * The identifier of this code path. + * Rules use it to store additional information of each rule. + * @type {string} + */ + this.id = id; + + /** + * An array of the next segments. + * @type {CodePathSegment[]} + */ + this.nextSegments = []; + + /** + * An array of the previous segments. + * @type {CodePathSegment[]} + */ + this.prevSegments = allPrevSegments.filter(isReachable); + + /** + * An array of the next segments. + * This array includes unreachable segments. + * @type {CodePathSegment[]} + */ + this.allNextSegments = []; + + /** + * An array of the previous segments. + * This array includes unreachable segments. + * @type {CodePathSegment[]} + */ + this.allPrevSegments = allPrevSegments; + + /** + * A flag which shows this is reachable. + * @type {boolean} + */ + this.reachable = reachable; + + // Internal data. + Object.defineProperty(this, 'internal', { + value: { + used: false, + loopedPrevSegments: [], + }, + }); + } + + /** + * Checks a given previous segment is coming from the end of a loop. + * @param {CodePathSegment} segment A previous segment to check. + * @returns {boolean} `true` if the segment is coming from the end of a loop. + */ + isLoopedPrevSegment(segment) { + return this.internal.loopedPrevSegments.includes(segment); + } + + /** + * Creates the root segment. + * @param {string} id An identifier. + * @returns {CodePathSegment} The created segment. + */ + static newRoot(id) { + return new CodePathSegment(id, [], true); + } + + /** + * Creates a segment that follows given segments. + * @param {string} id An identifier. + * @param {CodePathSegment[]} allPrevSegments An array of the previous segments. + * @returns {CodePathSegment} The created segment. + */ + static newNext(id, allPrevSegments) { + return new CodePathSegment( + id, + CodePathSegment.flattenUnusedSegments(allPrevSegments), + allPrevSegments.some(isReachable), + ); + } + + /** + * Creates an unreachable segment that follows given segments. + * @param {string} id An identifier. + * @param {CodePathSegment[]} allPrevSegments An array of the previous segments. + * @returns {CodePathSegment} The created segment. + */ + static newUnreachable(id, allPrevSegments) { + const segment = new CodePathSegment( + id, + CodePathSegment.flattenUnusedSegments(allPrevSegments), + false, + ); + + /* + * In `if (a) return a; foo();` case, the unreachable segment preceded by + * the return statement is not used but must not be remove. + */ + CodePathSegment.markUsed(segment); + + return segment; + } + + /** + * Creates a segment that follows given segments. + * This factory method does not connect with `allPrevSegments`. + * But this inherits `reachable` flag. + * @param {string} id An identifier. + * @param {CodePathSegment[]} allPrevSegments An array of the previous segments. + * @returns {CodePathSegment} The created segment. + */ + static newDisconnected(id, allPrevSegments) { + return new CodePathSegment(id, [], allPrevSegments.some(isReachable)); + } + + /** + * Makes a given segment being used. + * + * And this function registers the segment into the previous segments as a next. + * @param {CodePathSegment} segment A segment to mark. + * @returns {void} + */ + static markUsed(segment) { + if (segment.internal.used) { + return; + } + segment.internal.used = true; + + let i; + + if (segment.reachable) { + for (i = 0; i < segment.allPrevSegments.length; ++i) { + const prevSegment = segment.allPrevSegments[i]; + + prevSegment.allNextSegments.push(segment); + prevSegment.nextSegments.push(segment); + } + } else { + for (i = 0; i < segment.allPrevSegments.length; ++i) { + segment.allPrevSegments[i].allNextSegments.push(segment); + } + } + } + + /** + * Marks a previous segment as looped. + * @param {CodePathSegment} segment A segment. + * @param {CodePathSegment} prevSegment A previous segment to mark. + * @returns {void} + */ + static markPrevSegmentAsLooped(segment, prevSegment) { + segment.internal.loopedPrevSegments.push(prevSegment); + } + + /** + * Replaces unused segments with the previous segments of each unused segment. + * @param {CodePathSegment[]} segments An array of segments to replace. + * @returns {CodePathSegment[]} The replaced array. + */ + static flattenUnusedSegments(segments) { + const done = Object.create(null); + const retv = []; + + for (let i = 0; i < segments.length; ++i) { + const segment = segments[i]; + + // Ignores duplicated. + if (done[segment.id]) { + continue; + } + + // Use previous segments if unused. + if (!segment.internal.used) { + for (let j = 0; j < segment.allPrevSegments.length; ++j) { + const prevSegment = segment.allPrevSegments[j]; + + if (!done[prevSegment.id]) { + done[prevSegment.id] = true; + retv.push(prevSegment); + } + } + } else { + done[segment.id] = true; + retv.push(segment); + } + } + + return retv; + } +} + +module.exports = CodePathSegment; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-state.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-state.js new file mode 100644 index 0000000000000..9a107fecd7f9c --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path-state.js @@ -0,0 +1,1441 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +// eslint-disable-next-line +const CodePathSegment = require('./code-path-segment'); +// eslint-disable-next-line +const ForkContext = require('./fork-context'); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Adds given segments into the `dest` array. + * If the `others` array does not includes the given segments, adds to the `all` + * array as well. + * + * This adds only reachable and used segments. + * @param {CodePathSegment[]} dest A destination array (`returnedSegments` or `thrownSegments`). + * @param {CodePathSegment[]} others Another destination array (`returnedSegments` or `thrownSegments`). + * @param {CodePathSegment[]} all The unified destination array (`finalSegments`). + * @param {CodePathSegment[]} segments Segments to add. + * @returns {void} + */ +function addToReturnedOrThrown(dest, others, all, segments) { + for (let i = 0; i < segments.length; ++i) { + const segment = segments[i]; + + dest.push(segment); + if (!others.includes(segment)) { + all.push(segment); + } + } +} + +/** + * Gets a loop-context for a `continue` statement. + * @param {CodePathState} state A state to get. + * @param {string} label The label of a `continue` statement. + * @returns {LoopContext} A loop-context for a `continue` statement. + */ +function getContinueContext(state, label) { + if (!label) { + return state.loopContext; + } + + let context = state.loopContext; + + while (context) { + if (context.label === label) { + return context; + } + context = context.upper; + } + + /* c8 ignore next */ + return null; +} + +/** + * Gets a context for a `break` statement. + * @param {CodePathState} state A state to get. + * @param {string} label The label of a `break` statement. + * @returns {LoopContext|SwitchContext} A context for a `break` statement. + */ +function getBreakContext(state, label) { + let context = state.breakContext; + + while (context) { + if (label ? context.label === label : context.breakable) { + return context; + } + context = context.upper; + } + + /* c8 ignore next */ + return null; +} + +/** + * Gets a context for a `return` statement. + * @param {CodePathState} state A state to get. + * @returns {TryContext|CodePathState} A context for a `return` statement. + */ +function getReturnContext(state) { + let context = state.tryContext; + + while (context) { + if (context.hasFinalizer && context.position !== 'finally') { + return context; + } + context = context.upper; + } + + return state; +} + +/** + * Gets a context for a `throw` statement. + * @param {CodePathState} state A state to get. + * @returns {TryContext|CodePathState} A context for a `throw` statement. + */ +function getThrowContext(state) { + let context = state.tryContext; + + while (context) { + if ( + context.position === 'try' || + (context.hasFinalizer && context.position === 'catch') + ) { + return context; + } + context = context.upper; + } + + return state; +} + +/** + * Removes a given element from a given array. + * @param {any[]} xs An array to remove the specific element. + * @param {any} x An element to be removed. + * @returns {void} + */ +function remove(xs, x) { + xs.splice(xs.indexOf(x), 1); +} + +/** + * Disconnect given segments. + * + * This is used in a process for switch statements. + * If there is the "default" chunk before other cases, the order is different + * between node's and running's. + * @param {CodePathSegment[]} prevSegments Forward segments to disconnect. + * @param {CodePathSegment[]} nextSegments Backward segments to disconnect. + * @returns {void} + */ +function removeConnection(prevSegments, nextSegments) { + for (let i = 0; i < prevSegments.length; ++i) { + const prevSegment = prevSegments[i]; + const nextSegment = nextSegments[i]; + + remove(prevSegment.nextSegments, nextSegment); + remove(prevSegment.allNextSegments, nextSegment); + remove(nextSegment.prevSegments, prevSegment); + remove(nextSegment.allPrevSegments, prevSegment); + } +} + +/** + * Creates looping path. + * @param {CodePathState} state The instance. + * @param {CodePathSegment[]} unflattenedFromSegments Segments which are source. + * @param {CodePathSegment[]} unflattenedToSegments Segments which are destination. + * @returns {void} + */ +function makeLooped(state, unflattenedFromSegments, unflattenedToSegments) { + const fromSegments = CodePathSegment.flattenUnusedSegments( + unflattenedFromSegments, + ); + const toSegments = CodePathSegment.flattenUnusedSegments( + unflattenedToSegments, + ); + + const end = Math.min(fromSegments.length, toSegments.length); + + for (let i = 0; i < end; ++i) { + const fromSegment = fromSegments[i]; + const toSegment = toSegments[i]; + + if (toSegment.reachable) { + fromSegment.nextSegments.push(toSegment); + } + if (fromSegment.reachable) { + toSegment.prevSegments.push(fromSegment); + } + fromSegment.allNextSegments.push(toSegment); + toSegment.allPrevSegments.push(fromSegment); + + if (toSegment.allPrevSegments.length >= 2) { + CodePathSegment.markPrevSegmentAsLooped(toSegment, fromSegment); + } + + state.notifyLooped(fromSegment, toSegment); + } +} + +/** + * Finalizes segments of `test` chunk of a ForStatement. + * + * - Adds `false` paths to paths which are leaving from the loop. + * - Sets `true` paths to paths which go to the body. + * @param {LoopContext} context A loop context to modify. + * @param {ChoiceContext} choiceContext A choice context of this loop. + * @param {CodePathSegment[]} head The current head paths. + * @returns {void} + */ +function finalizeTestSegmentsOfFor(context, choiceContext, head) { + if (!choiceContext.processed) { + choiceContext.trueForkContext.add(head); + choiceContext.falseForkContext.add(head); + choiceContext.qqForkContext.add(head); + } + + if (context.test !== true) { + context.brokenForkContext.addAll(choiceContext.falseForkContext); + } + context.endOfTestSegments = choiceContext.trueForkContext.makeNext(0, -1); +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * A class which manages state to analyze code paths. + */ +class CodePathState { + /** + * @param {IdGenerator} idGenerator An id generator to generate id for code + * path segments. + * @param {Function} onLooped A callback function to notify looping. + */ + constructor(idGenerator, onLooped) { + this.idGenerator = idGenerator; + this.notifyLooped = onLooped; + this.forkContext = ForkContext.newRoot(idGenerator); + this.choiceContext = null; + this.switchContext = null; + this.tryContext = null; + this.loopContext = null; + this.breakContext = null; + this.chainContext = null; + + this.currentSegments = []; + this.initialSegment = this.forkContext.head[0]; + + // returnedSegments and thrownSegments push elements into finalSegments also. + const final = (this.finalSegments = []); + const returned = (this.returnedForkContext = []); + const thrown = (this.thrownForkContext = []); + + returned.add = addToReturnedOrThrown.bind(null, returned, thrown, final); + thrown.add = addToReturnedOrThrown.bind(null, thrown, returned, final); + } + + /** + * The head segments. + * @type {CodePathSegment[]} + */ + get headSegments() { + return this.forkContext.head; + } + + /** + * The parent forking context. + * This is used for the root of new forks. + * @type {ForkContext} + */ + get parentForkContext() { + const current = this.forkContext; + + return current && current.upper; + } + + /** + * Creates and stacks new forking context. + * @param {boolean} forkLeavingPath A flag which shows being in a + * "finally" block. + * @returns {ForkContext} The created context. + */ + pushForkContext(forkLeavingPath) { + this.forkContext = ForkContext.newEmpty(this.forkContext, forkLeavingPath); + + return this.forkContext; + } + + /** + * Pops and merges the last forking context. + * @returns {ForkContext} The last context. + */ + popForkContext() { + const lastContext = this.forkContext; + + this.forkContext = lastContext.upper; + this.forkContext.replaceHead(lastContext.makeNext(0, -1)); + + return lastContext; + } + + /** + * Creates a new path. + * @returns {void} + */ + forkPath() { + this.forkContext.add(this.parentForkContext.makeNext(-1, -1)); + } + + /** + * Creates a bypass path. + * This is used for such as IfStatement which does not have "else" chunk. + * @returns {void} + */ + forkBypassPath() { + this.forkContext.add(this.parentForkContext.head); + } + + //-------------------------------------------------------------------------- + // ConditionalExpression, LogicalExpression, IfStatement + //-------------------------------------------------------------------------- + + /** + * Creates a context for ConditionalExpression, LogicalExpression, AssignmentExpression (logical assignments only), + * IfStatement, WhileStatement, DoWhileStatement, or ForStatement. + * + * LogicalExpressions have cases that it goes different paths between the + * `true` case and the `false` case. + * + * For Example: + * + * if (a || b) { + * foo(); + * } else { + * bar(); + * } + * + * In this case, `b` is evaluated always in the code path of the `else` + * block, but it's not so in the code path of the `if` block. + * So there are 3 paths. + * + * a -> foo(); + * a -> b -> foo(); + * a -> b -> bar(); + * @param {string} kind A kind string. + * If the new context is LogicalExpression's or AssignmentExpression's, this is `"&&"` or `"||"` or `"??"`. + * If it's IfStatement's or ConditionalExpression's, this is `"test"`. + * Otherwise, this is `"loop"`. + * @param {boolean} isForkingAsResult A flag that shows that goes different + * paths between `true` and `false`. + * @returns {void} + */ + pushChoiceContext(kind, isForkingAsResult) { + this.choiceContext = { + upper: this.choiceContext, + kind, + isForkingAsResult, + trueForkContext: ForkContext.newEmpty(this.forkContext), + falseForkContext: ForkContext.newEmpty(this.forkContext), + qqForkContext: ForkContext.newEmpty(this.forkContext), + processed: false, + }; + } + + /** + * Pops the last choice context and finalizes it. + * @throws {Error} (Unreachable.) + * @returns {ChoiceContext} The popped context. + */ + popChoiceContext() { + const context = this.choiceContext; + + this.choiceContext = context.upper; + + const forkContext = this.forkContext; + const headSegments = forkContext.head; + + switch (context.kind) { + case '&&': + case '||': + case '??': + /* + * If any result were not transferred from child contexts, + * this sets the head segments to both cases. + * The head segments are the path of the right-hand operand. + */ + if (!context.processed) { + context.trueForkContext.add(headSegments); + context.falseForkContext.add(headSegments); + context.qqForkContext.add(headSegments); + } + + /* + * Transfers results to upper context if this context is in + * test chunk. + */ + if (context.isForkingAsResult) { + const parentContext = this.choiceContext; + + parentContext.trueForkContext.addAll(context.trueForkContext); + parentContext.falseForkContext.addAll(context.falseForkContext); + parentContext.qqForkContext.addAll(context.qqForkContext); + parentContext.processed = true; + + return context; + } + + break; + + case 'test': + if (!context.processed) { + /* + * The head segments are the path of the `if` block here. + * Updates the `true` path with the end of the `if` block. + */ + context.trueForkContext.clear(); + context.trueForkContext.add(headSegments); + } else { + /* + * The head segments are the path of the `else` block here. + * Updates the `false` path with the end of the `else` + * block. + */ + context.falseForkContext.clear(); + context.falseForkContext.add(headSegments); + } + + break; + + case 'loop': + /* + * Loops are addressed in popLoopContext(). + * This is called from popLoopContext(). + */ + return context; + + /* c8 ignore next */ + default: + throw new Error('unreachable'); + } + + // Merges all paths. + const prevForkContext = context.trueForkContext; + + prevForkContext.addAll(context.falseForkContext); + forkContext.replaceHead(prevForkContext.makeNext(0, -1)); + + return context; + } + + /** + * Makes a code path segment of the right-hand operand of a logical + * expression. + * @throws {Error} (Unreachable.) + * @returns {void} + */ + makeLogicalRight() { + const context = this.choiceContext; + const forkContext = this.forkContext; + + if (context.processed) { + /* + * This got segments already from the child choice context. + * Creates the next path from own true/false fork context. + */ + let prevForkContext; + + switch (context.kind) { + case '&&': // if true then go to the right-hand side. + prevForkContext = context.trueForkContext; + break; + case '||': // if false then go to the right-hand side. + prevForkContext = context.falseForkContext; + break; + case '??': // Both true/false can short-circuit, so needs the third path to go to the right-hand side. That's qqForkContext. + prevForkContext = context.qqForkContext; + break; + default: + throw new Error('unreachable'); + } + + forkContext.replaceHead(prevForkContext.makeNext(0, -1)); + prevForkContext.clear(); + context.processed = false; + } else { + /* + * This did not get segments from the child choice context. + * So addresses the head segments. + * The head segments are the path of the left-hand operand. + */ + switch (context.kind) { + case '&&': // the false path can short-circuit. + context.falseForkContext.add(forkContext.head); + break; + case '||': // the true path can short-circuit. + context.trueForkContext.add(forkContext.head); + break; + case '??': // both can short-circuit. + context.trueForkContext.add(forkContext.head); + context.falseForkContext.add(forkContext.head); + break; + default: + throw new Error('unreachable'); + } + + forkContext.replaceHead(forkContext.makeNext(-1, -1)); + } + } + + /** + * Makes a code path segment of the `if` block. + * @returns {void} + */ + makeIfConsequent() { + const context = this.choiceContext; + const forkContext = this.forkContext; + + /* + * If any result were not transferred from child contexts, + * this sets the head segments to both cases. + * The head segments are the path of the test expression. + */ + if (!context.processed) { + context.trueForkContext.add(forkContext.head); + context.falseForkContext.add(forkContext.head); + context.qqForkContext.add(forkContext.head); + } + + context.processed = false; + + // Creates new path from the `true` case. + forkContext.replaceHead(context.trueForkContext.makeNext(0, -1)); + } + + /** + * Makes a code path segment of the `else` block. + * @returns {void} + */ + makeIfAlternate() { + const context = this.choiceContext; + const forkContext = this.forkContext; + + /* + * The head segments are the path of the `if` block. + * Updates the `true` path with the end of the `if` block. + */ + context.trueForkContext.clear(); + context.trueForkContext.add(forkContext.head); + context.processed = true; + + // Creates new path from the `false` case. + forkContext.replaceHead(context.falseForkContext.makeNext(0, -1)); + } + + //-------------------------------------------------------------------------- + // ChainExpression + //-------------------------------------------------------------------------- + + /** + * Push a new `ChainExpression` context to the stack. + * This method is called on entering to each `ChainExpression` node. + * This context is used to count forking in the optional chain then merge them on the exiting from the `ChainExpression` node. + * @returns {void} + */ + pushChainContext() { + this.chainContext = { + upper: this.chainContext, + countChoiceContexts: 0, + }; + } + + /** + * Pop a `ChainExpression` context from the stack. + * This method is called on exiting from each `ChainExpression` node. + * This merges all forks of the last optional chaining. + * @returns {void} + */ + popChainContext() { + const context = this.chainContext; + + this.chainContext = context.upper; + + // pop all choice contexts of this. + for (let i = context.countChoiceContexts; i > 0; --i) { + this.popChoiceContext(); + } + } + + /** + * Create a choice context for optional access. + * This method is called on entering to each `(Call|Member)Expression[optional=true]` node. + * This creates a choice context as similar to `LogicalExpression[operator="??"]` node. + * @returns {void} + */ + makeOptionalNode() { + if (this.chainContext) { + this.chainContext.countChoiceContexts += 1; + this.pushChoiceContext('??', false); + } + } + + /** + * Create a fork. + * This method is called on entering to the `arguments|property` property of each `(Call|Member)Expression` node. + * @returns {void} + */ + makeOptionalRight() { + if (this.chainContext) { + this.makeLogicalRight(); + } + } + + //-------------------------------------------------------------------------- + // SwitchStatement + //-------------------------------------------------------------------------- + + /** + * Creates a context object of SwitchStatement and stacks it. + * @param {boolean} hasCase `true` if the switch statement has one or more + * case parts. + * @param {string|null} label The label text. + * @returns {void} + */ + pushSwitchContext(hasCase, label) { + this.switchContext = { + upper: this.switchContext, + hasCase, + defaultSegments: null, + defaultBodySegments: null, + foundDefault: false, + lastIsDefault: false, + countForks: 0, + }; + + this.pushBreakContext(true, label); + } + + /** + * Pops the last context of SwitchStatement and finalizes it. + * + * - Disposes all forking stack for `case` and `default`. + * - Creates the next code path segment from `context.brokenForkContext`. + * - If the last `SwitchCase` node is not a `default` part, creates a path + * to the `default` body. + * @returns {void} + */ + popSwitchContext() { + const context = this.switchContext; + + this.switchContext = context.upper; + + const forkContext = this.forkContext; + const brokenForkContext = this.popBreakContext().brokenForkContext; + + if (context.countForks === 0) { + /* + * When there is only one `default` chunk and there is one or more + * `break` statements, even if forks are nothing, it needs to merge + * those. + */ + if (!brokenForkContext.empty) { + brokenForkContext.add(forkContext.makeNext(-1, -1)); + forkContext.replaceHead(brokenForkContext.makeNext(0, -1)); + } + + return; + } + + const lastSegments = forkContext.head; + + this.forkBypassPath(); + const lastCaseSegments = forkContext.head; + + /* + * `brokenForkContext` is used to make the next segment. + * It must add the last segment into `brokenForkContext`. + */ + brokenForkContext.add(lastSegments); + + /* + * A path which is failed in all case test should be connected to path + * of `default` chunk. + */ + if (!context.lastIsDefault) { + if (context.defaultBodySegments) { + /* + * Remove a link from `default` label to its chunk. + * It's false route. + */ + removeConnection(context.defaultSegments, context.defaultBodySegments); + makeLooped(this, lastCaseSegments, context.defaultBodySegments); + } else { + /* + * It handles the last case body as broken if `default` chunk + * does not exist. + */ + brokenForkContext.add(lastCaseSegments); + } + } + + // Pops the segment context stack until the entry segment. + for (let i = 0; i < context.countForks; ++i) { + this.forkContext = this.forkContext.upper; + } + + /* + * Creates a path from all brokenForkContext paths. + * This is a path after switch statement. + */ + this.forkContext.replaceHead(brokenForkContext.makeNext(0, -1)); + } + + /** + * Makes a code path segment for a `SwitchCase` node. + * @param {boolean} isEmpty `true` if the body is empty. + * @param {boolean} isDefault `true` if the body is the default case. + * @returns {void} + */ + makeSwitchCaseBody(isEmpty, isDefault) { + const context = this.switchContext; + + if (!context.hasCase) { + return; + } + + /* + * Merge forks. + * The parent fork context has two segments. + * Those are from the current case and the body of the previous case. + */ + const parentForkContext = this.forkContext; + const forkContext = this.pushForkContext(); + + forkContext.add(parentForkContext.makeNext(0, -1)); + + /* + * Save `default` chunk info. + * If the `default` label is not at the last, we must make a path from + * the last `case` to the `default` chunk. + */ + if (isDefault) { + context.defaultSegments = parentForkContext.head; + if (isEmpty) { + context.foundDefault = true; + } else { + context.defaultBodySegments = forkContext.head; + } + } else { + if (!isEmpty && context.foundDefault) { + context.foundDefault = false; + context.defaultBodySegments = forkContext.head; + } + } + + context.lastIsDefault = isDefault; + context.countForks += 1; + } + + //-------------------------------------------------------------------------- + // TryStatement + //-------------------------------------------------------------------------- + + /** + * Creates a context object of TryStatement and stacks it. + * @param {boolean} hasFinalizer `true` if the try statement has a + * `finally` block. + * @returns {void} + */ + pushTryContext(hasFinalizer) { + this.tryContext = { + upper: this.tryContext, + position: 'try', + hasFinalizer, + + returnedForkContext: hasFinalizer + ? ForkContext.newEmpty(this.forkContext) + : null, + + thrownForkContext: ForkContext.newEmpty(this.forkContext), + lastOfTryIsReachable: false, + lastOfCatchIsReachable: false, + }; + } + + /** + * Pops the last context of TryStatement and finalizes it. + * @returns {void} + */ + popTryContext() { + const context = this.tryContext; + + this.tryContext = context.upper; + + if (context.position === 'catch') { + // Merges two paths from the `try` block and `catch` block merely. + this.popForkContext(); + return; + } + + /* + * The following process is executed only when there is the `finally` + * block. + */ + + const returned = context.returnedForkContext; + const thrown = context.thrownForkContext; + + if (returned.empty && thrown.empty) { + return; + } + + // Separate head to normal paths and leaving paths. + const headSegments = this.forkContext.head; + + this.forkContext = this.forkContext.upper; + const normalSegments = headSegments.slice(0, (headSegments.length / 2) | 0); + const leavingSegments = headSegments.slice((headSegments.length / 2) | 0); + + // Forwards the leaving path to upper contexts. + if (!returned.empty) { + getReturnContext(this).returnedForkContext.add(leavingSegments); + } + if (!thrown.empty) { + getThrowContext(this).thrownForkContext.add(leavingSegments); + } + + // Sets the normal path as the next. + this.forkContext.replaceHead(normalSegments); + + /* + * If both paths of the `try` block and the `catch` block are + * unreachable, the next path becomes unreachable as well. + */ + if (!context.lastOfTryIsReachable && !context.lastOfCatchIsReachable) { + this.forkContext.makeUnreachable(); + } + } + + /** + * Makes a code path segment for a `catch` block. + * @returns {void} + */ + makeCatchBlock() { + const context = this.tryContext; + const forkContext = this.forkContext; + const thrown = context.thrownForkContext; + + // Update state. + context.position = 'catch'; + context.thrownForkContext = ForkContext.newEmpty(forkContext); + context.lastOfTryIsReachable = forkContext.reachable; + + // Merge thrown paths. + thrown.add(forkContext.head); + const thrownSegments = thrown.makeNext(0, -1); + + // Fork to a bypass and the merged thrown path. + this.pushForkContext(); + this.forkBypassPath(); + this.forkContext.add(thrownSegments); + } + + /** + * Makes a code path segment for a `finally` block. + * + * In the `finally` block, parallel paths are created. The parallel paths + * are used as leaving-paths. The leaving-paths are paths from `return` + * statements and `throw` statements in a `try` block or a `catch` block. + * @returns {void} + */ + makeFinallyBlock() { + const context = this.tryContext; + let forkContext = this.forkContext; + const returned = context.returnedForkContext; + const thrown = context.thrownForkContext; + const headOfLeavingSegments = forkContext.head; + + // Update state. + if (context.position === 'catch') { + // Merges two paths from the `try` block and `catch` block. + this.popForkContext(); + forkContext = this.forkContext; + + context.lastOfCatchIsReachable = forkContext.reachable; + } else { + context.lastOfTryIsReachable = forkContext.reachable; + } + context.position = 'finally'; + + if (returned.empty && thrown.empty) { + // This path does not leave. + return; + } + + /* + * Create a parallel segment from merging returned and thrown. + * This segment will leave at the end of this finally block. + */ + const segments = forkContext.makeNext(-1, -1); + + for (let i = 0; i < forkContext.count; ++i) { + const prevSegsOfLeavingSegment = [headOfLeavingSegments[i]]; + + for (let j = 0; j < returned.segmentsList.length; ++j) { + prevSegsOfLeavingSegment.push(returned.segmentsList[j][i]); + } + for (let j = 0; j < thrown.segmentsList.length; ++j) { + prevSegsOfLeavingSegment.push(thrown.segmentsList[j][i]); + } + + segments.push( + CodePathSegment.newNext( + this.idGenerator.next(), + prevSegsOfLeavingSegment, + ), + ); + } + + this.pushForkContext(true); + this.forkContext.add(segments); + } + + /** + * Makes a code path segment from the first throwable node to the `catch` + * block or the `finally` block. + * @returns {void} + */ + makeFirstThrowablePathInTryBlock() { + const forkContext = this.forkContext; + + if (!forkContext.reachable) { + return; + } + + const context = getThrowContext(this); + + if ( + context === this || + context.position !== 'try' || + !context.thrownForkContext.empty + ) { + return; + } + + context.thrownForkContext.add(forkContext.head); + forkContext.replaceHead(forkContext.makeNext(-1, -1)); + } + + //-------------------------------------------------------------------------- + // Loop Statements + //-------------------------------------------------------------------------- + + /** + * Creates a context object of a loop statement and stacks it. + * @param {string} type The type of the node which was triggered. One of + * `WhileStatement`, `DoWhileStatement`, `ForStatement`, `ForInStatement`, + * and `ForStatement`. + * @param {string|null} label A label of the node which was triggered. + * @throws {Error} (Unreachable - unknown type.) + * @returns {void} + */ + pushLoopContext(type, label) { + const forkContext = this.forkContext; + const breakContext = this.pushBreakContext(true, label); + + switch (type) { + case 'WhileStatement': + this.pushChoiceContext('loop', false); + this.loopContext = { + upper: this.loopContext, + type, + label, + test: void 0, + continueDestSegments: null, + brokenForkContext: breakContext.brokenForkContext, + }; + break; + + case 'DoWhileStatement': + this.pushChoiceContext('loop', false); + this.loopContext = { + upper: this.loopContext, + type, + label, + test: void 0, + entrySegments: null, + continueForkContext: ForkContext.newEmpty(forkContext), + brokenForkContext: breakContext.brokenForkContext, + }; + break; + + case 'ForStatement': + this.pushChoiceContext('loop', false); + this.loopContext = { + upper: this.loopContext, + type, + label, + test: void 0, + endOfInitSegments: null, + testSegments: null, + endOfTestSegments: null, + updateSegments: null, + endOfUpdateSegments: null, + continueDestSegments: null, + brokenForkContext: breakContext.brokenForkContext, + }; + break; + + case 'ForInStatement': + case 'ForOfStatement': + this.loopContext = { + upper: this.loopContext, + type, + label, + prevSegments: null, + leftSegments: null, + endOfLeftSegments: null, + continueDestSegments: null, + brokenForkContext: breakContext.brokenForkContext, + }; + break; + + /* c8 ignore next */ + default: + throw new Error(`unknown type: "${type}"`); + } + } + + /** + * Pops the last context of a loop statement and finalizes it. + * @throws {Error} (Unreachable - unknown type.) + * @returns {void} + */ + popLoopContext() { + const context = this.loopContext; + + this.loopContext = context.upper; + + const forkContext = this.forkContext; + const brokenForkContext = this.popBreakContext().brokenForkContext; + + // Creates a looped path. + switch (context.type) { + case 'WhileStatement': + case 'ForStatement': + this.popChoiceContext(); + makeLooped(this, forkContext.head, context.continueDestSegments); + break; + + case 'DoWhileStatement': { + const choiceContext = this.popChoiceContext(); + + if (!choiceContext.processed) { + choiceContext.trueForkContext.add(forkContext.head); + choiceContext.falseForkContext.add(forkContext.head); + } + if (context.test !== true) { + brokenForkContext.addAll(choiceContext.falseForkContext); + } + + // `true` paths go to looping. + const segmentsList = choiceContext.trueForkContext.segmentsList; + + for (let i = 0; i < segmentsList.length; ++i) { + makeLooped(this, segmentsList[i], context.entrySegments); + } + break; + } + + case 'ForInStatement': + case 'ForOfStatement': + brokenForkContext.add(forkContext.head); + makeLooped(this, forkContext.head, context.leftSegments); + break; + + /* c8 ignore next */ + default: + throw new Error('unreachable'); + } + + // Go next. + if (brokenForkContext.empty) { + forkContext.replaceHead(forkContext.makeUnreachable(-1, -1)); + } else { + forkContext.replaceHead(brokenForkContext.makeNext(0, -1)); + } + } + + /** + * Makes a code path segment for the test part of a WhileStatement. + * @param {boolean|undefined} test The test value (only when constant). + * @returns {void} + */ + makeWhileTest(test) { + const context = this.loopContext; + const forkContext = this.forkContext; + const testSegments = forkContext.makeNext(0, -1); + + // Update state. + context.test = test; + context.continueDestSegments = testSegments; + forkContext.replaceHead(testSegments); + } + + /** + * Makes a code path segment for the body part of a WhileStatement. + * @returns {void} + */ + makeWhileBody() { + const context = this.loopContext; + const choiceContext = this.choiceContext; + const forkContext = this.forkContext; + + if (!choiceContext.processed) { + choiceContext.trueForkContext.add(forkContext.head); + choiceContext.falseForkContext.add(forkContext.head); + } + + // Update state. + if (context.test !== true) { + context.brokenForkContext.addAll(choiceContext.falseForkContext); + } + forkContext.replaceHead(choiceContext.trueForkContext.makeNext(0, -1)); + } + + /** + * Makes a code path segment for the body part of a DoWhileStatement. + * @returns {void} + */ + makeDoWhileBody() { + const context = this.loopContext; + const forkContext = this.forkContext; + const bodySegments = forkContext.makeNext(-1, -1); + + // Update state. + context.entrySegments = bodySegments; + forkContext.replaceHead(bodySegments); + } + + /** + * Makes a code path segment for the test part of a DoWhileStatement. + * @param {boolean|undefined} test The test value (only when constant). + * @returns {void} + */ + makeDoWhileTest(test) { + const context = this.loopContext; + const forkContext = this.forkContext; + + context.test = test; + + // Creates paths of `continue` statements. + if (!context.continueForkContext.empty) { + context.continueForkContext.add(forkContext.head); + const testSegments = context.continueForkContext.makeNext(0, -1); + + forkContext.replaceHead(testSegments); + } + } + + /** + * Makes a code path segment for the test part of a ForStatement. + * @param {boolean|undefined} test The test value (only when constant). + * @returns {void} + */ + makeForTest(test) { + const context = this.loopContext; + const forkContext = this.forkContext; + const endOfInitSegments = forkContext.head; + const testSegments = forkContext.makeNext(-1, -1); + + // Update state. + context.test = test; + context.endOfInitSegments = endOfInitSegments; + context.continueDestSegments = context.testSegments = testSegments; + forkContext.replaceHead(testSegments); + } + + /** + * Makes a code path segment for the update part of a ForStatement. + * @returns {void} + */ + makeForUpdate() { + const context = this.loopContext; + const choiceContext = this.choiceContext; + const forkContext = this.forkContext; + + // Make the next paths of the test. + if (context.testSegments) { + finalizeTestSegmentsOfFor(context, choiceContext, forkContext.head); + } else { + context.endOfInitSegments = forkContext.head; + } + + // Update state. + const updateSegments = forkContext.makeDisconnected(-1, -1); + + context.continueDestSegments = context.updateSegments = updateSegments; + forkContext.replaceHead(updateSegments); + } + + /** + * Makes a code path segment for the body part of a ForStatement. + * @returns {void} + */ + makeForBody() { + const context = this.loopContext; + const choiceContext = this.choiceContext; + const forkContext = this.forkContext; + + // Update state. + if (context.updateSegments) { + context.endOfUpdateSegments = forkContext.head; + + // `update` -> `test` + if (context.testSegments) { + makeLooped(this, context.endOfUpdateSegments, context.testSegments); + } + } else if (context.testSegments) { + finalizeTestSegmentsOfFor(context, choiceContext, forkContext.head); + } else { + context.endOfInitSegments = forkContext.head; + } + + let bodySegments = context.endOfTestSegments; + + if (!bodySegments) { + /* + * If there is not the `test` part, the `body` path comes from the + * `init` part and the `update` part. + */ + const prevForkContext = ForkContext.newEmpty(forkContext); + + prevForkContext.add(context.endOfInitSegments); + if (context.endOfUpdateSegments) { + prevForkContext.add(context.endOfUpdateSegments); + } + + bodySegments = prevForkContext.makeNext(0, -1); + } + context.continueDestSegments = context.continueDestSegments || bodySegments; + forkContext.replaceHead(bodySegments); + } + + /** + * Makes a code path segment for the left part of a ForInStatement and a + * ForOfStatement. + * @returns {void} + */ + makeForInOfLeft() { + const context = this.loopContext; + const forkContext = this.forkContext; + const leftSegments = forkContext.makeDisconnected(-1, -1); + + // Update state. + context.prevSegments = forkContext.head; + context.leftSegments = context.continueDestSegments = leftSegments; + forkContext.replaceHead(leftSegments); + } + + /** + * Makes a code path segment for the right part of a ForInStatement and a + * ForOfStatement. + * @returns {void} + */ + makeForInOfRight() { + const context = this.loopContext; + const forkContext = this.forkContext; + const temp = ForkContext.newEmpty(forkContext); + + temp.add(context.prevSegments); + const rightSegments = temp.makeNext(-1, -1); + + // Update state. + context.endOfLeftSegments = forkContext.head; + forkContext.replaceHead(rightSegments); + } + + /** + * Makes a code path segment for the body part of a ForInStatement and a + * ForOfStatement. + * @returns {void} + */ + makeForInOfBody() { + const context = this.loopContext; + const forkContext = this.forkContext; + const temp = ForkContext.newEmpty(forkContext); + + temp.add(context.endOfLeftSegments); + const bodySegments = temp.makeNext(-1, -1); + + // Make a path: `right` -> `left`. + makeLooped(this, forkContext.head, context.leftSegments); + + // Update state. + context.brokenForkContext.add(forkContext.head); + forkContext.replaceHead(bodySegments); + } + + //-------------------------------------------------------------------------- + // Control Statements + //-------------------------------------------------------------------------- + + /** + * Creates new context for BreakStatement. + * @param {boolean} breakable The flag to indicate it can break by + * an unlabeled BreakStatement. + * @param {string|null} label The label of this context. + * @returns {Object} The new context. + */ + pushBreakContext(breakable, label) { + this.breakContext = { + upper: this.breakContext, + breakable, + label, + brokenForkContext: ForkContext.newEmpty(this.forkContext), + }; + return this.breakContext; + } + + /** + * Removes the top item of the break context stack. + * @returns {Object} The removed context. + */ + popBreakContext() { + const context = this.breakContext; + const forkContext = this.forkContext; + + this.breakContext = context.upper; + + // Process this context here for other than switches and loops. + if (!context.breakable) { + const brokenForkContext = context.brokenForkContext; + + if (!brokenForkContext.empty) { + brokenForkContext.add(forkContext.head); + forkContext.replaceHead(brokenForkContext.makeNext(0, -1)); + } + } + + return context; + } + + /** + * Makes a path for a `break` statement. + * + * It registers the head segment to a context of `break`. + * It makes new unreachable segment, then it set the head with the segment. + * @param {string} label A label of the break statement. + * @returns {void} + */ + makeBreak(label) { + const forkContext = this.forkContext; + + if (!forkContext.reachable) { + return; + } + + const context = getBreakContext(this, label); + + if (context) { + context.brokenForkContext.add(forkContext.head); + } + + /* c8 ignore next */ + forkContext.replaceHead(forkContext.makeUnreachable(-1, -1)); + } + + /** + * Makes a path for a `continue` statement. + * + * It makes a looping path. + * It makes new unreachable segment, then it set the head with the segment. + * @param {string} label A label of the continue statement. + * @returns {void} + */ + makeContinue(label) { + const forkContext = this.forkContext; + + if (!forkContext.reachable) { + return; + } + + const context = getContinueContext(this, label); + + if (context) { + if (context.continueDestSegments) { + makeLooped(this, forkContext.head, context.continueDestSegments); + + // If the context is a for-in/of loop, this effects a break also. + if ( + context.type === 'ForInStatement' || + context.type === 'ForOfStatement' + ) { + context.brokenForkContext.add(forkContext.head); + } + } else { + context.continueForkContext.add(forkContext.head); + } + } + forkContext.replaceHead(forkContext.makeUnreachable(-1, -1)); + } + + /** + * Makes a path for a `return` statement. + * + * It registers the head segment to a context of `return`. + * It makes new unreachable segment, then it set the head with the segment. + * @returns {void} + */ + makeReturn() { + const forkContext = this.forkContext; + + if (forkContext.reachable) { + getReturnContext(this).returnedForkContext.add(forkContext.head); + forkContext.replaceHead(forkContext.makeUnreachable(-1, -1)); + } + } + + /** + * Makes a path for a `throw` statement. + * + * It registers the head segment to a context of `throw`. + * It makes new unreachable segment, then it set the head with the segment. + * @returns {void} + */ + makeThrow() { + const forkContext = this.forkContext; + + if (forkContext.reachable) { + getThrowContext(this).thrownForkContext.add(forkContext.head); + forkContext.replaceHead(forkContext.makeUnreachable(-1, -1)); + } + } + + /** + * Makes the final path. + * @returns {void} + */ + makeFinal() { + const segments = this.currentSegments; + + if (segments.length > 0 && segments[0].reachable) { + this.returnedForkContext.add(segments); + } + } +} + +module.exports = CodePathState; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path.js new file mode 100644 index 0000000000000..167d9be7af80f --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/code-path.js @@ -0,0 +1,239 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +// eslint-disable-next-line +const CodePathState = require('./code-path-state'); +// eslint-disable-next-line +const IdGenerator = require('./id-generator'); + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * A code path. + */ +class CodePath { + /** + * Creates a new instance. + * @param {Object} options Options for the function (see below). + * @param {string} options.id An identifier. + * @param {string} options.origin The type of code path origin. + * @param {CodePath|null} options.upper The code path of the upper function scope. + * @param {Function} options.onLooped A callback function to notify looping. + */ + constructor({id, origin, upper, onLooped}) { + /** + * The identifier of this code path. + * Rules use it to store additional information of each rule. + * @type {string} + */ + this.id = id; + + /** + * The reason that this code path was started. May be "program", + * "function", "class-field-initializer", or "class-static-block". + * @type {string} + */ + this.origin = origin; + + /** + * The code path of the upper function scope. + * @type {CodePath|null} + */ + this.upper = upper; + + /** + * The code paths of nested function scopes. + * @type {CodePath[]} + */ + this.childCodePaths = []; + + // Initializes internal state. + Object.defineProperty(this, 'internal', { + value: new CodePathState(new IdGenerator(`${id}_`), onLooped), + }); + + // Adds this into `childCodePaths` of `upper`. + if (upper) { + upper.childCodePaths.push(this); + } + } + + /** + * Gets the state of a given code path. + * @param {CodePath} codePath A code path to get. + * @returns {CodePathState} The state of the code path. + */ + static getState(codePath) { + return codePath.internal; + } + + /** + * The initial code path segment. + * @type {CodePathSegment} + */ + get initialSegment() { + return this.internal.initialSegment; + } + + /** + * Final code path segments. + * This array is a mix of `returnedSegments` and `thrownSegments`. + * @type {CodePathSegment[]} + */ + get finalSegments() { + return this.internal.finalSegments; + } + + /** + * Final code path segments which is with `return` statements. + * This array contains the last path segment if it's reachable. + * Since the reachable last path returns `undefined`. + * @type {CodePathSegment[]} + */ + get returnedSegments() { + return this.internal.returnedForkContext; + } + + /** + * Final code path segments which is with `throw` statements. + * @type {CodePathSegment[]} + */ + get thrownSegments() { + return this.internal.thrownForkContext; + } + + /** + * Current code path segments. + * @type {CodePathSegment[]} + */ + get currentSegments() { + return this.internal.currentSegments; + } + + /** + * Traverses all segments in this code path. + * + * codePath.traverseSegments(function(segment, controller) { + * // do something. + * }); + * + * This method enumerates segments in order from the head. + * + * The `controller` object has two methods. + * + * - `controller.skip()` - Skip the following segments in this branch. + * - `controller.break()` - Skip all following segments. + * @param {Object} [options] Omittable. + * @param {CodePathSegment} [options.first] The first segment to traverse. + * @param {CodePathSegment} [options.last] The last segment to traverse. + * @param {Function} callback A callback function. + * @returns {void} + */ + traverseSegments(options, callback) { + let resolvedOptions; + let resolvedCallback; + + if (typeof options === 'function') { + resolvedCallback = options; + resolvedOptions = {}; + } else { + resolvedOptions = options || {}; + resolvedCallback = callback; + } + + const startSegment = resolvedOptions.first || this.internal.initialSegment; + const lastSegment = resolvedOptions.last; + + let item = null; + let index = 0; + let end = 0; + let segment = null; + const visited = Object.create(null); + const stack = [[startSegment, 0]]; + let skippedSegment = null; + let broken = false; + const controller = { + skip() { + if (stack.length <= 1) { + broken = true; + } else { + skippedSegment = stack[stack.length - 2][0]; + } + }, + break() { + broken = true; + }, + }; + + /** + * Checks a given previous segment has been visited. + * @param {CodePathSegment} prevSegment A previous segment to check. + * @returns {boolean} `true` if the segment has been visited. + */ + function isVisited(prevSegment) { + return ( + visited[prevSegment.id] || segment.isLoopedPrevSegment(prevSegment) + ); + } + + while (stack.length > 0) { + item = stack[stack.length - 1]; + segment = item[0]; + index = item[1]; + + if (index === 0) { + // Skip if this segment has been visited already. + if (visited[segment.id]) { + stack.pop(); + continue; + } + + // Skip if all previous segments have not been visited. + if ( + segment !== startSegment && + segment.prevSegments.length > 0 && + !segment.prevSegments.every(isVisited) + ) { + stack.pop(); + continue; + } + + // Reset the flag of skipping if all branches have been skipped. + if (skippedSegment && segment.prevSegments.includes(skippedSegment)) { + skippedSegment = null; + } + visited[segment.id] = true; + + // Call the callback when the first time. + if (!skippedSegment) { + resolvedCallback.call(this, segment, controller); + if (segment === lastSegment) { + controller.skip(); + } + if (broken) { + break; + } + } + } + + // Update the stack. + end = segment.nextSegments.length - 1; + if (index < end) { + item[1] += 1; + stack.push([segment.nextSegments[index], 0]); + } else if (index === end) { + item[0] = segment.nextSegments[index]; + item[1] = 0; + } else { + stack.pop(); + } + } + } +} + +module.exports = CodePath; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/fork-context.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/fork-context.js new file mode 100644 index 0000000000000..528aaa41e15fb --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/fork-context.js @@ -0,0 +1,252 @@ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +// eslint-disable-next-line +const assert = require('./assert'); +// eslint-disable-next-line +const CodePathSegment = require('./code-path-segment'); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Gets whether or not a given segment is reachable. + * @param {CodePathSegment} segment A segment to get. + * @returns {boolean} `true` if the segment is reachable. + */ +function isReachable(segment) { + return segment.reachable; +} + +/** + * Creates new segments from the specific range of `context.segmentsList`. + * + * When `context.segmentsList` is `[[a, b], [c, d], [e, f]]`, `begin` is `0`, and + * `end` is `-1`, this creates `[g, h]`. This `g` is from `a`, `c`, and `e`. + * This `h` is from `b`, `d`, and `f`. + * @param {ForkContext} context An instance. + * @param {number} begin The first index of the previous segments. + * @param {number} end The last index of the previous segments. + * @param {Function} create A factory function of new segments. + * @returns {CodePathSegment[]} New segments. + */ +function makeSegments(context, begin, end, create) { + const list = context.segmentsList; + + const normalizedBegin = begin >= 0 ? begin : list.length + begin; + const normalizedEnd = end >= 0 ? end : list.length + end; + + const segments = []; + + for (let i = 0; i < context.count; ++i) { + const allPrevSegments = []; + + for (let j = normalizedBegin; j <= normalizedEnd; ++j) { + allPrevSegments.push(list[j][i]); + } + + segments.push(create(context.idGenerator.next(), allPrevSegments)); + } + + return segments; +} + +/** + * `segments` becomes doubly in a `finally` block. Then if a code path exits by a + * control statement (such as `break`, `continue`) from the `finally` block, the + * destination's segments may be half of the source segments. In that case, this + * merges segments. + * @param {ForkContext} context An instance. + * @param {CodePathSegment[]} segments Segments to merge. + * @returns {CodePathSegment[]} The merged segments. + */ +function mergeExtraSegments(context, segments) { + let currentSegments = segments; + + while (currentSegments.length > context.count) { + const merged = []; + + for ( + let i = 0, length = (currentSegments.length / 2) | 0; + i < length; + ++i + ) { + merged.push( + CodePathSegment.newNext(context.idGenerator.next(), [ + currentSegments[i], + currentSegments[i + length], + ]), + ); + } + currentSegments = merged; + } + return currentSegments; +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * A class to manage forking. + */ +class ForkContext { + /** + * @param {IdGenerator} idGenerator An identifier generator for segments. + * @param {ForkContext|null} upper An upper fork context. + * @param {number} count A number of parallel segments. + */ + constructor(idGenerator, upper, count) { + this.idGenerator = idGenerator; + this.upper = upper; + this.count = count; + this.segmentsList = []; + } + + /** + * The head segments. + * @type {CodePathSegment[]} + */ + get head() { + const list = this.segmentsList; + + return list.length === 0 ? [] : list[list.length - 1]; + } + + /** + * A flag which shows empty. + * @type {boolean} + */ + get empty() { + return this.segmentsList.length === 0; + } + + /** + * A flag which shows reachable. + * @type {boolean} + */ + get reachable() { + const segments = this.head; + + return segments.length > 0 && segments.some(isReachable); + } + + /** + * Creates new segments from this context. + * @param {number} begin The first index of previous segments. + * @param {number} end The last index of previous segments. + * @returns {CodePathSegment[]} New segments. + */ + makeNext(begin, end) { + return makeSegments(this, begin, end, CodePathSegment.newNext); + } + + /** + * Creates new segments from this context. + * The new segments is always unreachable. + * @param {number} begin The first index of previous segments. + * @param {number} end The last index of previous segments. + * @returns {CodePathSegment[]} New segments. + */ + makeUnreachable(begin, end) { + return makeSegments(this, begin, end, CodePathSegment.newUnreachable); + } + + /** + * Creates new segments from this context. + * The new segments don't have connections for previous segments. + * But these inherit the reachable flag from this context. + * @param {number} begin The first index of previous segments. + * @param {number} end The last index of previous segments. + * @returns {CodePathSegment[]} New segments. + */ + makeDisconnected(begin, end) { + return makeSegments(this, begin, end, CodePathSegment.newDisconnected); + } + + /** + * Adds segments into this context. + * The added segments become the head. + * @param {CodePathSegment[]} segments Segments to add. + * @returns {void} + */ + add(segments) { + assert( + segments.length >= this.count, + `${segments.length} >= ${this.count}`, + ); + + this.segmentsList.push(mergeExtraSegments(this, segments)); + } + + /** + * Replaces the head segments with given segments. + * The current head segments are removed. + * @param {CodePathSegment[]} segments Segments to add. + * @returns {void} + */ + replaceHead(segments) { + assert( + segments.length >= this.count, + `${segments.length} >= ${this.count}`, + ); + + this.segmentsList.splice(-1, 1, mergeExtraSegments(this, segments)); + } + + /** + * Adds all segments of a given fork context into this context. + * @param {ForkContext} context A fork context to add. + * @returns {void} + */ + addAll(context) { + assert(context.count === this.count); + + const source = context.segmentsList; + + for (let i = 0; i < source.length; ++i) { + this.segmentsList.push(source[i]); + } + } + + /** + * Clears all segments in this context. + * @returns {void} + */ + clear() { + this.segmentsList = []; + } + + /** + * Creates the root fork context. + * @param {IdGenerator} idGenerator An identifier generator for segments. + * @returns {ForkContext} New fork context. + */ + static newRoot(idGenerator) { + const context = new ForkContext(idGenerator, null, 1); + + context.add([CodePathSegment.newRoot(idGenerator.next())]); + + return context; + } + + /** + * Creates an empty fork context preceded by a given context. + * @param {ForkContext} parentContext The parent fork context. + * @param {boolean} forkLeavingPath A flag which shows inside of `finally` block. + * @returns {ForkContext} New fork context. + */ + static newEmpty(parentContext, forkLeavingPath) { + return new ForkContext( + parentContext.idGenerator, + parentContext, + (forkLeavingPath ? 2 : 1) * parentContext.count, + ); + } +} + +module.exports = ForkContext; diff --git a/packages/eslint-plugin-react-hooks/src/code-path-analysis/id-generator.js b/packages/eslint-plugin-react-hooks/src/code-path-analysis/id-generator.js new file mode 100644 index 0000000000000..ed0f47d83464a --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/code-path-analysis/id-generator.js @@ -0,0 +1,37 @@ +'use strict'; + +/* eslint-disable react-internal/safe-string-coercion */ + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +/** + * A generator for unique ids. + */ +class IdGenerator { + /** + * @param {string} prefix Optional. A prefix of generated ids. + */ + constructor(prefix) { + this.prefix = String(prefix); + this.n = 0; + } + + /** + * Generates id. + * @returns {string} A generated id. + */ + next() { + this.n = (1 + this.n) | 0; + + /* c8 ignore start */ + if (this.n < 0) { + this.n = 1; + } /* c8 ignore stop */ + + return this.prefix + this.n; + } +} + +module.exports = IdGenerator; diff --git a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts index 0ab0c5ff21e4b..6be40df53226a 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts @@ -9,6 +9,9 @@ import type {Rule, Scope} from 'eslint'; import type {CallExpression, DoWhileStatement, Node} from 'estree'; +// @ts-expect-error untyped module +import CodePathAnalyzer from '../code-path-analysis/code-path-analyzer'; + /** * Catch all identifiers that begin with "use" followed by an uppercase Latin * character to exclude identifiers like "user". @@ -184,9 +187,25 @@ const rule = { return getSourceCode().getScope(node); }; - return { + function hasFlowSuppression(node: Node, suppression: string) { + const sourceCode = getSourceCode(); + const comments = sourceCode.getAllComments(); + const flowSuppressionRegex = new RegExp( + '\\$FlowFixMe\\[' + suppression + '\\]', + ); + return comments.some( + commentNode => + flowSuppressionRegex.test(commentNode.value) && + commentNode.loc != null && + node.loc != null && + commentNode.loc.end.line === node.loc.start.line - 1, + ); + } + + const analyzer = new CodePathAnalyzer({ // Maintain code segment path stack as we traverse. - onCodePathSegmentStart: segment => codePathSegmentStack.push(segment), + onCodePathSegmentStart: (segment: Rule.CodePathSegment) => + codePathSegmentStack.push(segment), onCodePathSegmentEnd: () => codePathSegmentStack.pop(), // Maintain code path stack as we traverse. @@ -199,7 +218,7 @@ const rule = { // // Everything is ok if all React Hooks are both reachable from the initial // segment and reachable from every final segment. - onCodePathEnd(codePath, codePathNode) { + onCodePathEnd(codePath: any, codePathNode: Node) { const reactHooksMap = codePathReactHooksMapStack.pop(); if (reactHooksMap?.size === 0) { return; @@ -508,6 +527,11 @@ const rule = { const cycled = cyclic.has(segment.id); for (const hook of reactHooks) { + // Skip reporting if this hook already has a relevant flow suppression. + if (hasFlowSuppression(hook, 'react-rule-hook')) { + continue; + } + // Report an error if a hook may be called more then once. // `use(...)` can be called in loops. if ( @@ -611,6 +635,16 @@ const rule = { } } }, + }); + + return { + '*'(node: any) { + analyzer.enterNode(node); + }, + + '*:exit'(node: any) { + analyzer.leaveNode(node); + }, // Missed opportunity...We could visit all `Identifier`s instead of all // `CallExpression`s and check that _every use_ of a hook name is valid. @@ -696,6 +730,10 @@ const rule = { function getFunctionName(node: Node) { if ( + // @ts-expect-error parser-hermes produces these node types + node.type === 'ComponentDeclaration' || + // @ts-expect-error parser-hermes produces these node types + node.type === 'HookDeclaration' || node.type === 'FunctionDeclaration' || (node.type === 'FunctionExpression' && node.id) ) { From 0d695bea10c04d32df4fae7813aa5f448213014a Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 2 May 2025 16:03:06 -0400 Subject: [PATCH 02/12] [eslint-plugin-react-hooks] update fbsource build (#33104) In order to sync the lint rules directly to internal, include the eslint plugin in the build output for fbsource. --- .github/workflows/runtime_commit_artifacts.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/runtime_commit_artifacts.yml b/.github/workflows/runtime_commit_artifacts.yml index cc600d2085e50..a1c739331b09d 100644 --- a/.github/workflows/runtime_commit_artifacts.yml +++ b/.github/workflows/runtime_commit_artifacts.yml @@ -132,9 +132,9 @@ jobs: mkdir ./compiled/facebook-www/__test_utils__ mv build/__test_utils__/ReactAllWarnings.js ./compiled/facebook-www/__test_utils__/ReactAllWarnings.js - # Move eslint-plugin-react-hooks into eslint-plugin-react-hooks + # Copy eslint-plugin-react-hooks mkdir ./compiled/eslint-plugin-react-hooks - mv build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \ + cp build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \ ./compiled/eslint-plugin-react-hooks/index.js # Move unstable_server-external-runtime.js into facebook-www @@ -167,6 +167,13 @@ jobs: rm $RENDERER_FOLDER/ReactFabric-{dev,prod,profiling}.js rm $RENDERER_FOLDER/ReactNativeRenderer-{dev,prod,profiling}.js + # Copy eslint-plugin-react-hooks + # NOTE: This is different from www, here we include the full package + # including package.json to include dependencies in fbsource. + mkdir $BASE_FOLDER/RKJSModules/vendor/react/eslint-plugin-react-hooks + cp -r build/oss-experimental/eslint-plugin-react-hooks \ + $BASE_FOLDER/RKJSModules/vendor/react/eslint-plugin-react-hooks + # Move React Native version file mv build/facebook-react-native/VERSION_NATIVE_FB ./compiled-rn/VERSION_NATIVE_FB From 9de0304ad72bc3f8a77d2d84efa530b8051d1c15 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 2 May 2025 16:52:17 -0400 Subject: [PATCH 03/12] Add missing copyright header (#33106) This made the build fail since there was no file header comment. --- packages/eslint-plugin-react-hooks/npm/index.d.ts | 7 +++++++ packages/eslint-plugin-react-hooks/npm/index.js | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/packages/eslint-plugin-react-hooks/npm/index.d.ts b/packages/eslint-plugin-react-hooks/npm/index.d.ts index 62ca164ec2173..17bdf95c515a9 100644 --- a/packages/eslint-plugin-react-hooks/npm/index.d.ts +++ b/packages/eslint-plugin-react-hooks/npm/index.d.ts @@ -1 +1,8 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + export * from './cjs/eslint-plugin-react-hooks'; diff --git a/packages/eslint-plugin-react-hooks/npm/index.js b/packages/eslint-plugin-react-hooks/npm/index.js index 20458a7a9bf22..b819fc7023699 100644 --- a/packages/eslint-plugin-react-hooks/npm/index.js +++ b/packages/eslint-plugin-react-hooks/npm/index.js @@ -1,3 +1,10 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + 'use strict'; // TODO: this doesn't make sense for an ESLint rule. From e39b380a21aa4ef48d5880aa2c800ec5b1b044bd Mon Sep 17 00:00:00 2001 From: lauren Date: Fri, 2 May 2025 16:54:17 -0400 Subject: [PATCH 04/12] [mcp] Fix unresolved imports (#33105) We need to explicitly import the modules so they'll be inlined correctly into the bundle. --- .../react-mcp-server/src/tools/runtimePerf.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler/packages/react-mcp-server/src/tools/runtimePerf.ts b/compiler/packages/react-mcp-server/src/tools/runtimePerf.ts index bc46f496ec463..7f4d0a1efecd8 100644 --- a/compiler/packages/react-mcp-server/src/tools/runtimePerf.ts +++ b/compiler/packages/react-mcp-server/src/tools/runtimePerf.ts @@ -1,5 +1,11 @@ import * as babel from '@babel/core'; import puppeteer from 'puppeteer'; +// @ts-ignore +import * as babelPresetTypescript from '@babel/preset-typescript'; +// @ts-ignore +import * as babelPresetEnv from '@babel/preset-env'; +// @ts-ignore +import * as babelPresetReact from '@babel/preset-react'; type PerformanceResults = { renderTime: number; @@ -25,15 +31,11 @@ export async function measurePerformance( code: string, iterations: number, ): Promise { - const babelOptions = { + const babelOptions: babel.TransformOptions = { filename: 'anonymous.tsx', configFile: false, babelrc: false, - presets: [ - '@babel/preset-typescript', - '@babel/preset-env', - '@babel/preset-react', - ], + presets: [babelPresetTypescript, babelPresetEnv, babelPresetReact], }; const parsed = await babel.parseAsync(code, babelOptions); From f0ca53d1337780ccfd49c132170d378c789cb463 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 2 May 2025 17:05:56 -0400 Subject: [PATCH 05/12] [eslint-plugin-react-hooks] another CI update... (#33107) We currently created a nested directory, this should remove that. See: https://github.com/facebook/react/tree/builds/facebook-fbsource/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/eslint-plugin-react-hooks/eslint-plugin-react-hooks --- .github/workflows/runtime_commit_artifacts.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/runtime_commit_artifacts.yml b/.github/workflows/runtime_commit_artifacts.yml index a1c739331b09d..24bb897f8cd15 100644 --- a/.github/workflows/runtime_commit_artifacts.yml +++ b/.github/workflows/runtime_commit_artifacts.yml @@ -170,9 +170,7 @@ jobs: # Copy eslint-plugin-react-hooks # NOTE: This is different from www, here we include the full package # including package.json to include dependencies in fbsource. - mkdir $BASE_FOLDER/RKJSModules/vendor/react/eslint-plugin-react-hooks - cp -r build/oss-experimental/eslint-plugin-react-hooks \ - $BASE_FOLDER/RKJSModules/vendor/react/eslint-plugin-react-hooks + cp -r build/oss-experimental/eslint-plugin-react-hooks $BASE_FOLDER/RKJSModules/vendor/react # Move React Native version file mv build/facebook-react-native/VERSION_NATIVE_FB ./compiled-rn/VERSION_NATIVE_FB From 26ecc98a0014700524e78d938e3654c73213cf3b Mon Sep 17 00:00:00 2001 From: lauren Date: Fri, 2 May 2025 17:50:39 -0400 Subject: [PATCH 06/12] [mcp] Prompt improvements for smarter tool composition (#33108) Our runtime results are still bogus, but so far this tweak let's the llm use the tools a little more intelligently. example: --- ```js import React from 'react'; function App() { const [count, setCount] = React.useState(0); const [data, setData] = React.useState([]); React.useEffect(() => { const processData = () => { console.log("Processing data..."); const newData = []; for (let i = 0; i < 1000000000; i++) { newData.push({ id: i, value: Math.random() * count }); } return newData; }; const processedData = processData(); setData(processedData); }, [count]); return (

Count: {count}

Data Length: {data.length}

    {data.slice(0, 5).map((item) => (
  • Item {item.id}: {item.value.toFixed(2)}
    • ))}
); }; ``` why is this component slow? use the react tools to figure this out. > I'll analyze why this component is slow using the React tools. Let's first evaluate the performance of the original code. > [review-react-runtime ...] > > The performance measurement tool timed out, which is a strong indication of just how severe the performance issue is in this component. Let's now try to compile the code to see if React Compiler can identify any issues: > [compile...] > > Now I understand the issue. Let's analyze why this component > is extremely slow: > Main Performance Issues: > > Massive Data Processing: The component is trying to create an array with a billion (1,000,000,000) objects in the processData function. This is an enormous amount of data that will: > > Consume massive amounts of memory > Take an extremely long time to process > Likely crash the browser > > > Unnecessary Recalculation: The effect depends on [count], meaning every time the count changes, it will reprocess this enormous dataset. > Inefficient Data Handling: Even though only 5 items are displayed, the component processes a billion items. > > Let's fix these issues: > [compile...] > > Now let's check if our modified code performs better: > [review-react-runtime...] > > Excellent! The performance has dramatically improved. Now let's check the React documentation to make sure we're following the best practices: > [query-react-dev-docs...] (hit the maximum length conversation limit because i'm on a free plan) --- .../packages/react-mcp-server/src/index.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/compiler/packages/react-mcp-server/src/index.ts b/compiler/packages/react-mcp-server/src/index.ts index e250770c15125..138dc57dc14a9 100644 --- a/compiler/packages/react-mcp-server/src/index.ts +++ b/compiler/packages/react-mcp-server/src/index.ts @@ -29,7 +29,7 @@ const server = new McpServer({ server.tool( 'query-react-dev-docs', - 'This tool lets you search for official docs from react.dev. This always has the most up to date information on React. You can look for documentation on APIs such as , , and hooks like useOptimistic, useSyncExternalStore, useTransition, and more. Whenever you think hard about React, please use this tool to get more information before proceeding.', + 'This tool lets you search for official docs from react.dev. This always has the most up to date information on React. You can look for documentation on APIs such as , , and hooks like useOptimistic, useSyncExternalStore, useTransition, and more. Whenever you think hard about React, use this tool to get more information before proceeding.', { query: z.string(), }, @@ -72,7 +72,15 @@ server.tool( server.tool( 'compile', - 'Compile code with React Compiler. This tool will return the compiled output, which is automatically memoized React components and hooks, written in JavaScript or TypeScript. You can run this tool whenever you want to check if some React code will compile successfully. You can also run this tool every time you make a suggestion to code, to see how it affects the compiled output. If the compiler returns a diagnostic message, you should read the diagnostic message and try to fix the code and run the compiler again to verify.', + `Compile code with React Compiler. This tool will return the compiled output, which is automatically memoized React components and hooks, written in JavaScript or TypeScript. You can run this tool whenever you want to check if some React code will compile successfully. You can also run this tool every time you make a suggestion to code, to see how it affects the compiled output. If the compiler returns a diagnostic message, you should read the diagnostic message and try to fix the code and run the compiler again to verify. After compiling code successfully, you should run it through the review-react-runtime to verify the compiled code is faster than the original. + + + When you encounter a bailout or diagnostic message, first think and try to understand where the error is coming from. You can use tools such as puppeteer if available to browse the documentation links provided in the diagnostic, and then read that information to understand the error in more detail. You can propose fixes after doing so. + + This is a non-exhaustive list of bailouts where you should take specific actions: + - React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved: fix this by first removing the manual memo (useMemo/useCallback) and then try compiling again. use the review-react-runtime tool to verify that the compiled code can run without crashing. if it crashes, the original code likely breaks the Rules of React and so cannot be safely compiled. + + `, { text: z.string(), passName: z.enum(['HIR', 'ReactiveFunction', 'All', '@DEBUG']).optional(), @@ -278,6 +286,7 @@ server.tool( server.tool( 'review-react-runtime', `Run this tool every time you propose a performance related change to verify if your suggestion actually improves performance. + This tool has some requirements on the code input: - The react code that is passed into this tool MUST contain an App functional component without arrow function. @@ -298,12 +307,12 @@ server.tool( (repeat until every metric is good or two consecutive cycles show no gain) - - Apply one focused change based on the failing metric plus React-specific guidance: + - Always run the tool once on the original code before any modification + - Run the tool again after making the modification, and apply one focused change based on the failing metric plus React-specific guidance: - LCP: lazy-load off-screen images, inline critical CSS, preconnect, use React.lazy + Suspense for below-the-fold modules. if the user requests for it, use React Server Components for static content (Server Components). - INP: wrap non-critical updates in useTransition, avoid calling setState inside useEffect. - CLS: reserve space via explicit width/height or aspect-ratio, keep stable list keys, use fixed-size skeleton loaders, animate only transform/opacity, avoid inserting ads or banners without placeholders. - - Stop when every metric is classified as good. Return the final metric table and the list of applied changes. + - Compare the results of your modified code compared to the original to verify that your changes have improved performance. `, { From 66de8e5a9975a4b53734ebed8a1c1f07892426a8 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Fri, 2 May 2025 18:14:56 -0400 Subject: [PATCH 07/12] [eslint-plugin-react-hooks] move eslint plugin once again (#33110) --- .github/workflows/runtime_commit_artifacts.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/runtime_commit_artifacts.yml b/.github/workflows/runtime_commit_artifacts.yml index 24bb897f8cd15..ab0e71b83cfc7 100644 --- a/.github/workflows/runtime_commit_artifacts.yml +++ b/.github/workflows/runtime_commit_artifacts.yml @@ -170,7 +170,8 @@ jobs: # Copy eslint-plugin-react-hooks # NOTE: This is different from www, here we include the full package # including package.json to include dependencies in fbsource. - cp -r build/oss-experimental/eslint-plugin-react-hooks $BASE_FOLDER/RKJSModules/vendor/react + mkdir "$BASE_FOLDER/tools" + cp -r build/oss-experimental/eslint-plugin-react-hooks "$BASE_FOLDER/tools" # Move React Native version file mv build/facebook-react-native/VERSION_NATIVE_FB ./compiled-rn/VERSION_NATIVE_FB From ac2cae524576b8091a6d78d9ab05627053949df1 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 3 May 2025 09:07:50 +0900 Subject: [PATCH 08/12] [compiler] Fix for string attribute values with emoji If a JSX attribute value is a string that contains unicode or other characters that need special escaping, we wrap the attribute value in an expression container. However, our unicode to detect this only handled the basic unicode character plane, not the "astral" plane which includes emojis. This PR updates the regex to detect such extended characters and also use an expression container. ghstack-source-id: 6d9c8e4dd22285077108e2fa53d66154d1b781fb Pull Request resolved: https://github.com/facebook/react/pull/33096 --- .../src/ReactiveScopes/CodegenReactiveFunction.ts | 5 ++++- .../jsx-string-attribute-expression-container.expect.md | 4 +++- .../compiler/jsx-string-attribute-expression-container.js | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 790a64b407316..33a124dcec6e2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -2327,9 +2327,12 @@ function codegenInstructionValue( * u0080 to u009F: C1 control codes * u00A0 to uFFFF: All non-basic Latin characters * https://en.wikipedia.org/wiki/List_of_Unicode_characters#Control_codes + * + * u010000 to u10FFFF: Astral plane characters + * https://mathiasbynens.be/notes/javascript-unicode */ const STRING_REQUIRES_EXPR_CONTAINER_PATTERN = - /[\u{0000}-\u{001F}\u{007F}\u{0080}-\u{FFFF}]|"|\\/u; + /[\u{0000}-\u{001F}\u{007F}\u{0080}-\u{FFFF}\u{010000}-\u{10FFFF}]|"|\\/u; function codegenJsxAttribute( cx: Context, attribute: JsxAttribute, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.expect.md index a006c2d084996..2bcf48253742e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.expect.md @@ -11,6 +11,7 @@ function Component() { + ); } @@ -42,6 +43,7 @@ function Component() { + ); $[0] = t0; @@ -74,4 +76,4 @@ export const FIXTURE_ENTRYPOINT = { ### Eval output (kind: ok)
-A E나은Laurenசத்யாSathya
\ No newline at end of file +A E나은Laurenசத்யாSathyawelcome 👋 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.js index c9844983fb686..3234745a4c7f9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-string-attribute-expression-container.js @@ -7,6 +7,7 @@ function Component() { + ); } From 73d7e816b7746c700ab843964aaf11c17351fac1 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 3 May 2025 09:09:34 +0900 Subject: [PATCH 09/12] [compiler] ValidatePreservedManualMemoization reports detailed errors This pass didn't previously report the precise difference btw inferred/manual dependencies unless a debug flag was set. But the error message is really good (nice job mofeiz): the only catch is that in theory the inferred dep could be a temporary that can't trivially be reported to the user. But the messages are really useful for quickly verifying why the compiler couldn't preserve memoization. So here we switch to outputting a detailed message about the discrepancy btw inferred/manual deps so long as the inferred dep root is a named variable. I also slightly adjusted the message to handle the case where there is no diagnostic, which can occur if there were no manual deps but the compiler inferred a dependency. ghstack-source-id: 534f6f1fec0855e05e85077eba050eb2ba254ef8 Pull Request resolved: https://github.com/facebook/react/pull/33095 --- .../ValidatePreservedManualMemoization.ts | 33 ++++++++++--------- ...ession-with-conditional-optional.expect.md | 2 +- ...mber-expression-with-conditional.expect.md | 2 +- ...as-memo-dep-non-optional-in-body.expect.md | 2 +- .../error.ref-like-name-not-Ref.expect.md | 2 +- .../error.ref-like-name-not-a-ref.expect.md | 2 +- ...ack-conditional-access-own-scope.expect.md | 2 +- ...ck-infer-conditional-value-block.expect.md | 4 +-- ...nvalid-useCallback-read-maybeRef.expect.md | 2 +- ...be-invalid-useMemo-read-maybeRef.expect.md | 2 +- ...ve-use-memo-ref-missing-reactive.expect.md | 2 +- .../error.useCallback-aliased-var.expect.md | 2 +- ...lback-conditional-access-noAlloc.expect.md | 2 +- ...less-specific-conditional-access.expect.md | 2 +- ...or.useCallback-property-call-dep.expect.md | 2 +- .../error.useMemo-aliased-var.expect.md | 2 +- ...less-specific-conditional-access.expect.md | 2 +- ...specific-conditional-value-block.expect.md | 4 +-- ...emo-property-call-chained-object.expect.md | 2 +- .../error.useMemo-property-call-dep.expect.md | 2 +- ...o-unrelated-mutation-in-depslist.expect.md | 2 +- ...ession-with-conditional-optional.expect.md | 2 +- ...mber-expression-with-conditional.expect.md | 2 +- 23 files changed, 42 insertions(+), 39 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index 85fec7a75333b..1829d77822998 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -141,14 +141,14 @@ function getCompareDependencyResultDescription( ): string { switch (result) { case CompareDependencyResult.Ok: - return 'dependencies equal'; + return 'Dependencies equal'; case CompareDependencyResult.RootDifference: case CompareDependencyResult.PathDifference: - return 'inferred different dependency than source'; + return 'Inferred different dependency than source'; case CompareDependencyResult.RefAccessDifference: - return 'differences in ref.current access'; + return 'Differences in ref.current access'; case CompareDependencyResult.Subpath: - return 'inferred less specific property than source'; + return 'Inferred less specific property than source'; } } @@ -279,17 +279,20 @@ function validateInferredDep( severity: ErrorSeverity.CannotPreserveMemoization, reason: 'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected', - description: DEBUG - ? `The inferred dependency was \`${prettyPrintScopeDependency( - dep, - )}\`, but the source dependencies were [${validDepsInMemoBlock - .map(dep => printManualMemoDependency(dep, true)) - .join(', ')}]. Detail: ${ - errorDiagnostic - ? getCompareDependencyResultDescription(errorDiagnostic) - : 'none' - }` - : null, + description: + DEBUG || + // If the dependency is a named variable then we can report it. Otherwise only print in debug mode + (dep.identifier.name != null && dep.identifier.name.kind === 'named') + ? `The inferred dependency was \`${prettyPrintScopeDependency( + dep, + )}\`, but the source dependencies were [${validDepsInMemoBlock + .map(dep => printManualMemoDependency(dep, true)) + .join(', ')}]. ${ + errorDiagnostic + ? getCompareDependencyResultDescription(errorDiagnostic) + : 'Inferred dependency not present in source' + }` + : null, loc: memoLocation, suggestions: null, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md index d9c2b599998b7..048fee7ee1d58 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional-optional.expect.md @@ -41,7 +41,7 @@ function Component(props) { > 10 | return x; | ^^^^^^^^^^^^^^^^^ > 11 | }, [props?.items, props.cond]); - | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (4:11) + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source (4:11) 12 | return ( 13 | 14 | ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md index 57b7d48facbd5..ca3ee2ae1388e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hoist-optional-member-expression-with-conditional.expect.md @@ -41,7 +41,7 @@ function Component(props) { > 10 | return x; | ^^^^^^^^^^^^^^^^^ > 11 | }, [props?.items, props.cond]); - | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (4:11) + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `props.items`, but the source dependencies were [props?.items, props.cond]. Inferred different dependency than source (4:11) 12 | return ( 13 | 14 | ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md index ba5b30418069a..7d6c4b38a7a4b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-optional-member-expression-as-memo-dep-non-optional-in-body.expect.md @@ -29,7 +29,7 @@ function Component(props) { > 6 | // deps are optional | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 7 | }, [props.items?.edges?.nodes]); - | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (3:7) + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `props.items.edges.nodes`, but the source dependencies were [props.items?.edges?.nodes]. Inferred different dependency than source (3:7) 8 | return ; 9 | } 10 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md index cf15ab13d12e1..2b4943ba491b4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-like-name-not-Ref.expect.md @@ -38,7 +38,7 @@ export const FIXTURE_ENTRYPOINT = { > 12 | Ref.current?.click(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ > 13 | }, []); - | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (11:13) + | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected. The inferred dependency was `Ref.current`, but the source dependencies were []. Inferred dependency not present in source (11:13) 14 | 15 | return