From a64e5ceab9cbfe2f23e5ab5607357338c217163a Mon Sep 17 00:00:00 2001 From: Joe Lencioni Date: Fri, 5 Feb 2016 14:54:48 -0800 Subject: [PATCH] Teach sort-comp rule about static class methods A number of people have said that they think it makes sense for static class methods to appear at the very top of classes. This commit allows this to be configured by adding the `static-methods` keyword to the sort-comp rule. Fixes #128 --- docs/rules/sort-comp.md | 11 +++++-- lib/rules/sort-comp.js | 63 +++++++++++++++++++++++++++-------------- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/docs/rules/sort-comp.md b/docs/rules/sort-comp.md index f92201662e..a1e7a17cd8 100644 --- a/docs/rules/sort-comp.md +++ b/docs/rules/sort-comp.md @@ -6,9 +6,10 @@ When creating React components it is more convenient to always follow the same o With default configuration the following organisation must be followed: - 1. lifecycle methods: `displayName`, `propTypes`, `contextTypes`, `childContextTypes`, `mixins`, `statics`,`defaultProps`, `constructor`, `getDefaultProps`, `getInitialState`, `state`, `getChildContext`, `componentWillMount`, `componentDidMount`, `componentWillReceiveProps`, `shouldComponentUpdate`, `componentWillUpdate`, `componentDidUpdate`, `componentWillUnmount` (in this order). - 2. custom methods - 3. `render` method + 1. static methods + 2. lifecycle methods: `displayName`, `propTypes`, `contextTypes`, `childContextTypes`, `mixins`, `statics`,`defaultProps`, `constructor`, `getDefaultProps`, `getInitialState`, `state`, `getChildContext`, `componentWillMount`, `componentDidMount`, `componentWillReceiveProps`, `shouldComponentUpdate`, `componentWillUpdate`, `componentDidUpdate`, `componentWillUnmount` (in this order). + 3. custom methods + 4. `render` method The following patterns are considered warnings: @@ -51,6 +52,7 @@ The default configuration is: ```js { order: [ + 'static-methods', 'lifecycle', 'everything-else', 'render' @@ -81,6 +83,7 @@ The default configuration is: } ``` +* `static-methods` is a special keyword that refers to static class methods. * `lifecycle` is refering to the `lifecycle` group defined in `groups`. * `everything-else` is a special group that match all the methods that do not match any of the other groups. * `render` is refering to the `render` method. @@ -92,6 +95,7 @@ For example, if you want to place your event handlers (`onClick`, `onSubmit`, et ```js "react/sort-comp": [1, { order: [ + 'static-methods', 'lifecycle', '/^on.+$/', 'render', @@ -127,6 +131,7 @@ If you want to split your `render` method into smaller ones and keep them just b ```js "react/sort-comp": [1, { order: [ + 'static-methods', 'lifecycle', 'everything-else', 'rendering', diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 9ffbbd6f8a..ee07e3ddc4 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -46,6 +46,7 @@ module.exports = Components.detect(function(context, components) { var methodsOrder = getMethodsOrder({ order: [ + 'static-methods', 'lifecycle', 'everything-else', 'render' @@ -83,7 +84,7 @@ module.exports = Components.detect(function(context, components) { /** * Get indexes of the matching patterns in methods order configuration - * @param {String} method - Method name. + * @param {Object} method - Method metadata. * @returns {Array} The matching patterns indexes. Return [Infinity] if there is no match. */ function getRefPropIndexes(method) { @@ -92,15 +93,28 @@ module.exports = Components.detect(function(context, components) { var i; var j; var indexes = []; - for (i = 0, j = methodsOrder.length; i < j; i++) { - isRegExp = methodsOrder[i].match(regExpRegExp); - if (isRegExp) { - matching = new RegExp(isRegExp[1], isRegExp[2]).test(method); - } else { - matching = methodsOrder[i] === method; + + if (method.static) { + for (i = 0, j = methodsOrder.length; i < j; i++) { + if (methodsOrder[i] === 'static-methods') { + indexes.push(i); + } } - if (matching) { - indexes.push(i); + } + + // Either this is not a static method or static methods are not specified + // in the methodsOrder. + if (indexes.length === 0) { + for (i = 0, j = methodsOrder.length; i < j; i++) { + isRegExp = methodsOrder[i].match(regExpRegExp); + if (isRegExp) { + matching = new RegExp(isRegExp[1], isRegExp[2]).test(method.name); + } else { + matching = methodsOrder[i] === method.name; + } + if (matching) { + indexes.push(i); + } } } @@ -127,7 +141,6 @@ module.exports = Components.detect(function(context, components) { * @returns {String} Property name. */ function getPropertyName(node) { - // Special case for class properties // (babel-eslint does not expose property name so we have to rely on tokens) if (node.type === 'ClassProperty') { @@ -236,12 +249,12 @@ module.exports = Components.detect(function(context, components) { /** * Compare two properties and find out if they are in the right order - * @param {Array} propertiesNames Array containing all the properties names. - * @param {String} propA First property name. - * @param {String} propB Second property name. + * @param {Array} propertiesInfos Array containing all the properties metadata. + * @param {Object} propA First property name and metadata + * @param {Object} propB Second property name. * @returns {Object} Object containing a correct true/false flag and the correct indexes for the two properties. */ - function comparePropsOrder(propertiesNames, propA, propB) { + function comparePropsOrder(propertiesInfos, propA, propB) { var i; var j; var k; @@ -254,8 +267,8 @@ module.exports = Components.detect(function(context, components) { var refIndexesB = getRefPropIndexes(propB); // Get current indexes for given properties - var classIndexA = propertiesNames.indexOf(propA); - var classIndexB = propertiesNames.indexOf(propB); + var classIndexA = propertiesInfos.indexOf(propA); + var classIndexB = propertiesInfos.indexOf(propB); // Loop around the references indexes for the 1st property for (i = 0, j = refIndexesA.length; i < j; i++) { @@ -296,7 +309,13 @@ module.exports = Components.detect(function(context, components) { * @param {Array} properties Array containing all the properties. */ function checkPropsOrder(properties) { - var propertiesNames = properties.map(getPropertyName); + var propertiesInfos = properties.map(function(node) { + return { + name: getPropertyName(node), + static: node.static + }; + }); + var i; var j; var k; @@ -306,15 +325,15 @@ module.exports = Components.detect(function(context, components) { var order; // Loop around the properties - for (i = 0, j = propertiesNames.length; i < j; i++) { - propA = propertiesNames[i]; + for (i = 0, j = propertiesInfos.length; i < j; i++) { + propA = propertiesInfos[i]; // Loop around the properties a second time (for comparison) - for (k = 0, l = propertiesNames.length; k < l; k++) { - propB = propertiesNames[k]; + for (k = 0, l = propertiesInfos.length; k < l; k++) { + propB = propertiesInfos[k]; // Compare the properties order - order = comparePropsOrder(propertiesNames, propA, propB); + order = comparePropsOrder(propertiesInfos, propA, propB); // Continue to next comparison is order is correct if (order.correct === true) {