Repository: yannickcr/eslint-plugin-react
Pull request #623 (2016-06-05 21:06:23)

`jsx-curly-spacing`: fix “alternative” option
Rule now accepts a `spacing` object, which takes its defaults from the first always/never option. It accepts:
 - “default”: which controls all values unless otherwise specified.
 - “objectLiterals”: controls object literal values.
(in other words, omitted spacing option values default to the first option, which is "always" or "never")

Warnings around object literals themselves are removed, because the built-in `object-curly-spacing` rule handles that.

(Re https://github.com/yannickcr/eslint-plugin-react/pull/604#issuecomment-223824638)

ljharb: (hold up on this one, i want to add some more test cases)
ljharb: @yannickcr k, this should be gtg :-) thoughts?
ljharb: ping!

Patch:
diff --git a/README.md b/README.md
index d0758882..e4d09930 100644
--- a/README.md
+++ b/README.md
@@ -75,7 +75,7 @@ Finally, enable all of the rules that you would like to use.  Use [our preset](#
     "react/jsx-uses-react": "error",
     "react/jsx-uses-vars": "error",
   }
-```  
+```
 
 # List of supported rules
 
diff --git a/docs/rules/jsx-curly-spacing.md b/docs/rules/jsx-curly-spacing.md
index 7086e454..fbbde059 100644
--- a/docs/rules/jsx-curly-spacing.md
+++ b/docs/rules/jsx-curly-spacing.md
@@ -107,26 +107,34 @@ The following patterns are not warnings:
 <Hello name={ {firstname: 'John', lastname: 'Doe'} } />;
 ```
 
-#### Alternative
+#### Granular spacing controls
 
-When setting the `alternative` option to `true` you must collapse the curly braces:
+You can specify an additional `spacing` property that is an object with the following possible values:
 
 ```json
-"jsx-curly-spacing": [2, "always", {"alternative": true}]
+"jsx-curly-spacing": [2, "always", {"spacing": {
+  "objectLiterals": "never"
+}}]
 ```
 
-When `"always"` is used and `alternative` is `true`, the following pattern is not warnings:
+* `objectLiterals`: This controls different spacing requirements when the value inside the jsx curly braces is an object literal.
+
+All spacing options accept either the string `"always"` or the string `"never"`. Note that the default value for all "spacing" options matches the first "always"/"never" option provided.
+
+When `"always"` is used but `objectLiterals` is `"never"`, the following pattern is not considered a warning:
 
 ```js
-<App foo={{ bar: true, baz: true }} />;
+<App blah={ 3 } foo={{ bar: true, baz: true }} />;
 ```
 
-When `"always"` is used and `alternative` is `true`, the following pattern is considered warnings:
+When `"never"` is used and `objectLiterals` is `"always"`, the following pattern is not considered a warning:
 
 ```js
