diff --git a/.changeset/loud-mammals-dress.md b/.changeset/loud-mammals-dress.md new file mode 100644 index 00000000000..b2f9fafab28 --- /dev/null +++ b/.changeset/loud-mammals-dress.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-qwik': patch +--- + +FIX: eslint-plugin: detect node API usage more accurately diff --git a/packages/eslint-plugin-qwik/src/scope-use-task.ts b/packages/eslint-plugin-qwik/src/scope-use-task.ts index 9e616f29d80..aeb55958fd4 100644 --- a/packages/eslint-plugin-qwik/src/scope-use-task.ts +++ b/packages/eslint-plugin-qwik/src/scope-use-task.ts @@ -2,6 +2,8 @@ import { Rule } from 'eslint'; import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as eslint from 'eslint'; // For Scope types const ISSERVER = 'isServer'; +const GLOBALAPIS = ['process', '__dirname', '__filename', 'module']; +const PRESETNODEAPIS = ['fs', 'os', 'path', 'child_process', 'http', 'https', 'Buffer']; // Helper function: checks if a node is a descendant of another node function isNodeDescendantOf(descendantNode, ancestorNode): boolean { if (!ancestorNode) { @@ -35,18 +37,7 @@ export const scopeUseTask: Rule.RuleModule = { forbiddenApis: { type: 'array', items: { type: 'string' }, - default: [ - 'process', - 'fs', - 'os', - 'path', - 'child_process', - 'http', - 'https', - 'Buffer', - '__dirname', - '__filename', - ], + default: PRESETNODEAPIS, }, }, additionalProperties: false, @@ -62,18 +53,7 @@ export const scopeUseTask: Rule.RuleModule = { create(context: Rule.RuleContext): Rule.RuleListener { const options = context.options[0] || {}; const forbiddenApis = new Set( - options.forbiddenApis || [ - 'process', - 'fs', - 'os', - 'path', - 'child_process', - 'http', - 'https', - 'Buffer', - '__dirname', - '__filename', - ] + options.forbiddenApis || PRESETNODEAPIS.concat(GLOBALAPIS) ); const serverGuardIdentifier: string = ISSERVER; const sourceCode = context.sourceCode; @@ -93,7 +73,6 @@ export const scopeUseTask: Rule.RuleModule = { */ function isApiUsageGuarded(apiOrCallNode, functionContextNode): boolean { let currentParentNode: TSESTree.Node | undefined = apiOrCallNode.parent; - while ( currentParentNode && currentParentNode !== functionContextNode.body && @@ -161,6 +140,11 @@ export const scopeUseTask: Rule.RuleModule = { // Try to find the variable starting from the current scope and going upwards let currentScopeForSearch: eslint.Scope.Scope | null = scope; + + if (!GLOBALAPIS.includes(identifierNode.name)) { + return true; + } + while (currentScopeForSearch) { const foundVar = currentScopeForSearch.variables.find( (v) => v.name === identifierNode.name @@ -178,13 +162,9 @@ export const scopeUseTask: Rule.RuleModule = { currentScopeForSearch = currentScopeForSearch.upper; } - if (!variable) { - // Cannot find variable, assume it's not a shadowed global for safety, - // though this state implies an undeclared variable (another ESLint rule should catch this). - return false; - } + // If we didn't find a variable, it might be a global API or an undeclared variable. - if (variable.defs.length === 0) { + if (!variable || variable.defs.length === 0) { // No definitions usually means it's an implicit global (e.g., 'process' in Node.js environment). // Such a variable is NOT considered "shadowed by a user declaration". return false; @@ -192,7 +172,7 @@ export const scopeUseTask: Rule.RuleModule = { // If there are definitions, check if any of them are standard declaration types. // This means the identifier refers to a user-declared variable, parameter, function, class, or an import. - return variable.defs.some((def) => { + return variable?.defs.some((def) => { return ( def.type === 'Variable' || def.type === 'Parameter' || @@ -200,7 +180,7 @@ export const scopeUseTask: Rule.RuleModule = { def.type === 'ClassName' || def.type === 'ImportBinding' ); - }); + }) as boolean; } /** @@ -418,7 +398,6 @@ export const scopeUseTask: Rule.RuleModule = { targetFunctionNode.body.type === AST_NODE_TYPES.BlockStatement ? targetFunctionNode.body : targetFunctionNode.body; - analyzeNodeContent(nodeToAnalyze, targetFunctionNode, callNode); } } diff --git a/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-function.tsx b/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-function.tsx index a5c39b24cb7..f2cabd3de47 100644 --- a/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-function.tsx +++ b/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-function.tsx @@ -4,7 +4,6 @@ // Expect error: { "messageId": "unsafeApiUsageInCalledFunction" } import { component$, useTask$ } from '@qwik.dev/core'; - export default component$(() => { useTask$(() => { function foo() { diff --git a/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-track.tsx b/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-track.tsx index e6827caa40e..feb391180e1 100644 --- a/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-track.tsx +++ b/packages/eslint-plugin-qwik/tests/scope-use-task/invalid-scope-use-track.tsx @@ -1,12 +1,19 @@ // Expect error: { "messageId": "unsafeApiUsage" } - -import { component$, useSignal, useTask$ } from '@qwik.dev/core'; +// Expect error: { "messageId": "unsafeApiUsage" } +// Expect error: { "messageId": "unsafeApiUsage" } +// Expect error: { "messageId": "unsafeApiUsage" } +import { component$, isBrowser, useSignal, useTask$ } from '@qwik.dev/core'; export default component$(() => { const s = useSignal(0); useTask$(({ track }) => { track(() => { + if (isBrowser) { + process.env; + const m = process; + } process.env; + const m = process; return s.value; }); }); diff --git a/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-function.tsx b/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-function.tsx index d10e15de7d8..e03ac6170cc 100644 --- a/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-function.tsx +++ b/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-function.tsx @@ -1,13 +1,25 @@ import { component$, isServer, useTask$ } from '@qwik.dev/core'; - +import path from 'path'; export default component$(() => { useTask$(() => { + function child_process() {} function foo() { if (isServer) { process.env; + const m = process; + const _path = path; + const pathJoin = path.join('foo', 'bar'); } } + child_process(); + const foo2 = () => { + if (isServer) { + process.env; + const m = process; + } + }; foo(); + foo2(); }); return <>; }); diff --git a/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-return-object.tsx b/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-return-object.tsx new file mode 100644 index 00000000000..e4c042471ee --- /dev/null +++ b/packages/eslint-plugin-qwik/tests/scope-use-task/valid-scope-use-return-object.tsx @@ -0,0 +1,19 @@ +import { component$, useTask$, isBrowser, useSignal } from '@qwik.dev/core'; + +export default component$(() => { + const state = useSignal(true); + useTask$(({ track }) => { + if (isBrowser) { + track(() => { + if (state.value) { + const values = [ + { + path: '1', + }, + ]; + } + }); + } + }); + return <>; +});