Repository: jsx-eslint/eslint-plugin-react (js)

Issue:
Make jsx-no-script-url use shared settings to specify link components
Why do we have to specify Link components in `react/jsx-no-script-url` settings when we already have that in shared config? Can't shared config be used here, as it is in `react/jsx-no-target-blank`?

Pull request description:
[jsx-no-script-url]: use shared settings to specify link components
Fixes #2865 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8bd7d075..0aa9588e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,8 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [`jsx-wrap-multilines`]: add `never` option to prohibit wrapping parens on multiline JSX ([#3668][] @reedws)
 * [`jsx-filename-extension`]: add `ignoreFilesWithoutCode` option to allow empty files ([#3674][] @burtek)
 * [`jsx-boolean-value`]: add `assumeUndefinedIsFalse` option ([#3675][] @developer-bandi)
+* `linkAttribute` setting, [`jsx-no-target-blank`]: support multiple properties ([#3673][] @burtek)
+* [`jsx-no-script-url`]: add `includeFromSettings` option to support `linkAttributes` setting ([#3673][] @burtek)
 
 ### Fixed
 * [`jsx-no-leaked-render`]: preserve RHS parens for multiline jsx elements while fixing ([#3623][] @akulsr0)
@@ -32,6 +34,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 
 [#3675]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3675
 [#3674]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3674
+[#3673]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3673
 [#3668]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3668
 [#3666]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3666
 [#3662]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3662
diff --git a/README.md b/README.md
index 0cda1109..9c67ff12 100644
--- a/README.md
+++ b/README.md
@@ -63,12 +63,14 @@ You should also specify settings that will be shared across all the plugin rules
     "formComponents": [
       // Components used as alternatives to <form> for forms, eg. <Form endpoint={ url } />
       "CustomForm",
-      {"name": "Form", "formAttribute": "endpoint"}
+      {"name": "SimpleForm", "formAttribute": "endpoint"},
+      {"name": "Form", "formAttribute": ["registerEndpoint", "loginEndpoint"]}, // allows specifying multiple properties if necessary
     ],
     "linkComponents": [
       // Components used as alternatives to <a> for linking, eg. <Link to={ url } />
       "Hyperlink",
-      {"name": "Link", "linkAttribute": "to"}
+      {"name": "MyLink", "linkAttribute": "to"},
+      {"name": "Link", "linkAttribute": ["to", "href"]}, // allows specifying multiple properties if necessary
     ]
   }
 }
diff --git a/docs/rules/jsx-no-script-url.md b/docs/rules/jsx-no-script-url.md
index 8d4f7c8c..11fffdad 100644
--- a/docs/rules/jsx-no-script-url.md
+++ b/docs/rules/jsx-no-script-url.md
@@ -23,8 +23,14 @@ Examples of **correct** code for this rule:
 <a href={"javascript:"}></a>
 ```
 
+This rule takes the `linkComponents` setting into account.
+
 ## Rule Options
 
+This rule accepts array option (optional) and object option (optional).
+
+### Array option (default `[]`)
+
 ```json
 {
   "react/jsx-no-script-url": [
@@ -45,11 +51,11 @@ Examples of **correct** code for this rule:
 
 Allows you to indicate a specific list of properties used by a custom component to be checked.
 
-### name
+#### name
 
 Component name.
 
-### props
+#### props
 
 List of properties that should be validated.
 
@@ -60,3 +66,37 @@ Examples of **incorrect** code for this rule, when configured with the above opt
 <Foo href="javascript:void(0)"></Foo>
 <Foo to="javascript:void(0)"></Foo>
 ```
+
+### Object option
+
+#### includeFromSettings (default `false`)
+
+Indicates if the `linkComponents` config in [global shared settings](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/README.md#configuration) should also be taken into account. If enabled, components and properties defined in settings will be added to the list provided in first option (if provided):
+
+```json
+{
+  "react/jsx-no-script-url": [
+    "error",
+    [
+      {
+        "name": "Link",
+        "props": ["to"]
+      },
+      {
+        "name": "Foo",
+        "props": ["href", "to"]
+      }
+    ],
+    { "includeFromSettings": true }
+  ]
+}
+```
+
+If only global settings should be used for this rule, the array option can be omitted:
+
+```jsonc
+{
+  // same as ["error", [], { "includeFromSettings": true }]
+  "react/jsx-no-script-url": ["error", { "includeFromSettings": true }]
+}
+```
diff --git a/lib/rules/jsx-no-script-url.js b/lib/rules/jsx-no-script-url.js
index 425741cf..bcf9468a 100644
--- a/lib/rules/jsx-no-script-url.js
+++ b/lib/rules/jsx-no-script-url.js
@@ -5,7 +5,9 @@
 
 'use strict';
 
+const includes = require('array-includes');
 const docsUrl = require('../util/docsUrl');
+const linkComponentsUtil = require('../util/linkComponents');
 const report = require('../util/report');
 
 // ------------------------------------------------------------------------------
@@ -21,26 +23,20 @@ function hasJavaScriptProtocol(attr) {
     && isJavaScriptProtocol.test(attr.value.value);
 }
 
-function shouldVerifyElement(node, config) {
-  const name = node.name && node.name.name;
-  return name === 'a' || config.find((i) => i.name === name);
-}
-
 function shouldVerifyProp(node, config) {
   const name = node.name && node.name.name;
   const parentName = node.parent.name && node.parent.name.name;
 
-  if (parentName === 'a' && name === 'href') {
-    return true;
-  }
+  if (!name || !parentName || !config.has(parentName)) return false;
 
-  const el = config.find((i) => i.name === parentName);
-  if (!el) {
-    return false;
-  }
+  const attributes = config.get(parentName);
+  return includes(attributes, name);
+}
 
-  const props = el.props || [];
-  return node.name && props.indexOf(name) !== -1;
+function parseLegacyOption(config, option) {
+  option.forEach((opt) => {
+    config.set(opt.name, opt.props);
+  });
 }
 
 const messages = {
@@ -58,35 +54,84 @@ module.exports = {
 
     messages,
 
-    schema: [{
-      type: 'array',
-      uniqueItems: true,
-      items: {
-        type: 'object',
-        properties: {
-          name: {
-            type: 'string',
-          },
-          props: {
-            type: 'array',
-            items: {
-              type: 'string',
+    schema: {
+      anyOf: [
+        {
+          type: 'array',
+          items: [
+            {
+              type: 'array',
               uniqueItems: true,
+              items: {
+                type: 'object',
+                properties: {
+                  name: {
+                    type: 'string',
+                  },
+                  props: {
+                    type: 'array',
+                    items: {
+                      type: 'string',
+                      uniqueItems: true,
+                    },
+                  },
+                },
+                required: ['name', 'props'],
+                additionalProperties: false,
+              },
+            },
+            {
+              type: 'object',
+              properties: {
+                includeFromSettings: {
+                  type: 'boolean',
+                },
+              },
+              additionalItems: false,
             },
-          },
+          ],
+          additionalItems: false,
         },
-        required: ['name', 'props'],
-        additionalProperties: false,
-      },
-    }],
+        {
+          type: 'array',
+          items: [
+            {
+              type: 'object',
+              properties: {
+                includeFromSettings: {
+                  type: 'boolean',
+                },
+              },
+              additionalItems: false,
+            },
+          ],
+          additionalItems: false,
+        },
+      ],
+    },
   },
 
   create(context) {
-    const config = context.options[0] || [];
+    const options = context.options;
+    const hasLegacyOption = Array.isArray(options[0]);
+    const legacyOptions = hasLegacyOption ? options[0] : [];
+    // eslint-disable-next-line no-nested-ternary
+    const objectOption = (hasLegacyOption && options.length > 1)
+      ? options[1]
+      : (options.length > 0
+        ? options[0]
+        : {
+          includeFromSettings: false,
+        }
+      );
+    const includeFromSettings = objectOption.includeFromSettings;
+
+    const linkComponents = linkComponentsUtil.getLinkComponents(includeFromSettings ? context : {});
+    parseLegacyOption(linkComponents, legacyOptions);
+
     return {
       JSXAttribute(node) {
-        const parent = node.parent;
-        if (shouldVerifyElement(parent, config) && shouldVerifyProp(node, config) && hasJavaScriptProtocol(node)) {
+        if (shouldVerifyProp(node, linkComponents) && hasJavaScriptProtocol(node)) {
           report(context, messages.noScriptURL, 'noScriptURL', {
             node,
           });
diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js
index 9b77e37f..795de8a7 100644
--- a/lib/rules/jsx-no-target-blank.js
+++ b/lib/rules/jsx-no-target-blank.js
@@ -5,6 +5,7 @@
 
 'use strict';
 
+const includes = require('array-includes');
 const docsUrl = require('../util/docsUrl');
 const linkComponentsUtil = require('../util/linkComponents');
 const report = require('../util/report');
@@ -48,16 +49,16 @@ function attributeValuePossiblyBlank(attribute) {
   return false;
 }
 
-function hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex) {
-  const linkIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === linkAttribute);
+function hasExternalLink(node, linkAttributes, warnOnSpreadAttributes, spreadAttributeIndex) {
+  const linkIndex = findLastIndex(node.attributes, (attr) => attr.name && includes(linkAttributes, attr.name.name));
   const foundExternalLink = linkIndex !== -1 && ((attr) => attr.value && attr.value.type === 'Literal' && /^(?:\w+:|\/\/)/.test(attr.value.value))(
     node.attributes[linkIndex]);
   return foundExternalLink || (warnOnSpreadAttributes && linkIndex < spreadAttributeIndex);
 }
 
-function hasDynamicLink(node, linkAttribute) {
+function hasDynamicLink(node, linkAttributes) {
   const dynamicLinkIndex = findLastIndex(node.attributes, (attr) => attr.name
-    && attr.name.name === linkAttribute
+    && includes(linkAttributes, attr.name.name)
     && attr.value
     && attr.value.type === 'JSXExpressionContainer');
   if (dynamicLinkIndex !== -1) {
@@ -194,9 +195,9 @@ module.exports = {
             }
           }
 
-          const linkAttribute = linkComponents.get(node.name.name);
-          const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex)
-            || (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute));
+          const linkAttributes = linkComponents.get(node.name.name);
+          const hasDangerousLink = hasExternalLink(node, linkAttributes, warnOnSpreadAttributes, spreadAttributeIndex)
+            || (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttributes));
           if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) {
             const messageId = allowReferrer ? 'noTargetBlankWithoutNoopener' : 'noTargetBlankWithoutNoreferrer';
             const relValue = allowReferrer ? 'noopener' : 'noreferrer';
@@ -265,11 +266,11 @@ module.exports = {
             return;
           }
 
-          const formAttribute = formComponents.get(node.name.name);
+          const formAttributes = formComponents.get(node.name.name);
 
           if (
-            hasExternalLink(node, formAttribute)
-            || (enforceDynamicLinks === 'always' && hasDynamicLink(node, formAttribute))
+            hasExternalLink(node, formAttributes)
+            || (enforceDynamicLinks === 'always' && hasDynamicLink(node, formAttributes))
           ) {
             const messageId = allowReferrer ? 'noTargetBlankWithoutNoopener' : 'noTargetBlankWithoutNoreferrer';
             report(context, messages[messageId], messageId, {
diff --git a/lib/util/linkComponents.js b/lib/util/linkComponents.js
index 46d9e0e2..181ed377 100644
--- a/lib/util/linkComponents.js
+++ b/lib/util/linkComponents.js
@@ -24,9 +24,9 @@ function getFormComponents(context) {
   );
   return new Map(map(iterFrom(formComponents), (value) => {
     if (typeof value === 'string') {
-      return [value, DEFAULT_FORM_ATTRIBUTE];
+      return [value, [DEFAULT_FORM_ATTRIBUTE]];
     }
-    return [value.name, value.formAttribute];
+    return [value.name, [].concat(value.formAttribute)];
   }));
 }
 
@@ -37,9 +37,9 @@ function getLinkComponents(context) {
   );
   return new Map(map(iterFrom(linkComponents), (value) => {
     if (typeof value === 'string') {
-      return [value, DEFAULT_LINK_ATTRIBUTE];
+      return [value, [DEFAULT_LINK_ATTRIBUTE]];
     }
-    return [value.name, value.linkAttribute];
+    return [value.name, [].concat(value.linkAttribute)];
   }));
 }
 


Test patch:
diff --git a/tests/lib/rules/jsx-no-script-url.js b/tests/lib/rules/jsx-no-script-url.js
index 24cff790..4d0374e0 100644
--- a/tests/lib/rules/jsx-no-script-url.js
+++ b/tests/lib/rules/jsx-no-script-url.js
@@ -38,8 +38,22 @@ ruleTester.run('jsx-no-script-url', rule, {
     { code: '<a href={"javascript:"}></a>' },
     { code: '<Foo href="javascript:"></Foo>' },
     { code: '<a href />' },
+    {
+      code: '<Foo href="javascript:"></Foo>',
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: ['to', 'href'] }],
+      },
+    },
+    {
+      code: '<Foo href="javascript:"></Foo>',
+      options: [[], { includeFromSettings: false }],
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: ['to', 'href'] }],
+      },
+    },
   ]),
   invalid: parsers.all([
+    // defaults
     {
       code: '<a href="javascript:"></a>',
       errors: [{ messageId: 'noScriptURL' }],
@@ -52,6 +66,8 @@ ruleTester.run('jsx-no-script-url', rule, {
       code: '<a href="j\n\n\na\rv\tascript:"></a>',
       errors: [{ messageId: 'noScriptURL' }],
     },
+
+    // with component passed by options
     {
       code: '<Foo to="javascript:"></Foo>',
       errors: [{ messageId: 'noScriptURL' }],
@@ -66,6 +82,34 @@ ruleTester.run('jsx-no-script-url', rule, {
         [{ name: 'Foo', props: ['to', 'href'] }],
       ],
     },
+    { // make sure it still uses defaults when passed options
+      code: '<a href="javascript:void(0)"></a>',
+      errors: [{ messageId: 'noScriptURL' }],
+      options: [
+        [{ name: 'Foo', props: ['to', 'href'] }],
+      ],
+    },
+
+    // with components passed by settings
+    {
+      code: '<Foo to="javascript:"></Foo>',
+      errors: [{ messageId: 'noScriptURL' }],
+      options: [
+        [{ name: 'Bar', props: ['to', 'href'] }],
+        { includeFromSettings: true },
+      ],
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: 'to' }],
+      },
+    },
+    {
+      code: '<Foo href="javascript:"></Foo>',
+      errors: [{ messageId: 'noScriptURL' }],
+      options: [{ includeFromSettings: true }],
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: ['to', 'href'] }],
+      },
+    },
     {
       code: `
       <div>
@@ -78,11 +122,29 @@ ruleTester.run('jsx-no-script-url', rule, {
         { messageId: 'noScriptURL' },
       ],
       options: [
-        [
-          { name: 'Foo', props: ['to', 'href'] },
-          { name: 'Bar', props: ['link'] },
-        ],
+        [{ name: 'Bar', props: ['link'] }],
+        { includeFromSettings: true },
+      ],
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: ['to', 'href'] }],
+      },
+    },
+    {
+      code: `
+      <div>
+        <Foo href="javascript:"></Foo>
+        <Bar link="javascript:"></Bar>
+      </div>
+    `,
+      errors: [
+        { messageId: 'noScriptURL' },
+      ],
+      options: [
+        [{ name: 'Bar', props: ['link'] }],
       ],
+      settings: {
+        linkComponents: [{ name: 'Foo', linkAttribute: ['to', 'href'] }],
+      },
     },
   ]),
 });
diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js
index f5ab6b87..591a1a42 100644
--- a/tests/lib/rules/jsx-no-target-blank.js
+++ b/tests/lib/rules/jsx-no-target-blank.js
@@ -102,6 +102,11 @@ ruleTester.run('jsx-no-target-blank', rule, {
       options: [{ enforceDynamicLinks: 'never' }],
       settings: { linkComponents: { name: 'Link', linkAttribute: 'to' } },
     },
+    {
+      code: '<Link target="_blank" to={ dynamicLink }></Link>',
+      options: [{ enforceDynamicLinks: 'never' }],
+      settings: { linkComponents: { name: 'Link', linkAttribute: ['to'] } },
+    },
     {
       code: '<a href="foobar" target="_blank" rel="noopener"></a>',
       options: [{ allowReferrer: true }],
@@ -167,6 +172,14 @@ ruleTester.run('jsx-no-target-blank', rule, {
     {
       code: '<a href={href} target={isExternal ? "_blank" : undefined} rel={isExternal ? "noopener noreferrer" : undefined} />',
     },
+    {
+      code: '<form action={action} />',
+      options: [{ forms: true }],
+    },
+    {
+      code: '<form action={action} {...spread} />',
+      options: [{ forms: true }],
+    },
   ]),
   invalid: parsers.all([
     {
@@ -407,5 +420,20 @@ ruleTester.run('jsx-no-target-blank', rule, {
       options: [{ allowReferrer: true }],
       errors: allowReferrerErrors,
     },
+    {
+      code: '<form action={action} target="_blank" />',
+      options: [{ allowReferrer: true, forms: true }],
+      errors: allowReferrerErrors,
+    },
+    {
+      code: '<form action={action} target="_blank" />',
+      options: [{ forms: true }],
+      errors: defaultErrors,
+    },
+    {
+      code: '<form action={action} {...spread} />',
+      options: [{ forms: true, warnOnSpreadAttributes: true }],
+      errors: defaultErrors,
+    },
   ]),
 });
diff --git a/tests/util/linkComponents.js b/tests/util/linkComponents.js
index 09394def..741df261 100644
--- a/tests/util/linkComponents.js
+++ b/tests/util/linkComponents.js
@@ -8,7 +8,7 @@ describe('linkComponentsFunctions', () => {
     it('returns a default map of components', () => {
       const context = {};
       assert.deepStrictEqual(linkComponentsUtil.getLinkComponents(context), new Map([
-        ['a', 'href'],
+        ['a', ['href']],
       ]));
     });
 
@@ -19,6 +19,10 @@ describe('linkComponentsFunctions', () => {
           name: 'Link',
           linkAttribute: 'to',
         },
+        {
+          name: 'Link2',
+          linkAttribute: ['to1', 'to2'],
+        },
       ];
       const context = {
         settings: {
@@ -26,9 +30,44 @@ describe('linkComponentsFunctions', () => {
         },
       };
       assert.deepStrictEqual(linkComponentsUtil.getLinkComponents(context), new Map([
-        ['a', 'href'],
-        ['Hyperlink', 'href'],
-        ['Link', 'to'],
+        ['a', ['href']],
+        ['Hyperlink', ['href']],
+        ['Link', ['to']],
+        ['Link2', ['to1', 'to2']],
+      ]));
+    });
+  });
+
+  describe('getFormComponents', () => {
+    it('returns a default map of components', () => {
+      const context = {};
+      assert.deepStrictEqual(linkComponentsUtil.getFormComponents(context), new Map([
+        ['form', ['action']],
+      ]));
+    });
+
+    it('returns a map of components', () => {
+      const formComponents = [
+        'Form',
+        {
+          name: 'MyForm',
+          formAttribute: 'endpoint',
+        },
+        {
+          name: 'MyForm2',
+          formAttribute: ['endpoint1', 'endpoint2'],
+        },
+      ];
+      const context = {
+        settings: {
+          formComponents,
+        },
+      };
+      assert.deepStrictEqual(linkComponentsUtil.getFormComponents(context), new Map([
+        ['form', ['action']],
+        ['Form', ['action']],
+        ['MyForm', ['endpoint']],
+        ['MyForm2', ['endpoint1', 'endpoint2']],
       ]));
     });
   });