-<App foo={ {bar: true, baz: true} } />;
+<App blah={3} foo={ {bar: true, baz: true} } />;
 ```
 
+Please note that spacing of the object literal curly braces themselves is controlled by the built-in [`object-curly-spacing`](http://eslint.org/docs/rules/object-curly-spacing) rule.
+
 ## When Not To Use It
 
 You can turn this rule off if you are not concerned with the consistency around the spacing inside of JSX attributes.
diff --git a/lib/rules/jsx-curly-spacing.js b/lib/rules/jsx-curly-spacing.js
index f2b6cd64..0cdafb90 100644
--- a/lib/rules/jsx-curly-spacing.js
+++ b/lib/rules/jsx-curly-spacing.js
@@ -14,12 +14,20 @@
 // Rule Definition
 // ------------------------------------------------------------------------------
 
+var SPACING = {
+  always: 'always',
+  never: 'never'
+};
+var SPACING_VALUES = [SPACING.always, SPACING.never];
+
 module.exports = function(context) {
 
   var sourceCode = context.getSourceCode();
-  var spaced = context.options[0] === 'always';
+  var spaced = context.options[0] === SPACING.always;
   var multiline = context.options[1] ? context.options[1].allowMultiline : true;
-  var alternative = context.options[1] ? context.options[1].alternative : false;
+  var spacing = context.options[1] ? context.options[1].spacing || {} : {};
+  var defaultSpacing = spaced ? SPACING.always : SPACING.never;
+  var objectLiteralSpacing = spacing.objectLiterals || (spaced ? SPACING.always : SPACING.never);
 
   // --------------------------------------------------------------------------
   // Helpers
@@ -151,57 +159,59 @@ module.exports = function(context) {
     var last = sourceCode.getLastToken(node);
     var second = context.getTokenAfter(first);
     var penultimate = sourceCode.getTokenBefore(last);
-    var third = context.getTokenAfter(second);
-    var antepenultimate = sourceCode.getTokenBefore(penultimate);
-
-    if (spaced) {
-      if (!alternative) {
-        if (!sourceCode.isSpaceBetweenTokens(first, second)) {
-          reportRequiredBeginningSpace(node, first);
-        } else if (!multiline && isMultiline(first, second)) {
-          reportNoBeginningNewline(node, first);
-        }
-
-        if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
-          reportRequiredEndingSpace(node, last);
-        } else if (!multiline && isMultiline(penultimate, last)) {
-          reportNoEndingNewline(node, last);
-        }
 
-      // Object literal
-      } else if (first.value === second.value) {
+    var isObjectLiteral = first.value === second.value;
+    if (isObjectLiteral) {
+      if (objectLiteralSpacing === SPACING.never) {
         if (sourceCode.isSpaceBetweenTokens(first, second)) {
           reportNoBeginningSpace(node, first);
+        } else if (!multiline && isMultiline(first, second)) {
+          reportNoBeginningNewline(node, first);
         }
         if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
           reportNoEndingSpace(node, last);
+        } else if (!multiline && isMultiline(penultimate, last)) {
+          reportNoEndingNewline(node, last);
         }
-        if (!sourceCode.isSpaceBetweenTokens(second, third)) {
-          reportRequiredBeginningSpace(node, second);
-        }
-        if (!sourceCode.isSpaceBetweenTokens(antepenultimate, penultimate)) {
-          reportRequiredEndingSpace(node, penultimate);
-        }
-
-      } else {
+      } else if (objectLiteralSpacing === SPACING.always) {
         if (!sourceCode.isSpaceBetweenTokens(first, second)) {
           reportRequiredBeginningSpace(node, first);
+        } else if (!multiline && isMultiline(first, second)) {
+          reportNoBeginningNewline(node, first);
         }
         if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
           reportRequiredEndingSpace(node, last);
+        } else if (!multiline && isMultiline(penultimate, last)) {
+          reportNoEndingNewline(node, last);
         }
       }
+    } else if (defaultSpacing === SPACING.always) {
+      if (!sourceCode.isSpaceBetweenTokens(first, second)) {
+        reportRequiredBeginningSpace(node, first);
+      } else if (!multiline && isMultiline(first, second)) {
+        reportNoBeginningNewline(node, first);
+      }
 
-      return;
-    }
-
-    // "never" setting if we get here.
-    if (sourceCode.isSpaceBetweenTokens(first, second) && !(multiline && isMultiline(first, second))) {
-      reportNoBeginningSpace(node, first);
-    }
-
-    if (sourceCode.isSpaceBetweenTokens(penultimate, last) && !(multiline && isMultiline(penultimate, last))) {
-      reportNoEndingSpace(node, last);
+      if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
+        reportRequiredEndingSpace(node, last);
+      } else if (!multiline && isMultiline(penultimate, last)) {
+        reportNoEndingNewline(node, last);
+      }
+    } else if (defaultSpacing === SPACING.never) {
+      if (isMultiline(first, second)) {
+        if (!multiline) {
+          reportNoBeginningNewline(node, first);
+        }
+      } else if (sourceCode.isSpaceBetweenTokens(first, second)) {
+        reportNoBeginningSpace(node, first);
+      }
+      if (isMultiline(penultimate, last)) {
+        if (!multiline) {
+          reportNoEndingNewline(node, last);
+        }
+      } else if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
+        reportNoEndingSpace(node, last);
+      }
     }
   }
 
@@ -216,15 +226,20 @@ module.exports = function(context) {
 };
 
 module.exports.schema = [{
-  enum: ['always', 'never']
+  enum: SPACING_VALUES
 }, {
   type: 'object',
   properties: {
     allowMultiline: {
       type: 'boolean'
     },
-    alternative: {
-      type: 'boolean'
+    spacing: {
+      type: 'object',
+      properties: {
+        objectLiterals: {
+          enum: SPACING_VALUES
+        }
+      }
     }
   },
   additionalProperties: false
diff --git a/lib/rules/jsx-handler-names.js b/lib/rules/jsx-handler-names.js
index 849c489c..56d35e07 100644
--- a/lib/rules/jsx-handler-names.js
+++ b/lib/rules/jsx-handler-names.js
@@ -26,7 +26,7 @@ module.exports = function(context) {
       }
 
       var propKey = typeof node.name === 'object' ? node.name.name : node.name;
-      var propValue = sourceCode.getText(node.value.expression).replace(/^this\./, '');
+      var propValue = sourceCode.getText(node.value.expression).replace(/^this\.|.*::/, '');
 
       if (propKey === 'ref') {
         return;


Test patch:
diff --git a/tests/lib/rules/jsx-curly-spacing.js b/tests/lib/rules/jsx-curly-spacing.js
index e3b56d65..fc975a92 100644
--- a/tests/lib/rules/jsx-curly-spacing.js
+++ b/tests/lib/rules/jsx-curly-spacing.js
@@ -84,11 +84,11 @@ ruleTester.run('jsx-curly-spacing', rule, {
     parserOptions: parserOptions
   }, {
     code: '<App foo={ bar } />;',
-    options: ['always', {alternative: true}],
+    options: ['always', {spacing: {}}],
     parserOptions: parserOptions
   }, {
     code: '<App foo={{ bar: true, baz: true }} />;',
-    options: ['always', {alternative: true}],
+    options: ['always', {spacing: {objectLiterals: 'never'}}],
     parserOptions: parserOptions
   }, {
     code: [
@@ -96,7 +96,7 @@ ruleTester.run('jsx-curly-spacing', rule, {
       'bar',
       '} />;'
     ].join('\n'),
-    options: ['always', {alternative: true}],
+    options: ['always', {allowMultiline: true}],
     parserOptions: parserOptions
   }, {
     code: '<App {...bar} />;',
@@ -178,6 +178,10 @@ ruleTester.run('jsx-curly-spacing', rule, {
     code: '<App foo={bar/* comment */} {...baz/* comment */} />;',
     options: ['never'],
     parserOptions: parserOptions
+  }, {
+    code: '<App foo={3} bar={ {a: 2} } />',
+    options: ['never', {spacing: {objectLiterals: 'always'}}],
+    parserOptions: parserOptions
   }],
 
   invalid: [{
@@ -261,9 +265,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
     output: '<App foo={bar} />;',
     options: ['never', {allowMultiline: false}],
     errors: [{
-      message: 'There should be no space after \'{\''
+      message: 'There should be no newline after \'{\''
     }, {
-      message: 'There should be no space before \'}\''
+      message: 'There should be no newline before \'}\''
     }],
     parserOptions: parserOptions
   }, {
@@ -283,7 +287,7 @@ ruleTester.run('jsx-curly-spacing', rule, {
   }, {
     code: '<App foo={bar} />;',
     output: '<App foo={ bar } />;',
-    options: ['always', {alternative: true}],
+    options: ['always', {spacing: {}}],
     errors: [{
       message: 'A space is required after \'{\''
     }, {
@@ -293,7 +297,7 @@ ruleTester.run('jsx-curly-spacing', rule, {
   }, {
     code: '<App foo={ bar} />;',
     output: '<App foo={ bar } />;',
-    options: ['always', {alternative: true}],
+    options: ['always', {spacing: {}}],
     errors: [{
       message: 'A space is required before \'}\''
     }],
@@ -301,21 +305,17 @@ ruleTester.run('jsx-curly-spacing', rule, {
   }, {
     code: '<App foo={bar } />;',
     output: '<App foo={ bar } />;',
-    options: ['always', {alternative: true}],
+    options: ['always', {spacing: {}}],
     errors: [{
       message: 'A space is required after \'{\''
     }],
     parserOptions: parserOptions
   }, {
     code: '<App foo={ {bar: true, baz: true} } />;',
-    output: '<App foo={{ bar: true, baz: true }} />;',
-    options: ['always', {alternative: true}],
+    output: '<App foo={{bar: true, baz: true}} />;',
+    options: ['always', {spacing: {objectLiterals: 'never'}}],
     errors: [{
       message: 'There should be no space after \'{\''
-    }, {
-      message: 'A space is required after \'{\''
-    }, {
-      message: 'A space is required before \'}\''
     }, {
       message: 'There should be no space before \'}\''
     }],
@@ -401,9 +401,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
     output: '<App {...bar} />;',
     options: ['never', {allowMultiline: false}],
     errors: [{
-      message: 'There should be no space after \'{\''
+      message: 'There should be no newline after \'{\''
     }, {
-      message: 'There should be no space before \'}\''
+      message: 'There should be no newline before \'}\''
     }],
     parserOptions: parserOptions
   }, {
@@ -527,13 +527,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
     output: '<App foo={bar} {...baz} />;',
     options: ['never', {allowMultiline: false}],
     errors: [{
-      message: 'There should be no space after \'{\''
+      message: 'There should be no newline after \'{\''
     }, {
-      message: 'There should be no space before \'}\''
+      message: 'There should be no newline before \'}\''
     }, {
-      message: 'There should be no space after \'{\''
+      message: 'There should be no newline after \'{\''
     }, {
-      message: 'There should be no space before \'}\''
+      message: 'There should be no newline before \'}\''
     }],
     parserOptions: parserOptions
   }, {
@@ -556,5 +556,19 @@ ruleTester.run('jsx-curly-spacing', rule, {
       message: 'There should be no newline before \'}\''
     }],
     parserOptions: parserOptions
+  }, {
+    code: '<App foo={ 3 } bar={{a: 2}} />',
+    output: '<App foo={3} bar={ {a: 2} } />',
+    options: ['never', {spacing: {objectLiterals: 'always'}}],
+    errors: [{
+      message: 'There should be no space after \'{\''
+    }, {
+      message: 'There should be no space before \'}\''
+    }, {
+      message: 'A space is required after \'{\''
+    }, {
+      message: 'A space is required before \'}\''
+    }],
+    parserOptions: parserOptions
   }]
 });
diff --git a/tests/lib/rules/jsx-handler-names.js b/tests/lib/rules/jsx-handler-names.js
index 33b1c34c..be8deaf9 100644
--- a/tests/lib/rules/jsx-handler-names.js
+++ b/tests/lib/rules/jsx-handler-names.js
@@ -78,6 +78,11 @@ ruleTester.run('jsx-handler-names', rule, {
       eventHandlerPropPrefix: 'on'
     }],
     parserOptions: parserOptions
+  }, {
+    code: [
+      '<TestComponent onChange={props::handleChange} />'
+    ].join('\n'),
+    parser: 'babel-eslint'
   }],
 
   invalid: [{
@@ -98,5 +103,11 @@ ruleTester.run('jsx-handler-names', rule, {
     ].join('\n'),
     parserOptions: parserOptions,
     errors: [{message: 'Handler function for onChange prop key must begin with \'handle\''}]
+  }, {
+    code: [
+      '<TestComponent onChange={props::onChange} />'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    errors: [{message: 'Handler function for onChange prop key must begin with \'handle\''}]
   }]
 });