Skip to content

Commit 067882b

Browse files
authored
Merge pull request jsx-eslint#1311 from RDGthree/master
Teach sort-comp rule about getters and setters.
2 parents ef2054b + b1c1f84 commit 067882b

File tree

3 files changed

+118
-10
lines changed

3 files changed

+118
-10
lines changed

docs/rules/sort-comp.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ The default configuration is:
9090
* `everything-else` is a special group that match all the methods that do not match any of the other groups.
9191
* `render` is referring to the `render` method.
9292
* `type-annotations`. This group is not specified by default, but can be used to enforce flow annotations to be at the top.
93+
* `getters` This group is not specified by default, but can be used to enforce class getters positioning.
94+
* `setters` This group is not specified by default, but can be used to enforce class setters positioning.
9395

9496
You can override this configuration to match your needs.
9597

lib/rules/sort-comp.js

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,20 +128,30 @@ module.exports = {
128128
const indexes = [];
129129

130130
if (method.static) {
131-
for (i = 0, j = methodsOrder.length; i < j; i++) {
132-
if (methodsOrder[i] === 'static-methods') {
133-
indexes.push(i);
134-
break;
135-
}
131+
const staticIndex = methodsOrder.indexOf('static-methods');
132+
if (staticIndex >= 0) {
133+
indexes.push(staticIndex);
134+
}
135+
}
136+
137+
if (method.getter) {
138+
const getterIndex = methodsOrder.indexOf('getters');
139+
if (getterIndex >= 0) {
140+
indexes.push(getterIndex);
141+
}
142+
}
143+
144+
if (method.setter) {
145+
const setterIndex = methodsOrder.indexOf('setters');
146+
if (setterIndex >= 0) {
147+
indexes.push(setterIndex);
136148
}
137149
}
138150

139151
if (method.typeAnnotation) {
140-
for (i = 0, j = methodsOrder.length; i < j; i++) {
141-
if (methodsOrder[i] === 'type-annotations') {
142-
indexes.push(i);
143-
break;
144-
}
152+
const annotationIndex = methodsOrder.indexOf('type-annotations');
153+
if (annotationIndex >= 0) {
154+
indexes.push(annotationIndex);
145155
}
146156
}
147157

@@ -192,6 +202,14 @@ module.exports = {
192202
return tokens[1] && tokens[1].type === 'Identifier' ? tokens[1].value : tokens[0].value;
193203
}
194204

205+
if (node.kind === 'get') {
206+
return 'getter functions';
207+
}
208+
209+
if (node.kind === 'set') {
210+
return 'setter functions';
211+
}
212+
195213
return node.key.name;
196214
}
197215

@@ -363,6 +381,8 @@ module.exports = {
363381
function checkPropsOrder(properties) {
364382
const propertiesInfos = properties.map(node => ({
365383
name: getPropertyName(node),
384+
getter: node.kind === 'get',
385+
setter: node.kind === 'set',
366386
static: node.static,
367387
typeAnnotation: !!node.typeAnnotation && node.value === null
368388
}));

tests/lib/rules/sort-comp.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,48 @@ ruleTester.run('sort-comp', rule, {
268268
].join('\n'),
269269
parser: 'babel-eslint',
270270
parserOptions: parserOptions
271+
}, {
272+
// Getters should be at the top
273+
code: [
274+
'class Hello extends React.Component {',
275+
' get foo() {}',
276+
' constructor() {}',
277+
' render() {',
278+
' return <div>{this.props.text}</div>;',
279+
' }',
280+
'}'
281+
].join('\n'),
282+
parser: 'babel-eslint',
283+
options: [{
284+
order: [
285+
'getters',
286+
'static-methods',
287+
'lifecycle',
288+
'everything-else',
289+
'render'
290+
]
291+
}]
292+
}, {
293+
// Setters should be at the top
294+
code: [
295+
'class Hello extends React.Component {',
296+
' set foo(bar) {}',
297+
' constructor() {}',
298+
' render() {',
299+
' return <div>{this.props.text}</div>;',
300+
' }',
301+
'}'
302+
].join('\n'),
303+
parser: 'babel-eslint',
304+
options: [{
305+
order: [
306+
'setters',
307+
'static-methods',
308+
'lifecycle',
309+
'everything-else',
310+
'render'
311+
]
312+
}]
271313
}],
272314

273315
invalid: [{
@@ -429,5 +471,49 @@ ruleTester.run('sort-comp', rule, {
429471
'render'
430472
]
431473
}]
474+
}, {
475+
// Getters should at the top
476+
code: [
477+
'class Hello extends React.Component {',
478+
' constructor() {}',
479+
' get foo() {}',
480+
' render() {',
481+
' return <div>{this.props.text}</div>;',
482+
' }',
483+
'}'
484+
].join('\n'),
485+
parser: 'babel-eslint',
486+
errors: [{message: 'constructor should be placed after getter functions'}],
487+
options: [{
488+
order: [
489+
'getters',
490+
'static-methods',
491+
'lifecycle',
492+
'everything-else',
493+
'render'
494+
]
495+
}]
496+
}, {
497+
// Setters should at the top
498+
code: [
499+
'class Hello extends React.Component {',
500+
' constructor() {}',
501+
' set foo(bar) {}',
502+
' render() {',
503+
' return <div>{this.props.text}</div>;',
504+
' }',
505+
'}'
506+
].join('\n'),
507+
parser: 'babel-eslint',
508+
errors: [{message: 'constructor should be placed after setter functions'}],
509+
options: [{
510+
order: [
511+
'setters',
512+
'static-methods',
513+
'lifecycle',
514+
'everything-else',
515+
'render'
516+
]
517+
}]
432518
}]
433519
});

0 commit comments

Comments
 (0)