Repository: yannickcr/eslint-plugin-react
Pull request #1221 (2017-05-24 04:22:02)

Fix regression bug in jsx-sort-props
This is a fix for: https://github.com/yannickcr/eslint-plugin-react/issues/1175

I have explained the problem already in the issue, but a quick recap:

* Bug only occurs when using `ignoreCase: true` and `callbacksLast: true`
* `isCallbackPropName(propName)` looks for pattern: `on[A-Z]`
* This pattern can't be found after `toLowerCase()` has been executed (because of `ignoreCase: true`

Bug was introduced in 7.0.0. Reverted the code to how it was working originally, and added a test case to cover this specific combination of options.


Patch:
diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js
index 0e5f9ea3..176fc0fb 100644
--- a/lib/rules/display-name.js
+++ b/lib/rules/display-name.js
@@ -107,10 +107,10 @@ module.exports = {
         )
       );
       var namedObjectDeclaration = (
-        node.type === 'ObjectExpression' &&
-        node.parent &&
-        node.parent.parent &&
-        node.parent.parent.type === 'VariableDeclarator'
+          node.type === 'ObjectExpression' &&
+          node.parent &&
+          node.parent.parent &&
+          node.parent.parent.type === 'VariableDeclarator'
       );
       var namedClass = (
         (node.type === 'ClassDeclaration' || node.type === 'ClassExpression') &&
diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index 953352b6..1945115d 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -150,6 +150,8 @@ module.exports = {
           var currentPropName = propName(decl);
           var previousValue = memo.value;
           var currentValue = decl.value;
+          var previousIsCallback = isCallbackPropName(previousPropName);
+          var currentIsCallback = isCallbackPropName(currentPropName);
 
           if (ignoreCase) {
             previousPropName = previousPropName.toLowerCase();
@@ -186,9 +188,6 @@ module.exports = {
           }
 
           if (callbacksLast) {
-            var previousIsCallback = isCallbackPropName(previousPropName);
-            var currentIsCallback = isCallbackPropName(currentPropName);
-
             if (!previousIsCallback && currentIsCallback) {
               // Entering the callback prop section
               return decl;


Test patch:
diff --git a/tests/lib/rules/default-props-match-prop-types.js b/tests/lib/rules/default-props-match-prop-types.js
index 363df983..97aa952e 100644
--- a/tests/lib/rules/default-props-match-prop-types.js
+++ b/tests/lib/rules/default-props-match-prop-types.js
@@ -16,14 +16,13 @@ require('babel-eslint');
 
 var parserOptions = {
   ecmaVersion: 6,
-  sourceType: 'module',
   ecmaFeatures: {
     experimentalObjectRestSpread: true,
     jsx: true
   }
 };
 
-var ruleTester = new RuleTester({parserOptions});
+var ruleTester = new RuleTester({parserOptions: parserOptions});
 
 // ------------------------------------------------------------------------------
 // Tests
diff --git a/tests/lib/rules/jsx-no-undef.js b/tests/lib/rules/jsx-no-undef.js
index 0d03628f..6749282b 100644
--- a/tests/lib/rules/jsx-no-undef.js
+++ b/tests/lib/rules/jsx-no-undef.js
@@ -9,9 +9,9 @@
 // Requirements
 // -----------------------------------------------------------------------------
 
-var eslint = require('eslint');
+var eslint = require('eslint').linter;
 var rule = require('../../../lib/rules/jsx-no-undef');
-var RuleTester = eslint.RuleTester;
+var RuleTester = require('eslint').RuleTester;
 
 var parserOptions = {
   ecmaVersion: 8,
@@ -26,8 +26,7 @@ var parserOptions = {
 // -----------------------------------------------------------------------------
 
 var ruleTester = new RuleTester({parserOptions});
-var linter = ruleTester.linter || eslint.linter;
-linter.defineRule('no-undef', require('eslint/lib/rules/no-undef'));
+eslint.defineRule('no-undef', require('eslint/lib/rules/no-undef'));
 ruleTester.run('jsx-no-undef', rule, {
   valid: [{
     code: '/*eslint no-undef:1*/ var React, App; React.render(<App />);'
diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js
index 702243de..472cb046 100644
--- a/tests/lib/rules/jsx-sort-props.js
+++ b/tests/lib/rules/jsx-sort-props.js
@@ -56,6 +56,10 @@ var expectedInvalidReservedFirstError = {
 var callbacksLastArgs = [{
   callbacksLast: true
 }];
+var ignoreCaseAndCallbackLastArgs = [{
+  callbacksLast: true,
+  ignoreCase: true
+}];
 var shorthandFirstArgs = [{
   shorthandFirst: true
 }];
@@ -109,6 +113,7 @@ ruleTester.run('jsx-sort-props', rule, {
     {code: '<App A b C />;', options: ignoreCaseArgs},
     // Sorting callbacks below all other props
     {code: '<App a z onBar onFoo />;', options: callbacksLastArgs},
+    {code: '<App z onBar onFoo />;', options: ignoreCaseAndCallbackLastArgs},
     // Sorting shorthand props before others
     {code: '<App a b="b" />;', options: shorthandFirstArgs},
     {code: '<App z a="a" />;', options: shorthandFirstArgs},
diff --git a/tests/lib/rules/jsx-uses-react.js b/tests/lib/rules/jsx-uses-react.js
index 33a7e64e..e87cf044 100644
--- a/tests/lib/rules/jsx-uses-react.js
+++ b/tests/lib/rules/jsx-uses-react.js
@@ -9,9 +9,9 @@
 // Requirements
 // -----------------------------------------------------------------------------
 
-var eslint = require('eslint');
+var eslint = require('eslint').linter;
 var rule = require('eslint/lib/rules/no-unused-vars');
-var RuleTester = eslint.RuleTester;
+var RuleTester = require('eslint').RuleTester;
 
 var parserOptions = {
   ecmaVersion: 8,
@@ -33,8 +33,7 @@ var settings = {
 // -----------------------------------------------------------------------------
 
 var ruleTester = new RuleTester({parserOptions});
-var linter = ruleTester.linter || eslint.linter;
-linter.defineRule('jsx-uses-react', require('../../../lib/rules/jsx-uses-react'));
+eslint.defineRule('jsx-uses-react', require('../../../lib/rules/jsx-uses-react'));
 ruleTester.run('no-unused-vars', rule, {
   valid: [
     {code: '/*eslint jsx-uses-react:1*/ var React; <div />;'},
diff --git a/tests/lib/rules/jsx-uses-vars.js b/tests/lib/rules/jsx-uses-vars.js
index 28d56b53..80c27c67 100644
--- a/tests/lib/rules/jsx-uses-vars.js
+++ b/tests/lib/rules/jsx-uses-vars.js
@@ -9,10 +9,10 @@
 // Requirements
 // -----------------------------------------------------------------------------
 
-var eslint = require('eslint');
+var eslint = require('eslint').linter;
 var ruleNoUnusedVars = require('eslint/lib/rules/no-unused-vars');
 var rulePreferConst = require('eslint/lib/rules/prefer-const');
-var RuleTester = eslint.RuleTester;
+var RuleTester = require('eslint').RuleTester;
 
 var parserOptions = {
   ecmaVersion: 8,
@@ -30,8 +30,7 @@ require('babel-eslint');
 // -----------------------------------------------------------------------------
 
 var ruleTester = new RuleTester({parserOptions});
-var linter = ruleTester.linter || eslint.linter;
-linter.defineRule('jsx-uses-vars', require('../../../lib/rules/jsx-uses-vars'));
+eslint.defineRule('jsx-uses-vars', require('../../../lib/rules/jsx-uses-vars'));
 ruleTester.run('no-unused-vars', ruleNoUnusedVars, {
   valid: [
     {