diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 6befbc0ba8..8d98f4b476 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -131,84 +131,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 ([ + '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' + ].includes(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 +416,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/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' + ] + }] }] });