Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] no-invalid-html-attribute: convert autofix to suggestion #3474

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -17,13 +17,15 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`no-unknown-property`]: do not check `fbs` elements ([#3494][] @brianogilvie)
* [`jsx-newline`]: No newline between comments and jsx elements ([#3493][] @justmejulian)
* [`jsx-no-leaked-render`]: Don't report errors on empty strings if React >= v18 ([#3488][] @himanshu007-creator)
* [`no-invalid-html-attribute`]: convert autofix to suggestion ([#3474][] @himanshu007-creator @ljharb)

### Changed
* [Docs] [`jsx-no-leaked-render`]: Remove mentions of empty strings for React 18 ([#3468][] @karlhorky)

[#3494]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3494
[#3493]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3493
[#3488]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3488
[#3474]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3474
[#3471]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3471
[#3468]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3468
[#3461]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3461
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -352,7 +352,7 @@ module.exports = [
| [no-did-update-set-state](docs/rules/no-did-update-set-state.md) | Disallow usage of setState in componentDidUpdate | | | | |
| [no-direct-mutation-state](docs/rules/no-direct-mutation-state.md) | Disallow direct mutation of this.state | 💼 | | | |
| [no-find-dom-node](docs/rules/no-find-dom-node.md) | Disallow usage of findDOMNode | 💼 | | | |
| [no-invalid-html-attribute](docs/rules/no-invalid-html-attribute.md) | Disallow usage of invalid attributes | | 🔧 | | |
| [no-invalid-html-attribute](docs/rules/no-invalid-html-attribute.md) | Disallow usage of invalid attributes | | | 💡 | |
| [no-is-mounted](docs/rules/no-is-mounted.md) | Disallow usage of isMounted | 💼 | | | |
| [no-multi-comp](docs/rules/no-multi-comp.md) | Disallow multiple component definition per file | | | | |
| [no-namespace](docs/rules/no-namespace.md) | Enforce that namespaces are not used in React elements | | | | |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/no-invalid-html-attribute.md
@@ -1,6 +1,6 @@
# Disallow usage of invalid attributes (`react/no-invalid-html-attribute`)

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).

<!-- end auto-generated rule header -->

Expand Down
14 changes: 5 additions & 9 deletions lib/rules/hook-use-state.js
Expand Up @@ -114,16 +114,12 @@ module.exports = {
getMessageData('suggestPair', messages.suggestPair),
{
fix(fixer) {
if (expectedSetterVariableNames.length === 0) {
return;
if (expectedSetterVariableNames.length > 0) {
return fixer.replaceTextRange(
node.parent.id.range,
`[${valueVariableName}, ${expectedSetterVariableNames[0]}]`
);
}

const fix = fixer.replaceTextRange(
node.parent.id.range,
`[${valueVariableName}, ${expectedSetterVariableNames[0]}]`
);

return fix;
},
}
),
Expand Down
124 changes: 85 additions & 39 deletions lib/rules/no-invalid-html-attribute.js
Expand Up @@ -8,6 +8,7 @@
const matchAll = require('string.prototype.matchall');
const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const getMessageData = require('../util/message');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -232,6 +233,11 @@ const messages = {
onlyMeaningfulFor: 'The ”{{attributeName}}“ attribute only has meaning on the tags: {{tagNames}}',
onlyStrings: '“{{attributeName}}” attribute only supports strings.',
spaceDelimited: '”{{attributeName}}“ attribute values should be space delimited.',
suggestRemoveDefault: '"remove {{attributeName}}"',
suggestRemoveEmpty: '"remove empty attribute {{attributeName}}"',
suggestRemoveInvalid: '“remove invalid attribute {{reportingValue}}”',
suggestRemoveWhitespaces: 'remove whitespaces in “{{reportingValue}}”',
suggestRemoveNonString: 'remove non-string value in “{{reportingValue}}”',
};

function splitIntoRangedParts(node, regex) {
Expand All @@ -254,9 +260,12 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
report(context, messages.onlyStrings, 'onlyStrings', {
node,
data: { attributeName },
fix(fixer) {
return fixer.remove(parentNode);
},
suggest: [
Object.assign(
getMessageData('suggestRemoveNonString', messages.suggestRemoveNonString),
{ fix(fixer) { return fixer.remove(parentNode); } }
),
],
});
return;
}
Expand All @@ -265,9 +274,12 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
report(context, messages.noEmpty, 'noEmpty', {
node,
data: { attributeName },
fix(fixer) {
return fixer.remove(parentNode);
},
suggest: [
Object.assign(
getMessageData('suggestRemoveEmpty', messages.suggestRemoveEmpty),
{ fix(fixer) { return fixer.remove(node.parent); } }
),
],
});
return;
}
Expand All @@ -276,16 +288,23 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
for (const singlePart of singleAttributeParts) {
const allowedTags = VALID_VALUES.get(attributeName).get(singlePart.value);
const reportingValue = singlePart.reportingValue;

const suggest = [
Object.assign(
getMessageData('suggestRemoveInvalid', messages.suggestRemoveInvalid),
{ fix(fixer) { return fixer.removeRange(singlePart.range); } }
),
];

if (!allowedTags) {
const data = {
attributeName,
reportingValue,
};
report(context, messages.neverValid, 'neverValid', {
node,
data: {
attributeName,
reportingValue,
},
fix(fixer) {
return fixer.removeRange(singlePart.range);
},
data,
suggest,
});
} else if (!allowedTags.has(parentNodeName)) {
report(context, messages.notValidFor, 'notValidFor', {
Expand All @@ -295,9 +314,7 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
reportingValue,
elementName: parentNodeName,
},
fix(fixer) {
return fixer.removeRange(singlePart.range);
},
suggest,
});
}
}
Expand All @@ -324,6 +341,7 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
secondValue,
missingValue: Array.from(siblings).join(', '),
},
suggest: false,
});
}
}
Expand All @@ -337,17 +355,23 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
report(context, messages.spaceDelimited, 'spaceDelimited', {
node,
data: { attributeName },
fix(fixer) {
return fixer.removeRange(whitespacePart.range);
},
suggest: [
Object.assign(
getMessageData('suggestRemoveWhitespaces', messages.suggestRemoveWhitespaces),
{ fix(fixer) { return fixer.removeRange(whitespacePart.range); } }
),
],
});
} else if (whitespacePart.value !== '\u0020') {
report(context, messages.spaceDelimited, 'spaceDelimited', {
node,
data: { attributeName },
fix(fixer) {
return fixer.replaceTextRange(whitespacePart.range, '\u0020');
},
suggest: [
Object.assign(
getMessageData('suggestRemoveWhitespaces', messages.suggestRemoveWhitespaces),
{ fix(fixer) { return fixer.replaceTextRange(whitespacePart.range, '\u0020'); } }
),
],
});
}
}
Expand All @@ -358,10 +382,6 @@ const DEFAULT_ATTRIBUTES = ['rel'];
function checkAttribute(context, node) {
const attribute = node.name.name;

function fix(fixer) {
return fixer.remove(node);
}

const parentNodeName = node.parent.name.name;
if (!COMPONENT_ATTRIBUTE_MAP.has(attribute) || !COMPONENT_ATTRIBUTE_MAP.get(attribute).has(parentNodeName)) {
const tagNames = Array.from(
Expand All @@ -374,16 +394,28 @@ function checkAttribute(context, node) {
attributeName: attribute,
tagNames,
},
fix,
suggest: [
Object.assign(
getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
{ fix(fixer) { return fixer.remove(node); } }
),
],
});
return;
}

function fix(fixer) { return fixer.remove(node); }

if (!node.value) {
report(context, messages.emptyIsMeaningless, 'emptyIsMeaningless', {
node,
data: { attributeName: attribute },
fix,
suggest: [
Object.assign(
getMessageData('suggestRemoveEmpty', messages.suggestRemoveEmpty),
{ fix }
),
],
});
return;
}
Expand All @@ -404,16 +436,23 @@ function checkAttribute(context, node) {
report(context, messages.onlyStrings, 'onlyStrings', {
node,
data: { attributeName: attribute },
fix,
suggest: [
Object.assign(
getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
{ fix }
),
],
});
return;
}

if (node.value.expression.type === 'Identifier' && node.value.expression.name === 'undefined') {
} else if (node.value.expression.type === 'Identifier' && node.value.expression.name === 'undefined') {
report(context, messages.onlyStrings, 'onlyStrings', {
node,
data: { attributeName: attribute },
fix,
suggest: [
Object.assign(
getMessageData('suggestRemoveDefault', messages.suggestRemoveDefault),
{ fix }
),
],
});
}
}
Expand Down Expand Up @@ -441,18 +480,22 @@ function checkPropValidValue(context, node, value, attribute) {
attributeName: attribute,
reportingValue: value.value,
},
suggest: [
Object.assign(
getMessageData('suggestRemoveInvalid', messages.suggestRemoveInvalid),
{ fix(fixer) { return fixer.replaceText(value, value.raw.replace(value.value, '')); } }
),
],
});
return;
}

if (!validTagSet.has(node.arguments[0].value)) {
} else if (!validTagSet.has(node.arguments[0].value)) {
report(context, messages.notValidFor, 'notValidFor', {
node: value,
data: {
attributeName: attribute,
reportingValue: value.raw,
elementName: node.arguments[0].value,
},
suggest: false,
});
}
}
Expand Down Expand Up @@ -493,6 +536,7 @@ function checkCreateProps(context, node, attribute) {
attributeName: attribute,
tagNames,
},
suggest: false,
});

// eslint-disable-next-line no-continue
Expand All @@ -505,6 +549,7 @@ function checkCreateProps(context, node, attribute) {
data: {
attributeName: attribute,
},
suggest: false,
});

// eslint-disable-next-line no-continue
Expand All @@ -531,7 +576,6 @@ function checkCreateProps(context, node, attribute) {

module.exports = {
meta: {
fixable: 'code',
docs: {
description: 'Disallow usage of invalid attributes',
category: 'Possible Errors',
Expand All @@ -545,6 +589,8 @@ module.exports = {
enum: ['rel'],
},
}],
type: 'suggestion',
hasSuggestions: true, // eslint-disable-line eslint-plugin/require-meta-has-suggestions
},

create(context) {
Expand Down
2 changes: 2 additions & 0 deletions tests/helpers/parsers.js
Expand Up @@ -174,6 +174,8 @@ const parsers = {
tsNew ? addComment(Object.assign({}, test, { parser: parsers['@TYPESCRIPT_ESLINT'] }), '@typescript-eslint/parser') : []
);
});

// console.log(require('util').inspect(t, { depth: null }));
return t;
},
};
Expand Down