From 0073170168e1cbbb87f1da8e76a8871f28872536 Mon Sep 17 00:00:00 2001 From: Eduard Deisling Date: Fri, 22 Apr 2022 12:14:16 +0300 Subject: [PATCH 1/2] define-macros-order bugs --- lib/rules/define-macros-order.js | 51 ++++++++++++++++++++------ tests/lib/rules/define-macros-order.js | 42 ++++++++++++++++++--- 2 files changed, 76 insertions(+), 17 deletions(-) diff --git a/lib/rules/define-macros-order.js b/lib/rules/define-macros-order.js index 9c8ea45e6..c8a94fb84 100644 --- a/lib/rules/define-macros-order.js +++ b/lib/rules/define-macros-order.js @@ -19,6 +19,17 @@ const MACROS_PROPS = 'defineProps' const ORDER = [MACROS_EMITS, MACROS_PROPS] const DEFAULT_ORDER = [MACROS_PROPS, MACROS_EMITS] +/** + * @param {VElement} scriptSetup + * @param {ASTNode} node + */ +function inScriptSetup(scriptSetup, node) { + return ( + scriptSetup.range[0] <= node.range[0] && + node.range[1] <= scriptSetup.range[1] + ) +} + /** * @param {ASTNode} node */ @@ -33,9 +44,10 @@ function isUseStrictStatement(node) { /** * Get an index of the first statement after imports and interfaces in order * to place defineEmits and defineProps before this statement + * @param {VElement} scriptSetup * @param {Program} program */ -function getTargetStatementPosition(program) { +function getTargetStatementPosition(scriptSetup, program) { const skipStatements = new Set([ 'ImportDeclaration', 'TSInterfaceDeclaration', @@ -45,7 +57,11 @@ function getTargetStatementPosition(program) { ]) for (const [index, item] of program.body.entries()) { - if (!skipStatements.has(item.type) && !isUseStrictStatement(item)) { + if ( + inScriptSetup(scriptSetup, item) && + !skipStatements.has(item.type) && + !isUseStrictStatement(item) + ) { return index } } @@ -104,7 +120,10 @@ function create(context) { 'Program:exit'(program) { const shouldFirstNode = macrosNodes.get(order[0]) const shouldSecondNode = macrosNodes.get(order[1]) - const firstStatementIndex = getTargetStatementPosition(program) + const firstStatementIndex = getTargetStatementPosition( + scriptSetup, + program + ) const firstStatement = program.body[firstStatementIndex] // have both defineEmits and defineProps @@ -213,13 +232,12 @@ function create(context) { const targetComment = sourceCode.getTokenAfter(beforeTargetToken, { includeComments: true }) - const textSpace = getTextBetweenTokens(beforeTargetToken, targetComment) // make insert text: comments + node + space before target const textNode = sourceCode.getText( node, node.range[0] - nodeComment.range[0] ) - const insertText = textNode + textSpace + const insertText = getInsertText(textNode, target) return [ fixer.insertTextBefore(targetComment, insertText), @@ -228,17 +246,28 @@ function create(context) { } /** - * @param {ASTNode} tokenBefore - * @param {ASTNode} tokenAfter + * Get result text to insert + * @param {string} textNode + * @param {ASTNode} target */ - function getTextBetweenTokens(tokenBefore, tokenAfter) { - return sourceCode.text.slice(tokenBefore.range[1], tokenAfter.range[0]) + function getInsertText(textNode, target) { + const afterTargetComment = sourceCode.getTokenAfter(target, { + includeComments: true + }) + const afterText = sourceCode.text.slice( + target.range[1], + afterTargetComment.range[0] + ) + // handle case when a();b() -> b()a(); + const invalidResult = !textNode.endsWith(';') && !/[\n;]/.test(afterText) + + return textNode + afterText + (invalidResult ? ';' : '') } /** * Get position of the beginning of the token's line(or prevToken end if no line) - * @param {ASTNode} token - * @param {ASTNode} prevToken + * @param {ASTNode|Token} token + * @param {ASTNode|Token} prevToken */ function getLineStartIndex(token, prevToken) { // if we have next token on the same line - get index right before that token diff --git a/tests/lib/rules/define-macros-order.js b/tests/lib/rules/define-macros-order.js index ce5f30040..5b09c4a67 100644 --- a/tests/lib/rules/define-macros-order.js +++ b/tests/lib/rules/define-macros-order.js @@ -155,9 +155,7 @@ tester.run('define-macros-order', rule, { defineProps({ test: Boolean }) - defineEmits(['update:test']) - console.log('test1') console.log('test2') console.log('test3') @@ -192,7 +190,6 @@ tester.run('define-macros-order', rule, { defineProps({ test: Boolean }) - defineEmits(['update:test']) console.log('test1') @@ -261,7 +258,6 @@ tester.run('define-macros-order', rule, { } const emit = defineEmits<{(e: 'update:test'): void}>() - const props = withDefaults(defineProps(), { msg: 'hello', labels: () => ['one', 'two'] @@ -377,8 +373,7 @@ tester.run('define-macros-order', rule, { `, output: ` + defineEmits(['update:test']);const props = defineProps({ test: Boolean }); `, options: optionsEmitsFirst, errors: [ @@ -387,6 +382,41 @@ tester.run('define-macros-order', rule, { line: 3 } ] + }, + { + filename: 'test.vue', + code: ` + + + + `, + output: ` + + + + `, + errors: [ + { + message: message('defineProps'), + line: 11 + } + ] } ] }) From 5209095c8d3c378e69515dea4511fe10a9e4490d Mon Sep 17 00:00:00 2001 From: Eduard Deisling Date: Fri, 22 Apr 2022 12:50:16 +0300 Subject: [PATCH 2/2] define-macros-order bugs revert --- lib/rules/define-macros-order.js | 26 ++++++++------------------ tests/lib/rules/define-macros-order.js | 8 +++++++- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/rules/define-macros-order.js b/lib/rules/define-macros-order.js index c8a94fb84..1db259fa1 100644 --- a/lib/rules/define-macros-order.js +++ b/lib/rules/define-macros-order.js @@ -232,12 +232,13 @@ function create(context) { const targetComment = sourceCode.getTokenAfter(beforeTargetToken, { includeComments: true }) + const textSpace = getTextBetweenTokens(beforeTargetToken, targetComment) // make insert text: comments + node + space before target const textNode = sourceCode.getText( node, node.range[0] - nodeComment.range[0] ) - const insertText = getInsertText(textNode, target) + const insertText = textNode + textSpace return [ fixer.insertTextBefore(targetComment, insertText), @@ -246,28 +247,17 @@ function create(context) { } /** - * Get result text to insert - * @param {string} textNode - * @param {ASTNode} target + * @param {ASTNode} tokenBefore + * @param {ASTNode} tokenAfter */ - function getInsertText(textNode, target) { - const afterTargetComment = sourceCode.getTokenAfter(target, { - includeComments: true - }) - const afterText = sourceCode.text.slice( - target.range[1], - afterTargetComment.range[0] - ) - // handle case when a();b() -> b()a(); - const invalidResult = !textNode.endsWith(';') && !/[\n;]/.test(afterText) - - return textNode + afterText + (invalidResult ? ';' : '') + function getTextBetweenTokens(tokenBefore, tokenAfter) { + return sourceCode.text.slice(tokenBefore.range[1], tokenAfter.range[0]) } /** * Get position of the beginning of the token's line(or prevToken end if no line) - * @param {ASTNode|Token} token - * @param {ASTNode|Token} prevToken + * @param {ASTNode} token + * @param {ASTNode} prevToken */ function getLineStartIndex(token, prevToken) { // if we have next token on the same line - get index right before that token diff --git a/tests/lib/rules/define-macros-order.js b/tests/lib/rules/define-macros-order.js index 5b09c4a67..65a07887f 100644 --- a/tests/lib/rules/define-macros-order.js +++ b/tests/lib/rules/define-macros-order.js @@ -155,7 +155,9 @@ tester.run('define-macros-order', rule, { defineProps({ test: Boolean }) + defineEmits(['update:test']) + console.log('test1') console.log('test2') console.log('test3') @@ -190,6 +192,7 @@ tester.run('define-macros-order', rule, { defineProps({ test: Boolean }) + defineEmits(['update:test']) console.log('test1') @@ -258,6 +261,7 @@ tester.run('define-macros-order', rule, { } const emit = defineEmits<{(e: 'update:test'): void}>() + const props = withDefaults(defineProps(), { msg: 'hello', labels: () => ['one', 'two'] @@ -373,7 +377,8 @@ tester.run('define-macros-order', rule, { `, output: ` + defineEmits(['update:test']) + const props = defineProps({ test: Boolean }); `, options: optionsEmitsFirst, errors: [ @@ -408,6 +413,7 @@ tester.run('define-macros-order', rule, { import 'test' const props = defineProps({ test: Boolean }); + defineEmits(['update:test']) `,