Skip to content

Commit e0af60b

Browse files
Primajinljharb
authored andcommitted
[Fix] no-invalid-html-attribute: allow 'shortcut icon' on link
Fixes #3172
1 parent 4c3d00f commit e0af60b

File tree

3 files changed

+149
-7
lines changed

3 files changed

+149
-7
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
1010

1111
### Fixed
1212
* [`prop-types`], `propTypes`: add support for exported type inference ([#3163][] @vedadeepta)
13+
* [`no-invalid-html-attribute`]: allow 'shortcut icon' on `link` ([#3174][] @Primajin)
1314

1415
### Changed
1516
* [readme] change [`jsx-runtime`] link from branch to sha ([#3160][] @tatsushitoji)
1617
* [Docs] HTTP => HTTPS ([#3133][] @Schweinepriester)
1718

19+
[#3174]: https://github.com/yannickcr/eslint-plugin-react/pull/3174
1820
[#3163]: https://github.com/yannickcr/eslint-plugin-react/pull/3163
1921
[#3160]: https://github.com/yannickcr/eslint-plugin-react/pull/3160
2022
[#3133]: https://github.com/yannickcr/eslint-plugin-react/pull/3133

lib/rules/no-invalid-html-attribute.js

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,16 @@ const rel = new Map([
3939
['prerender', new Set(['link'])],
4040
['prev', new Set(['link', 'area', 'a', 'form'])],
4141
['search', new Set(['link', 'area', 'a', 'form'])],
42+
['shortcut', new Set(['link'])], // generally allowed but needs pair with "icon"
43+
['shortcut\u0020icon', new Set(['link'])],
4244
['stylesheet', new Set(['link'])],
4345
['tag', new Set(['area', 'a'])],
4446
]);
4547

48+
const pairs = new Map([
49+
['shortcut', new Set(['icon'])],
50+
]);
51+
4652
/**
4753
* Map between attributes and a mapping between valid values and a set of tags they are valid on
4854
* @type {Map<string, Map<string, Set<string>>>}
@@ -51,6 +57,14 @@ const VALID_VALUES = new Map([
5157
['rel', rel],
5258
]);
5359

60+
/**
61+
* Map between attributes and a mapping between pair-values and a set of values they are valid with
62+
* @type {Map<string, Map<string, Set<string>>>}
63+
*/
64+
const VALID_PAIR_VALUES = new Map([
65+
['rel', pairs],
66+
]);
67+
5468
/**
5569
* The set of all possible HTML elements. Used for skipping custom types
5670
* @type {Set<string>}
@@ -216,6 +230,8 @@ const messages = {
216230
noMethod: 'The ”{{attributeName}}“ attribute cannot be a method.',
217231
onlyMeaningfulFor: 'The ”{{attributeName}}“ attribute only has meaning on the tags: {{tagNames}}',
218232
emptyIsMeaningless: 'An empty “{{attributeName}}” attribute is meaningless.',
233+
notAlone: '“{{reportingValue}}” must be directly followed by “{{missingValue}}”.',
234+
notPaired: '“{{reportingValue}}” can not be directly followed by “{{secondValue}}” without “{{missingValue}}”.',
219235
};
220236

221237
function splitIntoRangedParts(node, regex) {
@@ -256,10 +272,10 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
256272
return;
257273
}
258274

259-
const parts = splitIntoRangedParts(node, /([^\s]+)/g);
260-
for (const part of parts) {
261-
const allowedTags = VALID_VALUES.get(attributeName).get(part.value);
262-
const reportingValue = part.reportingValue;
275+
const singleAttributeParts = splitIntoRangedParts(node, /(\S+)/g);
276+
for (const singlePart of singleAttributeParts) {
277+
const allowedTags = VALID_VALUES.get(attributeName).get(singlePart.value);
278+
const reportingValue = singlePart.reportingValue;
263279
if (!allowedTags) {
264280
report(context, messages.neverValid, 'neverValid', {
265281
node,
@@ -268,7 +284,7 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
268284
reportingValue,
269285
},
270286
fix(fixer) {
271-
return fixer.removeRange(part.range);
287+
return fixer.removeRange(singlePart.range);
272288
},
273289
});
274290
} else if (!allowedTags.has(parentNodeName)) {
@@ -280,22 +296,56 @@ function checkLiteralValueNode(context, attributeName, node, parentNode, parentN
280296
elementName: parentNodeName,
281297
},
282298
fix(fixer) {
283-
return fixer.removeRange(part.range);
299+
return fixer.removeRange(singlePart.range);
284300
},
285301
});
286302
}
287303
}
288304

305+
const allowedPairsForAttribute = VALID_PAIR_VALUES.get(attributeName);
306+
if (allowedPairsForAttribute) {
307+
const pairAttributeParts = splitIntoRangedParts(node, /(?=(\b\S+\s*\S+))/g);
308+
for (const pairPart of pairAttributeParts) {
309+
for (const [pairing, siblings] of allowedPairsForAttribute) {
310+
const attributes = pairPart.reportingValue.split('\u0020');
311+
const [firstValue, secondValue] = attributes;
312+
if (firstValue === pairing) {
313+
const lastValue = attributes[attributes.length - 1]; // in case of multiple white spaces
314+
if (!siblings.has(lastValue)) {
315+
const message = secondValue ? messages.notPaired : messages.notAlone;
316+
const messageId = secondValue ? 'notPaired' : 'notAlone';
317+
report(context, message, messageId, {
318+
node,
319+
data: {
320+
reportingValue: firstValue,
321+
secondValue,
322+
missingValue: [...siblings].join(', '),
323+
},
324+
});
325+
}
326+
}
327+
}
328+
}
329+
}
330+
289331
const whitespaceParts = splitIntoRangedParts(node, /(\s+)/g);
290332
for (const whitespacePart of whitespaceParts) {
291-
if (whitespacePart.value !== ' ' || whitespacePart.range[0] === (node.range[0] + 1) || whitespacePart.range[1] === (node.range[1] - 1)) {
333+
if (whitespacePart.range[0] === (node.range[0] + 1) || whitespacePart.range[1] === (node.range[1] - 1)) {
292334
report(context, messages.spaceDelimited, 'spaceDelimited', {
293335
node,
294336
data: { attributeName },
295337
fix(fixer) {
296338
return fixer.removeRange(whitespacePart.range);
297339
},
298340
});
341+
} else if (whitespacePart.value !== '\u0020') {
342+
report(context, messages.spaceDelimited, 'spaceDelimited', {
343+
node,
344+
data: { attributeName },
345+
fix(fixer) {
346+
return fixer.replaceTextRange(whitespacePart.range, '\u0020');
347+
},
348+
});
299349
}
300350
}
301351
}

tests/lib/rules/no-invalid-html-attribute.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ ruleTester.run('no-invalid-html-attribute', rule, {
130130
{ code: '<link rel="icon"></link>' },
131131
{ code: 'React.createElement("link", { rel: "icon" })' },
132132
{ code: 'React.createElement("link", { rel: ["icon"] })' },
133+
{ code: '<link rel="shortcut icon"></link>' },
134+
{ code: 'React.createElement("link", { rel: "shortcut icon" })' },
135+
{ code: 'React.createElement("link", { rel: ["shortcut icon"] })' },
133136
{ code: '<link rel="license"></link>' },
134137
{ code: 'React.createElement("link", { rel: "license" })' },
135138
{ code: 'React.createElement("link", { rel: ["license"] })' },
@@ -450,6 +453,26 @@ ruleTester.run('no-invalid-html-attribute', rule, {
450453
},
451454
],
452455
},
456+
{
457+
code: '<a rel="noreferrer noopener"></a>',
458+
output: '<a rel="noreferrer noopener"></a>',
459+
errors: [
460+
{
461+
messageId: 'spaceDelimited',
462+
data: { attributeName: 'rel' },
463+
},
464+
],
465+
},
466+
{
467+
code: '<a rel="noreferrer\xa0\xa0noopener"></a>',
468+
output: '<a rel="noreferrer noopener"></a>',
469+
errors: [
470+
{
471+
messageId: 'spaceDelimited',
472+
data: { attributeName: 'rel' },
473+
},
474+
],
475+
},
453476
{
454477
code: '<a rel={"noreferrer noopener foobar"}></a>',
455478
output: '<a rel={"noreferrer noopener "}></a>',
@@ -614,6 +637,73 @@ ruleTester.run('no-invalid-html-attribute', rule, {
614637
},
615638
],
616639
},
640+
{
641+
code: '<link rel="shortcut"></link>',
642+
errors: [
643+
{
644+
messageId: 'notAlone',
645+
data: {
646+
reportingValue: 'shortcut',
647+
missingValue: 'icon',
648+
},
649+
},
650+
],
651+
},
652+
{
653+
code: '<link rel="shortcut foo"></link>',
654+
output: '<link rel="shortcut "></link>',
655+
errors: [
656+
{
657+
messageId: 'neverValid',
658+
data: {
659+
reportingValue: 'foo',
660+
attributeName: 'rel',
661+
},
662+
},
663+
{
664+
messageId: 'notPaired',
665+
data: {
666+
reportingValue: 'shortcut',
667+
secondValue: 'foo',
668+
missingValue: 'icon',
669+
},
670+
},
671+
],
672+
},
673+
{
674+
code: '<link rel="shortcut icon"></link>',
675+
output: '<link rel="shortcut icon"></link>',
676+
errors: [
677+
{
678+
messageId: 'spaceDelimited',
679+
data: { attributeName: 'rel' },
680+
},
681+
],
682+
},
683+
{
684+
code: '<link rel="shortcut foo"></link>',
685+
output: '<link rel="shortcut foo"></link>',
686+
errors: [
687+
{
688+
messageId: 'neverValid',
689+
data: {
690+
reportingValue: 'foo',
691+
attributeName: 'rel',
692+
},
693+
},
694+
{
695+
messageId: 'notAlone',
696+
data: {
697+
reportingValue: 'shortcut',
698+
missingValue: 'icon',
699+
},
700+
},
701+
{
702+
messageId: 'spaceDelimited',
703+
data: { attributeName: 'rel' },
704+
},
705+
],
706+
},
617707
{
618708
code: '<a rel="manifest"></a>',
619709
output: '<a rel=""></a>',

0 commit comments

Comments
 (0)