Skip to content

Commit

Permalink
[Fix] no-invalid-html-attribute: convert autofix to suggestion
Browse files Browse the repository at this point in the history 
Fixes #3458.

Co-authored-by: himanshu007-creator <addyjeridiq@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
  • Loading branch information
@himanshu007-creator @ljharb
himanshu007-creator and ljharb committed Oct 27, 2022
1 parent 01ab399 commit a522476
Show file tree
Hide file tree
Showing 7 changed files with 605 additions and 126 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
@@ -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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit a522476

Please sign in to comment.