Skip to content

Commit

Permalink
Teach sort-comp rule about static class methods
Browse files Browse the repository at this point in the history
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 jsx-eslint#128
  • Loading branch information
lencioni committed Feb 5, 2016
1 parent 77723c4 commit a64e5ce
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
11 changes: 8 additions & 3 deletions docs/rules/sort-comp.md
Expand Up @@ -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:

Expand Down Expand Up @@ -51,6 +52,7 @@ The default configuration is:
```js
{
order: [
'static-methods',
'lifecycle',
'everything-else',
'render'
Expand Down Expand Up @@ -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.
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
63 changes: 41 additions & 22 deletions lib/rules/sort-comp.js
Expand Up @@ -46,6 +46,7 @@ module.exports = Components.detect(function(context, components) {

var methodsOrder = getMethodsOrder({
order: [
'static-methods',
'lifecycle',
'everything-else',
'render'
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}

Expand All @@ -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') {
Expand Down Expand Up @@ -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;
Expand All @@ -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++) {
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down

0 comments on commit a64e5ce

Please sign in to comment.