Skip to content

Commit 5a88ede

Browse files
authored
Add no-useless-concat rule (#3146)
1 parent 522e188 commit 5a88ede

7 files changed

Lines changed: 967 additions & 0 deletions

File tree

docs/rules/no-useless-concat.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# no-useless-concat
2+
3+
📝 Disallow useless concatenation of literals.
4+
5+
💼 This rule is enabled in the following [configs](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config): ✅ `recommended`, ☑️ `unopinionated`.
6+
7+
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
8+
9+
<!-- end auto-generated rule header -->
10+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
11+
12+
Concatenating two literals with `+` is pointless, they can be written as a single literal. This usually happens as a leftover from refactoring, for example after a variable is removed from the concatenation.
13+
14+
This is a fixable counterpart to the frozen [`no-useless-concat`](https://eslint.org/docs/latest/rules/no-useless-concat) ESLint core rule, and additionally catches adjacent literals inside longer concatenations.
15+
16+
## Examples
17+
18+
```js
19+
//
20+
const message = 'Hello, ' + 'world!';
21+
22+
//
23+
const message = 'Hello, world!';
24+
```
25+
26+
```js
27+
//
28+
const path = directory + '/' + 'file.js';
29+
30+
//
31+
const path = directory + '/file.js';
32+
```
33+
34+
```js
35+
//
36+
const greeting = `Hello, ${name}` + `! Welcome.`;
37+
38+
//
39+
const greeting = `Hello, ${name}! Welcome.`;
40+
```

readme.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ export default [
162162
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |
163163
| [no-useless-boolean-cast](docs/rules/no-useless-boolean-cast.md) | Disallow unnecessary `Boolean()` casts in array predicate callbacks. | ✅ ☑️ | 🔧 | |
164164
| [no-useless-collection-argument](docs/rules/no-useless-collection-argument.md) | Disallow useless values or fallbacks in `Set`, `Map`, `WeakSet`, or `WeakMap`. | ✅ ☑️ | 🔧 | 💡 |
165+
| [no-useless-concat](docs/rules/no-useless-concat.md) | Disallow useless concatenation of literals. | ✅ ☑️ | 🔧 | |
165166
| [no-useless-else](docs/rules/no-useless-else.md) | Disallow `else` after a statement that exits. || 🔧 | |
166167
| [no-useless-error-capture-stack-trace](docs/rules/no-useless-error-capture-stack-trace.md) | Disallow unnecessary `Error.captureStackTrace(…)`. | ✅ ☑️ | 🔧 | |
167168
| [no-useless-fallback-in-spread](docs/rules/no-useless-fallback-in-spread.md) | Disallow useless fallback when spreading in object literals. | ✅ ☑️ | 🔧 | |

rules/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export {default as 'no-unused-array-method-return'} from './no-unused-array-meth
101101
export {default as 'no-unused-properties'} from './no-unused-properties.js';
102102
export {default as 'no-useless-boolean-cast'} from './no-useless-boolean-cast.js';
103103
export {default as 'no-useless-collection-argument'} from './no-useless-collection-argument.js';
104+
export {default as 'no-useless-concat'} from './no-useless-concat.js';
104105
export {default as 'no-useless-else'} from './no-useless-else.js';
105106
export {default as 'no-useless-error-capture-stack-trace'} from './no-useless-error-capture-stack-trace.js';
106107
export {default as 'no-useless-fallback-in-spread'} from './no-useless-fallback-in-spread.js';

rules/no-useless-concat.js

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
import {isStringLiteral} from './ast/index.js';
2+
import {fixSpaceAroundKeyword} from './fix/index.js';
3+
import {
4+
escapeString,
5+
getParenthesizedRange,
6+
isParenthesized,
7+
needsSemicolon,
8+
} from './utils/index.js';
9+
import escapeTemplateElementRaw from './utils/escape-template-element-raw.js';
10+
11+
const MESSAGE_ID = 'no-useless-concat';
12+
const messages = {
13+
[MESSAGE_ID]: 'Do not concatenate two literals, combine them into one.',
14+
};
15+
16+
// A `+` operand that is a template literal can never be tagged, since tagging produces a `TaggedTemplateExpression` node instead.
17+
const isStringish = node => isStringLiteral(node) || node.type === 'TemplateLiteral';
18+
19+
// The string value of a literal, or `undefined` if it's a template literal with expressions.
20+
function getStringValue(node) {
21+
if (isStringLiteral(node)) {
22+
return node.value;
23+
}
24+
25+
if (node.expressions.length === 0) {
26+
return node.quasis[0].value.cooked;
27+
}
28+
}
29+
30+
// Legacy octal (`\1`, `\012`) and `\8`/`\9` escapes are valid in sloppy-mode string literals but are syntax errors inside template literals.
31+
const hasTemplateIncompatibleEscape = raw => /(?<=(?:^|[^\\])(?:\\\\)*)\\(?:[1-9]|0\d)/v.test(raw);
32+
33+
// The raw inner content of a literal as it would appear inside a template literal.
34+
function toTemplateElementRaw(node, sourceCode) {
35+
if (node.type === 'TemplateLiteral') {
36+
return sourceCode.getText(node).slice(1, -1);
37+
}
38+
39+
return escapeTemplateElementRaw(node.raw.slice(1, -1));
40+
}
41+
42+
/** @param {import('eslint').Rule.RuleContext} context */
43+
const create = context => {
44+
const {sourceCode} = context;
45+
46+
context.on('BinaryExpression', node => {
47+
const {right} = node;
48+
if (node.operator !== '+' || !isStringish(right)) {
49+
return;
50+
}
51+
52+
// The literal directly to the left of the `+`. For `'a' + 'b'` it's the left operand; for `foo + 'a' + 'b'` it's the right operand of the left `+`.
53+
let left;
54+
if (isStringish(node.left)) {
55+
left = node.left;
56+
} else if (node.left.type === 'BinaryExpression' && node.left.operator === '+' && isStringish(node.left.right)) {
57+
left = node.left.right;
58+
} else {
59+
return;
60+
}
61+
62+
// Allow concatenation spanning multiple lines, it's often used intentionally for readability.
63+
if (sourceCode.getLoc(left).end.line !== sourceCode.getLoc(right).start.line) {
64+
return;
65+
}
66+
67+
// Whether `left` is the right operand of a preceding `+`, as in `foo + 'a' + 'b'`.
68+
const isChain = left !== node.left;
69+
70+
const leftValue = getStringValue(left);
71+
const rightValue = getStringValue(right);
72+
const replacement = leftValue === undefined || rightValue === undefined
73+
? `\`${toTemplateElementRaw(left, sourceCode)}${toTemplateElementRaw(right, sourceCode)}\``
74+
: escapeString(leftValue + rightValue);
75+
76+
const operatorToken = sourceCode.getTokenBefore(right, token => token.type === 'Punctuator' && token.value === '+');
77+
78+
return {
79+
node,
80+
loc: sourceCode.getLoc(operatorToken),
81+
messageId: MESSAGE_ID,
82+
* fix(fixer, {abort}) {
83+
const range = [getParenthesizedRange(left, context)[0], getParenthesizedRange(right, context)[1]];
84+
85+
// Don't drop comments inside the replaced range.
86+
if (sourceCode.getCommentsInside(node).some(comment => {
87+
const [start, end] = sourceCode.getRange(comment);
88+
return start >= range[0] && end <= range[1];
89+
})) {
90+
return abort();
91+
}
92+
93+
// In a chain like `(foo + 'a') + 'b'`, the parentheses around the left side sit inside the replaced range, so folding would drop the closing parenthesis.
94+
if (isChain && isParenthesized(node.left, context)) {
95+
return abort();
96+
}
97+
98+
// In a chain like `foo + 'a' + `${bar()}``, folding moves the right template's expressions before the left side is coerced, changing the order of side effects.
99+
if (isChain && rightValue === undefined) {
100+
return abort();
101+
}
102+
103+
let text = replacement;
104+
105+
// A template literal replacement starts with a backtick, which needs care around what precedes it.
106+
if (text.startsWith('`')) {
107+
// Don't move a string literal's template-incompatible escape into a template literal, it would be a syntax error.
108+
if (hasTemplateIncompatibleEscape(text)) {
109+
return abort();
110+
}
111+
112+
// A backtick after a keyword like `return` would form a tagged template, so keep them apart.
113+
yield fixSpaceAroundKeyword(fixer, node, context);
114+
115+
// A backtick after an expression on a previous line would attach as a tagged template, so add the semicolon that automatic semicolon insertion no longer provides.
116+
if (needsSemicolon(sourceCode.getTokenBefore({range}), context, text)) {
117+
text = `;${text}`;
118+
}
119+
}
120+
121+
yield fixer.replaceTextRange(range, text);
122+
},
123+
};
124+
});
125+
};
126+
127+
/** @type {import('eslint').Rule.RuleModule} */
128+
const config = {
129+
create,
130+
meta: {
131+
type: 'suggestion',
132+
docs: {
133+
description: 'Disallow useless concatenation of literals.',
134+
recommended: 'unopinionated',
135+
},
136+
fixable: 'code',
137+
messages,
138+
languages: [
139+
'js/js',
140+
],
141+
},
142+
};
143+
144+
export default config;

test/no-useless-concat.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/* eslint-disable no-template-curly-in-string */
2+
import outdent from 'outdent';
3+
import {getTester, parsers} from './utils/test.js';
4+
5+
const {test} = getTester(import.meta);
6+
7+
test.snapshot({
8+
valid: [
9+
'\'a\' + b',
10+
'a + \'b\'',
11+
'a + b',
12+
'1 + 2',
13+
'\'a\' + 1',
14+
'1 + \'a\'',
15+
'\'a\' - \'b\'',
16+
// Different lines, often intentional for readability
17+
outdent`
18+
"a" +
19+
"b"
20+
`,
21+
// No two adjacent literals
22+
'a + \'b\' + c',
23+
'\'a\' + b + \'c\'',
24+
// Tagged template on the left is not a string-ish operand
25+
'foo`a` + `b`',
26+
// Not a concatenation
27+
'`a${b}`',
28+
'\'a\'',
29+
// TypeScript: assertions are not literals
30+
{code: '(\'a\' as string) + \'b\'', languageOptions: {parser: parsers.typescript}},
31+
{code: '(\'a\' as const) + \'b\'', languageOptions: {parser: parsers.typescript}},
32+
{code: '(\'a\' satisfies string) + \'b\'', languageOptions: {parser: parsers.typescript}},
33+
],
34+
invalid: [
35+
'\'a\' + \'b\'',
36+
'\'1\' + \'0\'',
37+
'`a` + `b`',
38+
'\'1\' + `0`',
39+
'`1` + \'0\'',
40+
'"a" + "b"',
41+
'\'\' + \'a\'',
42+
// Escaping
43+
String.raw`'a' + 'b\'c'`,
44+
// A trailing escape must not merge with the next operand into a new escape sequence
45+
String.raw`'\\' + 'n'`,
46+
'\'a\' + \'${b}\'',
47+
'`a` + `${b}`',
48+
'"a`b" + "c"',
49+
// Templates with expressions
50+
'`a${x}` + `b`',
51+
'\'a\' + `b${x}`',
52+
'`a${x}` + `b${y}`',
53+
'`a` + `b${x}`',
54+
// Chains
55+
'foo + \'a\' + \'b\'',
56+
'\'a\' + \'b\' + \'c\'',
57+
'\'a\' + \'b\' + c',
58+
'foo + \'a\' + \'b\' + \'c\'',
59+
// Parentheses
60+
'(\'a\') + \'b\'',
61+
'\'a\' + (\'b\')',
62+
'foo + (\'a\') + \'b\'',
63+
// A parenthesized left side in a chain can't be folded without dropping the parenthesis (fix aborts)
64+
'(foo + \'a\') + \'b\'',
65+
// Folding an expression template across a prior addition would reorder side effects (fix aborts)
66+
'foo + \'a\' + `${bar()}`',
67+
'foo + `a` + `b${baz()}`',
68+
// A statement-leading template replacement needs a semicolon to avoid becoming a tagged template
69+
outdent`
70+
foo
71+
'a' + \`b\${x}\`
72+
`,
73+
// Comments are preserved (fix aborts)
74+
'\'a\' /* comment */ + \'b\'',
75+
'\'a\' + /* comment */ \'b\'',
76+
// Legacy octal and `\8`/`\9` escapes are valid in sloppy-mode strings but can't move into a template literal (fix aborts)
77+
{code: '\'\\1\' + `${x}`', languageOptions: {sourceType: 'script'}},
78+
{code: '`${x}` + \'\\012\'', languageOptions: {sourceType: 'script'}},
79+
{code: '\'\\8\' + `${x}`', languageOptions: {sourceType: 'script'}},
80+
// `\0` not followed by a digit is fine in a template literal
81+
'\'\\0\' + `${x}`',
82+
// JSX
83+
{
84+
code: 'const element = <div>{\'a\' + \'b\'}</div>;',
85+
languageOptions: {parserOptions: {ecmaFeatures: {jsx: true}}},
86+
},
87+
// Keyword spacing
88+
'function foo() { return `a` + `b${x}` }',
89+
'function foo() { return \'a\' + \'b\' }',
90+
],
91+
});

0 commit comments

Comments
 (0)