From 7f3826e8e44caa6984232ab8d752d8b9441e4c99 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 17 Jan 2025 09:04:02 -0800 Subject: [PATCH] [compiler] validation against calling impure functions (#31960) For now we just reject all calls of impure functions, and the validation is off by default. Going forward we can make this more precise and only reject impure functions called during render. Note that I was intentionally imprecise in the return type of these functions in order to avoid changing output of existing code. We lie to the compiler and say that Date.now, performance.now, and Math.random return unknown mutable objects rather than primitives. Once the validation is complete and vetted we can switch this to be more precise. --- .../src/Entrypoint/Pipeline.ts | 5 ++ .../src/HIR/Environment.ts | 5 ++ .../src/HIR/Globals.ts | 50 ++++++++++++++++++ .../src/HIR/ObjectShape.ts | 4 ++ .../src/HIR/TypeSchema.ts | 4 ++ .../ValiateNoImpureFunctionsInRender.ts | 52 +++++++++++++++++++ ...valid-impure-functions-in-render.expect.md | 33 ++++++++++++ ...rror.invalid-impure-functions-in-render.js | 8 +++ 8 files changed, 161 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index ca6abc0748..a354acf692 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -99,6 +99,7 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI import {outlineJSX} from '../Optimization/OutlineJsx'; import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; import {transformFire} from '../Transform'; +import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -257,6 +258,10 @@ function runWithEnvironment( validateNoJSXInTryStatement(hir); } + if (env.config.validateNoImpureFunctionsInRender) { + validateNoImpureFunctionsInRender(hir); + } + inferReactivePlaces(hir); log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index f1db567660..9df34242ec 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -353,6 +353,11 @@ const EnvironmentConfigSchema = z.object({ validateNoCapitalizedCalls: z.nullable(z.array(z.string())).default(null), validateBlocklistedImports: z.nullable(z.array(z.string())).default(null), + /** + * Validate against impure functions called during render + */ + validateNoImpureFunctionsInRender: z.boolean().default(false), + /* * When enabled, the compiler assumes that hooks follow the Rules of React: * - Hooks may memoize computation based on any of their parameters, thus diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index ac5d44bab0..3fe2e938ce 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -141,6 +141,44 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ ], ]), ], + [ + 'performance', + addObject(DEFAULT_SHAPES, 'performance', [ + // Static methods (TODO) + [ + 'now', + // Date.now() + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'performance.now', + }), + ], + ]), + ], + [ + 'Date', + addObject(DEFAULT_SHAPES, 'Date', [ + // Static methods (TODO) + [ + 'now', + // Date.now() + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'Date.now', + }), + ], + ]), + ], [ 'Math', addObject(DEFAULT_SHAPES, 'Math', [ @@ -209,6 +247,18 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ returnValueKind: ValueKind.Primitive, }), ], + [ + 'random', + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'Math.random', + }), + ], ]), ], ['Infinity', {kind: 'Primitive'}], diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 4482d17890..7c05c05531 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -172,6 +172,10 @@ export type FunctionSignature = { * - Else uses the effects specified by this signature. */ mutableOnlyIfOperandsAreMutable?: boolean; + + impure?: boolean; + + canonicalName?: string; }; /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts index 362328db72..9aac2a264f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts @@ -40,6 +40,8 @@ export type FunctionTypeConfig = { returnValueKind: ValueKind; noAlias?: boolean | null | undefined; mutableOnlyIfOperandsAreMutable?: boolean | null | undefined; + impure?: boolean | null | undefined; + canonicalName?: string | null | undefined; }; export const FunctionTypeSchema: z.ZodType = z.object({ kind: z.literal('function'), @@ -50,6 +52,8 @@ export const FunctionTypeSchema: z.ZodType = z.object({ returnValueKind: ValueKindSchema, noAlias: z.boolean().nullable().optional(), mutableOnlyIfOperandsAreMutable: z.boolean().nullable().optional(), + impure: z.boolean().nullable().optional(), + canonicalName: z.string().nullable().optional(), }); export type HookTypeConfig = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts new file mode 100644 index 0000000000..287a0aa978 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts @@ -0,0 +1,52 @@ +/** + * 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. + */ + +import {CompilerError, ErrorSeverity} from '..'; +import {HIRFunction} from '../HIR'; +import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; + +/** + * Checks that known-impure functions are not called during render. Examples of invalid functions to + * call during render are `Math.random()` and `Date.now()`. Users may extend this set of + * impure functions via a module type provider and specifying functions with `impure: true`. + * + * TODO: add best-effort analysis of functions which are called during render. We have variations of + * this in several of our validation passes and should unify those analyses into a reusable helper + * and use it here. + */ +export function validateNoImpureFunctionsInRender(fn: HIRFunction): void { + const errors = new CompilerError(); + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const value = instr.value; + if (value.kind === 'MethodCall' || value.kind == 'CallExpression') { + const callee = + value.kind === 'MethodCall' ? value.property : value.callee; + const signature = getFunctionCallSignature( + fn.env, + callee.identifier.type, + ); + if (signature != null && signature.impure === true) { + errors.push({ + reason: + 'Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)', + description: + signature.canonicalName != null + ? `\`${signature.canonicalName}\` is an impure function whose results may change on every call` + : null, + severity: ErrorSeverity.InvalidReact, + loc: callee.loc, + suggestions: null, + }); + } + } + } + } + if (errors.hasErrors()) { + throw errors; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md new file mode 100644 index 0000000000..a67d467df8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender + +function Component() { + const date = Date.now(); + const now = performance.now(); + const rand = Math.random(); + return ; +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | const date = Date.now(); + | ^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4) + +InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `performance.now` is an impure function whose results may change on every call (5:5) + +InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Math.random` is an impure function whose results may change on every call (6:6) + 5 | const now = performance.now(); + 6 | const rand = Math.random(); + 7 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js new file mode 100644 index 0000000000..6faf98caff --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js @@ -0,0 +1,8 @@ +// @validateNoImpureFunctionsInRender + +function Component() { + const date = Date.now(); + const now = performance.now(); + const rand = Math.random(); + return ; +}