From 83b072fbc7256662e89fc7e4856c8eed4cc261ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Sun, 15 Oct 2017 05:29:21 +0800 Subject: [PATCH 1/9] New: rule prefer-replace-text (fixes #47). --- README.md | 1 + lib/rules/prefer-replace-text.js | 83 ++++++++++++++++++++++++++ tests/lib/rules/prefer-replace-text.js | 68 +++++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 lib/rules/prefer-replace-text.js create mode 100644 tests/lib/rules/prefer-replace-text.js diff --git a/README.md b/README.md index e18a0fe0..f6e1c680 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ Name | ✔️ | 🛠 | Description [no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 | Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken [prefer-output-null](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-output-null.md) | | 🛠 | disallows invalid RuleTester test cases with the output the same as the code. [prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | disallow template literals as report messages +[prefer-replace-text](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-replace-text.md) | | 🛠 | prefer using replaceText instead of replaceTextRange. [report-message-format](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/report-message-format.md) | | | enforce a consistent format for rule report messages [require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) | ✔️ | | require rules to implement a meta.fixable property [test-case-property-ordering](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/test-case-property-ordering.md) | | 🛠 | Requires the properties of a test case to be placed in a consistent order diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js new file mode 100644 index 00000000..cee66e42 --- /dev/null +++ b/lib/rules/prefer-replace-text.js @@ -0,0 +1,83 @@ +/** + * @fileoverview prefer using replaceText instead of replaceTextRange. + * @author 薛定谔的猫 + */ + +'use strict'; + +const utils = require('../utils'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'prefer using replaceText instead of replaceTextRange.', + category: 'Rules', + recommended: false, + }, + fixable: 'code', + schema: [], + }, + + create (context) { + const sourceCode = context.getSourceCode(); + const message = 'prefer using replaceText instead of replaceTextRange.'; + let funcInfo = { + upper: null, + codePath: null, + shouldCheck: false, + node: null, + }; + let contextIdentifiers; + + return { + Program (node) { + contextIdentifiers = utils.getContextIdentifiers(context, node); + }, + + // Stacks this function's information. + onCodePathStart (codePath, node) { + const parent = node.parent; + const shouldCheck = node.type === 'FunctionExpression' && + parent.parent.type === 'ObjectExpression' && + parent.parent.parent.type === 'CallExpression' && + contextIdentifiers.has(parent.parent.parent.callee.object) && + parent.parent.parent.callee.property.name === 'report' && + utils.getReportInfo(parent.parent.parent.arguments).fix === node; + + funcInfo = { + upper: funcInfo, + codePath, + shouldCheck, + node, + }; + }, + + // Pops this function's information. + onCodePathEnd () { + funcInfo = funcInfo.upper; + }, + + // Checks the return statement is valid. + 'CallExpression[arguments.length>1]' (node) { + if (funcInfo.shouldCheck && + node.callee.property.name === 'replaceTextRange') { + const arg = node.arguments[0]; + if ((arg.type === 'MemberExpression' && arg.property.name === 'range') + || ( + arg.type === 'ArrayExpression' && arg.elements.length === 2 && + arg.elements[0].type === 'MemberExpression' && arg.elements[0].type === 'MemberExpression' && sourceCode.getText(arg.elements[0].object) === sourceCode.getText(arg.elements[1].object)) + ) { + context.report({ + node, + message, + }); + } + } + }, + }; + }, +}; diff --git a/tests/lib/rules/prefer-replace-text.js b/tests/lib/rules/prefer-replace-text.js new file mode 100644 index 00000000..a2b06ed6 --- /dev/null +++ b/tests/lib/rules/prefer-replace-text.js @@ -0,0 +1,68 @@ +/** + * @fileoverview prefer using replaceText instead of replaceTextRange + * @author 薛定谔的猫 + */ + +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/prefer-replace-text'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); +const ERROR = { message: 'prefer using replaceText instead of replaceTextRange.' }; + + +ruleTester.run('prefer-placeholders', rule, { + valid: [ + ` + module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceTextRange([start, end], ''); + } + }); + } + }; + `, + ], + + invalid: [ + { + code: ` + module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceTextRange(node.range, ''); + } + }); + } + }; + `, + errors: [ERROR], + }, + { + code: ` + module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceTextRange([node.range[0], node.range[1]], ''); + } + }); + } + }; + `, + errors: [ERROR], + }, + ], +}); From f837f634fb650b454065889d7a4b0a9bc5352cf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 02:51:00 +0800 Subject: [PATCH 2/9] todo: docs. --- README.md | 2 +- lib/rules/prefer-replace-text.js | 17 +++---- tests/lib/rules/prefer-replace-text.js | 69 +++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index f6e1c680..ceb6ec04 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ Name | ✔️ | 🛠 | Description [no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 | Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken [prefer-output-null](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-output-null.md) | | 🛠 | disallows invalid RuleTester test cases with the output the same as the code. [prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | disallow template literals as report messages -[prefer-replace-text](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-replace-text.md) | | 🛠 | prefer using replaceText instead of replaceTextRange. +[prefer-replace-text](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-replace-text.md) | | | prefer using replaceText instead of replaceTextRange. [report-message-format](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/report-message-format.md) | | | enforce a consistent format for rule report messages [require-meta-fixable](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/require-meta-fixable.md) | ✔️ | | require rules to implement a meta.fixable property [test-case-property-ordering](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/test-case-property-ordering.md) | | 🛠 | Requires the properties of a test case to be placed in a consistent order diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js index cee66e42..ed1b55c6 100644 --- a/lib/rules/prefer-replace-text.js +++ b/lib/rules/prefer-replace-text.js @@ -18,7 +18,7 @@ module.exports = { category: 'Rules', recommended: false, }, - fixable: 'code', + fixable: null, schema: [], }, @@ -41,7 +41,7 @@ module.exports = { // Stacks this function's information. onCodePathStart (codePath, node) { const parent = node.parent; - const shouldCheck = node.type === 'FunctionExpression' && + const shouldCheck = (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') && parent.parent.type === 'ObjectExpression' && parent.parent.parent.type === 'CallExpression' && contextIdentifiers.has(parent.parent.parent.callee.object) && @@ -61,16 +61,15 @@ module.exports = { funcInfo = funcInfo.upper; }, - // Checks the return statement is valid. - 'CallExpression[arguments.length>1]' (node) { + // Checks the replaceTextRange params. + 'CallExpression[arguments.length=2]' (node) { if (funcInfo.shouldCheck && node.callee.property.name === 'replaceTextRange') { const arg = node.arguments[0]; - if ((arg.type === 'MemberExpression' && arg.property.name === 'range') - || ( - arg.type === 'ArrayExpression' && arg.elements.length === 2 && - arg.elements[0].type === 'MemberExpression' && arg.elements[0].type === 'MemberExpression' && sourceCode.getText(arg.elements[0].object) === sourceCode.getText(arg.elements[1].object)) - ) { + const isIdenticalNodeRange = arg.type === 'ArrayExpression' && + arg.elements[0].type === 'MemberExpression' && arg.elements[1].type === 'MemberExpression' && + sourceCode.getText(arg.elements[0].object) === sourceCode.getText(arg.elements[1].object); + if (isIdenticalNodeRange) { context.report({ node, message, diff --git a/tests/lib/rules/prefer-replace-text.js b/tests/lib/rules/prefer-replace-text.js index a2b06ed6..c9c3f980 100644 --- a/tests/lib/rules/prefer-replace-text.js +++ b/tests/lib/rules/prefer-replace-text.js @@ -33,6 +33,31 @@ ruleTester.run('prefer-placeholders', rule, { } }; `, + ` + module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceTextRange([node1[0], node2[1]], ''); + } + }); + } + }; + `, + ` + module.exports = { + create(context) {} + }; + `, + ` + module.exports = { + create(context) { + var fixer = function(fixer) { + return fixer.replaceTextRange([node.range[0], node.range[1]], ''); + } + } + }; + `, ], invalid: [ @@ -42,7 +67,7 @@ ruleTester.run('prefer-placeholders', rule, { create(context) { context.report({ fix(fixer) { - return fixer.replaceTextRange(node.range, ''); + return fixer.replaceTextRange([node.range[0], node.range[1]], ''); } }); } @@ -55,7 +80,7 @@ ruleTester.run('prefer-placeholders', rule, { module.exports = { create(context) { context.report({ - fix(fixer) { + fix: function(fixer) { return fixer.replaceTextRange([node.range[0], node.range[1]], ''); } }); @@ -64,5 +89,45 @@ ruleTester.run('prefer-placeholders', rule, { `, errors: [ERROR], }, + { + code: ` + module.exports = { + create(context) { + context.report({ + fix: function(fixer) { + if (foo) {return fixer.replaceTextRange([node.range[0], node.range[1]], '')} + } + }); + } + }; + `, + errors: [ERROR], + }, + { + code: ` + module.exports = { + create(context) { + context.report({ + fix: fixer => fixer.replaceTextRange([node.range[0], node.range[1]], '') + }); + } + }; + `, + errors: [ERROR], + }, + { + code: ` + module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceTextRange([node.start, node.end], ''); + } + }); + } + }; + `, + errors: [ERROR], + }, ], }); From b2ce85a2a17c53899020a70a4a46a5c225fe2f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 04:03:14 +0800 Subject: [PATCH 3/9] Docs: add prefer-replace-range.md. --- docs/rules/prefer-replace-range.md | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 docs/rules/prefer-replace-range.md diff --git a/docs/rules/prefer-replace-range.md b/docs/rules/prefer-replace-range.md new file mode 100644 index 00000000..a56c9e93 --- /dev/null +++ b/docs/rules/prefer-replace-range.md @@ -0,0 +1,52 @@ +# prefer using replaceText instead of replaceTextRange. (prefer-replace-text) + +## Rule Details + +The rule reports an error if `replaceTextRange`'s first argument is an array of identical object elements. It can be easily replaced by `replaceText` to improve readability. + +Examples of **incorrect** code for this rule: + +```js +/* eslint eslint-plugin/prefer-text-range: error */ +module.exports = { + create(context) { + context.report({ + fix(fixer) { + // error, can be writen: return fixer.replaceText([node, ''); + return fixer.replaceTextRange([node.range[0], node.range[1]], ''); + } + }); + } +}; +``` + +Examples of **correct** code for this rule: + +```js +/* eslint eslint-plugin/prefer-text-range: error */ +module.exports = { + create(context) { + context.report({ + fix(fixer) { + return fixer.replaceText(node, ''); + } + }); + } +}; + +module.exports = { + create(context) { + context.report({ + fix(fixer) { + // start = ... + // end = ... + return fixer.replaceTextRange([start, end], ''); + } + }); + } +}; +``` + +## Further Reading + +* [Applying Fixes](https://eslint.org/docs/developer-guide/working-with-rules#applying-fixes) From dd06a0dcd59ad3c8aefa5519d2b6340517f4dc44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 04:38:42 +0800 Subject: [PATCH 4/9] Fix: add type check. --- lib/rules/prefer-replace-text.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js index ed1b55c6..4107dda0 100644 --- a/lib/rules/prefer-replace-text.js +++ b/lib/rules/prefer-replace-text.js @@ -64,6 +64,7 @@ module.exports = { // Checks the replaceTextRange params. 'CallExpression[arguments.length=2]' (node) { if (funcInfo.shouldCheck && + node.callee.type === 'MemberExpression' && node.callee.property.name === 'replaceTextRange') { const arg = node.arguments[0]; const isIdenticalNodeRange = arg.type === 'ArrayExpression' && From 8854d4dc9becf3beec6790a59fed8d2b4d5a8a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 04:42:45 +0800 Subject: [PATCH 5/9] chore: update comment. --- lib/rules/prefer-replace-text.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js index 4107dda0..47b3121e 100644 --- a/lib/rules/prefer-replace-text.js +++ b/lib/rules/prefer-replace-text.js @@ -61,7 +61,7 @@ module.exports = { funcInfo = funcInfo.upper; }, - // Checks the replaceTextRange params. + // Checks the replaceTextRange arguments. 'CallExpression[arguments.length=2]' (node) { if (funcInfo.shouldCheck && node.callee.type === 'MemberExpression' && From 7248b912b3cba193b9ff282d66c3f870bb3b0db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Sun, 22 Oct 2017 17:04:42 -0500 Subject: [PATCH 6/9] Update prefer-replace-range.md --- docs/rules/prefer-replace-range.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/prefer-replace-range.md b/docs/rules/prefer-replace-range.md index a56c9e93..b85b19e8 100644 --- a/docs/rules/prefer-replace-range.md +++ b/docs/rules/prefer-replace-range.md @@ -2,7 +2,7 @@ ## Rule Details -The rule reports an error if `replaceTextRange`'s first argument is an array of identical object elements. It can be easily replaced by `replaceText` to improve readability. +The rule reports an error if `replaceTextRange`'s first argument is an array of identical object properties. It can be easily replaced by `replaceText` to improve readability. Examples of **incorrect** code for this rule: From d54b0493910b9e103c85c9f980f7a3ee790125cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 21:31:14 -0500 Subject: [PATCH 7/9] Update prefer-replace-range.md --- docs/rules/prefer-replace-range.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/prefer-replace-range.md b/docs/rules/prefer-replace-range.md index b85b19e8..7f14c99f 100644 --- a/docs/rules/prefer-replace-range.md +++ b/docs/rules/prefer-replace-range.md @@ -2,7 +2,7 @@ ## Rule Details -The rule reports an error if `replaceTextRange`'s first argument is an array of identical object properties. It can be easily replaced by `replaceText` to improve readability. +The rule reports an error if `replaceTextRange`'s first argument is an array of identical array elements. It can be easily replaced by `replaceText` to improve readability. Examples of **incorrect** code for this rule: From aa1d01c32b1dabac3fcb7b0c2069be6c5ae72c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Mon, 23 Oct 2017 23:24:00 -0500 Subject: [PATCH 8/9] Update prefer-replace-range.md --- docs/rules/prefer-replace-range.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/prefer-replace-range.md b/docs/rules/prefer-replace-range.md index 7f14c99f..48688d8a 100644 --- a/docs/rules/prefer-replace-range.md +++ b/docs/rules/prefer-replace-range.md @@ -12,7 +12,7 @@ module.exports = { create(context) { context.report({ fix(fixer) { - // error, can be writen: return fixer.replaceText([node, ''); +        // error, can be written: return fixer.replaceText([node, '']); return fixer.replaceTextRange([node.range[0], node.range[1]], ''); } }); From 4bebb6ff06684f18c48233b530236e8e64f84d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Tue, 24 Oct 2017 12:34:13 +0800 Subject: [PATCH 9/9] Fix: reveiew suggestions. --- lib/rules/prefer-replace-text.js | 2 +- tests/lib/rules/prefer-replace-text.js | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js index 47b3121e..a4b9856f 100644 --- a/lib/rules/prefer-replace-text.js +++ b/lib/rules/prefer-replace-text.js @@ -24,7 +24,7 @@ module.exports = { create (context) { const sourceCode = context.getSourceCode(); - const message = 'prefer using replaceText instead of replaceTextRange.'; + const message = 'Use replaceText instead of replaceTextRange.'; let funcInfo = { upper: null, codePath: null, diff --git a/tests/lib/rules/prefer-replace-text.js b/tests/lib/rules/prefer-replace-text.js index c9c3f980..2c339f16 100644 --- a/tests/lib/rules/prefer-replace-text.js +++ b/tests/lib/rules/prefer-replace-text.js @@ -17,7 +17,7 @@ const RuleTester = require('eslint').RuleTester; // ------------------------------------------------------------------------------ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); -const ERROR = { message: 'prefer using replaceText instead of replaceTextRange.' }; +const ERROR = { message: 'Use replaceText instead of replaceTextRange.' }; ruleTester.run('prefer-placeholders', rule, { @@ -50,13 +50,7 @@ ruleTester.run('prefer-placeholders', rule, { }; `, ` - module.exports = { - create(context) { - var fixer = function(fixer) { - return fixer.replaceTextRange([node.range[0], node.range[1]], ''); - } - } - }; + fixer.replaceTextRange([node.range[0], node.range[1]], ''); `, ],