From 5938b6a6046242f46a89068d0857a1ff62f50fb7 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 11:35:14 +0200 Subject: [PATCH 01/12] Add initial rule and test --- rules/prefer-string-slice.js | 20 +++++++++++++++ test/prefer-string-slice.js | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 rules/prefer-string-slice.js create mode 100644 test/prefer-string-slice.js diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js new file mode 100644 index 0000000000..e1063cef22 --- /dev/null +++ b/rules/prefer-string-slice.js @@ -0,0 +1,20 @@ +'use strict'; + +const create = context => { + return { + Identifier: node => { + const name = node.name; + + if (name === 'substr' || name === 'substring') { + context.report({ + node, + message: 'Use String.slice instead of String.substr or String.substring.' + }); + } + } + }; +}; + +module.exports = { + create +}; diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js new file mode 100644 index 0000000000..19d3557769 --- /dev/null +++ b/test/prefer-string-slice.js @@ -0,0 +1,47 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import rule from '../rules/prefer-string-slice'; + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +const error = { + ruleId: 'prefer-string-slice', + message: 'Use String.slice instead of String.substr or String.substring.' +}; + +ruleTester.run('prefer-string-slice', rule, { + valid: [ + 'const foo = bar.slice(1)', + `const foo = bar.slice(function() { return 1; }, 2);` + ], + invalid: [ + { + code: 'const foo = bar.substr(1)', + errors: [error] + }, + { + code: 'const foo = bar.substr(1,2)', + errors: [error] + }, + { + code: `const foo = bar.substr(function() { return 1; }, 2);`, + errors: [error] + }, + { + code: 'const foo = bar.substring(1)', + errors: [error] + }, + { + code: 'const foo = bar.substring(1,2)', + errors: [error] + }, + { + code: `const foo = bar.substr(function() { return 1; }, 2);`, + errors: [error] + } + ] +}); From 15c28b0aba50fc7b81075b8ab46d24d432c1b8bb Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 11:41:00 +0200 Subject: [PATCH 02/12] Add prefer-string-slice to index.js --- index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 5b6186e9e1..f243f0c02c 100644 --- a/index.js +++ b/index.js @@ -22,7 +22,8 @@ module.exports = { 'unicorn/number-literal-case': 'error', 'unicorn/no-array-instanceof': 'error', 'unicorn/no-new-buffer': 'error', - 'unicorn/no-hex-escape': 'error' + 'unicorn/no-hex-escape': 'error', + 'unicorn/prefer-string-slice': 'error' } } } From b67a7e1928ad334c3cbf84d2a93d48733221eb9f Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 12:32:39 +0200 Subject: [PATCH 03/12] Clarify error message. --- rules/prefer-string-slice.js | 2 +- test/prefer-string-slice.js | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index e1063cef22..37610bd6fb 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -8,7 +8,7 @@ const create = context => { if (name === 'substr' || name === 'substring') { context.report({ node, - message: 'Use String.slice instead of String.substr or String.substring.' + message: `Use String.slice instead of String.${name}.` }); } } diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js index 19d3557769..2b808f79c1 100644 --- a/test/prefer-string-slice.js +++ b/test/prefer-string-slice.js @@ -8,10 +8,10 @@ const ruleTester = avaRuleTester(test, { } }); -const error = { +const error = name => ({ ruleId: 'prefer-string-slice', - message: 'Use String.slice instead of String.substr or String.substring.' -}; + message: `Use String.slice instead of String.${name}.` +}); ruleTester.run('prefer-string-slice', rule, { valid: [ @@ -21,27 +21,27 @@ ruleTester.run('prefer-string-slice', rule, { invalid: [ { code: 'const foo = bar.substr(1)', - errors: [error] + errors: [error('substr')] }, { code: 'const foo = bar.substr(1,2)', - errors: [error] + errors: [error('substr')] }, { code: `const foo = bar.substr(function() { return 1; }, 2);`, - errors: [error] + errors: [error('substr')] }, { code: 'const foo = bar.substring(1)', - errors: [error] + errors: [error('substring')] }, { code: 'const foo = bar.substring(1,2)', - errors: [error] + errors: [error('substring')] }, { - code: `const foo = bar.substr(function() { return 1; }, 2);`, - errors: [error] + code: `const foo = bar.substring(function() { return 1; }, 2);`, + errors: [error('substring')] } ] }); From bf29199bfaedea8496ceb29d2d1ee89fd8f0c383 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 12:34:04 +0200 Subject: [PATCH 04/12] Add backticks for readability --- rules/prefer-string-slice.js | 2 +- test/prefer-string-slice.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index 37610bd6fb..74813c1cff 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -8,7 +8,7 @@ const create = context => { if (name === 'substr' || name === 'substring') { context.report({ node, - message: `Use String.slice instead of String.${name}.` + message: `Use \`String.slice\` instead of \`String.${name}\`.` }); } } diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js index 2b808f79c1..340438e04e 100644 --- a/test/prefer-string-slice.js +++ b/test/prefer-string-slice.js @@ -10,7 +10,7 @@ const ruleTester = avaRuleTester(test, { const error = name => ({ ruleId: 'prefer-string-slice', - message: `Use String.slice instead of String.${name}.` + message: `Use \`String.slice\` instead of \`String.${name}\`.` }); ruleTester.run('prefer-string-slice', rule, { From 67fba564247388923a00b02a7ef05aaee9b41e5a Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 17:11:13 +0200 Subject: [PATCH 05/12] Add documentation --- docs/rules/prefer-string-slice.md | 16 ++++++++++++++++ readme.md | 1 + 2 files changed, 17 insertions(+) create mode 100644 docs/rules/prefer-string-slice.md diff --git a/docs/rules/prefer-string-slice.md b/docs/rules/prefer-string-slice.md new file mode 100644 index 0000000000..296c80c2d0 --- /dev/null +++ b/docs/rules/prefer-string-slice.md @@ -0,0 +1,16 @@ +# Prefer the use of `String.slice` instead of `String.substr` or `String.substring` + +Prefer a convention of using `String.slice` instead of `String.substr` or `String.substring`. `String.slice()` has clearer behavior and has a counterpart with arrays. It is also better to be consistent. Anything that can be done with `String.substr()` or `String.substring()` can be done with `String.slice()`. + +## Fail + +```js +const foo = bar.substr(1, 2); +const baz = quux.substring(1, 3); +``` + +## Pass + +```js +const foo = bar.slice(1, 3); +``` diff --git a/readme.md b/readme.md index cd3b97c41e..37b7b66a01 100644 --- a/readme.md +++ b/readme.md @@ -61,6 +61,7 @@ Configure it in `package.json`. - [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)* - [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)* - [no-hex-escape](docs/rules/no-hex-escape.md) - Enforce the use of unicode escapes instead of hexadecimal escapes. *(fixable)* +- [prefer-string-slice](docs/rules/prefer-string-slice) - Prefer the use of `String.slice` instead of `String.substr` or `String.substring` ## Recommended config From 20894969c7790bdb212628422ad4cd8005462d81 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Mon, 24 Oct 2016 17:19:43 +0200 Subject: [PATCH 06/12] Update example package.json --- readme.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 37b7b66a01..44e3da5bcf 100644 --- a/readme.md +++ b/readme.md @@ -42,7 +42,8 @@ Configure it in `package.json`. "unicorn/number-literal-case": "error", "unicorn/no-array-instanceof": "error", "unicorn/no-new-buffer": "error", - "unicorn/no-hex-escape": "error" + "unicorn/no-hex-escape": "error", + "unicorn/prefer-string-slice": "error" } } } From 59946b424d54517377422cbcf8b0a977559d2cb8 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Tue, 25 Oct 2016 17:07:55 +0200 Subject: [PATCH 07/12] Fix nonsensical test, add more tests --- rules/prefer-string-slice.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index 74813c1cff..817cd9b277 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -3,12 +3,12 @@ const create = context => { return { Identifier: node => { - const name = node.name; - - if (name === 'substr' || name === 'substring') { + if (['substr', 'substring'].indexOf(node.name) > -1) { + const args = node.parent.parent.arguments; + console.dir(args[0].type); context.report({ node, - message: `Use \`String.slice\` instead of \`String.${name}\`.` + message: `Use \`String.slice\` instead of \`String.${node.name}\`.` }); } } From c96e8f03624421a9c5461f82e1c613a78583a37a Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Wed, 26 Oct 2016 14:49:42 +0200 Subject: [PATCH 08/12] Improve documentation. Add links. --- docs/rules/prefer-string-slice.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/rules/prefer-string-slice.md b/docs/rules/prefer-string-slice.md index 296c80c2d0..0f80be774f 100644 --- a/docs/rules/prefer-string-slice.md +++ b/docs/rules/prefer-string-slice.md @@ -1,6 +1,8 @@ -# Prefer the use of `String.slice` instead of `String.substr` or `String.substring` +# Prefer the use of `String#slice` instead of `String#substr` or `String#substring` -Prefer a convention of using `String.slice` instead of `String.substr` or `String.substring`. `String.slice()` has clearer behavior and has a counterpart with arrays. It is also better to be consistent. Anything that can be done with `String.substr()` or `String.substring()` can be done with `String.slice()`. +Prefer a convention of using [`String#slice`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/slice) instead of [`String#substr`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/substr) or [`String#substring`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring). `String#slice()` has clearer behavior and has a counterpart with arrays. It is also better to be consistent. Anything that can be done with `String#substr()` or `String#substring()` can be done with `String#slice()`. + +Read more in [this Stack Overflow question](http://stackoverflow.com/questions/2243824/what-is-the-difference-between-string-slice-and-string-substring) ## Fail From a3e6e23c5ab8560291168910183940f5154ba44b Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Wed, 26 Oct 2016 14:49:57 +0200 Subject: [PATCH 09/12] Improve tests --- test/prefer-string-slice.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js index 340438e04e..e930110e36 100644 --- a/test/prefer-string-slice.js +++ b/test/prefer-string-slice.js @@ -16,7 +16,8 @@ const error = name => ({ ruleTester.run('prefer-string-slice', rule, { valid: [ 'const foo = bar.slice(1)', - `const foo = bar.slice(function() { return 1; }, 2);` + `const foo = bar.slice(baz, 2);`, + `const foo = bar.slice(baz - 1, 2);` ], invalid: [ { @@ -28,7 +29,11 @@ ruleTester.run('prefer-string-slice', rule, { errors: [error('substr')] }, { - code: `const foo = bar.substr(function() { return 1; }, 2);`, + code: `const foo = bar.substr(baz, 2);`, + errors: [error('substr')] + }, + { + code: `const foo = bar.substr(baz - 1, 2);`, errors: [error('substr')] }, { @@ -40,7 +45,11 @@ ruleTester.run('prefer-string-slice', rule, { errors: [error('substring')] }, { - code: `const foo = bar.substring(function() { return 1; }, 2);`, + code: `const foo = bar.substring(baz, 2);`, + errors: [error('substring')] + }, + { + code: `const foo = bar.substring(baz - 1, 2);`, errors: [error('substring')] } ] From e8231268311c157d41ae62025416f6612ac3b792 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Wed, 26 Oct 2016 14:51:41 +0200 Subject: [PATCH 10/12] Remove console.dir --- rules/prefer-string-slice.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index 817cd9b277..f64eb3c688 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -3,9 +3,7 @@ const create = context => { return { Identifier: node => { - if (['substr', 'substring'].indexOf(node.name) > -1) { - const args = node.parent.parent.arguments; - console.dir(args[0].type); + if (['substr', 'substring'].indexOf(node.name) !== -1) { context.report({ node, message: `Use \`String.slice\` instead of \`String.${node.name}\`.` From a6352770a2237bc9637de1de9862f8393b195486 Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Wed, 26 Oct 2016 14:57:53 +0200 Subject: [PATCH 11/12] Update error message --- rules/prefer-string-slice.js | 2 +- test/prefer-string-slice.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index f64eb3c688..d7281ff3d8 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -6,7 +6,7 @@ const create = context => { if (['substr', 'substring'].indexOf(node.name) !== -1) { context.report({ node, - message: `Use \`String.slice\` instead of \`String.${node.name}\`.` + message: `Use \`String#slice\` instead of \`String#${node.name}\`.` }); } } diff --git a/test/prefer-string-slice.js b/test/prefer-string-slice.js index e930110e36..39631f0e96 100644 --- a/test/prefer-string-slice.js +++ b/test/prefer-string-slice.js @@ -10,7 +10,7 @@ const ruleTester = avaRuleTester(test, { const error = name => ({ ruleId: 'prefer-string-slice', - message: `Use \`String.slice\` instead of \`String.${name}\`.` + message: `Use \`String#slice\` instead of \`String#${name}\`.` }); ruleTester.run('prefer-string-slice', rule, { From 92e13462c99fef22e0f61c4217701810c87c738f Mon Sep 17 00:00:00 2001 From: Matijs Brinkhuis Date: Wed, 2 Nov 2016 09:32:06 +0100 Subject: [PATCH 12/12] Update docs --- docs/rules/prefer-string-slice.md | 4 ++-- readme.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-string-slice.md b/docs/rules/prefer-string-slice.md index 0f80be774f..b49eb51ab7 100644 --- a/docs/rules/prefer-string-slice.md +++ b/docs/rules/prefer-string-slice.md @@ -1,6 +1,6 @@ -# Prefer the use of `String#slice` instead of `String#substr` or `String#substring` +# Prefer the use of `String#slice()` instead of `String#substr()` or `String#substring()` -Prefer a convention of using [`String#slice`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/slice) instead of [`String#substr`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/substr) or [`String#substring`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring). `String#slice()` has clearer behavior and has a counterpart with arrays. It is also better to be consistent. Anything that can be done with `String#substr()` or `String#substring()` can be done with `String#slice()`. +Prefer a convention of using [`String#slice()`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/slice) instead of [`String#substr()`](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/substr) or [`String#substring()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring). `String#slice()` has clearer behavior and has a counterpart with arrays. It is also better to be consistent. Anything that can be done with `String#substr()` or `String#substring()` can be done with `String#slice()`. Read more in [this Stack Overflow question](http://stackoverflow.com/questions/2243824/what-is-the-difference-between-string-slice-and-string-substring) diff --git a/readme.md b/readme.md index 44e3da5bcf..e4c0cb5934 100644 --- a/readme.md +++ b/readme.md @@ -62,7 +62,7 @@ Configure it in `package.json`. - [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)* - [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)* - [no-hex-escape](docs/rules/no-hex-escape.md) - Enforce the use of unicode escapes instead of hexadecimal escapes. *(fixable)* -- [prefer-string-slice](docs/rules/prefer-string-slice) - Prefer the use of `String.slice` instead of `String.substr` or `String.substring` +- [prefer-string-slice](docs/rules/prefer-string-slice) - Prefer the use of `String#slice()` instead of `String#substr()` or `String#substring()` ## Recommended config