Skip to content

Commit 61234af

Browse files
fiskerfisker
authored
prefer-dom-node-dataset: Remove broken fix for element.setAttribute (#2169)
1 parent ca837a8 commit 61234af

6 files changed

+134
-37
lines changed

‎rules/prefer-dom-node-dataset.js

+49-37
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22
const {isIdentifierName} = require('@babel/helper-validator-identifier');
3-
const {escapeString} = require('./utils/index.js');
3+
const {escapeString, hasOptionalChainElement} = require('./utils/index.js');
44
const {isMethodCall, isStringLiteral} = require('./ast/index.js');
55

66
const MESSAGE_ID = 'prefer-dom-node-dataset';
@@ -10,50 +10,29 @@ const messages = {
1010

1111
const dashToCamelCase = string => string.replace(/-[a-z]/g, s => s[1].toUpperCase());
1212

13-
/** @param {import('eslint').Rule.RuleContext} context */
14-
const create = context => ({
15-
CallExpression(node) {
16-
if (!(
17-
(
18-
isMethodCall(node, {
19-
method: 'setAttribute',
20-
argumentsLength: 2,
21-
optionalCall: false,
22-
optionalMember: false,
23-
})
24-
|| isMethodCall(node, {
25-
methods: ['getAttribute', 'removeAttribute', 'hasAttribute'],
26-
argumentsLength: 1,
27-
optionalCall: false,
28-
optionalMember: false,
29-
})
30-
)
31-
&& isStringLiteral(node.arguments[0])
32-
)) {
33-
return;
34-
}
13+
function getFix(callExpression, context) {
14+
const method = callExpression.callee.property.name;
3515

36-
const [nameNode] = node.arguments;
37-
const attributeName = nameNode.value.toLowerCase();
38-
39-
if (!attributeName.startsWith('data-')) {
40-
return;
41-
}
42-
43-
const method = node.callee.property.name;
44-
const name = dashToCamelCase(attributeName.slice(5));
16+
// `foo?.bar = ''` is invalid
17+
// TODO: Remove this restriction if https://github.com/nicolo-ribaudo/ecma262/pull/4 get merged
18+
if (method === 'setAttribute' && hasOptionalChainElement(callExpression.callee)) {
19+
return;
20+
}
4521

22+
return fixer => {
23+
const [nameNode] = callExpression.arguments;
24+
const name = dashToCamelCase(nameNode.value.toLowerCase().slice(5));
4625
const {sourceCode} = context;
4726
let text = '';
48-
const datasetText = `${sourceCode.getText(node.callee.object)}.dataset`;
27+
const datasetText = `${sourceCode.getText(callExpression.callee.object)}.dataset`;
4928
switch (method) {
5029
case 'setAttribute':
5130
case 'getAttribute':
5231
case 'removeAttribute': {
5332
text = isIdentifierName(name) ? `.${name}` : `[${escapeString(name, nameNode.raw.charAt(0))}]`;
5433
text = `${datasetText}${text}`;
5534
if (method === 'setAttribute') {
56-
text += ` = ${sourceCode.getText(node.arguments[1])}`;
35+
text += ` = ${sourceCode.getText(callExpression.arguments[1])}`;
5736
} else if (method === 'removeAttribute') {
5837
text = `delete ${text}`;
5938
}
@@ -72,11 +51,44 @@ const create = context => ({
7251
// No default
7352
}
7453

54+
return fixer.replaceText(callExpression, text);
55+
};
56+
}
57+
58+
/** @param {import('eslint').Rule.RuleContext} context */
59+
const create = context => ({
60+
CallExpression(callExpression) {
61+
if (!(
62+
(
63+
isMethodCall(callExpression, {
64+
method: 'setAttribute',
65+
argumentsLength: 2,
66+
optionalCall: false,
67+
optionalMember: false,
68+
})
69+
|| isMethodCall(callExpression, {
70+
methods: ['getAttribute', 'removeAttribute', 'hasAttribute'],
71+
argumentsLength: 1,
72+
optionalCall: false,
73+
optionalMember: false,
74+
})
75+
)
76+
&& isStringLiteral(callExpression.arguments[0])
77+
)) {
78+
return;
79+
}
80+
81+
const attributeName = callExpression.arguments[0].value.toLowerCase();
82+
83+
if (!attributeName.startsWith('data-')) {
84+
return;
85+
}
86+
7587
return {
76-
node,
88+
node: callExpression,
7789
messageId: MESSAGE_ID,
78-
data: {method},
79-
fix: fixer => fixer.replaceText(node, text),
90+
data: {method: callExpression.callee.property.name},
91+
fix: getFix(callExpression, context),
8092
};
8193
},
8294
});

‎rules/utils/has-optional-chain-element.js

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
3+
const isChainElement = node => node.type === 'MemberExpression' || node.type === 'CallExpression';
4+
5+
function hasOptionalChainElement(node) {
6+
if (!isChainElement(node)) {
7+
return false;
8+
}
9+
10+
if (node.optional) {
11+
return true;
12+
}
13+
14+
if (node.type === 'MemberExpression') {
15+
return hasOptionalChainElement(node.object);
16+
}
17+
18+
return false;
19+
}
20+
21+
module.exports = hasOptionalChainElement;

‎rules/utils/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ module.exports = {
2525
getReferences: require('./get-references.js'),
2626
getScopes: require('./get-scopes.js'),
2727
getVariableIdentifiers: require('./get-variable-identifiers.js'),
28+
hasOptionalChainElement: require('./has-optional-chain-element.js'),
2829
isArrayPrototypeProperty,
2930
isBooleanNode,
3031
isFunctionSelfUsedInside: require('./is-function-self-used-inside.js'),

‎test/prefer-dom-node-dataset.mjs

+5
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ test.snapshot({
5252
'element.setAttribute("DATA--FOO", "🦄");',
5353
'element.setAttribute("DATA- ", "🦄");',
5454
'element.setAttribute("DATA-Foo-bar", "🦄");',
55+
// Not fixable
56+
'optional?.element.setAttribute("data-unicorn", "🦄");',
5557
],
5658
});
5759

@@ -101,6 +103,7 @@ test.snapshot({
101103
'element.removeAttribute("data-foo");',
102104
'element.querySelector("#selector").removeAttribute("data-AllowAccess");',
103105
'element.removeAttribute("data-");',
106+
'optional?.element.removeAttribute("data-unicorn");',
104107
],
105108
});
106109

@@ -152,6 +155,7 @@ test.snapshot({
152155
'element.hasAttribute("data-foo-bar");',
153156
'element.hasAttribute("data-foo");',
154157
'element.querySelector("#selector").hasAttribute("data-AllowAccess");',
158+
'optional?.element.hasAttribute("data-unicorn");',
155159
],
156160
});
157161

@@ -199,5 +203,6 @@ test.snapshot({
199203
'element.getAttribute("data-foo-bar");',
200204
'element.getAttribute("data-foo");',
201205
'element.querySelector("#selector").getAttribute("data-AllowAccess");',
206+
'optional?.element.getAttribute("data-unicorn");',
202207
],
203208
});

‎test/snapshots/prefer-dom-node-dataset.mjs.md

+58
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,16 @@ Generated by [AVA](https://avajs.dev).
269269
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`setAttribute(…)\`.␊
270270
`
271271

272+
## Invalid #17
273+
1 | optional?.element.setAttribute("data-unicorn", "🦄");
274+
275+
> Error 1/1
276+
277+
`␊
278+
> 1 | optional?.element.setAttribute("data-unicorn", "🦄");␊
279+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`setAttribute(…)\`.␊
280+
`
281+
272282
## Invalid #1
273283
1 | element.removeAttribute(
274284
2 | "data-foo", // comment
@@ -499,6 +509,22 @@ Generated by [AVA](https://avajs.dev).
499509
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`removeAttribute(…)\`.␊
500510
`
501511

512+
## Invalid #15
513+
1 | optional?.element.removeAttribute("data-unicorn");
514+
515+
> Output
516+
517+
`␊
518+
1 | delete optional?.element.dataset.unicorn;␊
519+
`
520+
521+
> Error 1/1
522+
523+
`␊
524+
> 1 | optional?.element.removeAttribute("data-unicorn");␊
525+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`removeAttribute(…)\`.␊
526+
`
527+
502528
## Invalid #1
503529
1 | element.hasAttribute(
504530
2 | "data-foo", // comment
@@ -713,6 +739,22 @@ Generated by [AVA](https://avajs.dev).
713739
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`hasAttribute(…)\`.␊
714740
`
715741

742+
## Invalid #14
743+
1 | optional?.element.hasAttribute("data-unicorn");
744+
745+
> Output
746+
747+
`␊
748+
1 | Object.hasOwn(optional?.element.dataset, "unicorn");␊
749+
`
750+
751+
> Error 1/1
752+
753+
`␊
754+
> 1 | optional?.element.hasAttribute("data-unicorn");␊
755+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`hasAttribute(…)\`.␊
756+
`
757+
716758
## Invalid #1
717759
1 | element.getAttribute(
718760
2 | "data-foo", // comment
@@ -926,3 +968,19 @@ Generated by [AVA](https://avajs.dev).
926968
> 1 | element.querySelector("#selector").getAttribute("data-AllowAccess");␊
927969
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`getAttribute(…)\`.␊
928970
`
971+
972+
## Invalid #14
973+
1 | optional?.element.getAttribute("data-unicorn");
974+
975+
> Output
976+
977+
`␊
978+
1 | optional?.element.dataset.unicorn;␊
979+
`
980+
981+
> Error 1/1
982+
983+
`␊
984+
> 1 | optional?.element.getAttribute("data-unicorn");␊
985+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`getAttribute(…)\`.␊
986+
`

‎test/snapshots/prefer-dom-node-dataset.mjs.snap

194 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)