diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts index 05a4b4b91f5..2bccda3a2e9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts @@ -184,25 +184,28 @@ function validateNoContextVariableAssignment( fn: HIRFunction, errors: CompilerError, ): void { + const context = new Set(fn.context.map(place => place.identifier.id)); for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { const value = instr.value; switch (value.kind) { case 'StoreContext': { - errors.pushDiagnostic( - CompilerDiagnostic.create({ - category: ErrorCategory.UseMemo, - reason: - 'useMemo() callbacks may not reassign variables declared outside of the callback', - description: - 'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function', - suggestions: null, - }).withDetails({ - kind: 'error', - loc: value.lvalue.place.loc, - message: 'Cannot reassign variable', - }), - ); + if (context.has(value.lvalue.place.identifier.id)) { + errors.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.UseMemo, + reason: + 'useMemo() callbacks may not reassign variables declared outside of the callback', + description: + 'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function', + suggestions: null, + }).withDetails({ + kind: 'error', + loc: value.lvalue.place.loc, + message: 'Cannot reassign variable', + }), + ); + } break; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md new file mode 100644 index 00000000000..29dc3afcd6f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +// @flow +export hook useItemLanguage(items) { + return useMemo(() => { + let language: ?string = null; + items.forEach(item => { + if (item.language != null) { + language = item.language; + } + }); + return language; + }, [items]); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +export function useItemLanguage(items) { + const $ = _c(2); + let language; + if ($[0] !== items) { + language = null; + items.forEach((item) => { + if (item.language != null) { + language = item.language; + } + }); + $[0] = items; + $[1] = language; + } else { + language = $[1]; + } + return language; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js new file mode 100644 index 00000000000..4ed89bfc643 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js @@ -0,0 +1,12 @@ +// @flow +export hook useItemLanguage(items) { + return useMemo(() => { + let language: ?string = null; + items.forEach(item => { + if (item.language != null) { + language = item.language; + } + }); + return language; + }, [items]); +} diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts index f89b049d100..19b4d692242 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts @@ -5,22 +5,15 @@ * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import ReactCompilerRule from '../src/rules/ReactCompilerRule'; -testRule( - 'no impure function calls rule', - allRules[getRuleForCategory(ErrorCategory.Purity).name].rule, - { - valid: [], - invalid: [ - { - name: 'Known impure function calls are caught', - code: normalizeIndent` +testRule('no impure function calls rule', ReactCompilerRule, { + valid: [], + invalid: [ + { + name: 'Known impure function calls are caught', + code: normalizeIndent` function Component() { const date = Date.now(); const now = performance.now(); @@ -28,12 +21,11 @@ testRule( return ; } `, - errors: [ - makeTestCaseError('Cannot call impure function during render'), - makeTestCaseError('Cannot call impure function during render'), - makeTestCaseError('Cannot call impure function during render'), - ], - }, - ], - }, -); + errors: [ + makeTestCaseError('Cannot call impure function during render'), + makeTestCaseError('Cannot call impure function during render'), + makeTestCaseError('Cannot call impure function during render'), + ], + }, + ], +}); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/InvalidHooksRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/InvalidHooksRule-test.ts index 1fd88935575..a0ca2bc844c 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/InvalidHooksRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/InvalidHooksRule-test.ts @@ -5,30 +5,23 @@ * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import {AllRules} from '../src/rules/ReactCompilerRule'; -testRule( - 'rules-of-hooks', - allRules[getRuleForCategory(ErrorCategory.Hooks).name].rule, - { - valid: [ - { - name: 'Basic example', - code: normalizeIndent` +testRule('rules-of-hooks', AllRules, { + valid: [ + { + name: 'Basic example', + code: normalizeIndent` function Component() { useHook(); return
Hello world
; } `, - }, - { - name: 'Violation with Flow suppression', - code: ` + }, + { + name: 'Violation with Flow suppression', + code: ` // Valid since error already suppressed with flow. function useHook() { if (cond) { @@ -37,11 +30,11 @@ testRule( } } `, - }, - { - // OK because invariants are only meant for the compiler team's consumption - name: '[Invariant] Defined after use', - code: normalizeIndent` + }, + { + // OK because invariants are only meant for the compiler team's consumption + name: '[Invariant] Defined after use', + code: normalizeIndent` function Component(props) { let y = function () { m(x); @@ -52,49 +45,42 @@ testRule( return y; } `, - }, - { - name: "Classes don't throw", - code: normalizeIndent` + }, + { + name: "Classes don't throw", + code: normalizeIndent` class Foo { #bar() {} } `, - }, - ], - invalid: [ - { - name: 'Simple violation', - code: normalizeIndent` + }, + ], + invalid: [ + { + name: 'Simple violation', + code: normalizeIndent` function useConditional() { if (cond) { useConditionalHook(); } } `, - errors: [ - makeTestCaseError( - 'Hooks must always be called in a consistent order', - ), - ], - }, - { - name: 'Multiple diagnostics within the same function are surfaced', - code: normalizeIndent` + errors: [ + makeTestCaseError('Hooks must always be called in a consistent order'), + ], + }, + { + name: 'Multiple diagnostics within the same function are surfaced', + code: normalizeIndent` function useConditional() { cond ?? useConditionalHook(); props.cond && useConditionalHook(); return
Hello world
; }`, - errors: [ - makeTestCaseError( - 'Hooks must always be called in a consistent order', - ), - makeTestCaseError( - 'Hooks must always be called in a consistent order', - ), - ], - }, - ], - }, -); + errors: [ + makeTestCaseError('Hooks must always be called in a consistent order'), + makeTestCaseError('Hooks must always be called in a consistent order'), + ], + }, + ], +}); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoAmbiguousJsxRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoAmbiguousJsxRule-test.ts index bb6f4b93b88..00a7843d81f 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoAmbiguousJsxRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoAmbiguousJsxRule-test.ts @@ -5,22 +5,15 @@ * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import ReactCompilerRule from '../src/rules/ReactCompilerRule'; -testRule( - 'no ambiguous JSX rule', - allRules[getRuleForCategory(ErrorCategory.ErrorBoundaries).name].rule, - { - valid: [], - invalid: [ - { - name: 'JSX in try blocks are warned against', - code: normalizeIndent` +testRule('no ambiguous JSX rule', ReactCompilerRule, { + valid: [], + invalid: [ + { + name: 'JSX in try blocks are warned against', + code: normalizeIndent` function Component(props) { let el; try { @@ -31,8 +24,7 @@ testRule( return el; } `, - errors: [makeTestCaseError('Avoid constructing JSX within try/catch')], - }, - ], - }, -); + errors: [makeTestCaseError('Avoid constructing JSX within try/catch')], + }, + ], +}); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoCapitalizedCallsRule-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoCapitalizedCallsRule-test.ts index 5b45a70fa1d..1efa80cd9f6 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoCapitalizedCallsRule-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoCapitalizedCallsRule-test.ts @@ -4,22 +4,15 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import ReactCompilerRule from '../src/rules/ReactCompilerRule'; -testRule( - 'no-capitalized-calls', - allRules[getRuleForCategory(ErrorCategory.CapitalizedCalls).name].rule, - { - valid: [], - invalid: [ - { - name: 'Simple violation', - code: normalizeIndent` +testRule('no-capitalized-calls', ReactCompilerRule, { + valid: [], + invalid: [ + { + name: 'Simple violation', + code: normalizeIndent` import Child from './Child'; function Component() { return <> @@ -27,15 +20,13 @@ testRule( ; } `, - errors: [ - makeTestCaseError( - 'Capitalized functions are reserved for components', - ), - ], - }, - { - name: 'Method call violation', - code: normalizeIndent` + errors: [ + makeTestCaseError('Capitalized functions are reserved for components'), + ], + }, + { + name: 'Method call violation', + code: normalizeIndent` import myModule from './MyModule'; function Component() { return <> @@ -43,15 +34,13 @@ testRule( ; } `, - errors: [ - makeTestCaseError( - 'Capitalized functions are reserved for components', - ), - ], - }, - { - name: 'Multiple diagnostics within the same function are surfaced', - code: normalizeIndent` + errors: [ + makeTestCaseError('Capitalized functions are reserved for components'), + ], + }, + { + name: 'Multiple diagnostics within the same function are surfaced', + code: normalizeIndent` import Child1 from './Child1'; import MyModule from './MyModule'; function Component() { @@ -60,12 +49,9 @@ testRule( {MyModule.Child2()} ; }`, - errors: [ - makeTestCaseError( - 'Capitalized functions are reserved for components', - ), - ], - }, - ], - }, -); + errors: [ + makeTestCaseError('Capitalized functions are reserved for components'), + ], + }, + ], +}); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoRefAccessInRender-tests.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoRefAccessInRender-tests.ts index 9953c8c2136..e0b75715f1d 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/NoRefAccessInRender-tests.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/NoRefAccessInRender-tests.ts @@ -5,30 +5,22 @@ * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import ReactCompilerRule from '../src/rules/ReactCompilerRule'; -testRule( - 'no ref access in render rule', - allRules[getRuleForCategory(ErrorCategory.Refs).name].rule, - { - valid: [], - invalid: [ - { - name: 'validate against simple ref access in render', - code: normalizeIndent` +testRule('no ref access in render rule', ReactCompilerRule, { + valid: [], + invalid: [ + { + name: 'validate against simple ref access in render', + code: normalizeIndent` function Component(props) { const ref = useRef(null); const value = ref.current; return value; } `, - errors: [makeTestCaseError('Cannot access refs during render')], - }, - ], - }, -); + errors: [makeTestCaseError('Cannot access refs during render')], + }, + ], +}); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts index e0063bc0a2c..5f08449d311 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts @@ -5,19 +5,10 @@ * LICENSE file in the root directory of this source tree. */ -import { - ErrorCategory, - getRuleForCategory, -} from 'babel-plugin-react-compiler/src/CompilerError'; -import { - normalizeIndent, - testRule, - makeTestCaseError, - TestRecommendedRules, -} from './shared-utils'; -import {allRules} from '../src/rules/ReactCompilerRule'; +import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils'; +import {AllRules} from '../src/rules/ReactCompilerRule'; -testRule('plugin-recommended', TestRecommendedRules, { +testRule('plugin-recommended', AllRules, { valid: [ { name: 'Basic example with component syntax', diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts index 87baf724e12..f255d2936ed 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts @@ -6,11 +6,8 @@ */ import {RuleTester} from 'eslint'; -import { - CompilerTestCases, - normalizeIndent, - TestRecommendedRules, -} from './shared-utils'; +import {CompilerTestCases, normalizeIndent} from './shared-utils'; +import ReactCompilerRule from '../src/rules/ReactCompilerRule'; const tests: CompilerTestCases = { valid: [ @@ -62,4 +59,4 @@ const eslintTester = new RuleTester({ // @ts-ignore[2353] - outdated types parser: require.resolve('@typescript-eslint/parser'), }); -eslintTester.run('react-compiler', TestRecommendedRules, tests); +eslintTester.run('react-compiler', ReactCompilerRule, tests); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/shared-utils.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/shared-utils.ts index b0523f522c9..8d3cf2cc9e2 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/shared-utils.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/shared-utils.ts @@ -1,8 +1,5 @@ import {RuleTester as ESLintTester, Rule} from 'eslint'; -import {type ErrorCategory} from 'babel-plugin-react-compiler/src/CompilerError'; import escape from 'regexp.escape'; -import {configs} from '../src/index'; -import {allRules} from '../src/rules/ReactCompilerRule'; /** * A string template tag that removes padding from the left side of multi-line strings @@ -46,31 +43,4 @@ export function testRule( eslintTester.run(name, rule, tests); } -/** - * Aggregates all recommended rules from the plugin. - */ -export const TestRecommendedRules: Rule.RuleModule = { - meta: { - type: 'problem', - docs: { - description: 'Disallow capitalized function calls', - category: 'Possible Errors', - recommended: true, - }, - // validation is done at runtime with zod - schema: [{type: 'object', additionalProperties: true}], - }, - create(context) { - for (const ruleConfig of Object.values( - configs.recommended.plugins['react-compiler'].rules, - )) { - const listener = ruleConfig.rule.create(context); - if (Object.entries(listener).length !== 0) { - throw new Error('TODO: handle rules that return listeners to eslint'); - } - } - return {}; - }, -}; - test('no test', () => {}); diff --git a/compiler/packages/eslint-plugin-react-compiler/src/index.ts b/compiler/packages/eslint-plugin-react-compiler/src/index.ts index 3f0c7bcdcb0..103cdbbbd32 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/index.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/index.ts @@ -5,37 +5,24 @@ * LICENSE file in the root directory of this source tree. */ -import {type Linter} from 'eslint'; -import { - allRules, - mapErrorSeverityToESlint, - recommendedRules, -} from './rules/ReactCompilerRule'; +import ReactCompilerRule from './rules/ReactCompilerRule'; -const meta = { - name: 'eslint-plugin-react-compiler', -}; - -const configs = { - recommended: { - plugins: { - 'react-compiler': { - rules: allRules, +module.exports = { + rules: { + 'react-compiler': ReactCompilerRule, + }, + configs: { + recommended: { + plugins: { + 'react-compiler': { + rules: { + 'react-compiler': ReactCompilerRule, + }, + }, + }, + rules: { + 'react-compiler/react-compiler': 'error', }, }, - rules: Object.fromEntries( - Object.entries(recommendedRules).map(([name, ruleConfig]) => { - return [ - 'react-compiler/' + name, - mapErrorSeverityToESlint(ruleConfig.severity), - ]; - }), - ) as Record, }, }; - -const rules = Object.fromEntries( - Object.entries(allRules).map(([name, {rule}]) => [name, rule]), -); - -export {configs, rules, meta}; diff --git a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts index 8f41b3afaba..4b75aa69e5f 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts @@ -14,7 +14,7 @@ import { import type {Linter, Rule} from 'eslint'; import runReactCompiler, {RunCacheEntry} from '../shared/RunReactCompiler'; import { - ErrorSeverity, + ErrorCategory, LintRulePreset, LintRules, type LintRule, @@ -108,14 +108,15 @@ function hasFlowSuppression( return false; } -function makeRule(rule: LintRule): Rule.RuleModule { +function makeRule(rules: Array): Rule.RuleModule { + const categories = new Set(rules.map(rule => rule.category)); const create = (context: Rule.RuleContext): Rule.RuleListener => { const result = getReactCompilerResult(context); for (const event of result.events) { if (event.kind === 'CompileError') { const detail = event.detail; - if (detail.category === rule.category) { + if (categories.has(detail.category)) { const loc = detail.primaryLocation(); if (loc == null || typeof loc === 'symbol') { continue; @@ -150,8 +151,8 @@ function makeRule(rule: LintRule): Rule.RuleModule { meta: { type: 'problem', docs: { - description: rule.description, - recommended: rule.preset === LintRulePreset.Recommended, + description: 'React Compiler diagnostics', + recommended: true, }, fixable: 'code', hasSuggestions: true, @@ -162,47 +163,13 @@ function makeRule(rule: LintRule): Rule.RuleModule { }; } -type RulesConfig = { - [name: string]: {rule: Rule.RuleModule; severity: ErrorSeverity}; -}; +export default makeRule( + LintRules.filter( + rule => + rule.preset === LintRulePreset.Recommended || + rule.preset === LintRulePreset.RecommendedLatest || + rule.category === ErrorCategory.CapitalizedCalls, + ), +); -export const allRules: RulesConfig = LintRules.reduce((acc, rule) => { - acc[rule.name] = {rule: makeRule(rule), severity: rule.severity}; - return acc; -}, {} as RulesConfig); - -export const recommendedRules: RulesConfig = LintRules.filter( - rule => rule.preset === LintRulePreset.Recommended, -).reduce((acc, rule) => { - acc[rule.name] = {rule: makeRule(rule), severity: rule.severity}; - return acc; -}, {} as RulesConfig); - -export const recommendedLatestRules: RulesConfig = LintRules.filter( - rule => - rule.preset === LintRulePreset.Recommended || - rule.preset === LintRulePreset.RecommendedLatest, -).reduce((acc, rule) => { - acc[rule.name] = {rule: makeRule(rule), severity: rule.severity}; - return acc; -}, {} as RulesConfig); - -export function mapErrorSeverityToESlint( - severity: ErrorSeverity, -): Linter.StringSeverity { - switch (severity) { - case ErrorSeverity.Error: { - return 'error'; - } - case ErrorSeverity.Warning: { - return 'warn'; - } - case ErrorSeverity.Hint: - case ErrorSeverity.Off: { - return 'off'; - } - default: { - assertExhaustive(severity, `Unhandled severity: ${severity}`); - } - } -} +export const AllRules = makeRule(LintRules);