diff --git a/package.json b/package.json index 7a6efdf2..02432080 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,8 @@ "release": "bash release.sh" }, "dependencies": { + "@babel/parser": "^7.26.2", + "@babel/traverse": "^7.25.9", "@vue/compiler-sfc": "^3.4.27", "cli-table3": "^0.6.5", "html-tags": "^4.0.0", @@ -63,6 +65,7 @@ }, "devDependencies": { "@antfu/eslint-config": "^3.0.0", + "@types/babel__traverse": "7.x", "@types/node": "^22.0.0", "@types/yargs": "^17.0.32", "@typescript-eslint/eslint-plugin": "^8.0.0", diff --git a/src/helpers/getFunctionParams.ts b/src/helpers/getFunctionParams.ts new file mode 100644 index 00000000..0b71eabf --- /dev/null +++ b/src/helpers/getFunctionParams.ts @@ -0,0 +1,40 @@ +import type { NodePath } from '@babel/traverse' +import * as t from '@babel/types' + +/** + * Extracts function parameters information from different types of function nodes + */ +interface FunctionParamsInfo { + name: string + params: t.Identifier[] + startLine: number +} + +function getFunctionParams(path: NodePath, parentName?: string): FunctionParamsInfo | null { + let name = 'anonymous' + let params: t.Identifier[] = [] + + // Get function name based on type + if (t.isFunctionDeclaration(path.node) && path.node.id) { + name = path.node.id.name + } + else if (t.isVariableDeclarator(path.parent) && t.isIdentifier(path.parent.id)) { + name = path.parent.id.name + } + else if (parentName) { + name = parentName + } + + // Get parameters + params = path.node.params.filter((param): param is t.Identifier => { + return t.isIdentifier(param) + }) as t.Identifier[] + + return { + name, + params, + startLine: path.node.loc?.start.line || 0, + } +} + +export { type FunctionParamsInfo, getFunctionParams } diff --git a/src/helpers/hasSideEffects.ts b/src/helpers/hasSideEffects.ts new file mode 100644 index 00000000..fc30391a --- /dev/null +++ b/src/helpers/hasSideEffects.ts @@ -0,0 +1,27 @@ +import traverse from '@babel/traverse' +import * as t from '@babel/types' + +/** + * Checks if a node contains side effects like assignments or array mutations + */ +export function hasSideEffects(node: t.Node): boolean { + let foundSideEffect = false + + // Convert the visitor object to a function that handles each node + traverse.cheap(node, (node) => { + if (t.isAssignmentExpression(node)) { + foundSideEffect = true + } + if (t.isCallExpression(node)) { + const callee = node.callee + if (t.isMemberExpression(callee)) { + const propertyName = t.isIdentifier(callee.property) ? callee.property.name : '' + const mutatingMethods = ['push', 'pop', 'shift', 'unshift', 'splice', 'reverse', 'sort'] + if (mutatingMethods.includes(propertyName)) { + foundSideEffect = true + } + } + } + }) + return foundSideEffect +} diff --git a/src/helpers/parseScript.ts b/src/helpers/parseScript.ts new file mode 100644 index 00000000..49487221 --- /dev/null +++ b/src/helpers/parseScript.ts @@ -0,0 +1,20 @@ +import * as parser from '@babel/parser' + +/** + * Parses JavaScript/TypeScript code into an AST (Abstract Syntax Tree) + * First attempts to parse with TypeScript and JSX support + * Falls back to basic JavaScript parsing if TypeScript parsing fails + */ +export function parseScript(content: string) { + try { + return parser.parse(content, { + sourceType: 'module', + plugins: ['typescript', 'jsx'], + }) + } + catch { + return parser.parse(content, { + sourceType: 'module', + }) + } +} diff --git a/src/rules/rrd/computedSideEffects.test.ts b/src/rules/rrd/computedSideEffects.test.ts index 2b1af8cf..381f2f35 100644 --- a/src/rules/rrd/computedSideEffects.test.ts +++ b/src/rules/rrd/computedSideEffects.test.ts @@ -43,6 +43,36 @@ describe('checkComputedSideEffects', () => { expect(result).toStrictEqual([]) }) + it('should not report files when a non reactive value is changed', () => { + const script = { + content: ` + + + + `, + } as SFCScriptBlock + const fileName = 'no-side-effects-non-reactive.vue' + checkComputedSideEffects(script, fileName) + const result = reportComputedSideEffects() + expect(result.length).toBe(0) + expect(result).toStrictEqual([]) + }) + it('should report files with array mutation side effects in computed properties', () => { const script = { content: ` @@ -66,7 +96,7 @@ describe('checkComputedSideEffects', () => { file: fileName, rule: `rrd ~ computed side effects`, description: `👉 Avoid side effects in computed properties. Computed properties should only derive and return a value. See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`, - message: `line #6 side effect detected in computed property (someArray.value.push) 🚨`, + message: `line #6 side effect detected in computed property (const computedValue...) 🚨`, }]) }) @@ -93,7 +123,7 @@ describe('checkComputedSideEffects', () => { file: fileName, rule: `rrd ~ computed side effects`, description: `👉 Avoid side effects in computed properties. Computed properties should only derive and return a value. See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`, - message: `line #6 side effect detected in computed property (someVariable.value =) 🚨`, + message: `line #6 side effect detected in computed property (const computedValue...) 🚨`, }]) }) @@ -126,12 +156,12 @@ describe('checkComputedSideEffects', () => { file: fileName, rule: `rrd ~ computed side effects`, description: `👉 Avoid side effects in computed properties. Computed properties should only derive and return a value. See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`, - message: `line #7 side effect detected in computed property (someArray.value.push) 🚨`, + message: `line #7 side effect detected in computed property (const computedValue1...) 🚨`, }, { file: fileName, rule: `rrd ~ computed side effects`, description: `👉 Avoid side effects in computed properties. Computed properties should only derive and return a value. See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`, - message: `line #12 side effect detected in computed property (otherVariable.value ) 🚨`, + message: `line #12 side effect detected in computed property (const computedValue2...) 🚨`, }]) }) @@ -152,7 +182,7 @@ describe('checkComputedSideEffects', () => { file: fileName, rule: `rrd ~ computed side effects`, description: `👉 Avoid side effects in computed properties. Computed properties should only derive and return a value. See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`, - message: `line #1 side effect detected in computed property (someVariable.value =) 🚨`, + message: `line #1 side effect detected in computed property (dValue = computed(()...) 🚨`, }]) }) }) diff --git a/src/rules/rrd/computedSideEffects.ts b/src/rules/rrd/computedSideEffects.ts index 73758ee4..f8845c37 100644 --- a/src/rules/rrd/computedSideEffects.ts +++ b/src/rules/rrd/computedSideEffects.ts @@ -1,7 +1,10 @@ +import type { NodePath } from '@babel/traverse' import type { SFCScriptBlock } from '@vue/compiler-sfc' - import type { FileCheckResult, Offense } from '../../types' -import getLineNumber from '../getLineNumber' +import traverse from '@babel/traverse' +import * as t from '@babel/types' +import { hasSideEffects } from '../../helpers/hasSideEffects' +import { parseScript } from '../../helpers/parseScript' const results: FileCheckResult[] = [] @@ -12,23 +15,36 @@ const checkComputedSideEffects = (script: SFCScriptBlock | null, filePath: strin return } - const computedRegex = /computed\s*\(\s*\(\s*\)\s*=>\s*\{([\s\S]*?)\}\s*\)/g - // eslint-disable-next-line regexp/no-unused-capturing-group - const sideEffectRegex = /\b(set|push|pop|shift|unshift|splice|reverse|sort)\b|(? { - const computedBody = match[1] - if (sideEffectRegex.test(computedBody)) { - const lineNumber = getLineNumber(script.content.trim(), match[0]) - const trimmedBody = computedBody.trim() - const truncatedBody = trimmedBody.length > 20 ? trimmedBody.slice(0, 20) : trimmedBody - results.push({ - filePath, - message: `line #${lineNumber} side effect detected in computed property (${truncatedBody})`, - }) - } + const content = script.content.trim().replace(/]*>|<\/script>/gi, '') + const ast = parseScript(content) + + traverse(ast, { + CallExpression(path: NodePath) { + // Check if this is a computed() call + if (t.isIdentifier(path.node.callee) && path.node.callee.name === 'computed') { + const [arg] = path.node.arguments + + // Check if the argument is an arrow function + if (t.isArrowFunctionExpression(arg)) { + const functionBody = t.isBlockStatement(arg.body) ? arg.body : t.blockStatement([t.returnStatement(arg.body)]) + + if (hasSideEffects(functionBody)) { + const loc = path.node.loc + if (loc) { + const lineNumber = loc.start.line + // Get the computed function's code for the message + const computedCode = script.content.slice(path.node.start!, path.node.end!) + const truncatedCode = computedCode.length > 20 ? `${computedCode.slice(0, 20)}...` : computedCode + + results.push({ + filePath, + message: `line #${lineNumber} side effect detected in computed property (${truncatedCode.trim()})`, + }) + } + } + } + } + }, }) } diff --git a/src/rules/rrd/functionSize.test.ts b/src/rules/rrd/functionSize.test.ts index 5d65f0fd..23293b91 100644 --- a/src/rules/rrd/functionSize.test.ts +++ b/src/rules/rrd/functionSize.test.ts @@ -53,7 +53,7 @@ describe('checkFunctionSize', () => { expect(result).toStrictEqual([]) }) - it.todo('should not report files where no function exceeds the limit', () => { + it('should not report files where no function exceeds the limit', () => { const script = { content: ` `, @@ -156,7 +155,7 @@ describe('checkFunctionSize', () => { file: fileName, rule: `rrd ~ function size`, description: `👉 Functions must be shorter than ${maxSize} lines. See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`, - message: `function (${funcName}#2) is too long: 23 lines 🚨`, + message: `function (${funcName}#2) is too long: 25 lines 🚨`, }]) }) @@ -233,12 +232,12 @@ describe('checkFunctionSize', () => { file: fileName, rule: `rrd ~ function size`, description: `👉 Functions must be shorter than ${maxSize} lines. See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`, - message: `function (dummyRegularFunction#2) is too long: 29 lines 🚨`, + message: `function (dummyRegularFunction#2) is too long: 31 lines 🚨`, }, { file: fileName, rule: `rrd ~ function size`, description: `👉 Functions must be shorter than ${maxSize} lines. See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`, - message: `function (dummyArrowFunction#34) is too long: 23 lines 🚨`, + message: `function (dummyArrowFunction#34) is too long: 25 lines 🚨`, }]) }) @@ -299,7 +298,7 @@ describe('checkFunctionSize', () => { file: fileName, rule: `rrd ~ function size`, description: `👉 Functions must be shorter than ${maxSize} lines. See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`, - message: `function (getOpenBookings#2) is too long: 41 lines 🚨`, + message: `function (getOpenBookings#2) is too long: 39 lines 🚨`, }]) }) @@ -347,7 +346,7 @@ describe('checkFunctionSize', () => { file: fileName, rule: `rrd ~ function size`, description: `👉 Functions must be shorter than ${maxSize} lines. See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`, - message: `function (createFiles#10) is too long: 20 lines 🚨`, + message: `function (createFiles#10) is too long: 22 lines 🚨`, }]) }) }) diff --git a/src/rules/rrd/functionSize.ts b/src/rules/rrd/functionSize.ts index 19f1bf7d..06e86a22 100644 --- a/src/rules/rrd/functionSize.ts +++ b/src/rules/rrd/functionSize.ts @@ -1,175 +1,117 @@ +import type { NodePath } from '@babel/traverse' import type { SFCScriptBlock } from '@vue/compiler-sfc' import type { FileCheckResult, Offense } from '../../types' -import getLineNumber from '../getLineNumber' +import traverse from '@babel/traverse' +import * as t from '@babel/types' +import { parseScript } from '../../helpers/parseScript' interface AddFunctionToFilesParams { - funcName: string - funcBody: string - lineNumber: number - filePath: string - max: number + funcName: string // Name of the function being analyzed + startLine: number // Starting line of the function in the file + endLine: number // Ending line of the function in the file + filePath: string // Path to the file being analyzed + max: number // Maximum allowed lines for a function } const results: FileCheckResult[] = [] -const CONST_KEYWORD_LENGTH = 'const'.length -const FUNCTION_KEYWORD_LENGTH = 'function'.length - -function addFunctionToFiles({ funcName, funcBody, lineNumber, filePath, max }: AddFunctionToFilesParams) { - const lineCount = funcBody.split('\n').length - - const cleanedFuncName = cleanFunctionName(funcName) +/** + * Analyzes a function's size and adds violations to the results array + * - If size > 2*max: Reports as error (red highlight) + * - If max <= size <= 2*max: Reports as warning (yellow highlight) + */ +function addFunctionToFiles({ funcName, startLine, endLine, filePath, max }: AddFunctionToFilesParams) { + const lineCount = endLine - startLine + 1 if (lineCount > 2 * max) { - results.push({ filePath, message: `function (${cleanedFuncName}#${lineNumber}) is too long: ${lineCount} lines` }) + results.push({ + filePath, + message: `function (${funcName}#${startLine}) is too long: ${lineCount} lines`, + }) return } if (lineCount >= max) { - results.push({ filePath, message: `function (${cleanedFuncName}#${lineNumber}) is too long: ${lineCount} lines` }) - } -} - -function parseRegularFunction(content: string, startIndex: number) { - // eslint-disable-next-line regexp/prefer-w - const functionRegex = /function\s+([a-zA-Z_$][0-9a-zA-Z_$]*)\s*\([^)]*\)\s*\{/g - functionRegex.lastIndex = startIndex // Start search from the given index - - const match = functionRegex.exec(content) - - if (match) { - const name = match[1] - const bodyStartIndex = functionRegex.lastIndex - let braceCount = 1 // To track the opening brace we just found - let currentIndex = bodyStartIndex - - while (braceCount > 0 && currentIndex < content.length) { - if (content[currentIndex] === '{') { - braceCount++ - } - else if (content[currentIndex] === '}') { - braceCount-- - } - currentIndex++ - } - - const body = content.slice(bodyStartIndex, currentIndex - 1).trim() // Extract the function body - - return { - name, - body, - end: currentIndex, // Returns the position after the matched function - } - } - else { - return null - } -} - -function parseArrowFunction(content: string, index: number) { - // Define regex to match arrow functions with or without curly braces - // eslint-disable-next-line regexp/prefer-w, regexp/no-unused-capturing-group - const regex = /const\s+([a-zA-Z_$][0-9a-zA-Z_$]*)\s*=\s*(async\s+)?\(([^)]*)\)\s*=>\s*/ - - // Extract the substring starting from the current index - const substring = content.slice(index) - const match = regex.exec(substring) - - if (match) { - const [, name] = match - const bodyStartIndex = index + match.index + match[0].length // Calculate the start index of the function body - let bodyEndIndex = bodyStartIndex - let body = '' - - if (content[bodyStartIndex] === '{') { - // If the function body starts with '{', we are dealing with a block arrow function - let braceCount = 1 // Start with 1 to account for the opening brace - bodyEndIndex = bodyStartIndex + 1 // Move past the opening brace - - while (bodyEndIndex < content.length && braceCount > 0) { - // Traverse the content to find the closing brace - if (content[bodyEndIndex] === '{') { - braceCount++ - } - else if (content[bodyEndIndex] === '}') { - braceCount-- - } - bodyEndIndex++ // Move to the next character - } - - // Extract the function body, excluding the final closing brace - body = content.slice(bodyStartIndex + 1, bodyEndIndex - 1).trim() - } - else { - // If the function body does not start with '{', it's a concise body arrow function - while (bodyEndIndex < content.length && content[bodyEndIndex] !== ';') { - // Traverse until we find the end of the concise body (usually a semicolon) - bodyEndIndex++ - } - - // Extract the function body up to the semicolon or end of content - body = content.slice(bodyStartIndex, bodyEndIndex).trim() - } - - return { - name, - body, - end: bodyEndIndex, // Position after the end of the function body - } - } - else { - return null + results.push({ + filePath, + message: `function (${funcName}#${startLine}) is too long: ${lineCount} lines`, + }) } } -// Cleans the function name by removing any leading 'const' keyword. -function cleanFunctionName(funcName: string): string { - return funcName.replace(/^const\s*/, '') -} - const resetResults = () => (results.length = 0) +/** + * Main function to analyze function sizes in a Vue component's script block + * Uses Babel to parse the code and traverse the AST to find all function definitions + * + * @param script - The script block from a Vue component + * @param filePath - Path to the file being analyzed + * @param max - Maximum allowed function size in lines + */ const checkFunctionSize = (script: SFCScriptBlock | null, filePath: string, max: number) => { if (!script) { return } - const content = script.content - const length = content.length - let index = 0 - - while (index < length) { - let funcName = '' - let funcBody = '' - let isFunction = false - - // Check for function declarations - if (content.slice(index, index + FUNCTION_KEYWORD_LENGTH) === 'function') { - const regularFunctionInfo = parseRegularFunction(content, index) - if (regularFunctionInfo) { - isFunction = true - funcName = regularFunctionInfo.name - funcBody = regularFunctionInfo.body - index = regularFunctionInfo.end + // Clean up script content by removing script tags + const content = script.content.trim().replace(/]*>|<\/script>/gi, '') + // Parse the code into an AST for analysis + const ast = parseScript(content) + + // Walk through the AST and check each type of function definition + traverse(ast, { + // Handles regular function declarations: function myFunc() { ... } + FunctionDeclaration(path: NodePath) { + const name = path.node.id?.name || 'anonymous' + const startLine = path.node.loc?.start.line || 0 + const endLine = path.node.loc?.end.line || 0 + + addFunctionToFiles({ + funcName: name, + startLine, + endLine, + filePath, + max, + }) + }, + + // Handles arrow functions: const myFunc = () => { ... } + ArrowFunctionExpression(path: NodePath) { + // Only check named arrow functions (those assigned to variables) + if (t.isVariableDeclarator(path.parent)) { + const name = t.isIdentifier(path.parent.id) ? path.parent.id.name : 'anonymous' + const startLine = path.node.loc?.start.line || 0 + const endLine = path.node.loc?.end.line || 0 + + addFunctionToFiles({ + funcName: name, + startLine, + endLine, + filePath, + max, + }) } - } - if (content.slice(index, index + CONST_KEYWORD_LENGTH) === 'const') { // Check for arrow functions - const arrowFunctionInfo = parseArrowFunction(content, index) - if (arrowFunctionInfo) { - isFunction = true - funcName = arrowFunctionInfo.name - funcBody = arrowFunctionInfo.body - index = arrowFunctionInfo.end // move the index past the end of the function + }, + + // Handles function expressions: const myFunc = function() { ... } + FunctionExpression(path: NodePath) { + let name = 'anonymous' + if (t.isVariableDeclarator(path.parent) && t.isIdentifier(path.parent.id)) { + name = path.parent.id.name } - } - - if (isFunction) { - const lineNumber = getLineNumber(content.trim(), funcName) - addFunctionToFiles({ funcName, funcBody, lineNumber, filePath, max }) - } - else { - index++ - } - } + + const startLine = path.node.loc?.start.line || 0 + const endLine = path.node.loc?.end.line || 0 + + addFunctionToFiles({ + funcName: name, + startLine, + endLine, + filePath, + max, + }) + }, + }) } const reportFunctionSize = (max: number) => { diff --git a/src/rules/rrd/parameterCount.test.ts b/src/rules/rrd/parameterCount.test.ts index b9b737ed..33afc851 100644 --- a/src/rules/rrd/parameterCount.test.ts +++ b/src/rules/rrd/parameterCount.test.ts @@ -22,7 +22,7 @@ describe('checkParameterCount', () => { expect(result).toStrictEqual([]) }) - it.todo('should not report files where functions do not exceed the recommended limit with destructuring', () => { + it('should not report files where functions do not exceed the recommended limit with destructuring', () => { const script = { content: `