From b2fefdcdd82fef51de3a7f19f0dc4289de0169f5 Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Tue, 10 Oct 2017 17:28:30 -0400 Subject: [PATCH 1/4] Add instance-methods and instance-variables to sort-comp. --- lib/rules/sort-comp.js | 16 +++++++ tests/lib/rules/sort-comp.js | 88 ++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index e214905081..d31035e3db 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -171,6 +171,20 @@ module.exports = { } } + 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++) { @@ -384,6 +398,8 @@ module.exports = { getter: node.kind === 'get', setter: node.kind === 'set', static: node.static, + instanceVariable: node.value && node.value.type !== 'ArrowFunctionExpression' && node.value.type !== 'FunctionExpression', + instanceMethod: node.value && (node.value.type === 'ArrowFunctionExpression' || node.value.type === 'FunctionExpression'), typeAnnotation: !!node.typeAnnotation && node.value === null })); diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index df1944040e..8e0a5492b5 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -310,6 +310,49 @@ ruleTester.run('sort-comp', rule, { 'render' ] }] + }, { + // Instance methods should be at the top + code: [ + 'class Hello extends React.Component {', + ' foo = () => {}', + ' constructor() {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + order: [ + 'instance-methods', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] + }, { + // Instance variables should be at the top + code: [ + 'class Hello extends React.Component {', + ' foo = "bar"', + ' constructor() {}', + ' state = {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + order: [ + 'instance-variables', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] }], invalid: [{ @@ -515,5 +558,50 @@ ruleTester.run('sort-comp', rule, { 'render' ] }] + }, { + // Instance methods should not be at the top + code: [ + 'class Hello extends React.Component {', + ' constructor() {}', + ' foo = () => {}', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{message: 'constructor should be placed after foo'}], + options: [{ + order: [ + 'instance-methods', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] + }, { + // Instance variables should not be at the top + code: [ + 'class Hello extends React.Component {', + ' constructor() {}', + ' state = {}', + ' foo = "bar"', + ' render() {', + ' return
{this.props.text}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{message: 'foo should be placed before constructor'}], + options: [{ + order: [ + 'instance-variables', + 'static-methods', + 'lifecycle', + 'everything-else', + 'render' + ] + }] }] }); From 3d0dc239b1b8db794ec9054e1980fcb8edeae63f Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Tue, 10 Oct 2017 17:32:18 -0400 Subject: [PATCH 2/4] Update docs. --- docs/rules/sort-comp.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/rules/sort-comp.md b/docs/rules/sort-comp.md index c1ec19b0bb..49fc85f9bd 100644 --- a/docs/rules/sort-comp.md +++ b/docs/rules/sort-comp.md @@ -89,9 +89,11 @@ The default configuration is: * `lifecycle` is referring 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 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. +* `type-annotations`. This group is not specified by default, but can be used to enforce flow annotations positioning. * `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. +* `instance-variables` This group is not specified by default, but can be used to enforce all other instance variables positioning. +* `instance-methods` This group is not specified by default, but can be used to enforce all other instance methods positioning. You can override this configuration to match your needs. From 0eacd40996496319f792fd58b8bf83af16e705b2 Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Tue, 10 Oct 2017 17:55:11 -0400 Subject: [PATCH 3/4] Improve tests. --- lib/rules/sort-comp.js | 10 ++++++++-- tests/lib/rules/sort-comp.js | 14 +++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index d31035e3db..8e53530b9f 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -398,8 +398,14 @@ module.exports = { getter: node.kind === 'get', setter: node.kind === 'set', static: node.static, - instanceVariable: node.value && node.value.type !== 'ArrowFunctionExpression' && node.value.type !== 'FunctionExpression', - instanceMethod: node.value && (node.value.type === 'ArrowFunctionExpression' || node.value.type === 'FunctionExpression'), + instanceVariable: !node.static && + node.value && + node.value.type !== 'ArrowFunctionExpression' && + node.value.type !== 'FunctionExpression', + instanceMethod: !node.static && + node.value && + (node.value.type === 'ArrowFunctionExpression' || + node.value.type === 'FunctionExpression'), typeAnnotation: !!node.typeAnnotation && node.value === null })); diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 8e0a5492b5..032ec7f1d0 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -316,6 +316,7 @@ ruleTester.run('sort-comp', rule, { 'class Hello extends React.Component {', ' foo = () => {}', ' constructor() {}', + ' static bar = () => {}', ' render() {', ' return
{this.props.text}
;', ' }', @@ -325,7 +326,6 @@ ruleTester.run('sort-comp', rule, { options: [{ order: [ 'instance-methods', - 'static-methods', 'lifecycle', 'everything-else', 'render' @@ -335,9 +335,10 @@ ruleTester.run('sort-comp', rule, { // Instance variables should be at the top code: [ 'class Hello extends React.Component {', - ' foo = "bar"', + ' foo = \'bar\'', ' constructor() {}', ' state = {}', + ' static bar = \'foo\'', ' render() {', ' return
{this.props.text}
;', ' }', @@ -347,7 +348,6 @@ ruleTester.run('sort-comp', rule, { options: [{ order: [ 'instance-variables', - 'static-methods', 'lifecycle', 'everything-else', 'render' @@ -563,6 +563,7 @@ ruleTester.run('sort-comp', rule, { code: [ 'class Hello extends React.Component {', ' constructor() {}', + ' static bar = () => {}', ' foo = () => {}', ' render() {', ' return
{this.props.text}
;', @@ -570,11 +571,10 @@ ruleTester.run('sort-comp', rule, { '}' ].join('\n'), parser: 'babel-eslint', - errors: [{message: 'constructor should be placed after foo'}], + errors: [{message: 'foo should be placed before constructor'}], options: [{ order: [ 'instance-methods', - 'static-methods', 'lifecycle', 'everything-else', 'render' @@ -586,7 +586,8 @@ ruleTester.run('sort-comp', rule, { 'class Hello extends React.Component {', ' constructor() {}', ' state = {}', - ' foo = "bar"', + ' static bar = {}', + ' foo = {}', ' render() {', ' return
{this.props.text}
;', ' }', @@ -597,7 +598,6 @@ ruleTester.run('sort-comp', rule, { options: [{ order: [ 'instance-variables', - 'static-methods', 'lifecycle', 'everything-else', 'render' From 7efe31a60c5bf83f31e408c88ce677f037c3232c Mon Sep 17 00:00:00 2001 From: Roger Graham Date: Tue, 10 Oct 2017 18:16:43 -0400 Subject: [PATCH 4/4] Account for class methods. --- lib/rules/sort-comp.js | 2 ++ tests/lib/rules/sort-comp.js | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 8e53530b9f..acb6c8158f 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -399,10 +399,12 @@ module.exports = { setter: node.kind === 'set', static: node.static, instanceVariable: !node.static && + node.type === 'ClassProperty' && node.value && node.value.type !== 'ArrowFunctionExpression' && node.value.type !== 'FunctionExpression', instanceMethod: !node.static && + node.type === 'ClassProperty' && node.value && (node.value.type === 'ArrowFunctionExpression' || node.value.type === 'FunctionExpression'), diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 032ec7f1d0..ac9da200a3 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -316,6 +316,7 @@ ruleTester.run('sort-comp', rule, { 'class Hello extends React.Component {', ' foo = () => {}', ' constructor() {}', + ' classMethod() {}', ' static bar = () => {}', ' render() {', ' return
{this.props.text}
;', @@ -564,7 +565,8 @@ ruleTester.run('sort-comp', rule, { 'class Hello extends React.Component {', ' constructor() {}', ' static bar = () => {}', - ' foo = () => {}', + ' classMethod() {}', + ' foo = function() {}', ' render() {', ' return
{this.props.text}
;', ' }',