Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add babel parser and traverse and migrate some rules #448

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
40 changes: 40 additions & 0 deletions src/helpers/getFunctionParams.ts
Original file line number Diff line number Diff line change
@@ -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<t.Function>, 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 }
27 changes: 27 additions & 0 deletions src/helpers/hasSideEffects.ts
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 20 additions & 0 deletions src/helpers/parseScript.ts
Original file line number Diff line number Diff line change
@@ -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',
})
}
}
40 changes: 35 additions & 5 deletions src/rules/rrd/computedSideEffects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,36 @@ describe('checkComputedSideEffects', () => {
expect(result).toStrictEqual([])
})

it('should not report files when a non reactive value is changed', () => {
const script = {
content: `
<script setup>
import { ref, computed } from 'vue';

const str1 = ref('abc');

const str2 = computed(() => {
const inner = 'def';
return str1.value + inner;
});
</script>

<template>
<div>
<p>Test</p>
<p>{{ str1 }}</p>
<p>{{ str2 }}</p>
</div>
</template>
`,
} 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: `
Expand All @@ -66,7 +96,7 @@ describe('checkComputedSideEffects', () => {
file: fileName,
rule: `<text_info>rrd ~ computed side effects</text_info>`,
description: `👉 <text_warn>Avoid side effects in computed properties. Computed properties should only derive and return a value.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`,
message: `line #6 side effect detected in computed property <bg_err>(someArray.value.push)</bg_err> 🚨`,
message: `line #6 side effect detected in computed property <bg_err>(const computedValue...)</bg_err> 🚨`,
}])
})

Expand All @@ -93,7 +123,7 @@ describe('checkComputedSideEffects', () => {
file: fileName,
rule: `<text_info>rrd ~ computed side effects</text_info>`,
description: `👉 <text_warn>Avoid side effects in computed properties. Computed properties should only derive and return a value.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`,
message: `line #6 side effect detected in computed property <bg_err>(someVariable.value =)</bg_err> 🚨`,
message: `line #6 side effect detected in computed property <bg_err>(const computedValue...)</bg_err> 🚨`,
}])
})

Expand Down Expand Up @@ -126,12 +156,12 @@ describe('checkComputedSideEffects', () => {
file: fileName,
rule: `<text_info>rrd ~ computed side effects</text_info>`,
description: `👉 <text_warn>Avoid side effects in computed properties. Computed properties should only derive and return a value.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`,
message: `line #7 side effect detected in computed property <bg_err>(someArray.value.push)</bg_err> 🚨`,
message: `line #7 side effect detected in computed property <bg_err>(const computedValue1...)</bg_err> 🚨`,
}, {
file: fileName,
rule: `<text_info>rrd ~ computed side effects</text_info>`,
description: `👉 <text_warn>Avoid side effects in computed properties. Computed properties should only derive and return a value.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`,
message: `line #12 side effect detected in computed property <bg_err>(otherVariable.value )</bg_err> 🚨`,
message: `line #12 side effect detected in computed property <bg_err>(const computedValue2...)</bg_err> 🚨`,
}])
})

Expand All @@ -152,7 +182,7 @@ describe('checkComputedSideEffects', () => {
file: fileName,
rule: `<text_info>rrd ~ computed side effects</text_info>`,
description: `👉 <text_warn>Avoid side effects in computed properties. Computed properties should only derive and return a value.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/computed-side-effects.html`,
message: `line #1 side effect detected in computed property <bg_err>(someVariable.value =)</bg_err> 🚨`,
message: `line #1 side effect detected in computed property <bg_err>(dValue = computed(()...)</bg_err> 🚨`,
}])
})
})
54 changes: 35 additions & 19 deletions src/rules/rrd/computedSideEffects.ts
Original file line number Diff line number Diff line change
@@ -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[] = []

Expand All @@ -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 matches = [...script.content.matchAll(computedRegex)]

matches.forEach((match) => {
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 <bg_err>(${truncatedBody})</bg_err>`,
})
}
const content = script.content.trim().replace(/<script\b[^>]*>|<\/script>/gi, '')
const ast = parseScript(content)

traverse(ast, {
CallExpression(path: NodePath<t.CallExpression>) {
// 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 <bg_err>(${truncatedCode.trim()})</bg_err>`,
})
}
}
}
}
},
})
}

Expand Down
13 changes: 6 additions & 7 deletions src/rules/rrd/functionSize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
<script setup>
Expand All @@ -79,7 +79,6 @@ describe('checkFunctionSize', () => {
// ...
// ...
// ...
// ...
}
</script>
`,
Expand Down Expand Up @@ -156,7 +155,7 @@ describe('checkFunctionSize', () => {
file: fileName,
rule: `<text_info>rrd ~ function size</text_info>`,
description: `👉 <text_warn>Functions must be shorter than ${maxSize} lines.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`,
message: `function <bg_warn>(${funcName}#2)</bg_warn> is too long: <bg_warn>23 lines</bg_warn> 🚨`,
message: `function <bg_warn>(${funcName}#2)</bg_warn> is too long: <bg_warn>25 lines</bg_warn> 🚨`,
}])
})

Expand Down Expand Up @@ -233,12 +232,12 @@ describe('checkFunctionSize', () => {
file: fileName,
rule: `<text_info>rrd ~ function size</text_info>`,
description: `👉 <text_warn>Functions must be shorter than ${maxSize} lines.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`,
message: `function <bg_warn>(dummyRegularFunction#2)</bg_warn> is too long: <bg_warn>29 lines</bg_warn> 🚨`,
message: `function <bg_warn>(dummyRegularFunction#2)</bg_warn> is too long: <bg_warn>31 lines</bg_warn> 🚨`,
}, {
file: fileName,
rule: `<text_info>rrd ~ function size</text_info>`,
description: `👉 <text_warn>Functions must be shorter than ${maxSize} lines.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`,
message: `function <bg_warn>(dummyArrowFunction#34)</bg_warn> is too long: <bg_warn>23 lines</bg_warn> 🚨`,
message: `function <bg_warn>(dummyArrowFunction#34)</bg_warn> is too long: <bg_warn>25 lines</bg_warn> 🚨`,
}])
})

Expand Down Expand Up @@ -299,7 +298,7 @@ describe('checkFunctionSize', () => {
file: fileName,
rule: `<text_info>rrd ~ function size</text_info>`,
description: `👉 <text_warn>Functions must be shorter than ${maxSize} lines.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`,
message: `function <bg_err>(getOpenBookings#2)</bg_err> is too long: <bg_err>41 lines</bg_err> 🚨`,
message: `function <bg_warn>(getOpenBookings#2)</bg_warn> is too long: <bg_warn>39 lines</bg_warn> 🚨`,
}])
})

Expand Down Expand Up @@ -347,7 +346,7 @@ describe('checkFunctionSize', () => {
file: fileName,
rule: `<text_info>rrd ~ function size</text_info>`,
description: `👉 <text_warn>Functions must be shorter than ${maxSize} lines.</text_warn> See: https://vue-mess-detector.webmania.cc/rules/rrd/function-size.html`,
message: `function <bg_warn>(createFiles#10)</bg_warn> is too long: <bg_warn>20 lines</bg_warn> 🚨`,
message: `function <bg_warn>(createFiles#10)</bg_warn> is too long: <bg_warn>22 lines</bg_warn> 🚨`,
}])
})
})
Loading
Loading