prefer-dom-node-dataset: Remove broken fix for element.setAttribute

rules/prefer-dom-node-dataset.js

@@ -1,6 +1,6 @@
 'use strict';
 const {isIdentifierName} = require('@babel/helper-validator-identifier');
-const {escapeString} = require('./utils/index.js');
+const {escapeString, hasOptionalChainElement} = require('./utils/index.js');
 const {isMethodCall, isStringLiteral} = require('./ast/index.js');
 
 const MESSAGE_ID = 'prefer-dom-node-dataset';
@@ -10,50 +10,29 @@ const messages = {
 
 const dashToCamelCase = string => string.replace(/-[a-z]/g, s => s[1].toUpperCase());
 
-/** @param {import('eslint').Rule.RuleContext} context */
-const create = context => ({
-	CallExpression(node) {
-		if (!(
-			(
-				isMethodCall(node, {
-					method: 'setAttribute',
-					argumentsLength: 2,
-					optionalCall: false,
-					optionalMember: false,
-				})
-				|| isMethodCall(node, {
-					methods: ['getAttribute', 'removeAttribute', 'hasAttribute'],
-					argumentsLength: 1,
-					optionalCall: false,
-					optionalMember: false,
-				})
-			)
-			&& isStringLiteral(node.arguments[0])
-		)) {
-			return;
-		}
+function getFix(callExpression, context) {
+	const method = callExpression.callee.property.name;
 
-		const [nameNode] = node.arguments;
-		const attributeName = nameNode.value.toLowerCase();
-
-		if (!attributeName.startsWith('data-')) {
-			return;
-		}
-
-		const method = node.callee.property.name;
-		const name = dashToCamelCase(attributeName.slice(5));
+	// `foo?.bar = ''` is invalid
+	// TODO: Remove this restriction if https://github.com/nicolo-ribaudo/ecma262/pull/4 get merged
+	if (method === 'setAttribute' && hasOptionalChainElement(callExpression.callee)) {
+		return;
+	}
 
+	return fixer => {
+		const [nameNode] = callExpression.arguments;
+		const name = dashToCamelCase(nameNode.value.toLowerCase().slice(5));
 		const {sourceCode} = context;
 		let text = '';
-		const datasetText = `${sourceCode.getText(node.callee.object)}.dataset`;
+		const datasetText = `${sourceCode.getText(callExpression.callee.object)}.dataset`;
 		switch (method) {
 			case 'setAttribute':
 			case 'getAttribute':
 			case 'removeAttribute': {
 				text = isIdentifierName(name) ? `.${name}` : `[${escapeString(name, nameNode.raw.charAt(0))}]`;
 				text = `${datasetText}${text}`;
 				if (method === 'setAttribute') {
-					text += ` = ${sourceCode.getText(node.arguments[1])}`;
+					text += ` = ${sourceCode.getText(callExpression.arguments[1])}`;
 				} else if (method === 'removeAttribute') {
 					text = `delete ${text}`;
 				}
@@ -72,11 +51,44 @@ const create = context => ({
 			// No default
 		}
 
+		return fixer.replaceText(callExpression, text);
+	};
+}
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => ({
+	CallExpression(callExpression) {
+		if (!(
+			(
+				isMethodCall(callExpression, {
+					method: 'setAttribute',
+					argumentsLength: 2,
+					optionalCall: false,
+					optionalMember: false,
+				})
+				|| isMethodCall(callExpression, {
+					methods: ['getAttribute', 'removeAttribute', 'hasAttribute'],
+					argumentsLength: 1,
+					optionalCall: false,
+					optionalMember: false,
+				})
+			)
+			&& isStringLiteral(callExpression.arguments[0])
+		)) {
+			return;
+		}
+
+		const attributeName = callExpression.arguments[0].value.toLowerCase();
+
+		if (!attributeName.startsWith('data-')) {
+			return;
+		}
+
 		return {
-			node,
+			node: callExpression,
 			messageId: MESSAGE_ID,
-			data: {method},
-			fix: fixer => fixer.replaceText(node, text),
+			data: {method: callExpression.callee.property.name},
+			fix: getFix(callExpression, context),
 		};
 	},
 });

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

@@ -0,0 +1,21 @@
+'use strict';
+
+const isChainElement = node => node.type === 'MemberExpression' || node.type === 'CallExpression';
+
+function hasOptionalChainElement(node) {
+	if (!isChainElement(node)) {
+		return false;
+	}
+
+	if (node.optional) {
+		return true;
+	}
+
+	if (node.type === 'MemberExpression') {
+		return hasOptionalChainElement(node.object);
+	}
+
+	return false;
+}
+
+module.exports = hasOptionalChainElement;

rules/utils/index.js

@@ -25,6 +25,7 @@ module.exports = {
 	getReferences: require('./get-references.js'),
 	getScopes: require('./get-scopes.js'),
 	getVariableIdentifiers: require('./get-variable-identifiers.js'),
+	hasOptionalChainElement: require('./has-optional-chain-element.js'),
 	isArrayPrototypeProperty,
 	isBooleanNode,
 	isFunctionSelfUsedInside: require('./is-function-self-used-inside.js'),

test/prefer-dom-node-dataset.mjs

@@ -52,6 +52,8 @@ test.snapshot({
 		'element.setAttribute("DATA--FOO", "🦄");',
 		'element.setAttribute("DATA- ", "🦄");',
 		'element.setAttribute("DATA-Foo-bar", "🦄");',
+		// Not fixable
+		'optional?.element.setAttribute("data-unicorn", "🦄");',
 	],
 });
 
@@ -101,6 +103,7 @@ test.snapshot({
 		'element.removeAttribute("data-foo");',
 		'element.querySelector("#selector").removeAttribute("data-AllowAccess");',
 		'element.removeAttribute("data-");',
+		'optional?.element.removeAttribute("data-unicorn");',
 	],
 });
 
@@ -152,6 +155,7 @@ test.snapshot({
 		'element.hasAttribute("data-foo-bar");',
 		'element.hasAttribute("data-foo");',
 		'element.querySelector("#selector").hasAttribute("data-AllowAccess");',
+		'optional?.element.hasAttribute("data-unicorn");',
 	],
 });
 
@@ -199,5 +203,6 @@ test.snapshot({
 		'element.getAttribute("data-foo-bar");',
 		'element.getAttribute("data-foo");',
 		'element.querySelector("#selector").getAttribute("data-AllowAccess");',
+		'optional?.element.getAttribute("data-unicorn");',
 	],
 });

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

@@ -269,6 +269,16 @@ Generated by [AVA](https://avajs.dev).
         | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`setAttribute(…)\`.␊
     `
 
+## Invalid #17
+      1 | optional?.element.setAttribute("data-unicorn", "🦄");
+
+> Error 1/1
+
+    `␊
+    > 1 | optional?.element.setAttribute("data-unicorn", "🦄");␊
+        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`setAttribute(…)\`.␊
+    `
+
 ## Invalid #1
       1 | element.removeAttribute(
       2 | 	"data-foo", // comment
@@ -499,6 +509,22 @@ Generated by [AVA](https://avajs.dev).
         | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`removeAttribute(…)\`.␊
     `
 
+## Invalid #15
+      1 | optional?.element.removeAttribute("data-unicorn");
+
+> Output
+
+    `␊
+      1 | delete optional?.element.dataset.unicorn;␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | optional?.element.removeAttribute("data-unicorn");␊
+        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`removeAttribute(…)\`.␊
+    `
+
 ## Invalid #1
       1 | element.hasAttribute(
       2 | 	"data-foo", // comment
@@ -713,6 +739,22 @@ Generated by [AVA](https://avajs.dev).
         | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`hasAttribute(…)\`.␊
     `
 
+## Invalid #14
+      1 | optional?.element.hasAttribute("data-unicorn");
+
+> Output
+
+    `␊
+      1 | Object.hasOwn(optional?.element.dataset, "unicorn");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | optional?.element.hasAttribute("data-unicorn");␊
+        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`hasAttribute(…)\`.␊
+    `
+
 ## Invalid #1
       1 | element.getAttribute(
       2 | 	"data-foo", // comment
@@ -926,3 +968,19 @@ Generated by [AVA](https://avajs.dev).
     > 1 | element.querySelector("#selector").getAttribute("data-AllowAccess");␊
         | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`getAttribute(…)\`.␊
     `
+
+## Invalid #14
+      1 | optional?.element.getAttribute("data-unicorn");
+
+> Output
+
+    `␊
+      1 | optional?.element.dataset.unicorn;␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | optional?.element.getAttribute("data-unicorn");␊
+        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer \`.dataset\` over \`getAttribute(…)\`.␊
+    `