Repository: yannickcr/eslint-plugin-react
Pull request #796 (2016-08-31 06:15:41)

Case insensitive comparisons for no-target-blank
`<a href="foo" target="_BLANK">` should also trigger no-target-blank.
Now it does.

ljharb: Do browsers natively treat them as case-insensitive (and does react normalize the case)? If not, they should *not* be targeted by this rule.
dmnd: Looks like I was overzealous with the HTML attribute name. Those seem to be case sensitive. But target="_BLANK" definitely works: http://codepen.io/dmnd/pen/LRPmZE?editors=1010
dmnd: Amended to not include all-caps html attributes.
yannickcr: Thanks. Can you rebase your branch ? Then I think we'll be good to go.
dmnd: Rebased.

Patch:
diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js
index d6210037..6d41a207 100644
--- a/lib/rules/jsx-no-target-blank.js
+++ b/lib/rules/jsx-no-target-blank.js
@@ -25,12 +25,12 @@ module.exports = {
           return;
         }
 
-        if (node.name.name === 'target' && node.value.value === '_blank') {
+        if (node.name.name === 'target' && node.value.value.toLowerCase() === '_blank') {
           var relFound = false;
           var attrs = node.parent.attributes;
           for (var idx in attrs) {
             if (attrs[idx].name && attrs[idx].name.name === 'rel') {
-              var tags = attrs[idx].value.value.split(' ');
+              var tags = attrs[idx].value.value.toLowerCase().split(' ');
               if (tags.indexOf('noopener') >= 0 && tags.indexOf('noreferrer') >= 0) {
                 relFound = true;
                 break;
diff --git a/lib/rules/jsx-uses-react.js b/lib/rules/jsx-uses-react.js
index e0c5951a..7b22aed5 100644
--- a/lib/rules/jsx-uses-react.js
+++ b/lib/rules/jsx-uses-react.js
@@ -4,7 +4,6 @@
  */
 'use strict';
 
-var variableUtil = require('../util/variable');
 var pragmaUtil = require('../util/pragma');
 
 // ------------------------------------------------------------------------------
@@ -32,7 +31,7 @@ module.exports = {
     return {
 
       JSXOpeningElement: function() {
-        variableUtil.markVariableAsUsed(context, pragma);
+        context.markVariableAsUsed(pragma);
       },
 
       BlockComment: function(node) {
diff --git a/lib/rules/jsx-uses-vars.js b/lib/rules/jsx-uses-vars.js
index 2433410e..e2495811 100644
--- a/lib/rules/jsx-uses-vars.js
+++ b/lib/rules/jsx-uses-vars.js
@@ -4,8 +4,6 @@
  */
 'use strict';
 
-var variableUtil = require('../util/variable');
-
 // ------------------------------------------------------------------------------
 // Rule Definition
 // ------------------------------------------------------------------------------
@@ -42,7 +40,7 @@ module.exports = {
           return;
         }
 
-        variableUtil.markVariableAsUsed(context, name);
+        context.markVariableAsUsed(name);
       }
 
     };
diff --git a/lib/util/variable.js b/lib/util/variable.js
index ae39a988..e7c7b5f8 100644
--- a/lib/util/variable.js
+++ b/lib/util/variable.js
@@ -4,41 +4,6 @@
  */
 'use strict';
 
-/**
- * Record that a particular variable has been used in code
- *
- * @param {Object} context The current rule context.
- * @param {String} name The name of the variable to mark as used.
- * @returns {Boolean} True if the variable was found and marked as used, false if not.
- */
-function markVariableAsUsed(context, name) {
-  var scope = context.getScope();
-  var variables;
-  var i;
-  var len;
-  var found = false;
-
-  // Special Node.js scope means we need to start one level deeper
-  if (scope.type === 'global') {
-    while (scope.childScopes.length) {
-      scope = scope.childScopes[0];
-    }
-  }
-
-  do {
-    variables = scope.variables;
-    for (i = 0, len = variables.length; i < len; i++) {
-      if (variables[i].name === name) {
-        variables[i].eslintUsed = true;
-        found = true;
-      }
-    }
-    scope = scope.upper;
-  } while (scope);
-
-  return found;
-}
-
 /**
  * Search a particular variable in a list
  * @param {Array} variables The variables list.
@@ -88,6 +53,5 @@ function variablesInScope(context) {
 
 module.exports = {
   findVariable: findVariable,
-  variablesInScope: variablesInScope,
-  markVariableAsUsed: markVariableAsUsed
+  variablesInScope: variablesInScope
 };


Test patch:
diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js
index 06f2a496..d3e06877 100644
--- a/tests/lib/rules/jsx-no-target-blank.js
+++ b/tests/lib/rules/jsx-no-target-blank.js
@@ -30,7 +30,8 @@ ruleTester.run('jsx-no-target-blank', rule, {
     {code: '<a href="foobar" target="_blank" rel="noopener noreferrer"></a>', parserOptions: parserOptions},
     {code: '<a target="_blank" {...spreadProps} rel="noopener noreferrer"></a>', parserOptions: parserOptions},
     {code: '<a target="_blank" rel="noopener noreferrer" {...spreadProps}></a>', parserOptions: parserOptions},
-    {code: '<p target="_blank"></p>', parserOptions: parserOptions}
+    {code: '<p target="_blank"></p>', parserOptions: parserOptions},
+    {code: '<a href="foobar" target="_BLANK" rel="NOOPENER noreferrer"></a>', parserOptions: parserOptions}
   ],
   invalid: [
     {code: '<a target="_blank"></a>', parserOptions: parserOptions,
@@ -40,6 +41,9 @@ ruleTester.run('jsx-no-target-blank', rule, {
      errors: [{message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'}]},
     {code: '<a target="_blank" rel="noopenernoreferrer"></a>', parserOptions: parserOptions,
+     errors: [{message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
+      ' see https://mathiasbynens.github.io/rel-noopener'}]},
+    {code: '<a target="_BLANK"></a>', parserOptions: parserOptions,
      errors: [{message: 'Using target="_blank" without rel="noopener noreferrer" is a security risk:' +
       ' see https://mathiasbynens.github.io/rel-noopener'}]}
   ]
diff --git a/tests/lib/rules/jsx-uses-vars.js b/tests/lib/rules/jsx-uses-vars.js
index 506bcc37..dd3dd225 100644
--- a/tests/lib/rules/jsx-uses-vars.js
+++ b/tests/lib/rules/jsx-uses-vars.js
@@ -184,6 +184,21 @@ ruleTester.run('no-unused-vars', ruleNoUnusedVars, {
       }],
       parser: 'babel-eslint',
       parserOptions: parserOptions
+    }, {
+      code: '\
+        /*eslint jsx-uses-vars:1*/\
+        import {Hello} from \'Hello\';\
+        function Greetings() {\
+          const Hello = require(\'Hello\').default;\
+          return <Hello />;\
+        }\
+        Greetings();',
+      errors: [{
+        message: '\'Hello\' is defined but never used.',
+        line: 1
+      }],
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
     }
   ]
 });