From 75b78bae016795811a06fcfaf9eef1e597cf4c4a Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Thu, 20 Jul 2017 13:08:48 -0400 Subject: [PATCH 1/2] Teach sort-comp rule about getters and setters. --- docs/rules/sort-comp.md | 2 ++ lib/rules/sort-comp.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/docs/rules/sort-comp.md b/docs/rules/sort-comp.md index 33c94a1b14..52e8deadbe 100644 --- a/docs/rules/sort-comp.md +++ b/docs/rules/sort-comp.md @@ -90,6 +90,8 @@ The default configuration is: * `everything-else` is a special group that match all the methods that do not match any of the other groups. * `render` is referring to the `render` method. * `type-annotations`. This group is not specified by default, but can be used to enforce flow annotations to be at the top. +* `getters` This group is not specified by default, but can be used to enforce class getters positioning. +* `setters` This group is not specified by default, but can be used to enforce class setters positioning. You can override this configuration to match your needs. diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index ffa952f642..6557992648 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -136,6 +136,24 @@ module.exports = { } } + if (method.getter) { + for (i = 0, j = methodsOrder.length; i < j; i++) { + if (methodsOrder[i] === 'getters') { + indexes.push(i); + break; + } + } + } + + if (method.setter) { + for (i = 0, j = methodsOrder.length; i < j; i++) { + if (methodsOrder[i] === 'setters') { + indexes.push(i); + break; + } + } + } + if (method.typeAnnotation) { for (i = 0, j = methodsOrder.length; i < j; i++) { if (methodsOrder[i] === 'type-annotations') { @@ -363,6 +381,8 @@ module.exports = { function checkPropsOrder(properties) { const propertiesInfos = properties.map(node => ({ name: getPropertyName(node), + getter: node.kind === 'get', + setter: node.kind === 'set', static: node.static, typeAnnotation: !!node.typeAnnotation && node.value === null })); From b1c1f84348fa86807a69632255f4f85cbe6a253f Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Thu, 20 Jul 2017 14:47:40 -0400 Subject: [PATCH 2/2] Small index tweak, tests, and getter + setter property name handling. --- lib/rules/sort-comp.js | 40 ++++++++--------- tests/lib/rules/sort-comp.js | 86 ++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 20 deletions(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 6557992648..e214905081 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -128,38 +128,30 @@ module.exports = { const indexes = []; if (method.static) { - for (i = 0, j = methodsOrder.length; i < j; i++) { - if (methodsOrder[i] === 'static-methods') { - indexes.push(i); - break; - } + const staticIndex = methodsOrder.indexOf('static-methods'); + if (staticIndex >= 0) { + indexes.push(staticIndex); } } if (method.getter) { - for (i = 0, j = methodsOrder.length; i < j; i++) { - if (methodsOrder[i] === 'getters') { - indexes.push(i); - break; - } + const getterIndex = methodsOrder.indexOf('getters'); + if (getterIndex >= 0) { + indexes.push(getterIndex); } } if (method.setter) { - for (i = 0, j = methodsOrder.length; i < j; i++) { - if (methodsOrder[i] === 'setters') { - indexes.push(i); - break; - } + const setterIndex = methodsOrder.indexOf('setters'); + if (setterIndex >= 0) { + indexes.push(setterIndex); } } if (method.typeAnnotation) { - for (i = 0, j = methodsOrder.length; i < j; i++) { - if (methodsOrder[i] === 'type-annotations') { - indexes.push(i); - break; - } + const annotationIndex = methodsOrder.indexOf('type-annotations'); + if (annotationIndex >= 0) { + indexes.push(annotationIndex); } } @@ -210,6 +202,14 @@ module.exports = { return tokens[1] && tokens[1].type === 'Identifier' ? tokens[1].value : tokens[0].value; } + if (node.kind === 'get') { + return 'getter functions'; + } + + if (node.kind === 'set') { + return 'setter functions'; + } + return node.key.name; } diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index f2713cbd50..df1944040e 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -268,6 +268,48 @@ ruleTester.run('sort-comp', rule, { ].join('\n'), parser: 'babel-eslint', parserOptions: parserOptions + }, { + // Getters should be at the top + code: [ + 'class Hello extends React.Component {', + ' get foo() {}', + ' constructor() {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + order: [ + 'getters', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] + }, { + // Setters should be at the top + code: [ + 'class Hello extends React.Component {', + ' set foo(bar) {}', + ' constructor() {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + order: [ + 'setters', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] }], invalid: [{ @@ -429,5 +471,49 @@ ruleTester.run('sort-comp', rule, { 'render' ] }] + }, { + // Getters should at the top + code: [ + 'class Hello extends React.Component {', + ' constructor() {}', + ' get foo() {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{message: 'constructor should be placed after getter functions'}], + options: [{ + order: [ + 'getters', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] + }, { + // Setters should at the top + code: [ + 'class Hello extends React.Component {', + ' constructor() {}', + ' set foo(bar) {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{message: 'constructor should be placed after setter functions'}], + options: [{ + order: [ + 'setters', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] }] });