[compiler] Validate against mutable functions being frozen

This revisits a validation I built a while ago, trying to make it more strict this time to ensure that it's high-signal.

We detect function expressions which are *known* mutable — they definitely can modify a variable defined outside of the function expression itself (modulo control flow). This uses types to look for known Store and Mutate effects only, and disregards mutations of effects. Any such function passed to a location with a Freeze effect is reported as a validation error.

This is behind a flag and disabled by default. If folks agree this makes sense to revisit, i'll test out internally and we can consider enabling by default.

ghstack-source-id: 075a731
Pull Request resolved: #33079

DiffTrain build for [0db8db1](0db8db1)

Author: josephsavona

compiled/eslint-plugin-react-hooks/index.js

@@ -17792,6 +17792,11 @@ function isUseStateType(id) {
 function isRefOrRefValue(id) {
     return isUseRefType(id) || isRefValueType(id);
 }
+function isRefOrRefLikeMutableType(type) {
+    return (type.kind === 'Object' &&
+        (type.shapeId === 'BuiltInUseRefId' ||
+            type.shapeId == 'ReanimatedSharedValueId'));
+}
 function isSetStateType(id) {
     return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInSetState';
 }
@@ -29584,6 +29589,8 @@ const BuiltInPropsId = 'BuiltInProps';
 const BuiltInArrayId = 'BuiltInArray';
 const BuiltInSetId = 'BuiltInSet';
 const BuiltInMapId = 'BuiltInMap';
+const BuiltInWeakSetId = 'BuiltInWeakSet';
+const BuiltInWeakMapId = 'BuiltInWeakMap';
 const BuiltInFunctionId = 'BuiltInFunction';
 const BuiltInJsxId = 'BuiltInJsx';
 const BuiltInObjectId = 'BuiltInObject';
@@ -29605,6 +29612,7 @@ const BuiltInUseTransitionId = 'BuiltInUseTransition';
 const BuiltInStartTransitionId = 'BuiltInStartTransition';
 const BuiltInFireId = 'BuiltInFire';
 const BuiltInFireFunctionId = 'BuiltInFireFunction';
+const ReanimatedSharedValueId = 'ReanimatedSharedValueId';
 const BUILTIN_SHAPES = new Map();
 addObject(BUILTIN_SHAPES, BuiltInPropsId, [
     ['ref', { kind: 'Object', shapeId: BuiltInUseRefId }],
@@ -30024,6 +30032,80 @@ addObject(BUILTIN_SHAPES, BuiltInMapId, [
         }),
     ],
 ]);
