Disallow unnecessary concatenation of strings (no-useless-concat)

[feross]

It’s unnecessary to concatenate two strings together, such as:

var foo = 'a' + 'b'

This code is likely the result of refactoring where a variable was removed from the concatenation (such as 'a' + b + 'b'). In such a case, the concatenation isn’t important and the code can be rewritten as:

var foo = 'ab'

http://eslint.org/docs/rules/no-useless-concat

[dcousens]

I wonder if this will break the "optional dependecy" browserify workaround some people use: require('buf' + 'fer') for example.

ACK anyway, there is probably other ways to fool browserify

[dougwilson]

I had the same though, but the whole Buffer issue has the same issue for modules that use standard but want to support Node.js versions without an alternative to the deprecated Node.js things. In my opinion, it's really more often a good rule, but the few times there is an exception, the eslint-disable comment can be used (and when I use them, I usually explain why in the comment, which you'd probably have a comment explaining why you're doing a useless concat anyway: "prevent browserify from resolving this require").

[feross]

@dcousens Heh, didn't know people were using that hack. The main use case I've seen for omitting a module is for big core things like Buffer, which sometimes get imported just because there's a Buffer.isBuffer() somewhere. For that, there's is-buffer, or the browser field spec.

[dcousens]

@feross or the alterior, where they want to detect if they should support methods like toBuffer, but only for people that want it.

[feross]

Ran the test suite and found minor to moderate ecosystem impact (2.5%). The rule is not automatically fixable.

# tests 287
# pass  280
# fail  7

Specifically, I found several cases that I think should be allowed. They are:

1. Making unicode surrogate pairs stand out explicitly:

var text = '\uD83D\uDE38' + '\uD83D\uDCAD' + '\uD83D\uDC4D'

2. Explicitly separating the parts of a complicated file format, or encoding:

test('encode', function (t) {
  var buf = txtBin.encode(obj)
  var expected = new Buffer('0a' + '537472696e67' + '3d' + '666f6f' +
                            '09' + '6e756d626572' + '3d' + '3432' +
                            '06' + '656d707479' + '3d' +
                            '09' + '6e756c6c' + '3d' + '6e756c6c' +
                            '04' + '626f6f6c' +
                            '0a' + '627566666572' + '3d' + '626172', 'hex')

3. Grouping related parts of a string in a SQL query:

this.query('select info from "' + this.infoTable + '" where id=\'' + (id + ':info') + "'", function (err, result) {

In this last example, I think using a `template string` is a better choice, so I don't feel too bad about this one.

Does anyone have thoughts on this particular rule, in light of these findings? cc @dcousens @dougwilson

(Going to push this to the v11 milestone in the meantime)

[dougwilson]

Nothing in particular.

[dcousens]

var text = '\uD83D\uDE38' + '\uD83D\uDCAD' + '\uD83D\uDC4D'

This reads like magic to me, why are they unique?
Maybe they should use

var PREFIX = '\uD83D\uDE38'
var text = PREFIX + '\uD83D\uDC4D'

[tunnckoCore]

Absolutely agree with @dcousens. There are other possible workarounds, so no need for "specials". Both 1 and 2 would look better if array + join is used. 2c