From 6beb5368ab7c12f7199a4e440dfe89b85126fd64 Mon Sep 17 00:00:00 2001 From: Hugo <60015232+hugop95@users.noreply.github.com> Date: Sat, 7 Sep 2024 16:10:57 +0200 Subject: [PATCH] fix: improve dependency detection in sort-variable-declarations rule --- rules/sort-variable-declarations.ts | 52 ++- test/sort-classes.test.ts | 94 +++- test/sort-variable-declarations.test.ts | 560 +++++++++++++++++++----- 3 files changed, 596 insertions(+), 110 deletions(-) diff --git a/rules/sort-variable-declarations.ts b/rules/sort-variable-declarations.ts index b650bf02..76f47dd7 100644 --- a/rules/sort-variable-declarations.ts +++ b/rules/sort-variable-declarations.ts @@ -89,16 +89,44 @@ export default createEslintRule({ let dependencies: string[] = [] let checkNode = (nodeValue: TSESTree.Node) => { + /** + * No need to check the body of functions and arrow functions + */ + if ( + nodeValue.type === 'ArrowFunctionExpression' || + nodeValue.type === 'FunctionExpression' + ) { + return + } + if (nodeValue.type === 'Identifier') { dependencies.push(nodeValue.name) } + if (nodeValue.type === 'Property') { + traverseNode(nodeValue.key) + traverseNode(nodeValue.value) + } + + if (nodeValue.type === 'ConditionalExpression') { + traverseNode(nodeValue.test) + traverseNode(nodeValue.consequent) + traverseNode(nodeValue.alternate) + } + if ( - 'body' in nodeValue && - nodeValue.body && - !Array.isArray(nodeValue.body) + 'expression' in nodeValue && + typeof nodeValue.expression !== 'boolean' ) { - traverseNode(nodeValue.body) + traverseNode(nodeValue.expression) + } + + if ('object' in nodeValue) { + traverseNode(nodeValue.object) + } + + if ('callee' in nodeValue) { + traverseNode(nodeValue.callee) } if ('left' in nodeValue) { @@ -113,9 +141,23 @@ export default createEslintRule({ nodeValue.elements .filter(currentNode => currentNode !== null) .forEach(traverseNode) - } else if ('arguments' in nodeValue) { + } + + if ('argument' in nodeValue && nodeValue.argument) { + traverseNode(nodeValue.argument) + } + + if ('arguments' in nodeValue) { nodeValue.arguments.forEach(traverseNode) } + + if ('properties' in nodeValue) { + nodeValue.properties.forEach(traverseNode) + } + + if ('expressions' in nodeValue) { + nodeValue.expressions.forEach(traverseNode) + } } let traverseNode = (nodeValue: TSESTree.Node) => { diff --git a/test/sort-classes.test.ts b/test/sort-classes.test.ts index d472bb3d..4267b611 100644 --- a/test/sort-classes.test.ts +++ b/test/sort-classes.test.ts @@ -2681,7 +2681,7 @@ describe(ruleName, () => { ) ruleTester.run( - `${ruleName}(${type}) detects arrow function expression dependencies`, + `${ruleName}(${type}) detects function expression dependencies`, rule, { valid: [ @@ -2705,6 +2705,26 @@ describe(ruleName, () => { }, ], }, + { + code: dedent` + class Class { + b = function() { + return 1 + } + a = this.b() + static b = function() { + return 1 + } + static a = this.b() + } + `, + options: [ + { + ...options, + groups: [['property', 'method']], + }, + ], + }, { code: dedent` class Class { @@ -2725,6 +2745,26 @@ describe(ruleName, () => { }, ], }, + { + code: dedent` + class Class { + b = function() { + return 1 + } + a = [1].map(this.b) + static b = function() { + return 1 + } + static a = [1].map(this.b) + } + `, + options: [ + { + ...options, + groups: [['property', 'method']], + }, + ], + }, { code: dedent` class Class { @@ -2745,6 +2785,26 @@ describe(ruleName, () => { }, ], }, + { + code: dedent` + class Class { + b = function() { + return 1 + } + a = [1].map(this.b) + static b = function() { + return 1 + } + static a = [1].map(Class.b) + } + `, + options: [ + { + ...options, + groups: [['property', 'method']], + }, + ], + }, ], invalid: [], }, @@ -3253,6 +3313,38 @@ describe(ruleName, () => { }, ], }, + { + code: dedent` + class Class { + b = [] + a = [...this.b] + static b = [] + static a = [...this.b] + } + `, + options: [ + { + ...options, + groups: ['property'], + }, + ], + }, + { + code: dedent` + class Class { + b = [] + a = [...this.b] + static b = [] + static a = [...Class.b] + } + `, + options: [ + { + ...options, + groups: ['property'], + }, + ], + }, ], invalid: [], }, diff --git a/test/sort-variable-declarations.test.ts b/test/sort-variable-declarations.test.ts index 78416907..581ef159 100644 --- a/test/sort-variable-declarations.test.ts +++ b/test/sort-variable-declarations.test.ts @@ -95,188 +95,540 @@ describe(ruleName, () => { }, ) + describe(`${ruleName}(${type}): detects dependencies`, () => { + ruleTester.run( + `${ruleName}(${type}): does not sort properties if the right value depends on the left value`, + rule, + { + valid: [ + { + code: dedent` + const bb = 1, + aaa = bb + 2, + c = aaa + 3 + `, + options: [options], + }, + { + code: dedent` + let a = 1, + b = a + 2, + c = b + 3, + d = [a, b, c]; + `, + options: [options], + }, + { + code: dedent` + var x = 10, + y = x * 2, + z = y + 5 - x; + `, + options: [options], + }, + { + code: dedent` + const arr = [1, 2, 3], + sum = arr.reduce((acc, val) => acc + val, 0), + avg = sum / arr.length; + `, + options: [options], + }, + { + code: dedent` + const getValue = () => 1, + value = getValue(), + result = value + 2; + `, + options: [options], + }, + { + code: dedent` + let position = editor.state.selection.$anchor, + depth = position.depth; + `, + options: [options], + }, + ], + invalid: [ + { + code: dedent` + const b, + a, + c; + `, + output: dedent` + const a, + b, + c; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'b', right: 'a' }, + }, + ], + }, + { + code: dedent` + const aaa = bb + 2, + bb = 1, + c = aaa + 3; + `, + output: dedent` + const bb = 1, + aaa = bb + 2, + c = aaa + 3; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'aaa', right: 'bb' }, + }, + ], + }, + { + code: dedent` + let b = a + 2, + a = 1, + c = b + 3; + `, + output: dedent` + let a = 1, + b = a + 2, + c = b + 3; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'b', right: 'a' }, + }, + ], + }, + { + code: dedent` + var y = x * 2, + x = 10, + z = y + 5; + `, + output: dedent` + var x = 10, + y = x * 2, + z = y + 5; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'y', right: 'x' }, + }, + ], + }, + { + code: dedent` + const sum = arr.reduce((acc, val) => acc + val, 0), + arr = [1, 2, 3], + avg = sum / arr.length; + `, + output: dedent` + const arr = [1, 2, 3], + sum = arr.reduce((acc, val) => acc + val, 0), + avg = sum / arr.length; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'sum', right: 'arr' }, + }, + ], + }, + { + code: dedent` + const value = getValue(), + getValue = () => 1, + result = value + 2; + `, + output: dedent` + const getValue = () => 1, + value = getValue(), + result = value + 2; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'value', right: 'getValue' }, + }, + ], + }, + { + code: dedent` + const a = c, + b = 10, + c = 10; + `, + output: dedent` + const c = 10, + a = c, + b = 10; + `, + options: [options], + errors: [ + { + messageId: 'unexpectedVariableDeclarationsOrder', + data: { left: 'b', right: 'c' }, + }, + ], + }, + ], + }, + ) + }) + ruleTester.run( - `${ruleName}(${type}): does not sort properties if the right value depends on the left value`, + `${ruleName}(${type}) detects function expression dependencies`, rule, { valid: [ { code: dedent` - const bb = 1, - aaa = bb + 2, - c = aaa + 3 + let b = () => 1, + a = b(); `, - options: [options], + options: [ + { + ...options, + }, + ], }, { code: dedent` - let a = 1, - b = a + 2, - c = b + 3, - d = [a, b, c]; + let b = function() { return 1 }, + a = b(); `, - options: [options], + options: [ + { + ...options, + }, + ], }, { code: dedent` - var x = 10, - y = x * 2, - z = y + 5 - x; + let b = () => 1, + a = a.map(b); `, - options: [options], + options: [ + { + ...options, + }, + ], }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects dependencies in objects`, + rule, + { + valid: [ { code: dedent` - const arr = [1, 2, 3], - sum = arr.reduce((acc, val) => acc + val, 0), - avg = sum / arr.length; + let b = 1, + a = {x: b}; `, - options: [options], + options: [ + { + ...options, + }, + ], }, { code: dedent` - const getValue = () => 1, - value = getValue(), - result = value + 2; + let b = 1, + a = {[b]: 0}; `, - options: [options], + options: [ + { + ...options, + }, + ], }, ], - invalid: [ + invalid: [], + }, + ) + + ruleTester.run(`${ruleName}(${type}) detects chained dependencies`, rule, { + valid: [ + { + code: dedent` + let b = {x: 1}, + a = b.x; + `, + options: [ + { + ...options, + }, + ], + }, + { + code: dedent` + let b = new Subject(), + a = b.asObservable(); + `, + options: [ + { + ...options, + }, + ], + }, + ], + invalid: [], + }) + + ruleTester.run( + `${ruleName}(${type}) detects optional chained dependencies`, + rule, + { + valid: [ { code: dedent` - const b, - a, - c; - `, - output: dedent` - const a, - b, - c; + let b = {x: 1}, + a = b?.x; `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'b', right: 'a' }, + ...options, }, ], }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects non-null asserted dependencies`, + rule, + { + valid: [ { code: dedent` - const aaa = bb + 2, - bb = 1, - c = aaa + 3; - `, - output: dedent` - const bb = 1, - aaa = bb + 2, - c = aaa + 3; + let b = 1, + a = b!; `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'aaa', right: 'bb' }, + ...options, }, ], }, + ], + invalid: [], + }, + ) + + ruleTester.run(`${ruleName}(${type}) detects unary dependencies`, rule, { + valid: [ + { + code: dedent` + let b = true, + a = !b; + `, + options: [ + { + ...options, + }, + ], + }, + ], + invalid: [], + }) + + ruleTester.run( + `${ruleName}(${type}) detects spread elements dependencies`, + rule, + { + valid: [ { code: dedent` - let b = a + 2, - a = 1, - c = b + 3; + let b = {x: 1}, + a = {...b}; `, - output: dedent` - let a = 1, - b = a + 2, - c = b + 3; - `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'b', right: 'a' }, + ...options, }, ], }, { code: dedent` - var y = x * 2, - x = 10, - z = y + 5; - `, - output: dedent` - var x = 10, - y = x * 2, - z = y + 5; + let b = [1] + a = [...b]; `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'y', right: 'x' }, + ...options, }, ], }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects dependencies in conditional expressions`, + rule, + { + valid: [ { code: dedent` - const sum = arr.reduce((acc, val) => acc + val, 0), - arr = [1, 2, 3], - avg = sum / arr.length; + let b = 0, + a = b ? 1 : 0; `, - output: dedent` - const arr = [1, 2, 3], - sum = arr.reduce((acc, val) => acc + val, 0), - avg = sum / arr.length; + options: [ + { + ...options, + }, + ], + }, + { + code: dedent` + let b = 0, + a = x ? b : 0; `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'sum', right: 'arr' }, + ...options, }, ], }, { code: dedent` - const value = getValue(), - getValue = () => 1, - result = value + 2; + let b = 0, + a = x ? 0 : b; `, - output: dedent` - const getValue = () => 1, - value = getValue(), - result = value + 2; + options: [ + { + ...options, + }, + ], + }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects dependencies in 'as' expressions`, + rule, + { + valid: [ + { + code: dedent` + let b = a, + a = b as any; `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'value', right: 'getValue' }, + ...options, }, ], }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects dependencies in type assertion expressions`, + rule, + { + valid: [ { code: dedent` - const a = c, - b = 10, - c = 10; + let b = a, + a = b; `, - output: dedent` - const c = 10, - a = c, - b = 10; + options: [ + { + ...options, + }, + ], + }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) detects dependencies in template literal expressions`, + rule, + { + valid: [ + { + code: dedent` + let b = a, + a = \`\${b}\` `, - options: [options], - errors: [ + options: [ { - messageId: 'unexpectedVariableDeclarationsOrder', - data: { left: 'b', right: 'c' }, + ...options, + }, + ], + }, + ], + invalid: [], + }, + ) + + ruleTester.run( + `${ruleName}(${type}) ignores function body dependencies`, + rule, + { + valid: [ + { + code: dedent` + let a = () => b, + b = 1; + `, + options: [ + { + ...options, + }, + ], + }, + { + code: dedent` + let a = function() { return b }, + b = 1; + `, + options: [ + { + ...options, + }, + ], + }, + { + code: dedent` + let a = () => {return b}, + b = 1; + `, + options: [ + { + ...options, }, ], }, ], + invalid: [], }, ) })