From 6715b877759d35ba6e970781edddc8b312b229b2 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Wed, 2 Jul 2025 17:21:17 -0400 Subject: [PATCH 01/16] New rule: `isolated-functions` --- configs/recommended.js | 1 + docs/rules/isolated-functions.md | 216 +++++++++++++++++++++++++++++++ readme.md | 1 + rules/isolated-functions.js | 172 ++++++++++++++++++++++++ test/isolated-functions.mjs | 193 +++++++++++++++++++++++++++ 5 files changed, 583 insertions(+) create mode 100644 docs/rules/isolated-functions.md create mode 100644 rules/isolated-functions.js create mode 100644 test/isolated-functions.mjs diff --git a/configs/recommended.js b/configs/recommended.js index d8ee1eb42a..3ddac40955 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -22,6 +22,7 @@ module.exports = { 'unicorn/expiring-todo-comments': 'error', 'unicorn/explicit-length-check': 'error', 'unicorn/filename-case': 'error', + 'unicorn/isolated-functions': 'error', 'unicorn/import-style': 'error', 'unicorn/new-for-builtins': 'error', 'unicorn/no-abusive-eslint-disable': 'error', diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md new file mode 100644 index 0000000000..83f74800ec --- /dev/null +++ b/docs/rules/isolated-functions.md @@ -0,0 +1,216 @@ +# Prevent usage of variables from outside the scope of isolated functions + +💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs). + + + + +Some functions need to be isolated from their surrounding scope due to execution context constraints. For example, functions passed to `makeSynchronous()` are executed in a subprocess and cannot access variables from outside their scope. This rule helps identify when functions are using external variables that may cause runtime errors. + +Common scenarios where functions must be isolated: +- Functions passed to `makeSynchronous()` (executed in subprocess) +- Functions that will be serialized via `Function.prototype.toString()` +- Server actions or other remote execution contexts +- Functions with specific JSDoc annotations + +## Fail + +```js +import makeSynchronous from 'make-synchronous'; + +export const fetchSync = () => { + const url = 'https://example.com'; + const getText = makeSynchronous(async () => { + const res = await fetch(url); // ❌ 'url' is not defined in isolated function scope + return res.text(); + }); + console.log(getText()); +}; +``` + +```js +const foo = 'hi'; + +/** @isolated */ +function abc() { + return foo.slice(); // ❌ 'foo' is not defined in isolated function scope +} +``` + +```js +const foo = 'hi'; + +/** @isolated */ +const abc = () => foo.slice(); // ❌ 'foo' is not defined in isolated function scope +``` + +## Pass + +```js +import makeSynchronous from 'make-synchronous'; + +export const fetchSync = () => { + const getText = makeSynchronous(async () => { + const url = 'https://example.com'; // ✅ Variable defined within function scope + const res = await fetch(url); + return res.text(); + }); + console.log(getText()); +}; +``` + +```js +import makeSynchronous from 'make-synchronous'; + +export const fetchSync = () => { + const getText = makeSynchronous(async (url) => { // ✅ Variable passed as parameter + const res = await fetch(url); + return res.text(); + }); + console.log(getText('https://example.com')); +}; +``` + +```js +/** @isolated */ +function abc() { + const foo = 'hi'; // ✅ Variable defined within function scope + return foo.slice(); +} +``` + +## Options + +Type: `object` + +### functions + +Type: `string[]`\ +Default: `['makeSynchronous']` + +Array of function names that create isolated execution contexts. Functions passed as arguments to these functions will be considered isolated. + +### selectors + +Type: `string[]`\ +Default: `[]` + +Array of [ESLint selectors](https://eslint.org/docs/developer-guide/selectors) to identify isolated functions. Useful for custom naming conventions or framework-specific patterns. + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + selectors: ['FunctionDeclaration[id.name=/lambdaHandler.*/]'] + } + ] +} +``` + +### comments + +Type: `string[]`\ +Default: `['@isolated']` + +Array of comment strings that mark functions as isolated. Functions with JSDoc comments containing these strings will be considered isolated. + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + comments: ['@isolated', '@remote'] + } + ] +} +``` + +### globals + +Type: `boolean | string[]`\ +Default: `false` + +Controls how global variables are handled: + +- `false` (default): Global variables are not allowed in isolated functions +- `true`: All globals from ESLint's language options are allowed +- `string[]`: Only the specified global variable names are allowed + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + globals: ['console', 'fetch'] // Only allow these globals + } + ] +} +``` + +## Examples + +### Custom function names + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + functions: ['makeSynchronous', 'createWorker', 'serializeFunction'] + } + ] +} +``` + +### Lambda function naming convention + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + selectors: ['FunctionDeclaration[id.name=/lambdaHandler.*/]'] + } + ] +} +``` + +```js +const foo = 'hi'; + +function lambdaHandlerFoo() { // ❌ Will be flagged as isolated + return foo.slice(); +} + +function someOtherFunction() { // ✅ Not flagged + return foo.slice(); +} + +createLambda({ + name: 'fooLambda', + code: lambdaHandlerFoo.toString(), // Function will be serialized +}); +``` + +### Allowing specific globals + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + globals: ['console', 'fetch', 'URL'] + } + ] +} +``` + +```js +makeSynchronous(async () => { + console.log('Starting...'); // ✅ Allowed global + const response = await fetch('https://api.example.com'); // ✅ Allowed global + const url = new URL(response.url); // ✅ Allowed global + return response.text(); +}); +``` \ No newline at end of file diff --git a/readme.md b/readme.md index e223570eac..8c7000e40b 100644 --- a/readme.md +++ b/readme.md @@ -67,6 +67,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c | [explicit-length-check](docs/rules/explicit-length-check.md) | Enforce explicitly comparing the `length` or `size` property of a value. | ✅ | 🔧 | 💡 | | [filename-case](docs/rules/filename-case.md) | Enforce a case style for filenames. | ✅ | | | | [import-style](docs/rules/import-style.md) | Enforce specific import styles per module. | ✅ | | | +| [isolated-functions](docs/rules/isolated-functions.md) | Prevent usage of variables from outside the scope of isolated functions. | ✅ | | | | [new-for-builtins](docs/rules/new-for-builtins.md) | Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. | ✅ | 🔧 | | | [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. | ✅ | | | | [no-array-callback-reference](docs/rules/no-array-callback-reference.md) | Prevent passing a function reference directly to iterator methods. | ✅ | | 💡 | diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js new file mode 100644 index 0000000000..6b30a7fd71 --- /dev/null +++ b/rules/isolated-functions.js @@ -0,0 +1,172 @@ +'use strict'; +const esquery = require('esquery'); +const functionTypes = require('./ast/function-types.js'); + +const MESSAGE_ID_EXTERNALLY_SCOPED_VARIABLE = 'externally-scoped-variable'; +const messages = { + [MESSAGE_ID_EXTERNALLY_SCOPED_VARIABLE]: 'Variable {{name}} not defined in scope of isolated function. Function is isolated because: {{reason}}.', +}; + +const parsedEsquerySelectors = new Map(); +const parseEsquerySelector = selector => { + if (!parsedEsquerySelectors.has(selector)) { + parsedEsquerySelectors.set(selector, esquery.parse(selector)); + } + + return parsedEsquerySelectors.get(selector); +}; + +/** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ +const defaultOptions = { + functions: ['makeSynchronous'], + selectors: [], + comments: ['@isolated'], + globals: false, +}; + +/** @param {import('eslint').Rule.RuleContext} context */ +const create = context => { + const {sourceCode} = context; + /** @type {typeof defaultOptions} */ + const userOptions = context.options[0]; + const options = { + ...defaultOptions, + ...userOptions, + }; + + options.comments = options.comments.map(comment => comment.toLowerCase()); + const allowedGlobals = options.globals === true + ? Object.keys(context.languageOptions.globals) + : (Array.isArray(options.globals) + ? options.globals + : []); + + /** @param {import('estree').Node} node */ + const checkForExternallyScopedVariables = node => { + const reason = reasaonForBeingIsolatedFunction(node); + if (!reason) { + return; + } + + const nodeScope = sourceCode.getScope(node); + + for (const ref of nodeScope.through) { + const {identifier} = ref; + + if (allowedGlobals.includes(identifier.name)) { + continue; + } + + // If (!options.considerTypeOf && hasTypeOfOperator(identifier)) { + // return; + // } + + context.report({ + node: identifier, + messageId: MESSAGE_ID_EXTERNALLY_SCOPED_VARIABLE, + // Message: `Variable ${identifier.name} is used from outside the scope of an isolated function. Function is isolated because: ${reason}.`, + data: {name: identifier.name, reason}, + }); + } + }; + + /** @param {import('estree').Node & {parent?: import('estree').Node}} node */ + const reasaonForBeingIsolatedFunction = node => { + if (options.comments.length > 0) { + let previousToken = sourceCode.getTokenBefore(node, {includeComments: true}); + let commentableNode = node; + while (previousToken?.type !== 'Block' && (commentableNode.parent.type === 'VariableDeclarator' || commentableNode.parent.type === 'VariableDeclaration')) { + // Search up to find jsdoc comments on the parent declaration `/** @isolated */ const foo = () => abc` + commentableNode = commentableNode.parent; + previousToken = sourceCode.getTokenBefore(commentableNode, {includeComments: true}); + } + + if (previousToken?.type === 'Block') { + const previousComment = previousToken.value.trim().toLowerCase(); + const match = options.comments.find(comment => previousComment.includes(comment)); + if (match) { + return `follows comment containing ${JSON.stringify(match)}`; + } + } + } + + if ( + options.functions.length > 0 + && node.parent.type === 'CallExpression' + && node.parent.arguments.includes(node) + && node.parent.callee.type === 'Identifier' + && options.functions.includes(node.parent.callee.name) + ) { + return `callee of function named ${JSON.stringify(node.parent.callee.name)}`; + } + + if (options.selectors.length > 0) { + const ancestors = sourceCode.getAncestors(node).reverse(); + const matchedSelector = options.selectors.find(selector => esquery.matches(node, parseEsquerySelector(selector), ancestors)); + if (matchedSelector) { + return `matches selector ${JSON.stringify(matchedSelector)}`; + } + } + + return undefined; + }; + + return Object.fromEntries(functionTypes.map(type => [ + `${type}:exit`, + checkForExternallyScopedVariables, + ])); +}; + +/** @type {import('json-schema').JSONSchema7[]} */ +const schema = [ + { + type: 'object', + additionalProperties: false, + properties: { + tags: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + globals: { + oneOf: [{type: 'boolean'}, {type: 'array', items: {type: 'string'}}], + }, + functions: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + selectors: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + comments: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + }, + }, +]; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + create, + meta: { + type: 'problem', + docs: { + description: 'Prevent usage of variables from outside the scope of isolated functions.', + }, + schema, + messages, + }, +}; diff --git a/test/isolated-functions.mjs b/test/isolated-functions.mjs new file mode 100644 index 0000000000..308a93c89a --- /dev/null +++ b/test/isolated-functions.mjs @@ -0,0 +1,193 @@ +import stripIndent from 'strip-indent'; +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +const error = data => ({messageId: 'externally-scoped-variable', data}); +const fooInMakeSynchronousError = error({name: 'foo', reason: 'callee of function named "makeSynchronous"'}); + +test({ + /** @type {import('eslint').RuleTester.InvalidTestCase[]} */ + invalid: [ + { + name: 'out of scope variable under makeSynchronous (arrow function)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(() => foo.slice()); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'out of scope variable under makeSynchronous (async arrow function)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(async () => foo.slice()); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'out of scope variable under makeSynchronous (function expression)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(function () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'out of scope variable under makeSynchronous (async function expression)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(async function () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'out of scope variable under makeSynchronous (named function expression)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(function abc () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'out of scope variable under makeSynchronous (named async function expression)', + code: stripIndent(` + const foo = 'hi'; + makeSynchronous(async function abc () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: '@isolated comment on function declaration', + code: stripIndent(` + const foo = 'hi'; + /** @isolated */ + function abc () { + return foo.slice(); + } + `), + errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], + }, + { + name: '@isolated comment on arrow function', + code: stripIndent(` + const foo = 'hi'; + /** @isolated */ + const abc = () => foo.slice(); + `), + errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], + }, + { + name: 'global variables not allowed by default', + globals: {foo: true}, + code: stripIndent(` + makeSynchronous(function () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'global variables can be explicitly disallowed', + globals: {foo: true}, + options: [{globals: false}], + code: stripIndent(` + makeSynchronous(function () { + return foo.slice(); + }); + `), + errors: [fooInMakeSynchronousError], + }, + { + name: 'make a function isolated by a selector', + // In this case, we're imagining some naming convention for lambda functions that will be created via `fn.toString()` + options: [{selectors: ['FunctionDeclaration[id.name=/lambdaHandler.*/]']}], + code: stripIndent(` + const foo = 'hi'; + + function lambdaHandlerFoo() { + return foo.slice(); + } + + function someOtherFunction() { + return foo.slice(); + } + + createLambda({ + name: 'fooLambda', + code: lambdaHandlerFoo.toString(), + }); + `), + errors: [ + error({name: 'foo', reason: 'matches selector "FunctionDeclaration[id.name=/lambdaHandler.*/]"'}), + ], + }, + ], + /** @type {import('eslint').RuleTester.ValidTestCase[]} */ + valid: [ + { + name: 'variable defined in scope of isolated function', + code: stripIndent(` + makeSynchronous(() => { + const foo = 'hi'; + return foo.slice(); + }); + `), + }, + { + name: 'variable defined as parameter of isolated function', + code: stripIndent(` + makeSynchronous(foo => { + return foo.slice(); + }); + `), + }, + { + name: 'inner function can access outer function parameters', + code: stripIndent(` + /** @isolated */ + function abc () { + const foo = 'hi'; + const slice = () => foo.slice(); + return slice(); + } + `), + }, + { + name: 'variable defined as parameter of isolated function (async)', + code: stripIndent(` + makeSynchronous(async function (foo) { + return foo.slice(); + }); + `), + }, + { + name: 'can allow global variables from language options', + globals: {foo: true}, + options: [{globals: true}], + code: stripIndent(` + makeSynchronous(function () { + return foo.slice(); + }); + `), + }, + { + name: 'allow global variables separate from language options', + globals: {abc: true}, + options: [{globals: ['foo']}], + code: stripIndent(` + makeSynchronous(function () { + return foo.slice(); + }); + `), + }, + ], +}); From 27ac79cb776b115ab413d7dc9da6d6ebecf30148 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Wed, 2 Jul 2025 17:47:15 -0400 Subject: [PATCH 02/16] esm is cool --- docs/rules/isolated-functions.md | 2 +- readme.md | 1 + rules/index.js | 1 + rules/isolated-functions.js | 13 +++---------- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 83f74800ec..c521e8dd15 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -1,6 +1,6 @@ # Prevent usage of variables from outside the scope of isolated functions -💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs). +💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config). diff --git a/readme.md b/readme.md index 13e74df1e4..66063e4f58 100644 --- a/readme.md +++ b/readme.md @@ -72,6 +72,7 @@ export default [ | [explicit-length-check](docs/rules/explicit-length-check.md) | Enforce explicitly comparing the `length` or `size` property of a value. | ✅ | 🔧 | 💡 | | [filename-case](docs/rules/filename-case.md) | Enforce a case style for filenames. | ✅ | | | | [import-style](docs/rules/import-style.md) | Enforce specific import styles per module. | ✅ | | | +| [isolated-functions](docs/rules/isolated-functions.md) | Prevent usage of variables from outside the scope of isolated functions. | ✅ | | | | [new-for-builtins](docs/rules/new-for-builtins.md) | Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. | ✅ | 🔧 | 💡 | | [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. | ✅ | | | | [no-accessor-recursion](docs/rules/no-accessor-recursion.md) | Disallow recursive access to `this` within getters and setters. | ✅ | | | diff --git a/rules/index.js b/rules/index.js index b0a372a5aa..8f5fcd91eb 100644 --- a/rules/index.js +++ b/rules/index.js @@ -16,6 +16,7 @@ export {default as 'expiring-todo-comments'} from './expiring-todo-comments.js'; export {default as 'explicit-length-check'} from './explicit-length-check.js'; export {default as 'filename-case'} from './filename-case.js'; export {default as 'import-style'} from './import-style.js'; +export {default as 'isolated-functions'} from './isolated-functions.js'; export {default as 'new-for-builtins'} from './new-for-builtins.js'; export {default as 'no-abusive-eslint-disable'} from './no-abusive-eslint-disable.js'; export {default as 'no-accessor-recursion'} from './no-accessor-recursion.js'; diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index f97f2abd25..0a0567581d 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -1,6 +1,6 @@ 'use strict'; -const esquery = require('esquery'); -const functionTypes = require('./ast/function-types.js'); +import esquery from 'esquery'; +import functionTypes from './ast/function-types.js'; const MESSAGE_ID_EXTERNALLY_SCOPED_VARIABLE = 'externally-scoped-variable'; const messages = { @@ -123,13 +123,6 @@ const schema = [ type: 'object', additionalProperties: false, properties: { - tags: { - type: 'array', - uniqueItems: true, - items: { - type: 'string', - }, - }, globals: { oneOf: [{type: 'boolean'}, {type: 'array', items: {type: 'string'}}], }, @@ -159,7 +152,7 @@ const schema = [ ]; /** @type {import('eslint').Rule.RuleModule} */ -module.exports = { +export default { create, meta: { type: 'problem', From 4fbb759e7fc7e6ee436b7ea0f921a275eabe40da Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Wed, 2 Jul 2025 17:52:20 -0400 Subject: [PATCH 03/16] lint --- rules/isolated-functions.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index 0a0567581d..b8b2f46563 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -50,21 +50,18 @@ const create = context => { const nodeScope = sourceCode.getScope(node); - for (const ref of nodeScope.through) { - const {identifier} = ref; + for (const reference of nodeScope.through) { + const {identifier} = reference; if (allowedGlobals.includes(identifier.name)) { continue; } - // If (!options.considerTypeOf && hasTypeOfOperator(identifier)) { - // return; - // } + // Could consider checking for typeof operator here, like in no-undef? context.report({ node: identifier, messageId: MESSAGE_ID_EXTERNALLY_SCOPED_VARIABLE, - // Message: `Variable ${identifier.name} is used from outside the scope of an isolated function. Function is isolated because: ${reason}.`, data: {name: identifier.name, reason}, }); } @@ -108,7 +105,7 @@ const create = context => { } } - return undefined; + }; return Object.fromEntries(functionTypes.map(type => [ From 55036f155a4f2d4156d9f114e1a043ab34199ca7 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Thu, 3 Jul 2025 00:29:24 -0400 Subject: [PATCH 04/16] exts --- docs/rules/isolated-functions.md | 3 +- rules/isolated-functions.js | 29 +++++++++---------- ...ed-functions.mjs => isolated-functions.js} | 19 ++++++++---- 3 files changed, 29 insertions(+), 22 deletions(-) rename test/{isolated-functions.mjs => isolated-functions.js} (91%) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index c521e8dd15..c72038663c 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -8,6 +8,7 @@ Some functions need to be isolated from their surrounding scope due to execution context constraints. For example, functions passed to `makeSynchronous()` are executed in a subprocess and cannot access variables from outside their scope. This rule helps identify when functions are using external variables that may cause runtime errors. Common scenarios where functions must be isolated: + - Functions passed to `makeSynchronous()` (executed in subprocess) - Functions that will be serialized via `Function.prototype.toString()` - Server actions or other remote execution contexts @@ -213,4 +214,4 @@ makeSynchronous(async () => { const url = new URL(response.url); // ✅ Allowed global return response.text(); }); -``` \ No newline at end of file +``` diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index b8b2f46563..6811ef5926 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -16,21 +16,17 @@ const parseEsquerySelector = selector => { return parsedEsquerySelectors.get(selector); }; -/** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ -const defaultOptions = { - functions: ['makeSynchronous'], - selectors: [], - comments: ['@isolated'], - globals: false, -}; - /** @param {import('eslint').Rule.RuleContext} context */ const create = context => { const {sourceCode} = context; - /** @type {typeof defaultOptions} */ + /** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ const userOptions = context.options[0]; + /** @type {typeof userOptions} */ const options = { - ...defaultOptions, + functions: ['makeSynchronous'], + selectors: [], + comments: ['@isolated'], + globals: false, ...userOptions, }; @@ -43,7 +39,7 @@ const create = context => { /** @param {import('estree').Node} node */ const checkForExternallyScopedVariables = node => { - const reason = reasaonForBeingIsolatedFunction(node); + const reason = reasonForBeingIsolatedFunction(node); if (!reason) { return; } @@ -68,17 +64,20 @@ const create = context => { }; /** @param {import('estree').Node & {parent?: import('estree').Node}} node */ - const reasaonForBeingIsolatedFunction = node => { + const reasonForBeingIsolatedFunction = node => { if (options.comments.length > 0) { let previousToken = sourceCode.getTokenBefore(node, {includeComments: true}); let commentableNode = node; - while (previousToken?.type !== 'Block' && (commentableNode.parent.type === 'VariableDeclarator' || commentableNode.parent.type === 'VariableDeclaration')) { + while ( + (previousToken?.type !== 'Block' && previousToken?.type !== 'Line') + && (commentableNode.parent.type === 'VariableDeclarator' || commentableNode.parent.type === 'VariableDeclaration') + ) { // Search up to find jsdoc comments on the parent declaration `/** @isolated */ const foo = () => abc` commentableNode = commentableNode.parent; previousToken = sourceCode.getTokenBefore(commentableNode, {includeComments: true}); } - if (previousToken?.type === 'Block') { + if (previousToken?.type === 'Block' || previousToken?.type === 'Line') { const previousComment = previousToken.value.trim().toLowerCase(); const match = options.comments.find(comment => previousComment.includes(comment)); if (match) { @@ -104,8 +103,6 @@ const create = context => { return `matches selector ${JSON.stringify(matchedSelector)}`; } } - - }; return Object.fromEntries(functionTypes.map(type => [ diff --git a/test/isolated-functions.mjs b/test/isolated-functions.js similarity index 91% rename from test/isolated-functions.mjs rename to test/isolated-functions.js index 308a93c89a..718bb02d7c 100644 --- a/test/isolated-functions.mjs +++ b/test/isolated-functions.js @@ -1,5 +1,5 @@ import stripIndent from 'strip-indent'; -import {getTester} from './utils/test.mjs'; +import {getTester} from './utils/test.js'; const {test} = getTester(import.meta); @@ -85,9 +85,18 @@ test({ `), errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], }, + { + name: '@isolated inline comment', + code: stripIndent(` + const foo = 'hi'; + // @isolated + const abc = () => foo.slice(); + `), + errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], + }, { name: 'global variables not allowed by default', - globals: {foo: true}, + languageOptions: {globals: {foo: true}}, code: stripIndent(` makeSynchronous(function () { return foo.slice(); @@ -97,7 +106,7 @@ test({ }, { name: 'global variables can be explicitly disallowed', - globals: {foo: true}, + languageOptions: {globals: {foo: true}}, options: [{globals: false}], code: stripIndent(` makeSynchronous(function () { @@ -171,7 +180,7 @@ test({ }, { name: 'can allow global variables from language options', - globals: {foo: true}, + languageOptions: {globals: {foo: true}}, options: [{globals: true}], code: stripIndent(` makeSynchronous(function () { @@ -181,7 +190,7 @@ test({ }, { name: 'allow global variables separate from language options', - globals: {abc: true}, + languageOptions: {globals: {abc: true}}, options: [{globals: ['foo']}], code: stripIndent(` makeSynchronous(function () { From ea09c9ec8cdda7bfca5919b06a0f872e602c0548 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Thu, 3 Jul 2025 08:40:02 -0400 Subject: [PATCH 05/16] allow globals by default --- docs/rules/isolated-functions.md | 21 +++++++++++++++++--- rules/isolated-functions.js | 33 ++++++++++++++++++-------------- test/isolated-functions.js | 21 ++++++++++---------- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index c72038663c..20d1d5af0a 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -14,6 +14,8 @@ Common scenarios where functions must be isolated: - Server actions or other remote execution contexts - Functions with specific JSDoc annotations +By default, this rule allows global variables (like `console`, `fetch`, etc.) in isolated functions, but prevents usage of variables from the surrounding scope. + ## Fail ```js @@ -80,6 +82,19 @@ function abc() { } ``` +```js +import makeSynchronous from 'make-synchronous'; + +export const fetchSync = () => { + const getText = makeSynchronous(async () => { + console.log('Starting...'); // ✅ Global variables are allowed by default + const res = await fetch('https://example.com'); // ✅ Global variables are allowed by default + return res.text(); + }); + console.log(getText()); +}; +``` + ## Options Type: `object` @@ -130,12 +145,12 @@ Array of comment strings that mark functions as isolated. Functions with JSDoc c ### globals Type: `boolean | string[]`\ -Default: `false` +Default: `true` Controls how global variables are handled: -- `false` (default): Global variables are not allowed in isolated functions -- `true`: All globals from ESLint's language options are allowed +- `false`: Global variables are not allowed in isolated functions +- `true` (default): All globals from ESLint's language options are allowed - `string[]`: Only the specified global variable names are allowed ```js diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index 6811ef5926..50945edaff 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -1,4 +1,3 @@ -'use strict'; import esquery from 'esquery'; import functionTypes from './ast/function-types.js'; @@ -16,26 +15,31 @@ const parseEsquerySelector = selector => { return parsedEsquerySelectors.get(selector); }; +/** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ +const defaultOptions = { + functions: ['makeSynchronous'], + selectors: [], + comments: ['@isolated'], + globals: true, +}; + /** @param {import('eslint').Rule.RuleContext} context */ const create = context => { const {sourceCode} = context; - /** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ - const userOptions = context.options[0]; - /** @type {typeof userOptions} */ + /** @type {typeof defaultOptions} */ const options = { - functions: ['makeSynchronous'], - selectors: [], - comments: ['@isolated'], - globals: false, - ...userOptions, + ...defaultOptions, + ...context.options[0], }; options.comments = options.comments.map(comment => comment.toLowerCase()); - const allowedGlobals = options.globals === true - ? Object.keys(context.languageOptions.globals) - : (Array.isArray(options.globals) - ? options.globals - : []); + /** @type {string[]} */ + let allowedGlobals = []; + if (options.globals === true) { + allowedGlobals = Object.keys(context.languageOptions.globals); + } else if (Array.isArray(options.globals)) { + allowedGlobals = options.globals; + } /** @param {import('estree').Node} node */ const checkForExternallyScopedVariables = node => { @@ -155,6 +159,7 @@ export default { recommended: true, }, schema, + defaultOptions: [defaultOptions], messages, }, }; diff --git a/test/isolated-functions.js b/test/isolated-functions.js index 718bb02d7c..ba8c5710b6 100644 --- a/test/isolated-functions.js +++ b/test/isolated-functions.js @@ -94,16 +94,6 @@ test({ `), errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], }, - { - name: 'global variables not allowed by default', - languageOptions: {globals: {foo: true}}, - code: stripIndent(` - makeSynchronous(function () { - return foo.slice(); - }); - `), - errors: [fooInMakeSynchronousError], - }, { name: 'global variables can be explicitly disallowed', languageOptions: {globals: {foo: true}}, @@ -179,7 +169,16 @@ test({ `), }, { - name: 'can allow global variables from language options', + name: 'can implicitly allow global variables from language options', + languageOptions: {globals: {foo: true}}, + code: stripIndent(` + makeSynchronous(function () { + return foo.slice(); + }); + `), + }, + { + name: 'can explicitly allow global variables from language options', languageOptions: {globals: {foo: true}}, options: [{globals: true}], code: stripIndent(` From 26e70a526e7b556bb2098c9bbee1a3e6227a5a66 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 07:03:28 -0400 Subject: [PATCH 06/16] set (updated 07:04) --- rules/isolated-functions.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index 50945edaff..fa74d6258c 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -33,13 +33,9 @@ const create = context => { }; options.comments = options.comments.map(comment => comment.toLowerCase()); - /** @type {string[]} */ - let allowedGlobals = []; - if (options.globals === true) { - allowedGlobals = Object.keys(context.languageOptions.globals); - } else if (Array.isArray(options.globals)) { - allowedGlobals = options.globals; - } + const allowedGlobals = options.globals === true + ? new Set(Object.keys(context.languageOptions.globals)) + : new Set(options.globals || []); /** @param {import('estree').Node} node */ const checkForExternallyScopedVariables = node => { @@ -53,7 +49,7 @@ const create = context => { for (const reference of nodeScope.through) { const {identifier} = reference; - if (allowedGlobals.includes(identifier.name)) { + if (allowedGlobals.has(identifier.name)) { continue; } From a52c3ee2bc93a6dc2ea5e9ddbcfd8d4f7a96b2d6 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 07:17:34 -0400 Subject: [PATCH 07/16] rm .reverse() --- rules/isolated-functions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index fa74d6258c..7cbfce1a12 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -97,7 +97,7 @@ const create = context => { } if (options.selectors.length > 0) { - const ancestors = sourceCode.getAncestors(node).reverse(); + const ancestors = sourceCode.getAncestors(node); const matchedSelector = options.selectors.find(selector => esquery.matches(node, parseEsquerySelector(selector), ancestors)); if (matchedSelector) { return `matches selector ${JSON.stringify(matchedSelector)}`; From 60f072de33dd2b5940cce6988286ca86a641d4aa Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Mon, 7 Jul 2025 13:41:30 +0200 Subject: [PATCH 08/16] Update isolated-functions.md --- docs/rules/isolated-functions.md | 34 +++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 20d1d5af0a..1bb3c7421d 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -5,11 +5,11 @@ -Some functions need to be isolated from their surrounding scope due to execution context constraints. For example, functions passed to `makeSynchronous()` are executed in a subprocess and cannot access variables from outside their scope. This rule helps identify when functions are using external variables that may cause runtime errors. +Some functions need to be isolated from their surrounding scope due to execution context constraints. For example, functions passed to [`makeSynchronous()`](https://github.com/sindresorhus/make-synchronous) are executed in a worker or subprocess and cannot access variables from outside their scope. This rule helps identify when functions are using external variables that may cause runtime errors. Common scenarios where functions must be isolated: -- Functions passed to `makeSynchronous()` (executed in subprocess) +- Functions passed to `makeSynchronous()` (executed in worker) - Functions that will be serialized via `Function.prototype.toString()` - Server actions or other remote execution contexts - Functions with specific JSDoc annotations @@ -23,10 +23,12 @@ import makeSynchronous from 'make-synchronous'; export const fetchSync = () => { const url = 'https://example.com'; + const getText = makeSynchronous(async () => { const res = await fetch(url); // ❌ 'url' is not defined in isolated function scope return res.text(); }); + console.log(getText()); }; ``` @@ -58,6 +60,7 @@ export const fetchSync = () => { const res = await fetch(url); return res.text(); }); + console.log(getText()); }; ``` @@ -70,6 +73,7 @@ export const fetchSync = () => { const res = await fetch(url); return res.text(); }); + console.log(getText('https://example.com')); }; ``` @@ -91,6 +95,7 @@ export const fetchSync = () => { const res = await fetch('https://example.com'); // ✅ Global variables are allowed by default return res.text(); }); + console.log(getText()); }; ``` @@ -118,7 +123,9 @@ Array of [ESLint selectors](https://eslint.org/docs/developer-guide/selectors) t 'unicorn/isolated-functions': [ 'error', { - selectors: ['FunctionDeclaration[id.name=/lambdaHandler.*/]'] + selectors: [ + 'FunctionDeclaration[id.name=/lambdaHandler.*/]' + ] } ] } @@ -136,7 +143,10 @@ Array of comment strings that mark functions as isolated. Functions with JSDoc c 'unicorn/isolated-functions': [ 'error', { - comments: ['@isolated', '@remote'] + comments: [ + '@isolated', + '@remote' + ] } ] } @@ -173,7 +183,11 @@ Controls how global variables are handled: 'unicorn/isolated-functions': [ 'error', { - functions: ['makeSynchronous', 'createWorker', 'serializeFunction'] + functions: [ + 'makeSynchronous', + 'createWorker', + 'serializeFunction' + ] } ] } @@ -186,7 +200,9 @@ Controls how global variables are handled: 'unicorn/isolated-functions': [ 'error', { - selectors: ['FunctionDeclaration[id.name=/lambdaHandler.*/]'] + selectors: [ + 'FunctionDeclaration[id.name=/lambdaHandler.*/]' + ] } ] } @@ -216,7 +232,11 @@ createLambda({ 'unicorn/isolated-functions': [ 'error', { - globals: ['console', 'fetch', 'URL'] + globals: [ + 'console', + 'fetch', + 'URL' + ] } ] } From 7786865af66146664a51791d05c69fbdf118e26c Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Mon, 7 Jul 2025 13:48:57 +0200 Subject: [PATCH 09/16] Update isolated-functions.js --- rules/isolated-functions.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index 7cbfce1a12..53262f9009 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -33,6 +33,7 @@ const create = context => { }; options.comments = options.comments.map(comment => comment.toLowerCase()); + const allowedGlobals = options.globals === true ? new Set(Object.keys(context.languageOptions.globals)) : new Set(options.globals || []); @@ -118,7 +119,17 @@ const schema = [ additionalProperties: false, properties: { globals: { - oneOf: [{type: 'boolean'}, {type: 'array', items: {type: 'string'}}], + oneOf: [ + { + type: 'boolean', + }, + { + type: 'array', + items: { + type: 'string', + }, + }, + ], }, functions: { type: 'array', From b3649e9a3ce922a373641163817649b189b36da4 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 07:52:03 -0400 Subject: [PATCH 10/16] docs: group fail/pass examples closer --- docs/rules/isolated-functions.md | 63 +++++++++++++++++++------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 1bb3c7421d..585bcfa1cc 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -16,7 +16,7 @@ Common scenarios where functions must be isolated: By default, this rule allows global variables (like `console`, `fetch`, etc.) in isolated functions, but prevents usage of variables from the surrounding scope. -## Fail +## Examples ```js import makeSynchronous from 'make-synchronous'; @@ -31,45 +31,36 @@ export const fetchSync = () => { console.log(getText()); }; -``` -```js -const foo = 'hi'; - -/** @isolated */ -function abc() { - return foo.slice(); // ❌ 'foo' is not defined in isolated function scope -} -``` - -```js -const foo = 'hi'; +// ✅ +export const fetchSync = () => { + const getText = makeSynchronous(async () => { + const url = 'https://example.com'; // Variable defined within function scope + const res = await fetch(url); + return res.text(); + }); -/** @isolated */ -const abc = () => foo.slice(); // ❌ 'foo' is not defined in isolated function scope + console.log(getText()); +}; ``` -## Pass - ```js import makeSynchronous from 'make-synchronous'; export const fetchSync = () => { + const url = 'https://example.com'; + const getText = makeSynchronous(async () => { - const url = 'https://example.com'; // ✅ Variable defined within function scope - const res = await fetch(url); + const res = await fetch(url); // ❌ 'url' is not defined in isolated function scope return res.text(); }); console.log(getText()); }; -``` - -```js -import makeSynchronous from 'make-synchronous'; +// ✅ export const fetchSync = () => { - const getText = makeSynchronous(async (url) => { // ✅ Variable passed as parameter + const getText = makeSynchronous(async (url) => { // Variable passed as parameter const res = await fetch(url); return res.text(); }); @@ -79,13 +70,35 @@ export const fetchSync = () => { ``` ```js +const foo = 'hi'; + +/** @isolated */ +function abc() { + return foo.slice(); // ❌ 'foo' is not defined in isolated function scope +} + +// ✅ /** @isolated */ function abc() { - const foo = 'hi'; // ✅ Variable defined within function scope + const foo = 'hi'; // Variable defined within function scope return foo.slice(); } ``` +```js +const foo = 'hi'; + +/** @isolated */ +const abc = () => foo.slice(); // ❌ 'foo' is not defined in isolated function scope + +// ✅ +/** @isolated */ +const abc = () => { + const foo = 'hi'; // Variable defined within function scope + return foo.slice(); +}; +``` + ```js import makeSynchronous from 'make-synchronous'; From 9c92506ac2aac67d6af6444fe2449eac740af8f7 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 07:54:43 -0400 Subject: [PATCH 11/16] docs: block fn --- docs/rules/isolated-functions.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 585bcfa1cc..a08e9c8bdd 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -89,7 +89,9 @@ function abc() { const foo = 'hi'; /** @isolated */ -const abc = () => foo.slice(); // ❌ 'foo' is not defined in isolated function scope +const abc = () => { + return foo.slice(); // ❌ 'foo' is not defined in isolated function scope +}; // ✅ /** @isolated */ From 30eaa9d7683d542523e76ad385fb4f6ce25a0a3a Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 08:01:45 -0400 Subject: [PATCH 12/16] more better docs (updated 08:02) --- docs/rules/isolated-functions.md | 61 ++++++++------------------------ 1 file changed, 14 insertions(+), 47 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index a08e9c8bdd..28e3280249 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -32,7 +32,7 @@ export const fetchSync = () => { console.log(getText()); }; -// ✅ +// ✅ Define all variables within isolated function's scope export const fetchSync = () => { const getText = makeSynchronous(async () => { const url = 'https://example.com'; // Variable defined within function scope @@ -42,23 +42,8 @@ export const fetchSync = () => { console.log(getText()); }; -``` - -```js -import makeSynchronous from 'make-synchronous'; -export const fetchSync = () => { - const url = 'https://example.com'; - - const getText = makeSynchronous(async () => { - const res = await fetch(url); // ❌ 'url' is not defined in isolated function scope - return res.text(); - }); - - console.log(getText()); -}; - -// ✅ +// ✅ Alternative: Pass as parameter export const fetchSync = () => { const getText = makeSynchronous(async (url) => { // Variable passed as parameter const res = await fetch(url); @@ -68,6 +53,7 @@ export const fetchSync = () => { console.log(getText('https://example.com')); }; ``` +``` ```js const foo = 'hi'; @@ -85,36 +71,6 @@ function abc() { } ``` -```js -const foo = 'hi'; - -/** @isolated */ -const abc = () => { - return foo.slice(); // ❌ 'foo' is not defined in isolated function scope -}; - -// ✅ -/** @isolated */ -const abc = () => { - const foo = 'hi'; // Variable defined within function scope - return foo.slice(); -}; -``` - -```js -import makeSynchronous from 'make-synchronous'; - -export const fetchSync = () => { - const getText = makeSynchronous(async () => { - console.log('Starting...'); // ✅ Global variables are allowed by default - const res = await fetch('https://example.com'); // ✅ Global variables are allowed by default - return res.text(); - }); - - console.log(getText()); -}; -``` - ## Options Type: `object` @@ -258,10 +214,21 @@ createLambda({ ``` ```js +// ✅ All globals used are explicitly allowed makeSynchronous(async () => { console.log('Starting...'); // ✅ Allowed global const response = await fetch('https://api.example.com'); // ✅ Allowed global const url = new URL(response.url); // ✅ Allowed global return response.text(); }); + +makeSynchronous(async () => { + const response = await fetch('https://api.example.com', { + headers: { + 'Authorization': `Bearer ${process.env.API_TOKEN}` // ❌ 'process' is not in allowed globals + } + }); + const url = new URL(response.url); + return response.text(); +}); ``` From f4b8b666ab8c879feacba30ed50328a6541d83dd Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 13:52:01 -0400 Subject: [PATCH 13/16] match builtin eslint globals behavior --- docs/rules/isolated-functions.md | 69 ++++++++++++++++++++++++++------ rules/isolated-functions.js | 37 +++++++++-------- test/isolated-functions.js | 43 ++++++++++++++------ 3 files changed, 104 insertions(+), 45 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 28e3280249..5bf3dc09a6 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -14,7 +14,7 @@ Common scenarios where functions must be isolated: - Server actions or other remote execution contexts - Functions with specific JSDoc annotations -By default, this rule allows global variables (like `console`, `fetch`, etc.) in isolated functions, but prevents usage of variables from the surrounding scope. +By default, this rule uses ESLint's language options globals and allows global variables (like `console`, `fetch`, etc.) in isolated functions, but prevents usage of variables from the surrounding scope. ## Examples @@ -125,21 +125,25 @@ Array of comment strings that mark functions as isolated. Functions with JSDoc c ### globals -Type: `boolean | string[]`\ -Default: `true` +Type: `object`\ +Default: `undefined` (uses ESLint's language options globals) -Controls how global variables are handled: +Controls how global variables are handled. When not specified, uses ESLint's language options globals. When specified as an object, each key is a global variable name and the value controls its behavior: -- `false`: Global variables are not allowed in isolated functions -- `true` (default): All globals from ESLint's language options are allowed -- `string[]`: Only the specified global variable names are allowed +- `'readonly'`: Global variable is allowed but cannot be written to (depreacted form `false` also accepted) +- `'writable'`: Global variable is allowed and can be read/written (deprecated forms `true` and `'writeable'` also accepted) +- `'off'`: Global variable is not allowed ```js { 'unicorn/isolated-functions': [ 'error', { - globals: ['console', 'fetch'] // Only allow these globals + globals: { + console: 'writable', // Allowed and writable + fetch: 'readonly', // Allowed but readonly + process: 'off' // Not allowed + } } ] } @@ -196,6 +200,39 @@ createLambda({ }); ``` +### Default behavior (using ESLint's language options) + +```js +// Uses ESLint's language options globals by default +makeSynchronous(async () => { + console.log('Starting...'); // ✅ Allowed if console is in language options + const response = await fetch('https://api.example.com'); // ✅ Allowed if fetch is in language options + return response.text(); +}); +``` + +### Disallowing all globals + +```js +{ + 'unicorn/isolated-functions': [ + 'error', + { + globals: {} // Empty object disallows all globals + } + ] +} +``` + +```js +// ❌ All globals are disallowed +makeSynchronous(async () => { + console.log('Starting...'); // ❌ 'console' is not allowed + const response = await fetch('https://api.example.com'); // ❌ 'fetch' is not allowed + return response.text(); +}); +``` + ### Allowing specific globals ```js @@ -203,11 +240,11 @@ createLambda({ 'unicorn/isolated-functions': [ 'error', { - globals: [ - 'console', - 'fetch', - 'URL' - ] + globals: { + console: 'writable', // Allowed and writable + fetch: 'readonly', // Allowed but readonly + URL: 'readonly' // Allowed but readonly + } } ] } @@ -231,4 +268,10 @@ makeSynchronous(async () => { const url = new URL(response.url); return response.text(); }); + +// ❌ Attempting to write to readonly global +makeSynchronous(async () => { + fetch = null; // ❌ 'fetch' is readonly + console.log('Starting...'); +}); ``` diff --git a/rules/isolated-functions.js b/rules/isolated-functions.js index 53262f9009..8393a2215c 100644 --- a/rules/isolated-functions.js +++ b/rules/isolated-functions.js @@ -15,12 +15,11 @@ const parseEsquerySelector = selector => { return parsedEsquerySelectors.get(selector); }; -/** @type {{functions: string[], selectors: string[], comments: string[], globals: boolean | string[]}} */ +/** @type {{functions: string[], selectors: string[], comments: string[], globals?: import('eslint').Linter.Globals}} */ const defaultOptions = { functions: ['makeSynchronous'], selectors: [], comments: ['@isolated'], - globals: true, }; /** @param {import('eslint').Rule.RuleContext} context */ @@ -34,24 +33,32 @@ const create = context => { options.comments = options.comments.map(comment => comment.toLowerCase()); - const allowedGlobals = options.globals === true - ? new Set(Object.keys(context.languageOptions.globals)) - : new Set(options.globals || []); + const allowedGlobals = options.globals ?? context.languageOptions.globals; /** @param {import('estree').Node} node */ const checkForExternallyScopedVariables = node => { - const reason = reasonForBeingIsolatedFunction(node); + let reason = reasonForBeingIsolatedFunction(node); if (!reason) { return; } const nodeScope = sourceCode.getScope(node); + // `through`: "The array of references which could not be resolved in this scope" https://eslint.org/docs/latest/extend/scope-manager-interface#scope-interface for (const reference of nodeScope.through) { const {identifier} = reference; + if (identifier.name in allowedGlobals && allowedGlobals[identifier.name] !== 'off') { + if (reference.isReadOnly()) { + continue; + } + + const globalsValue = allowedGlobals[identifier.name]; + const isGlobalWritable = globalsValue === true || globalsValue === 'writable' || globalsValue === 'writeable'; + if (isGlobalWritable) { + continue; + } - if (allowedGlobals.has(identifier.name)) { - continue; + reason += ' (global variable is not writable)'; } // Could consider checking for typeof operator here, like in no-undef? @@ -119,17 +126,9 @@ const schema = [ additionalProperties: false, properties: { globals: { - oneOf: [ - { - type: 'boolean', - }, - { - type: 'array', - items: { - type: 'string', - }, - }, - ], + additionalProperties: { + anyOf: [{type: 'boolean'}, {type: 'string', enum: ['readonly', 'writable', 'writeable', 'off']}], + }, }, functions: { type: 'array', diff --git a/test/isolated-functions.js b/test/isolated-functions.js index ba8c5710b6..71ff1b90ba 100644 --- a/test/isolated-functions.js +++ b/test/isolated-functions.js @@ -95,9 +95,9 @@ test({ errors: [error({name: 'foo', reason: 'follows comment containing "@isolated"'})], }, { - name: 'global variables can be explicitly disallowed', + name: 'all global variables can be explicitly disallowed', languageOptions: {globals: {foo: true}}, - options: [{globals: false}], + options: [{globals: {}}], code: stripIndent(` makeSynchronous(function () { return foo.slice(); @@ -105,6 +105,33 @@ test({ `), errors: [fooInMakeSynchronousError], }, + { + name: 'individual global variables can be explicitly disallowed', + options: [{globals: {URLSearchParams: 'readonly', URL: 'off'}}], + code: stripIndent(` + makeSynchronous(function () { + return new URL('https://example.com?') + new URLSearchParams({a: 'b'}).toString(); + }); + `), + errors: [error({name: 'URL', reason: 'callee of function named "makeSynchronous"'})], + }, + { + name: 'check globals writability', + code: stripIndent(` + makeSynchronous(function () { + location = new URL('https://example.com'); + process = {env: {}}; + process.env.FOO = 'bar'; + }); + `), + errors: [ + // Only one error, `location = new URL('https://example.com')` and `process.env.FOO = 'bar'` are fine, the problem is `process = {...}`. + error({ + name: 'process', + reason: 'callee of function named "makeSynchronous" (global variable is not writable)', + }), + ], + }, { name: 'make a function isolated by a selector', // In this case, we're imagining some naming convention for lambda functions that will be created via `fn.toString()` @@ -177,20 +204,10 @@ test({ }); `), }, - { - name: 'can explicitly allow global variables from language options', - languageOptions: {globals: {foo: true}}, - options: [{globals: true}], - code: stripIndent(` - makeSynchronous(function () { - return foo.slice(); - }); - `), - }, { name: 'allow global variables separate from language options', languageOptions: {globals: {abc: true}}, - options: [{globals: ['foo']}], + options: [{globals: {foo: true}}], code: stripIndent(` makeSynchronous(function () { return foo.slice(); From 543f7517f1cefd58e126f3cc71e4de4b4dfdde66 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky Date: Mon, 7 Jul 2025 13:57:10 -0400 Subject: [PATCH 14/16] Document how to use predefined global variables --- docs/rules/isolated-functions.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 5bf3dc09a6..f0c0ffaa3f 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -275,3 +275,28 @@ makeSynchronous(async () => { console.log('Starting...'); }); ``` + +### Predefined global variables + +To enable a predefined set of globals, use the [`globals` package](https://npmjs.com/package/globals) similarly to how you would use it in `languageOptions` (see [ESLint docs on globals](https://eslint.org/docs/latest/use/configure/language-options#predefined-global-variables)): + +```js +import globals from 'globals' + +export default [ + { + rules: { + 'unicorn/isolated-functions': [ + 'error', + { + globals: { + ...globals.builtin, + ...globals.applescript, + ...globals.greasemonkey, + }, + }, + ], + }, + }, +] +``` From c712120ef5f4e6790688d7001bd470147f442b6d Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Wed, 9 Jul 2025 19:38:01 -0400 Subject: [PATCH 15/16] fix markdown --- docs/rules/isolated-functions.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index f0c0ffaa3f..629b3538ba 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -53,7 +53,6 @@ export const fetchSync = () => { console.log(getText('https://example.com')); }; ``` -``` ```js const foo = 'hi'; From 8f21993826f3531abbd7f04048b410e71ef3df17 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 16 Jul 2025 15:52:24 +0200 Subject: [PATCH 16/16] Update isolated-functions.md --- docs/rules/isolated-functions.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/rules/isolated-functions.md b/docs/rules/isolated-functions.md index 629b3538ba..567a52601f 100644 --- a/docs/rules/isolated-functions.md +++ b/docs/rules/isolated-functions.md @@ -25,8 +25,8 @@ export const fetchSync = () => { const url = 'https://example.com'; const getText = makeSynchronous(async () => { - const res = await fetch(url); // ❌ 'url' is not defined in isolated function scope - return res.text(); + const response = await fetch(url); // ❌ 'url' is not defined in isolated function scope + return response.text(); }); console.log(getText()); @@ -36,8 +36,8 @@ export const fetchSync = () => { export const fetchSync = () => { const getText = makeSynchronous(async () => { const url = 'https://example.com'; // Variable defined within function scope - const res = await fetch(url); - return res.text(); + const response = await fetch(url); + return response.text(); }); console.log(getText()); @@ -46,8 +46,8 @@ export const fetchSync = () => { // ✅ Alternative: Pass as parameter export const fetchSync = () => { const getText = makeSynchronous(async (url) => { // Variable passed as parameter - const res = await fetch(url); - return res.text(); + const response = await fetch(url); + return response.text(); }); console.log(getText('https://example.com')); @@ -129,7 +129,7 @@ Default: `undefined` (uses ESLint's language options globals) Controls how global variables are handled. When not specified, uses ESLint's language options globals. When specified as an object, each key is a global variable name and the value controls its behavior: -- `'readonly'`: Global variable is allowed but cannot be written to (depreacted form `false` also accepted) +- `'readonly'`: Global variable is allowed but cannot be written to (deprecated form `false` also accepted) - `'writable'`: Global variable is allowed and can be read/written (deprecated forms `true` and `'writeable'` also accepted) - `'off'`: Global variable is not allowed @@ -264,7 +264,9 @@ makeSynchronous(async () => { 'Authorization': `Bearer ${process.env.API_TOKEN}` // ❌ 'process' is not in allowed globals } }); + const url = new URL(response.url); + return response.text(); });