diff --git a/packages/eslint-plugin/src/configs.ts b/packages/eslint-plugin/src/configs.ts index e8a2e8d387..31eb3e62b9 100644 --- a/packages/eslint-plugin/src/configs.ts +++ b/packages/eslint-plugin/src/configs.ts @@ -1,10 +1,12 @@ import type { TSESLint } from '@typescript-eslint/utils'; import { integerDivision } from './rules/integerDivision.ts'; import { unwrappedPojos } from './rules/unwrappedPojos.ts'; +import { invalidAssignment } from './rules/invalidAssignment.ts'; export const rules = { 'integer-division': integerDivision, 'unwrapped-pojo': unwrappedPojos, + 'invalid-assignment': invalidAssignment, } as const; type Rules = Record< @@ -15,9 +17,11 @@ type Rules = Record< export const recommendedRules: Rules = { 'typegpu/integer-division': 'warn', 'typegpu/unwrapped-pojo': 'warn', + 'typegpu/invalid-assignment': 'warn', }; export const allRules: Rules = { 'typegpu/integer-division': 'error', 'typegpu/unwrapped-pojo': 'error', + 'typegpu/invalid-assignment': 'error', }; diff --git a/packages/eslint-plugin/src/enhancers/directiveTracking.ts b/packages/eslint-plugin/src/enhancers/directiveTracking.ts index d8efde55d0..b490760afb 100644 --- a/packages/eslint-plugin/src/enhancers/directiveTracking.ts +++ b/packages/eslint-plugin/src/enhancers/directiveTracking.ts @@ -2,8 +2,13 @@ import type { TSESTree } from '@typescript-eslint/utils'; import type { RuleListener } from '@typescript-eslint/utils/ts-eslint'; import type { RuleEnhancer } from '../enhanceRule.ts'; +export type FunctionNode = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; + export type DirectiveData = { - insideUseGpu: () => boolean; + getEnclosingTypegpuFunction: () => FunctionNode | undefined; }; /** @@ -16,17 +21,17 @@ export type DirectiveData = { * - top level directives. */ export const directiveTracking: RuleEnhancer = () => { - const stack: string[][] = []; + const stack: { node: FunctionNode; directives: string[] }[] = []; const visitors: RuleListener = { FunctionDeclaration(node) { - stack.push(getDirectives(node)); + stack.push({ node, directives: getDirectives(node) }); }, FunctionExpression(node) { - stack.push(getDirectives(node)); + stack.push({ node, directives: getDirectives(node) }); }, ArrowFunctionExpression(node) { - stack.push(getDirectives(node)); + stack.push({ node, directives: getDirectives(node) }); }, 'FunctionDeclaration:exit'() { @@ -42,7 +47,15 @@ export const directiveTracking: RuleEnhancer = () => { return { visitors, - state: { insideUseGpu: () => (stack.at(-1) ?? []).includes('use gpu') }, + state: { + getEnclosingTypegpuFunction: () => { + const current = stack.at(-1); + if (current && current.directives.includes('use gpu')) { + return current.node; + } + return undefined; + }, + }, }; }; diff --git a/packages/eslint-plugin/src/rules/invalidAssignment.ts b/packages/eslint-plugin/src/rules/invalidAssignment.ts new file mode 100644 index 0000000000..947da9ebdd --- /dev/null +++ b/packages/eslint-plugin/src/rules/invalidAssignment.ts @@ -0,0 +1,99 @@ +import { ASTUtils, type TSESTree } from '@typescript-eslint/utils'; +import { createRule } from '../ruleCreator.ts'; +import { enhanceRule } from '../enhanceRule.ts'; +import { directiveTracking } from '../enhancers/directiveTracking.ts'; +import type { RuleContext } from '@typescript-eslint/utils/ts-eslint'; + +export const invalidAssignment = createRule({ + name: 'invalid-assignment', + meta: { + type: 'problem', + docs: { + description: `Avoid assignments that will not generate valid WGSL.`, + }, + messages: { + parameterAssignment: + "Cannot assign to '{{snippet}}' since WGSL parameters are immutable. If you're using d.ref, please either use '.$' or disable this rule.", + jsAssignment: + "Cannot assign to '{{snippet}}' since it is a JS variable defined outside of the current TypeGPU function's scope. Use buffers, workgroup variables or local variables instead.", + }, + schema: [], + }, + defaultOptions: [], + + create: enhanceRule({ directives: directiveTracking }, (context, state) => { + const { directives } = state; + + return { + UpdateExpression(node) { + const enclosingFn = directives.getEnclosingTypegpuFunction(); + validateAssignment(context, node, enclosingFn, node.argument); + }, + + AssignmentExpression(node) { + const enclosingFn = directives.getEnclosingTypegpuFunction(); + validateAssignment(context, node, enclosingFn, node.left); + }, + }; + }), +}); + +function validateAssignment( + context: Readonly>, + node: TSESTree.Node, + enclosingFn: TSESTree.Node | undefined, + leftNode: TSESTree.Node, +) { + if (!enclosingFn) { + return; + } + + // follow the member expression chain + let assignee = leftNode; + while (assignee.type === 'MemberExpression') { + if ( + assignee.property.type === 'Identifier' && + assignee.property.name === '$' + ) { + // a dollar was used so we assume this assignment is fine + return; + } + assignee = assignee.object; + } + if (assignee.type !== 'Identifier') { + return; + } + + // look for a scope that defines the variable + const variable = ASTUtils.findVariable( + context.sourceCode.getScope(assignee), + assignee, + ); + // defs is an array because there may be multiple definitions with `var` + const def = variable?.defs[0]; + + // check if variable is global or was defined outside of current function by checking ranges + // NOTE: if the variable is an outer function parameter, then the enclosingFn range will be encompassed by node range + if ( + !def || + def && ( + def.node.range[0] < enclosingFn.range[0] || + enclosingFn.range[1] < def.node.range[1] + ) + ) { + context.report({ + messageId: 'jsAssignment', + node, + data: { snippet: context.sourceCode.getText(leftNode) }, + }); + return; + } + + if (def.type === 'Parameter') { + context.report({ + messageId: 'parameterAssignment', + node, + data: { snippet: context.sourceCode.getText(leftNode) }, + }); + } +} diff --git a/packages/eslint-plugin/src/rules/unwrappedPojos.ts b/packages/eslint-plugin/src/rules/unwrappedPojos.ts index babe1a3f8b..f2d6900076 100644 --- a/packages/eslint-plugin/src/rules/unwrappedPojos.ts +++ b/packages/eslint-plugin/src/rules/unwrappedPojos.ts @@ -22,7 +22,7 @@ export const unwrappedPojos = createRule({ return { ObjectExpression(node) { - if (!directives.insideUseGpu()) { + if (!directives.getEnclosingTypegpuFunction()) { return; } if (node.parent?.type === 'Property') { diff --git a/packages/eslint-plugin/tests/invalidAssignment.test.ts b/packages/eslint-plugin/tests/invalidAssignment.test.ts new file mode 100644 index 0000000000..abe4db1805 --- /dev/null +++ b/packages/eslint-plugin/tests/invalidAssignment.test.ts @@ -0,0 +1,190 @@ +import { describe } from 'vitest'; +import { ruleTester } from './ruleTester.ts'; +import { invalidAssignment } from '../src/rules/invalidAssignment.ts'; + +describe('invalidAssignment', () => { + ruleTester.run('parameterAssignment', invalidAssignment, { + valid: [ + // not inside 'use gpu' function + 'const fn = (a) => { a = {}; }', + 'const fn = (a) => { a.prop = 1; }', + "const fn = (a) => { a['prop'] = 1; }", + 'const fn = (a) => { a[0] = 1; }', + + // not using parameter + "const fn = (a) => { 'use gpu'; let b = 0; b = 1; }", + "const fn = (a) => { 'use gpu'; { let a = 1; a = 2; } }", + + // correctly accessed + "const fn = (a) => { 'use gpu'; a.$ = 1 }", + "const fn = (a) => { 'use gpu'; a.$++; }", + "const fn = (a) => { 'use gpu'; a.$ += 1; }", + ], + invalid: [ + { + code: "const fn = (a) => { 'use gpu'; a = 1; }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "let a; const fn = (a) => { 'use gpu'; a = 1; }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "const fn = (a) => { 'use gpu'; a.prop = 1; }", + errors: [{ + messageId: 'parameterAssignment', + data: { snippet: 'a.prop' }, + }], + }, + { + code: "const fn = (a) => { 'use gpu'; a['prop'] = 1; }", + errors: [{ + messageId: 'parameterAssignment', + data: { snippet: "a['prop']" }, + }], + }, + { + code: "const fn = (a) => { 'use gpu'; a[0] = 1; }", + errors: [{ + messageId: 'parameterAssignment', + data: { snippet: 'a[0]' }, + }], + }, + { + code: "const fn = (a) => { 'use gpu'; a++; }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "const fn = (a) => { 'use gpu'; a += 1; }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "const fn = (a) => { 'use gpu'; a.prop1.prop2 = 1; }", + errors: [{ + messageId: 'parameterAssignment', + data: { snippet: 'a.prop1.prop2' }, + }], + }, + { + code: "const fn = (a) => { 'use gpu'; if (true) { a = 1; } }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "const fn = (a) => { 'use gpu'; a = 1; { let a; } }", + errors: [{ messageId: 'parameterAssignment', data: { snippet: 'a' } }], + }, + { + code: "const fn = (a, b) => { 'use gpu'; a = 1; b = 2; }", + errors: [ + { messageId: 'parameterAssignment', data: { snippet: 'a' } }, + { messageId: 'parameterAssignment', data: { snippet: 'b' } }, + ], + }, + { + code: "const fn = (a) => { 'use gpu'; a.$prop = 1; }", + errors: [{ + messageId: 'parameterAssignment', + data: { snippet: 'a.$prop' }, + }], + }, + ], + }); + + ruleTester.run('jsAssignment', invalidAssignment, { + valid: [ + // not inside 'use gpu' function + 'let a; const fn = () => { a = 1 }', + 'const outer = (a) => { const fn = () => { a = 1 } }', + 'const vars = []; const fn = () => { vars[0] = 1 }', + + // correctly accessed + "const buffer = {}; const fn = () => { 'use gpu'; buffer.$ = 1 }", + "const outer = (buffer) => { const fn = () => { 'use gpu'; buffer.$ = 1 } }", + "const buffers = []; const fn = () => { 'use gpu'; buffers[0].$ = 1 }", + ], + invalid: [ + { + code: "let a; const fn = () => { 'use gpu'; a = 1 }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "var a; const fn = () => { 'use gpu'; a = 1 }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "const outer = (a) => { const fn = () => { 'use gpu'; a = 1 } }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "const a = {}; const fn = () => { 'use gpu'; a.prop = 1; }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: 'a.prop' }, + }], + }, + { + code: "const a = {}; const fn = () => { 'use gpu'; a['prop'] = 1; }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: "a['prop']" }, + }], + }, + { + code: "const a = []; const fn = () => { 'use gpu'; a[0] = 1; }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: 'a[0]' }, + }], + }, + { + code: "const vars = []; const fn = () => { 'use gpu'; vars[0] = 1 }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'vars[0]' } }], + }, + { + code: "const fn = () => { 'use gpu'; a += 1; }; let a;", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "let a; const fn = () => { 'use gpu'; a++; }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "let a; const fn = () => { 'use gpu'; a += 1; }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: + "const a = {}; const fn = () => { 'use gpu'; a.prop1.prop2 = 1; }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: 'a.prop1.prop2' }, + }], + }, + { + code: "let a; const fn = () => { 'use gpu'; if (true) { a = 1; } }", + errors: [{ messageId: 'jsAssignment', data: { snippet: 'a' } }], + }, + { + code: "let a, b; const fn = () => { 'use gpu'; a = 1; b = 2; }", + errors: [ + { messageId: 'jsAssignment', data: { snippet: 'a' } }, + { messageId: 'jsAssignment', data: { snippet: 'b' } }, + ], + }, + { + code: "const a = {}; const fn = () => { 'use gpu'; a.$prop = 1; }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: 'a.$prop' }, + }], + }, + { + code: "const fn = () => { 'use gpu'; globalThis.prop = 1 }", + errors: [{ + messageId: 'jsAssignment', + data: { snippet: 'globalThis.prop' }, + }], + }, + ], + }); +}); diff --git a/packages/typegpu/tests/ref.test.ts b/packages/typegpu/tests/ref.test.ts index 1470936a17..36a9db3acd 100644 --- a/packages/typegpu/tests/ref.test.ts +++ b/packages/typegpu/tests/ref.test.ts @@ -114,7 +114,7 @@ describe('d.ref', () => { const clearPosition = (entity: d.ref) => { 'use gpu'; - entity.pos = d.vec3f(); + entity.$.pos = d.vec3f(); }; const main = () => { @@ -149,7 +149,7 @@ describe('d.ref', () => { it('allows updating a vector component from another function', () => { const clearX = (pos: d.ref) => { 'use gpu'; - pos.x = 0; + pos.$.x = 0; }; const main = () => { diff --git a/packages/typegpu/tests/tgsl/argumentOrigin.test.ts b/packages/typegpu/tests/tgsl/argumentOrigin.test.ts index ed2ad8494a..e56a9d3972 100644 --- a/packages/typegpu/tests/tgsl/argumentOrigin.test.ts +++ b/packages/typegpu/tests/tgsl/argumentOrigin.test.ts @@ -6,6 +6,7 @@ describe('function argument origin tracking', () => { it('should fail on mutation of primitive arguments', () => { const foo = (a: number) => { 'use gpu'; + // oxlint-disable-next-line typegpu/invalid-assignment -- this is a test a += 1; }; @@ -28,6 +29,7 @@ describe('function argument origin tracking', () => { const foo = ({ a }: { a: number }) => { 'use gpu'; + // oxlint-disable-next-line typegpu/invalid-assignment -- this is a test a += 1; }; @@ -48,6 +50,7 @@ describe('function argument origin tracking', () => { it('should fail on mutation of non-primitive arguments', () => { const foo = (a: d.v3f) => { 'use gpu'; + // oxlint-disable-next-line typegpu/invalid-assignment -- this is a test a.x += 1; };