+addObject(BUILTIN_SHAPES, BuiltInWeakSetId, [
+    [
+        'add',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Capture],
+            restParam: null,
+            returnType: { kind: 'Object', shapeId: BuiltInWeakSetId },
+            calleeEffect: Effect.Store,
+            returnValueKind: ValueKind.Mutable,
+        }),
+    ],
+    [
+        'delete',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Read],
+            restParam: null,
+            returnType: PRIMITIVE_TYPE,
+            calleeEffect: Effect.Store,
+            returnValueKind: ValueKind.Primitive,
+        }),
+    ],
+    [
+        'has',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Read],
+            restParam: null,
+            returnType: PRIMITIVE_TYPE,
+            calleeEffect: Effect.Read,
+            returnValueKind: ValueKind.Primitive,
+        }),
+    ],
+]);
+addObject(BUILTIN_SHAPES, BuiltInWeakMapId, [
+    [
+        'delete',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Read],
+            restParam: null,
+            returnType: PRIMITIVE_TYPE,
+            calleeEffect: Effect.Store,
+            returnValueKind: ValueKind.Primitive,
+        }),
+    ],
+    [
+        'get',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Read],
+            restParam: null,
+            returnType: { kind: 'Poly' },
+            calleeEffect: Effect.Capture,
+            returnValueKind: ValueKind.Mutable,
+        }),
+    ],
+    [
+        'has',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Read],
+            restParam: null,
+            returnType: PRIMITIVE_TYPE,
+            calleeEffect: Effect.Read,
+            returnValueKind: ValueKind.Primitive,
+        }),
+    ],
+    [
+        'set',
+        addFunction(BUILTIN_SHAPES, [], {
+            positionalParams: [Effect.Capture, Effect.Capture],
+            restParam: null,
+            returnType: { kind: 'Object', shapeId: BuiltInWeakMapId },
+            calleeEffect: Effect.Store,
+            returnValueKind: ValueKind.Mutable,
+        }),
+    ],
+]);
 addObject(BUILTIN_SHAPES, BuiltInUseStateId, [
     ['0', { kind: 'Poly' }],
     [
@@ -37957,6 +38039,26 @@ const TYPED_GLOBALS = [
             returnValueKind: ValueKind.Mutable,
         }, null, true),
     ],
+    [
+        'WeakMap',
+        addFunction(DEFAULT_SHAPES, [], {
+            positionalParams: [Effect.ConditionallyMutateIterator],
+            restParam: null,
+            returnType: { kind: 'Object', shapeId: BuiltInWeakMapId },
+            calleeEffect: Effect.Read,
+            returnValueKind: ValueKind.Mutable,
+        }, null, true),
+    ],
+    [
+        'WeakSet',
+        addFunction(DEFAULT_SHAPES, [], {
+            positionalParams: [Effect.ConditionallyMutateIterator],
+            restParam: null,
+            returnType: { kind: 'Object', shapeId: BuiltInWeakSetId },
+            calleeEffect: Effect.Read,
+            returnValueKind: ValueKind.Mutable,
+        }, null, true),
+    ],
 ];
 const REACT_APIS = [
     [
@@ -38282,7 +38384,7 @@ function getReanimatedModuleType(registry) {
             addHook(registry, {
                 positionalParams: [],
                 restParam: Effect.Freeze,
-                returnType: { kind: 'Poly' },
+                returnType: { kind: 'Object', shapeId: ReanimatedSharedValueId },
                 returnValueKind: ValueKind.Mutable,
                 noAlias: true,
                 calleeEffect: Effect.Read,
@@ -38426,6 +38528,7 @@ const EnvironmentConfigSchema = zod.z.object({
     validateNoCapitalizedCalls: zod.z.nullable(zod.z.array(zod.z.string())).default(null),
     validateBlocklistedImports: zod.z.nullable(zod.z.array(zod.z.string())).default(null),
     validateNoImpureFunctionsInRender: zod.z.boolean().default(false),
+    validateNoFreezingKnownMutableFunctions: zod.z.boolean().default(false),
     enableAssumeHooksFollowRulesOfReact: zod.z.boolean().default(true),
     enableTransitivelyFreezeFunctionExpressions: zod.z.boolean().default(true),
     enableEmitFreeze: ExternalFunctionSchema.nullable().default(null),
@@ -48389,6 +48492,49 @@ class RewriteBlockIds extends ReactiveFunctionVisitor {
     }
 }
 
+function inferAliasForUncalledFunctions(fn, aliases) {
+    var _a;
+    for (const block of fn.body.blocks.values()) {
+        instrs: for (const instr of block.instructions) {
+            const { lvalue, value } = instr;
+            if (value.kind !== 'ObjectMethod' &&
+                value.kind !== 'FunctionExpression') {
+                continue;
+            }
+            const range = lvalue.identifier.mutableRange;
+            if (range.end > range.start + 1) {
+                continue;
+            }
+            for (const operand of eachInstructionValueOperand(value)) {
+                if (isMutable(instr, operand)) {
+                    continue instrs;
+                }
+            }
+            const operands = new Set();
+            for (const effect of (_a = value.loweredFunc.func.effects) !== null && _a !== void 0 ? _a : []) {
+                if (effect.kind !== 'ContextMutation') {
+                    continue;
+                }
+                if (effect.effect === Effect.Store || effect.effect === Effect.Mutate) {
+                    for (const operand of effect.places) {
+                        if (isMutableEffect(operand.effect, operand.loc) &&
+                            !isRefOrRefLikeMutableType(operand.identifier.type)) {
+                            operands.add(operand.identifier);
+                        }
+                    }
+                }
+            }
+            if (operands.size !== 0) {
+                operands.add(lvalue.identifier);
+                aliases.union([...operands]);
+                for (const operand of operands) {
+                    operand.mutableRange.end = makeInstructionId(instr.id + 1);
+                }
+            }
+        }
+    }
+}
+
 function inferAliases(func) {
     const aliases = new DisjointSet();
     for (const [_, block] of func.body.blocks) {
@@ -48630,6 +48776,7 @@ function inferMutableRanges(ir) {
     while (true) {
         inferMutableRangesForAlias(ir, aliases);
         inferAliasForPhis(ir, aliases);
+        inferAliasForUncalledFunctions(ir, aliases);
         const nextAliases = aliases.canonicalize();
         if (areEqualMaps(prevAliases, nextAliases)) {
             break;
@@ -55140,6 +55287,76 @@ function validateStaticComponents(fn) {
     return error.asResult();
 }
 
+function validateNoFreezingKnownMutableFunctions(fn) {
+    var _a;
+    const errors = new CompilerError();
+    const contextMutationEffects = new Map();
+    function visitOperand(operand) {
+        if (operand.effect === Effect.Freeze) {
+            const effect = contextMutationEffects.get(operand.identifier.id);
+            if (effect != null) {
+                errors.push({
+                    reason: `This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update`,
+                    description: `Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables`,
+                    loc: operand.loc,
+                    severity: ErrorSeverity.InvalidReact,
+                });
+                errors.push({
+                    reason: `The function modifies a local variable here`,
+                    loc: effect.loc,
+                    severity: ErrorSeverity.InvalidReact,
+                });
+            }
+        }
+    }
+    for (const block of fn.body.blocks.values()) {
+        for (const instr of block.instructions) {
+            const { lvalue, value } = instr;
+            switch (value.kind) {
+                case 'LoadLocal': {
+                    const effect = contextMutationEffects.get(value.place.identifier.id);
+                    if (effect != null) {
+                        contextMutationEffects.set(lvalue.identifier.id, effect);
+                    }
+                    break;
+                }
+                case 'StoreLocal': {
+                    const effect = contextMutationEffects.get(value.value.identifier.id);
+                    if (effect != null) {
+                        contextMutationEffects.set(lvalue.identifier.id, effect);
+                        contextMutationEffects.set(value.lvalue.place.identifier.id, effect);
+                    }
+                    break;
+                }
+                case 'FunctionExpression': {
+                    const knownMutation = ((_a = value.loweredFunc.func.effects) !== null && _a !== void 0 ? _a : []).find(effect => {
+                        return (effect.kind === 'ContextMutation' &&
+                            (effect.effect === Effect.Store ||
+                                effect.effect === Effect.Mutate) &&
+                            Iterable_some(effect.places, place => {
+                                return (isMutableEffect(place.effect, place.loc) &&
+                                    !isRefOrRefLikeMutableType(place.identifier.type));
+                            }));
+                    });
+                    if (knownMutation && knownMutation.kind === 'ContextMutation') {
+                        contextMutationEffects.set(lvalue.identifier.id, knownMutation);
+                    }
+                    break;
+                }
+                default: {
+                    for (const operand of eachInstructionValueOperand(value)) {
+                        visitOperand(operand);
+                    }
+                }
+            }
+        }
+        for (const operand of eachTerminalOperand(block.terminal)) {
+            visitOperand(operand);
+        }
+    }
+    return errors.asResult();
+}
+
 function run(func, config, fnType, mode, programContext, logger, filename, code) {
     var _a, _b;
     const contextIdentifiers = findContextIdentifiers(func);
@@ -55244,6 +55461,9 @@ function runWithEnvironment(func, env) {
         if (env.config.validateNoImpureFunctionsInRender) {
             validateNoImpureFunctionsInRender(hir).unwrap();
         }
+        if (env.config.validateNoFreezingKnownMutableFunctions) {
+            validateNoFreezingKnownMutableFunctions(hir).unwrap();
+        }
     }
     inferReactivePlaces(hir);
     log({ kind: 'hir', name: 'InferReactivePlaces', value: hir });

compiled/facebook-www/REVISION

@@ -1 +1 @@
-73d7e816b7746c700ab843964aaf11c17351fac1
+0db8db178c1521f979535bdba32bf9db9f47ca05

compiled/facebook-www/REVISION_TRANSFORMS

@@ -1 +1 @@
-73d7e816b7746c700ab843964aaf11c17351fac1
+0db8db178c1521f979535bdba32bf9db9f47ca05

compiled/facebook-www/React-dev.classic.js

@@ -1538,7 +1538,7 @@ __DEV__ &&
     exports.useTransition = function () {
       return resolveDispatcher().useTransition();
     };
-    exports.version = "19.2.0-www-classic-73d7e816-20250503";
+    exports.version = "19.2.0-www-classic-0db8db17-20250503";
     "undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
       "function" ===
         typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-dev.modern.js

@@ -1538,7 +1538,7 @@ __DEV__ &&
     exports.useTransition = function () {
       return resolveDispatcher().useTransition();
     };
-    exports.version = "19.2.0-www-modern-73d7e816-20250503";
+    exports.version = "19.2.0-www-modern-0db8db17-20250503";
     "undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
       "function" ===
         typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-prod.classic.js

@@ -636,4 +636,4 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactSharedInternals.H.useTransition();
 };
-exports.version = "19.2.0-www-classic-73d7e816-20250503";
+exports.version = "19.2.0-www-classic-0db8db17-20250503";

compiled/facebook-www/React-prod.modern.js

@@ -636,4 +636,4 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactSharedInternals.H.useTransition();
 };
-exports.version = "19.2.0-www-modern-73d7e816-20250503";
+exports.version = "19.2.0-www-modern-0db8db17-20250503";

compiled/facebook-www/React-profiling.classic.js

@@ -640,7 +640,7 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactSharedInternals.H.useTransition();
 };
-exports.version = "19.2.0-www-classic-73d7e816-20250503";
+exports.version = "19.2.0-www-classic-0db8db17-20250503";
 "undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
   "function" ===
     typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-profiling.modern.js

@@ -640,7 +640,7 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactSharedInternals.H.useTransition();
 };
-exports.version = "19.2.0-www-modern-73d7e816-20250503";
+exports.version = "19.2.0-www-modern-0db8db17-20250503";
 "undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
   "function" ===
     typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/ReactART-dev.classic.js

@@ -19014,10 +19014,10 @@ __DEV__ &&
     (function () {
       var internals = {
         bundleType: 1,
-        version: "19.2.0-www-classic-73d7e816-20250503",
+        version: "19.2.0-www-classic-0db8db17-20250503",
         rendererPackageName: "react-art",
         currentDispatcherRef: ReactSharedInternals,
-        reconcilerVersion: "19.2.0-www-classic-73d7e816-20250503"
+        reconcilerVersion: "19.2.0-www-classic-0db8db17-20250503"
       };
       internals.overrideHookState = overrideHookState;
       internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -19051,7 +19051,7 @@ __DEV__ &&
     exports.Shape = Shape;
     exports.Surface = Surface;
     exports.Text = Text;
-    exports.version = "19.2.0-www-classic-73d7e816-20250503";
+    exports.version = "19.2.0-www-classic-0db8db17-20250503";
     "undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
       "function" ===
         typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&