Add `prefer-string-slice` rule - fixes #65 #67

matijs · 2016-10-24T09:40:10Z

Prefer String.slice instead of String.substr or String.substring.

Documentation
Process feedback
Add fixer

SamVerschueren · 2016-10-24T09:42:33Z

rules/prefer-string-slice.js

+			if (name === 'substr' || name === 'substring') {
+				context.report({
+					node,
+					message: 'Use String.slice instead of String.substr or String.substring.'


Use \`String.slice\` instead of \`String.${name}\`.

Ah, of course, much better, sorted :)

sindresorhus · 2016-10-26T09:27:30Z

docs/rules/prefer-string-slice.md

@@ -0,0 +1,16 @@
+# 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

sindresorhus · 2016-10-26T09:28:10Z

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()`.


Linkify these methods to their MDN page.

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/slice

. => # (like the header)

Also link to the article I shared in the original issue about the difference.

sindresorhus · 2016-10-26T09:29:37Z

rules/prefer-string-slice.js

+const create = context => {
+	return {
+		Identifier: node => {
+			if (['substr', 'substring'].indexOf(node.name) > -1) {


> -1 => !== -1

sindresorhus · 2016-10-26T09:32:27Z

rules/prefer-string-slice.js

+		Identifier: node => {
+			if (['substr', 'substring'].indexOf(node.name) > -1) {
+				const args = node.parent.parent.arguments;
+				console.dir(args[0].type);


You left a console.dir in here.

sindresorhus · 2016-10-26T09:33:13Z

rules/prefer-string-slice.js

+				console.dir(args[0].type);
+				context.report({
+					node,
+					message: `Use \`String.slice\` instead of \`String.${node.name}\`.`


message: `Use \`String#slice\` instead of \`String#${node.name}\`.`

sindresorhus · 2016-10-26T09:35:46Z

Nice start! :)

Would be interesting if it could show in the error message what needs changing to switch to .slice(). Mainly when you're going from .substr as it uses length instead of end as the second argument.

Should also be fixable for literal arguments.

matijs · 2016-10-26T10:07:39Z

@sindresorhus thanks for the feedback! Thoughts on fixing so far… (WIP)

`String#substr` with one argument

Argument can be anything
String.substr(a) can be replaced by String.slice(a)

`String#substring` with one argument

Argument has to be a Literal
String.substring(a) with a >= 0 can be replaced by String.slice(a)
String.substring(a) with a < 0 can be replaced by `String.slice(0), this probably never happens

`String#substr` with two arguments

Arguments have to be Literal
String.substr(a, b) with b > 0 can be replaced by String.slice(a, a + b)
String.substr(a, b) with b <= 0 is unfixable

`String#substring` with two arguments

Arguments have to be Literal and > 0
String.substring(a, b) with a >= 0 && b >= 0 && can be replaced
if a <= b, replace by String.slice(a, b)
else replace by String.slice(b, a)

sindresorhus · 2016-10-26T10:13:21Z

Sounds good

sindresorhus · 2016-11-01T17:13:43Z

docs/rules/prefer-string-slice.md

@@ -0,0 +1,18 @@
+# Prefer the use of `String#slice` instead of `String#substr` or `String#substring`


We should use () here. E.g. String#slice()

sindresorhus · 2016-11-01T17:14:17Z

docs/rules/prefer-string-slice.md

+
+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)


Should end in a .

sindresorhus · 2016-11-01T17:14:37Z

readme.md

@@ -61,6 +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`


Description should be the same as the docs title.

sindresorhus · 2016-11-01T17:15:04Z

test/prefer-string-slice.js

+	valid: [
+		'const foo = bar.slice(1)',
+		`const foo = bar.slice(baz, 2);`,
+		`const foo = bar.slice(baz - 1, 2);`


No point in using template literal here.

Same elsewhere in this file.

Laziness, will fix.

sindresorhus · 2016-11-01T17:15:49Z

rules/prefer-string-slice.js

+			if (['substr', 'substring'].indexOf(node.name) !== -1) {
+				context.report({
+					node,
+					message: `Use \`String#slice\` instead of \`String#${node.name}\`.`


Add () here too.

Rewrote this bit quite a bit, fixed in the rewrite.

matijs · 2016-11-02T08:41:21Z

Still working on this, just a bit short on time at the moment.

SamVerschueren · 2016-11-02T08:51:33Z

@matijs No problem, just continue when you find the time :).

sindresorhus · 2017-01-23T09:01:56Z

Hey @matijs. Still interested in finishing this? :)

matijs · 2017-01-23T09:11:10Z

Yes, very much! Life is calming down a little bit, I hope to spend some time on this towards the end of the week.

sindresorhus · 2017-05-12T07:53:51Z

@matijs Still interested? Otherwise I'll close this.

matijs · 2017-05-12T20:31:35Z

I am, but I just don't have the time at the moment :/

sindresorhus · 2017-05-12T20:36:44Z

Ok. No worries. I'll close this, but happy to reopen if you get a chance to finish this at some point :)

florianb · 2017-12-04T12:55:15Z

@sindresorhus, @matijs - If no one objects, i'll take the PR over.

sindresorhus · 2017-12-08T00:19:16Z

@florianb Go ahead :)

matijs added 2 commits October 24, 2016 11:35

Add initial rule and test

5938b6a

Add prefer-string-slice to index.js

15c28b0

SamVerschueren reviewed Oct 24, 2016

View reviewed changes

matijs added 3 commits October 24, 2016 12:32

Clarify error message.

b67a7e1

Add backticks for readability

bf29199

Add documentation

67fba56

matijs changed the title ~~Prefer string slice~~ Prefer string slice - fixes #65 Oct 24, 2016

matijs added 2 commits October 24, 2016 17:19

Update example package.json

2089496

Fix nonsensical test, add more tests

59946b4

sindresorhus changed the title ~~Prefer string slice - fixes #65~~ Add prefer-string-slice rule - fixes #65 Oct 26, 2016

sindresorhus requested changes Oct 26, 2016

View reviewed changes

matijs added 4 commits October 26, 2016 14:49

Improve documentation. Add links.

c96e8f0

Improve tests

a3e6e23

Remove console.dir

e823126

Update error message

a635277

This was referenced Oct 26, 2016

prefer-string-slice rule #65

Closed

Make no-hex-escape more concise #68

Closed

sindresorhus requested changes Nov 1, 2016

View reviewed changes

sindresorhus mentioned this pull request Nov 2, 2016

no-useless-template rule #71

Closed

Update docs

92e1346

sindresorhus referenced this pull request in sindresorhus/camelcase Nov 8, 2016

ES2015ify and require Node.js 4

2296030

sindresorhus closed this May 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `prefer-string-slice` rule - fixes #65 #67

Add `prefer-string-slice` rule - fixes #65 #67

matijs commented Oct 24, 2016 •

edited

Loading

SamVerschueren Oct 24, 2016

matijs Oct 24, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus Oct 26, 2016

sindresorhus commented Oct 26, 2016

matijs commented Oct 26, 2016

sindresorhus commented Oct 26, 2016 •

edited

Loading

sindresorhus Nov 1, 2016

sindresorhus Nov 1, 2016

sindresorhus Nov 1, 2016

sindresorhus Nov 1, 2016

matijs Nov 2, 2016

sindresorhus Nov 1, 2016

matijs Nov 2, 2016

matijs commented Nov 2, 2016

SamVerschueren commented Nov 2, 2016

sindresorhus commented Jan 23, 2017

matijs commented Jan 23, 2017

sindresorhus commented May 12, 2017

matijs commented May 12, 2017

sindresorhus commented May 12, 2017

florianb commented Dec 4, 2017

sindresorhus commented Dec 8, 2017

		@@ -0,0 +1,16 @@
		# Prefer the use of `String.slice` instead of `String.substr` or `String.substring`

		@@ -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()`.

		@@ -0,0 +1,18 @@
		# 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()`.

		Read more in [this Stack Overflow question](http://stackoverflow.com/questions/2243824/what-is-the-difference-between-string-slice-and-string-substring)

Add prefer-string-slice rule - fixes #65 #67

Add prefer-string-slice rule - fixes #65 #67

Conversation

matijs commented Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Oct 26, 2016

matijs commented Oct 26, 2016

String#substr with one argument

String#substring with one argument

String#substr with two arguments

String#substring with two arguments

sindresorhus commented Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matijs commented Nov 2, 2016

SamVerschueren commented Nov 2, 2016

sindresorhus commented Jan 23, 2017

matijs commented Jan 23, 2017

sindresorhus commented May 12, 2017

matijs commented May 12, 2017

sindresorhus commented May 12, 2017

florianb commented Dec 4, 2017

sindresorhus commented Dec 8, 2017

Add `prefer-string-slice` rule - fixes #65 #67

Add `prefer-string-slice` rule - fixes #65 #67

matijs commented Oct 24, 2016 •

edited

Loading

`String#substr` with one argument

`String#substring` with one argument

`String#substr` with two arguments

`String#substring` with two arguments

sindresorhus commented Oct 26, 2016 •

edited

Loading