diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js
index 6befbc0ba8..ed364a8217 100644
--- a/lib/rules/sort-comp.js
+++ b/lib/rules/sort-comp.js
@@ -8,6 +8,7 @@ const has = require('has');
const util = require('util');
const Components = require('../util/Components');
+const arrayIncludes = require('array-includes');
const astUtil = require('../util/ast');
const docsUrl = require('../util/docsUrl');
@@ -131,84 +132,91 @@ module.exports = {
* @returns {Array} The matching patterns indexes. Return [Infinity] if there is no match.
*/
function getRefPropIndexes(method) {
- let isRegExp;
- let matching;
- let i;
- let j;
- const indexes = [];
-
- if (method.getter) {
- const getterIndex = methodsOrder.indexOf('getters');
- if (getterIndex >= 0) {
- indexes.push(getterIndex);
- }
- }
-
- if (method.setter) {
- const setterIndex = methodsOrder.indexOf('setters');
- if (setterIndex >= 0) {
- indexes.push(setterIndex);
- }
- }
-
- if (method.typeAnnotation) {
- const annotationIndex = methodsOrder.indexOf('type-annotations');
- if (annotationIndex >= 0) {
- indexes.push(annotationIndex);
- }
- }
+ const methodGroupIndexes = [];
- 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;
+ methodsOrder.forEach((currentGroup, groupIndex) => {
+ if (currentGroup === 'getters') {
+ if (method.getter) {
+ methodGroupIndexes.push(groupIndex);
}
- if (matching) {
- indexes.push(i);
+ } else if (currentGroup === 'setters') {
+ if (method.setter) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (currentGroup === 'type-annotations') {
+ if (method.typeAnnotation) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (currentGroup === 'static-methods') {
+ if (method.static) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (currentGroup === 'instance-variables') {
+ if (method.instanceVariable) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (currentGroup === 'instance-methods') {
+ if (method.instanceMethod) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (arrayIncludes([
+ 'displayName',
+ 'propTypes',
+ 'contextTypes',
+ 'childContextTypes',
+ 'mixins',
+ 'statics',
+ 'defaultProps',
+ 'constructor',
+ 'getDefaultProps',
+ 'state',
+ 'getInitialState',
+ 'getChildContext',
+ 'getDerivedStateFromProps',
+ 'componentWillMount',
+ 'UNSAFE_componentWillMount',
+ 'componentDidMount',
+ 'componentWillReceiveProps',
+ 'UNSAFE_componentWillReceiveProps',
+ 'shouldComponentUpdate',
+ 'componentWillUpdate',
+ 'UNSAFE_componentWillUpdate',
+ 'getSnapshotBeforeUpdate',
+ 'componentDidUpdate',
+ 'componentDidCatch',
+ 'componentWillUnmount',
+ 'render'
+ ], currentGroup)) {
+ if (currentGroup === method.name) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else {
+ // Is the group a regex?
+ const isRegExp = currentGroup.match(regExpRegExp);
+ if (isRegExp) {
+ const isMatching = new RegExp(isRegExp[1], isRegExp[2]).test(method.name);
+ if (isMatching) {
+ methodGroupIndexes.push(groupIndex);
+ }
+ } else if (currentGroup === method.name) {
+ methodGroupIndexes.push(groupIndex);
}
}
- }
-
- if (indexes.length === 0 && method.static) {
- const staticIndex = methodsOrder.indexOf('static-methods');
- if (staticIndex >= 0) {
- indexes.push(staticIndex);
- }
- }
-
- if (indexes.length === 0 && method.instanceVariable) {
- const annotationIndex = methodsOrder.indexOf('instance-variables');
- if (annotationIndex >= 0) {
- indexes.push(annotationIndex);
- }
- }
-
- if (indexes.length === 0 && method.instanceMethod) {
- const annotationIndex = methodsOrder.indexOf('instance-methods');
- if (annotationIndex >= 0) {
- indexes.push(annotationIndex);
- }
- }
+ });
// No matching pattern, return 'everything-else' index
- if (indexes.length === 0) {
- for (i = 0, j = methodsOrder.length; i < j; i++) {
- if (methodsOrder[i] === 'everything-else') {
- indexes.push(i);
- break;
- }
- }
- }
+ if (methodGroupIndexes.length === 0) {
+ const everythingElseIndex = methodsOrder.indexOf('everything-else');
- // No matching pattern and no 'everything-else' group
- if (indexes.length === 0) {
- indexes.push(Infinity);
+ if (everythingElseIndex !== -1) {
+ methodGroupIndexes.push(everythingElseIndex);
+ } else {
+ // No matching pattern and no 'everything-else' group
+ methodGroupIndexes.push(Infinity);
+ }
}
- return indexes;
+ return methodGroupIndexes;
}
/**
@@ -409,6 +417,10 @@ module.exports = {
// Loop around the properties a second time (for comparison)
for (k = 0, l = propertiesInfos.length; k < l; k++) {
+ if (i === k) {
+ continue;
+ }
+
propB = propertiesInfos[k];
// Compare the properties order
diff --git a/package.json b/package.json
index 15d1cfe585..13b420802c 100644
--- a/package.json
+++ b/package.json
@@ -24,6 +24,7 @@
"homepage": "https://github.com/yannickcr/eslint-plugin-react",
"bugs": "https://github.com/yannickcr/eslint-plugin-react/issues",
"dependencies": {
+ "array-includes": "^3.0.3",
"doctrine": "^2.1.0",
"has": "^1.0.3",
"jsx-ast-utils": "^2.0.1",
diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js
index fc26b96d2f..8277b10ab3 100644
--- a/tests/lib/rules/sort-comp.js
+++ b/tests/lib/rules/sort-comp.js
@@ -29,8 +29,8 @@ const ruleTester = new RuleTester({parserOptions});
ruleTester.run('sort-comp', rule, {
valid: [{
- // Must validate a full class
code: [
+ '// Must validate a full class',
'var Hello = createReactClass({',
' displayName : \'\',',
' propTypes: {},',
@@ -54,8 +54,8 @@ ruleTester.run('sort-comp', rule, {
'});'
].join('\n')
}, {
- // Must validate a class with missing groups
code: [
+ '// Must validate a class with missing groups',
'var Hello = createReactClass({',
' render: function() {',
' return Hello
;',
@@ -63,8 +63,8 @@ ruleTester.run('sort-comp', rule, {
'});'
].join('\n')
}, {
- // Must put a custom method in 'everything-else'
code: [
+ '// Must put a custom method in \'everything-else\'',
'var Hello = createReactClass({',
' onClick: function() {},',
' render: function() {',
@@ -73,8 +73,8 @@ ruleTester.run('sort-comp', rule, {
'});'
].join('\n')
}, {
- // Must allow us to re-order the groups
code: [
+ '// Must allow us to re-order the groups',
'var Hello = createReactClass({',
' displayName : \'Hello\',',
' render: function() {',
@@ -91,8 +91,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Must validate a full React 16.3 createReactClass class
code: [
+ '// Must validate a full React 16.3 createReactClass class',
'var Hello = createReactClass({',
' displayName : \'\',',
' propTypes: {},',
@@ -118,8 +118,8 @@ ruleTester.run('sort-comp', rule, {
'});'
].join('\n')
}, {
- // Must validate React 16.3 lifecycle methods with the default parser
code: [
+ '// Must validate React 16.3 lifecycle methods with the default parser',
'class Hello extends React.Component {',
' constructor() {}',
' static getDerivedStateFromProps() {}',
@@ -137,8 +137,8 @@ ruleTester.run('sort-comp', rule, {
'}'
].join('\n')
}, {
- // Must validate a full React 16.3 ES6 class
code: [
+ '// Must validate a full React 16.3 ES6 class',
'class Hello extends React.Component {',
' static displayName = \'\'',
' static propTypes = {}',
@@ -162,8 +162,8 @@ ruleTester.run('sort-comp', rule, {
].join('\n'),
parser: 'babel-eslint'
}, {
- // Must allow us to create a RegExp-based group
code: [
+ '// Must allow us to create a RegExp-based group',
'class Hello extends React.Component {',
' customHandler() {}',
' render() {',
@@ -181,8 +181,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Must allow us to create a named group
code: [
+ '// Must allow us to create a named group',
'class Hello extends React.Component {',
' customHandler() {}',
' render() {',
@@ -205,8 +205,8 @@ ruleTester.run('sort-comp', rule, {
}
}]
}, {
- // Must allow a method to be in different places if it's matches multiple patterns
code: [
+ '// Must allow a method to be in different places if it\'s matches multiple patterns',
'class Hello extends React.Component {',
' render() {',
' return Hello
;',
@@ -222,8 +222,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Must allow us to use 'constructor' as a method name
code: [
+ '// Must allow us to use \'constructor\' as a method name',
'class Hello extends React.Component {',
' constructor() {}',
' displayName() {}',
@@ -241,24 +241,24 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Must ignore stateless components
code: [
+ '// Must ignore stateless components',
'function Hello(props) {',
' return Hello {props.name}
',
'}'
].join('\n'),
parser: 'babel-eslint'
}, {
- // Must ignore stateless components (arrow function with explicit return)
code: [
+ '// Must ignore stateless components (arrow function with explicit return)',
'var Hello = props => (',
' Hello {props.name}
',
')'
].join('\n'),
parser: 'babel-eslint'
}, {
- // Must ignore spread operator
code: [
+ '// Must ignore spread operator',
'var Hello = createReactClass({',
' ...proto,',
' render: function() {',
@@ -268,8 +268,8 @@ ruleTester.run('sort-comp', rule, {
].join('\n'),
parser: 'babel-eslint'
}, {
- // Type Annotations should be first
code: [
+ '// Type Annotations should be first',
'class Hello extends React.Component {',
' props: { text: string };',
' constructor() {}',
@@ -289,8 +289,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Properties with Type Annotations should not be at the top
code: [
+ '// Properties with Type Annotations should not be at the top',
'class Hello extends React.Component {',
' props: { text: string };',
' constructor() {}',
@@ -311,8 +311,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Non-react classes should be ignored, even in expressions
code: [
+ '// Non-react classes should be ignored, even in expressions',
'return class Hello {',
' render() {',
' return {this.props.text}
;',
@@ -325,8 +325,8 @@ ruleTester.run('sort-comp', rule, {
parser: 'babel-eslint',
parserOptions: parserOptions
}, {
- // Non-react classes should be ignored, even in expressions
code: [
+ '// Non-react classes should be ignored, even in expressions',
'return class {',
' render() {',
' return {this.props.text}
;',
@@ -339,8 +339,8 @@ ruleTester.run('sort-comp', rule, {
parser: 'babel-eslint',
parserOptions: parserOptions
}, {
- // Getters should be at the top
code: [
+ '// Getters should be at the top',
'class Hello extends React.Component {',
' get foo() {}',
' constructor() {}',
@@ -360,8 +360,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Setters should be at the top
code: [
+ '// Setters should be at the top',
'class Hello extends React.Component {',
' set foo(bar) {}',
' constructor() {}',
@@ -381,8 +381,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Instance methods should be at the top
code: [
+ '// Instance methods should be at the top',
'class Hello extends React.Component {',
' foo = () => {}',
' constructor() {}',
@@ -403,8 +403,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Instance variables should be at the top
code: [
+ '// Instance variables should be at the top',
'class Hello extends React.Component {',
' foo = \'bar\'',
' constructor() {}',
@@ -424,11 +424,69 @@ ruleTester.run('sort-comp', rule, {
'render'
]
}]
+ }, {
+ code: [
+ '// Methods can be grouped with any matching group (with statics)',
+ 'class Hello extends React.Component {',
+ ' static onFoo() {}',
+ ' static renderFoo() {}',
+ ' render() {',
+ ' return {this.props.text}
;',
+ ' }',
+ ' getFoo() {}',
+ '}'
+ ].join('\n'),
+ options: [{
+ order: [
+ 'static-methods',
+ 'render',
+ '/^get.+$/',
+ '/^on.+$/',
+ '/^render.+$/'
+ ]
+ }]
+ }, {
+ code: [
+ '// Methods can be grouped with any matching group (with RegExp)',
+ 'class Hello extends React.Component {',
+ ' render() {',
+ ' return {this.props.text}
;',
+ ' }',
+ ' getFoo() {}',
+ ' static onFoo() {}',
+ ' static renderFoo() {}',
+ '}'
+ ].join('\n'),
+ options: [{
+ order: [
+ 'static-methods',
+ 'render',
+ '/^get.+$/',
+ '/^on.+$/',
+ '/^render.+$/'
+ ]
+ }]
+ }, {
+ code: [
+ '// static lifecycle methods can be grouped (with statics)',
+ 'class Hello extends React.Component {',
+ ' static getDerivedStateFromProps() {}',
+ ' constructor() {}',
+ '}'
+ ].join('\n')
+ }, {
+ code: [
+ '// static lifecycle methods can be grouped (with lifecycle)',
+ 'class Hello extends React.Component {',
+ ' constructor() {}',
+ ' static getDerivedStateFromProps() {}',
+ '}'
+ ].join('\n')
}],
invalid: [{
- // Must force a lifecycle method to be placed before render
code: [
+ '// Must force a lifecycle method to be placed before render',
'var Hello = createReactClass({',
' render: function() {',
' return Hello
;',
@@ -438,8 +496,8 @@ ruleTester.run('sort-comp', rule, {
].join('\n'),
errors: [{message: 'render should be placed after displayName'}]
}, {
- // Must run rule when render uses createElement instead of JSX
code: [
+ '// Must run rule when render uses createElement instead of JSX',
'var Hello = createReactClass({',
' render: function() {',
' return React.createElement("div", null, "Hello");',
@@ -449,8 +507,8 @@ ruleTester.run('sort-comp', rule, {
].join('\n'),
errors: [{message: 'render should be placed after displayName'}]
}, {
- // Must force a custom method to be placed before render
code: [
+ '// Must force a custom method to be placed before render',
'var Hello = createReactClass({',
' render: function() {',
' return Hello
;',
@@ -460,8 +518,8 @@ ruleTester.run('sort-comp', rule, {
].join('\n'),
errors: [{message: 'render should be placed after onClick'}]
}, {
- // Must force a custom method to be placed before render, even in function
code: [
+ '// Must force a custom method to be placed before render, even in function',
'var Hello = () => {',
' return class Test extends React.Component {',
' render () {',
@@ -474,8 +532,8 @@ ruleTester.run('sort-comp', rule, {
parserOptions: parserOptions,
errors: [{message: 'render should be placed after onClick'}]
}, {
- // Must force a custom method to be placed after render if no 'everything-else' group is specified
code: [
+ '// Must force a custom method to be placed after render if no \'everything-else\' group is specified',
'var Hello = createReactClass({',
' displayName: \'Hello\',',
' onClick: function() {},',
@@ -492,8 +550,8 @@ ruleTester.run('sort-comp', rule, {
}],
errors: [{message: 'onClick should be placed after render'}]
}, {
- // Must validate static properties
code: [
+ '// Must validate static properties',
'class Hello extends React.Component {',
' render() {',
' return ',
@@ -504,17 +562,8 @@ ruleTester.run('sort-comp', rule, {
parser: 'babel-eslint',
errors: [{message: 'render should be placed after displayName'}]
}, {
- // Must validate static lifecycle methods
- code: [
- 'class Hello extends React.Component {',
- ' static getDerivedStateFromProps() {}',
- ' constructor() {}',
- '}'
- ].join('\n'),
- errors: [{message: 'getDerivedStateFromProps should be placed after constructor'}]
- }, {
- // Type Annotations should not be at the top by default
code: [
+ '// Type Annotations should not be at the top by default',
'class Hello extends React.Component {',
' props: { text: string };',
' constructor() {}',
@@ -527,8 +576,8 @@ ruleTester.run('sort-comp', rule, {
parser: 'babel-eslint',
errors: [{message: 'props should be placed after state'}]
}, {
- // Type Annotations should be first
code: [
+ '// Type Annotations should be first',
'class Hello extends React.Component {',
' constructor() {}',
' props: { text: string };',
@@ -549,8 +598,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Properties with Type Annotations should not be at the top
code: [
+ '// Properties with Type Annotations should not be at the top',
'class Hello extends React.Component {',
' props: { text: string };',
' state: Object = {};',
@@ -573,6 +622,7 @@ ruleTester.run('sort-comp', rule, {
}]
}, {
code: [
+ '// componentDidMountOk should be placed after getA',
'export default class View extends React.Component {',
' componentDidMountOk() {}',
' getB() {}',
@@ -595,8 +645,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Getters should at the top
code: [
+ '// Getters should at the top',
'class Hello extends React.Component {',
' constructor() {}',
' get foo() {}',
@@ -617,8 +667,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Setters should at the top
code: [
+ '// Setters should at the top',
'class Hello extends React.Component {',
' constructor() {}',
' set foo(bar) {}',
@@ -639,8 +689,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Instance methods should not be at the top
code: [
+ '// Instance methods should not be at the top',
'class Hello extends React.Component {',
' constructor() {}',
' static bar = () => {}',
@@ -662,8 +712,8 @@ ruleTester.run('sort-comp', rule, {
]
}]
}, {
- // Instance variables should not be at the top
code: [
+ '// Instance variables should not be at the top',
'class Hello extends React.Component {',
' constructor() {}',
' state = {}',
@@ -684,5 +734,37 @@ ruleTester.run('sort-comp', rule, {
'render'
]
}]
+ }, {
+ code: [
+ '// Should not confuse method names with group names',
+ 'class Hello extends React.Component {',
+ ' setters() {}',
+ ' constructor() {}',
+ ' render() {}',
+ '}'
+ ].join('\n'),
+ errors: [{message: 'setters should be placed after render'}],
+ options: [{
+ order: [
+ 'setters',
+ 'lifecycle',
+ 'render'
+ ]
+ }]
+ }, {
+ code: [
+ '// Explicitly named methods should appear in the correct order',
+ 'class Hello extends React.Component {',
+ ' render() {}',
+ ' foo() {}',
+ '}'
+ ].join('\n'),
+ errors: [{message: 'render should be placed after foo'}],
+ options: [{
+ order: [
+ 'foo',
+ 'render'
+ ]
+ }]
}]
});