From a312b16ea41203fc02054308f590f0e65784e2c3 Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sun, 12 Aug 2018 23:46:41 +0700 Subject: [PATCH 1/7] Init rule state-in-constructor --- index.js | 1 + lib/rules/state-in-constructor.js | 39 ++++++ tests/lib/rules/state-in-constructor.js | 155 ++++++++++++++++++++++++ 3 files changed, 195 insertions(+) create mode 100644 lib/rules/state-in-constructor.js create mode 100644 tests/lib/rules/state-in-constructor.js diff --git a/index.js b/index.js index 45a96d7da3..05c1d97daf 100644 --- a/index.js +++ b/index.js @@ -78,6 +78,7 @@ const allRules = { 'self-closing-comp': require('./lib/rules/self-closing-comp'), 'sort-comp': require('./lib/rules/sort-comp'), 'sort-prop-types': require('./lib/rules/sort-prop-types'), + 'state-in-constructor': require('./lib/rules/state-in-constructor'), 'style-prop-object': require('./lib/rules/style-prop-object'), 'void-dom-elements-no-children': require('./lib/rules/void-dom-elements-no-children') }; diff --git a/lib/rules/state-in-constructor.js b/lib/rules/state-in-constructor.js new file mode 100644 index 0000000000..a0c7ecd514 --- /dev/null +++ b/lib/rules/state-in-constructor.js @@ -0,0 +1,39 @@ +/** + * @fileoverview State initialization in an ES6 class component should be in a constructor + * @author Kanitkorn Sujautra + */ +'use strict'; + +const Components = require('../util/Components'); +const docsUrl = require('../util/docsUrl'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'State initialization in an ES6 class component should be in a constructor', + category: 'Stylistic Issues', + recommended: false, + url: docsUrl('state-in-constructor') + }, + schema: [] + }, + + create: Components.detect((context, components, utils) => ({ + ClassProperty: function (node) { + if ( + !node.static && + node.key.name === 'state' && + utils.isES6Component(node.parent.parent) + ) { + context.report({ + node, + message: 'State initialization should be in a constructor' + }); + } + } + })) +}; diff --git a/tests/lib/rules/state-in-constructor.js b/tests/lib/rules/state-in-constructor.js new file mode 100644 index 0000000000..198bc0d852 --- /dev/null +++ b/tests/lib/rules/state-in-constructor.js @@ -0,0 +1,155 @@ +/** + * @fileoverview State initialization in an ES6 class component should be in a constructor + * @author Kanitkorn Sujautra + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/state-in-constructor'); +const RuleTester = require('eslint').RuleTester; + +const ruleTesterConfig = { + parser: 'babel-eslint', + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true + } + } +}; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(ruleTesterConfig); +ruleTester.run('state-in-constructor', rule, { + valid: [{ + code: ` + class Foo extends React.Component { + render() { + return
Foo
+ } + } + ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + render() { + return
Foo
+ } + } + ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + baz = { bar: 0 } + render() { + return
Foo
+ } + } + ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + render() { + return
Foo
+ } + } + ` + }, { + code: ` + class Foo extends React.Component { + baz = { bar: 0 } + render() { + return
Foo
+ } + } + ` + }, { + code: ` + const Foo = () =>
Foo
+ ` + }, { + code: ` + function Foo () { + return
Foo
+ } + ` + }], + + invalid: [{ + code: ` + class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } + } + `, + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + state = { bar: 0 } + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, + errors: [{ + message: 'State initialization should be in a constructor' + }] + }] +}); From 2f0667759a1632af07e1fb8f06dd99051f898821 Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sat, 18 Aug 2018 23:10:06 +0700 Subject: [PATCH 2/7] Add option for state-in-constructor rule --- lib/rules/state-in-constructor.js | 50 +++-- tests/lib/rules/state-in-constructor.js | 263 +++++++++++++++++++++++- 2 files changed, 297 insertions(+), 16 deletions(-) diff --git a/lib/rules/state-in-constructor.js b/lib/rules/state-in-constructor.js index a0c7ecd514..2243c0a40f 100644 --- a/lib/rules/state-in-constructor.js +++ b/lib/rules/state-in-constructor.js @@ -1,5 +1,5 @@ /** - * @fileoverview State initialization in an ES6 class component should be in a constructor + * @fileoverview Enforce the state initialization style to be either in a constructor or with a class property * @author Kanitkorn Sujautra */ 'use strict'; @@ -19,21 +19,41 @@ module.exports = { recommended: false, url: docsUrl('state-in-constructor') }, - schema: [] + schema: [{ + enum: ['always', 'never'] + }] }, - create: Components.detect((context, components, utils) => ({ - ClassProperty: function (node) { - if ( - !node.static && - node.key.name === 'state' && - utils.isES6Component(node.parent.parent) - ) { - context.report({ - node, - message: 'State initialization should be in a constructor' - }); + create: Components.detect((context, components, utils) => { + const option = context.options[0] || 'always'; + return { + ClassProperty: function (node) { + if ( + option === 'always' && + !node.static && + node.key.name === 'state' && + utils.isES6Component(node.parent.parent) + ) { + context.report({ + node, + message: 'State initialization should be in a constructor' + }); + } + }, + AssignmentExpression(node) { + if ( + option === 'never' && + node.left.object.type === 'ThisExpression' && + node.left.property.name === 'state' && + node.parent.parent.parent.parent.kind === 'constructor' && + utils.isES6Component(node.parent.parent.parent.parent.parent.parent) + ) { + context.report({ + node, + message: 'State initialization should be in a class property' + }); + } } - } - })) + }; + }) }; diff --git a/tests/lib/rules/state-in-constructor.js b/tests/lib/rules/state-in-constructor.js index 198bc0d852..ec8057fae2 100644 --- a/tests/lib/rules/state-in-constructor.js +++ b/tests/lib/rules/state-in-constructor.js @@ -1,5 +1,5 @@ /** - * @fileoverview State initialization in an ES6 class component should be in a constructor + * @fileoverview Enforce the state initialization style to be either in a constructor or with a class property * @author Kanitkorn Sujautra */ 'use strict'; @@ -36,6 +36,24 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + render() { + return
Foo
+ } + } + `, + options: ['always'] + }, { + code: ` + class Foo extends React.Component { + render() { + return
Foo
+ } + } + `, + options: ['never'] }, { code: ` class Foo extends React.Component { @@ -48,6 +66,19 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + render() { + return
Foo
+ } + } + `, + options: ['always'] }, { code: ` class Foo extends React.Component { @@ -61,6 +92,20 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['always'] }, { code: ` class Foo extends React.Component { @@ -73,6 +118,32 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + render() { + return
Foo
+ } + } + `, + options: ['always'] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + render() { + return
Foo
+ } + } + `, + options: ['never'] }, { code: ` class Foo extends React.Component { @@ -82,22 +153,160 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['always'] + }, { + code: ` + class Foo extends React.Component { + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'] }, { code: ` const Foo = () =>
Foo
` + }, { + code: ` + const Foo = () =>
Foo
+ `, + options: ['always'] + }, { + code: ` + const Foo = () =>
Foo
+ `, + options: ['never'] }, { code: ` function Foo () { return
Foo
} ` + }, { + code: ` + function Foo () { + return
Foo
+ } + `, + options: ['always'] + }, { + code: ` + function Foo () { + return
Foo
+ } + `, + options: ['never'] + }, { + code: ` + class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'] + }, { + code: ` + class Foo extends React.Component { + state = { bar: 0 } + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'] }], invalid: [{ + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + render() { + return
Foo
+ } + } + `, + options: ['never'], + errors: [{ + message: 'State initialization should be in a class property' + }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + baz = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'], + errors: [{ + message: 'State initialization should be in a class property' + }] + }, { + code: ` + class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } + } + `, + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } + } + `, + options: ['always'], + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { code: ` class Foo extends React.Component { state = { bar: 0 } + baz = { bar: 0 } render() { return
Foo
} @@ -116,6 +325,23 @@ ruleTester.run('state-in-constructor', rule, { } } `, + options: ['always'], + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.baz = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, errors: [{ message: 'State initialization should be in a constructor' }] @@ -132,6 +358,7 @@ ruleTester.run('state-in-constructor', rule, { } } `, + options: ['always'], errors: [{ message: 'State initialization should be in a constructor' }] @@ -151,5 +378,39 @@ ruleTester.run('state-in-constructor', rule, { errors: [{ message: 'State initialization should be in a constructor' }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, + options: ['always'], + errors: [{ + message: 'State initialization should be in a constructor' + }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + state = { baz: 0 } + render() { + return
Foo
+ } + } + `, + options: ['never'], + errors: [{ + message: 'State initialization should be in a class property' + }] }] }); From 4706e8696edaedc122eb138885de50c6c967c52c Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sat, 18 Aug 2018 23:30:48 +0700 Subject: [PATCH 3/7] Add doc for state-in-constructor rule --- docs/rules/state-in-constructor.md | 74 ++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 docs/rules/state-in-constructor.md diff --git a/docs/rules/state-in-constructor.md b/docs/rules/state-in-constructor.md new file mode 100644 index 0000000000..395862402d --- /dev/null +++ b/docs/rules/state-in-constructor.md @@ -0,0 +1,74 @@ +# Enforce state initialization style (react/state-in-constructor) + +This rule will enforce the state initialization style to be either in a constructor or with a class property. + +## Rule Options + +```js +... +"react/state-in-constructor": [, ] +... +``` + +### `always` mode + +Will enforce the state initialization style to be in a constructor. This is the default mode. + +The following patterns are considered warnings: + +```jsx +class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } +} +``` + +The following patterns are **not** considered warnings: + +```jsx +class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + render() { + return
Foo
+ } +} +``` + +### `never` mode + +Will enforce the state initialization style to be with a class property. + +The following patterns are considered warnings: + +```jsx +class Foo extends React.Component { + constructor(props) { + super(props) + this.state = { bar: 0 } + } + render() { + return
Foo
+ } +} +``` + +The following patterns are **not** considered warnings: + +```jsx +class Foo extends React.Component { + state = { bar: 0 } + render() { + return
Foo
+ } +} +``` + + +## When Not To Use It + +When the way a component state is being initialized doesn't matter. From 81d0e8bea4dc3dac17ea47bf797a257ee266698b Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sun, 19 Aug 2018 09:38:12 +0700 Subject: [PATCH 4/7] Remove test cases for `always` option in state-in-constructor rule Since `always` is the default option, we can use the no-option cases to cover these `always` cases. I left one `always` case to make sure that having an option as `always` will act the same way as no-option. --- tests/lib/rules/state-in-constructor.js | 119 ------------------------ 1 file changed, 119 deletions(-) diff --git a/tests/lib/rules/state-in-constructor.js b/tests/lib/rules/state-in-constructor.js index ec8057fae2..a09233ad1c 100644 --- a/tests/lib/rules/state-in-constructor.js +++ b/tests/lib/rules/state-in-constructor.js @@ -36,15 +36,6 @@ ruleTester.run('state-in-constructor', rule, { } } ` - }, { - code: ` - class Foo extends React.Component { - render() { - return
Foo
- } - } - `, - options: ['always'] }, { code: ` class Foo extends React.Component { @@ -92,20 +83,6 @@ ruleTester.run('state-in-constructor', rule, { } } ` - }, { - code: ` - class Foo extends React.Component { - constructor(props) { - super(props) - this.state = { bar: 0 } - } - baz = { bar: 0 } - render() { - return
Foo
- } - } - `, - options: ['always'] }, { code: ` class Foo extends React.Component { @@ -118,19 +95,6 @@ ruleTester.run('state-in-constructor', rule, { } } ` - }, { - code: ` - class Foo extends React.Component { - constructor(props) { - super(props) - this.baz = { bar: 0 } - } - render() { - return
Foo
- } - } - `, - options: ['always'] }, { code: ` class Foo extends React.Component { @@ -153,16 +117,6 @@ ruleTester.run('state-in-constructor', rule, { } } ` - }, { - code: ` - class Foo extends React.Component { - baz = { bar: 0 } - render() { - return
Foo
- } - } - `, - options: ['always'] }, { code: ` class Foo extends React.Component { @@ -177,11 +131,6 @@ ruleTester.run('state-in-constructor', rule, { code: ` const Foo = () =>
Foo
` - }, { - code: ` - const Foo = () =>
Foo
- `, - options: ['always'] }, { code: ` const Foo = () =>
Foo
@@ -193,13 +142,6 @@ ruleTester.run('state-in-constructor', rule, { return
Foo
} ` - }, { - code: ` - function Foo () { - return
Foo
- } - `, - options: ['always'] }, { code: ` function Foo () { @@ -289,19 +231,6 @@ ruleTester.run('state-in-constructor', rule, { errors: [{ message: 'State initialization should be in a constructor' }] - }, { - code: ` - class Foo extends React.Component { - state = { bar: 0 } - render() { - return
Foo
- } - } - `, - options: ['always'], - errors: [{ - message: 'State initialization should be in a constructor' - }] }, { code: ` class Foo extends React.Component { @@ -315,20 +244,6 @@ ruleTester.run('state-in-constructor', rule, { errors: [{ message: 'State initialization should be in a constructor' }] - }, { - code: ` - class Foo extends React.Component { - state = { bar: 0 } - baz = { bar: 0 } - render() { - return
Foo
- } - } - `, - options: ['always'], - errors: [{ - message: 'State initialization should be in a constructor' - }] }, { code: ` class Foo extends React.Component { @@ -345,39 +260,6 @@ ruleTester.run('state-in-constructor', rule, { errors: [{ message: 'State initialization should be in a constructor' }] - }, { - code: ` - class Foo extends React.Component { - constructor(props) { - super(props) - this.baz = { bar: 0 } - } - state = { baz: 0 } - render() { - return
Foo
- } - } - `, - options: ['always'], - errors: [{ - message: 'State initialization should be in a constructor' - }] - }, { - code: ` - class Foo extends React.Component { - constructor(props) { - super(props) - this.state = { bar: 0 } - } - state = { baz: 0 } - render() { - return
Foo
- } - } - `, - errors: [{ - message: 'State initialization should be in a constructor' - }] }, { code: ` class Foo extends React.Component { @@ -391,7 +273,6 @@ ruleTester.run('state-in-constructor', rule, { } } `, - options: ['always'], errors: [{ message: 'State initialization should be in a constructor' }] From 3516a813e6f2e737fa7450cffcc891020a2bd36a Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sun, 19 Aug 2018 09:49:23 +0700 Subject: [PATCH 5/7] Fix a case where AssignmentExpression is deeply nested This commit also refactor `inConstructor` function from `prop-types` rule into `utils/Components` so that another rule could use that logic as well. --- lib/rules/prop-types.js | 23 ++++-------------- lib/rules/state-in-constructor.js | 6 ++--- lib/util/Components.js | 15 ++++++++++++ tests/lib/rules/state-in-constructor.js | 32 +++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 239c58bcec..3b4280dfad 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -62,21 +62,6 @@ module.exports = { const MISSING_MESSAGE = '\'{{name}}\' is missing in props validation'; - /** - * Check if we are in a class constructor - * @return {boolean} true if we are in a class constructor, false if not - */ - function inConstructor() { - let scope = context.getScope(); - while (scope) { - if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') { - return true; - } - scope = scope.upper; - } - return false; - } - /** * Check if we are in a class constructor * @return {boolean} true if we are in a class constructor, false if not @@ -290,7 +275,7 @@ module.exports = { function getPropertyName(node) { const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - const isNotInConstructor = !inConstructor(); + const isNotInConstructor = !utils.inConstructor(); const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps(); const isNotInShouldComponentUpdate = !inShouldComponentUpdate(); if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps @@ -382,7 +367,7 @@ module.exports = { // let {firstname} = props const directDestructuring = PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) + (utils.getParentStatelessComponent() || utils.inConstructor() || inComponentWillReceiveProps()) ; if (thisDestructuring) { @@ -416,7 +401,7 @@ module.exports = { name: name, allNames: allNames, node: ( - !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? + !isDirectProp && !utils.inConstructor() && !inComponentWillReceiveProps() ? node.parent.property : node.property ) @@ -502,7 +487,7 @@ module.exports = { const directDestructuring = destructuring && PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) + (utils.getParentStatelessComponent() || utils.inConstructor() || inComponentWillReceiveProps()) ; if (!thisDestructuring && !directDestructuring) { diff --git a/lib/rules/state-in-constructor.js b/lib/rules/state-in-constructor.js index 2243c0a40f..f416ffde8c 100644 --- a/lib/rules/state-in-constructor.js +++ b/lib/rules/state-in-constructor.js @@ -32,7 +32,7 @@ module.exports = { option === 'always' && !node.static && node.key.name === 'state' && - utils.isES6Component(node.parent.parent) + utils.getParentES6Component() ) { context.report({ node, @@ -45,8 +45,8 @@ module.exports = { option === 'never' && node.left.object.type === 'ThisExpression' && node.left.property.name === 'state' && - node.parent.parent.parent.parent.kind === 'constructor' && - utils.isES6Component(node.parent.parent.parent.parent.parent.parent) + utils.inConstructor() && + utils.getParentES6Component() ) { context.report({ node, diff --git a/lib/util/Components.js b/lib/util/Components.js index 1859600f35..28e834d665 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -306,6 +306,21 @@ function componentRule(rule, context) { return calledOnReact; }, + /** + * Check if we are in a class constructor + * @return {boolean} true if we are in a class constructor, false if not + */ + inConstructor: function() { + let scope = context.getScope(); + while (scope) { + if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') { + return true; + } + scope = scope.upper; + } + return false; + }, + getReturnPropertyAndNode(ASTnode) { let property; let node = ASTnode; diff --git a/tests/lib/rules/state-in-constructor.js b/tests/lib/rules/state-in-constructor.js index a09233ad1c..fe4804b65d 100644 --- a/tests/lib/rules/state-in-constructor.js +++ b/tests/lib/rules/state-in-constructor.js @@ -184,6 +184,20 @@ ruleTester.run('state-in-constructor', rule, { } `, options: ['never'] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + if (foobar) { + this.state = { bar: 0 } + } + } + render() { + return
Foo
+ } + } + ` }], invalid: [{ @@ -293,5 +307,23 @@ ruleTester.run('state-in-constructor', rule, { errors: [{ message: 'State initialization should be in a class property' }] + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + if (foobar) { + this.state = { bar: 0 } + } + } + render() { + return
Foo
+ } + } + `, + options: ['never'], + errors: [{ + message: 'State initialization should be in a class property' + }] }] }); From b1024c08c6ac39404a4713a05d01fb316ffc90ba Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Sun, 19 Aug 2018 11:18:35 +0700 Subject: [PATCH 6/7] Fix a case where left side of assignment is not MemberExpression This commit also move `isStateMemberExpression` from `no-direct-mutation-state` rule into `util/Components` so that the logic could be share with other rules. --- lib/rules/no-direct-mutation-state.js | 13 ++----------- lib/rules/state-in-constructor.js | 3 +-- lib/util/Components.js | 9 +++++++++ tests/lib/rules/state-in-constructor.js | 25 +++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/lib/rules/no-direct-mutation-state.js b/lib/rules/no-direct-mutation-state.js index a4daed44ec..bbb9c62a12 100644 --- a/lib/rules/no-direct-mutation-state.js +++ b/lib/rules/no-direct-mutation-state.js @@ -59,15 +59,6 @@ module.exports = { return node; } - /** - * Determine if this MemberExpression is for `this.state` - * @param {Object} node The node to process - * @returns {Boolean} - */ - function isStateMemberExpression(node) { - return node.object.type === 'ThisExpression' && node.property.name === 'state'; - } - /** * Determine if we should currently ignore assignments in this component. * @param {?Object} component The component to process @@ -101,7 +92,7 @@ module.exports = { return; } const item = getOuterMemberExpression(node.left); - if (isStateMemberExpression(item)) { + if (utils.isStateMemberExpression(item)) { const mutations = (component && component.mutations) || []; mutations.push(node.left.object); components.set(node, { @@ -117,7 +108,7 @@ module.exports = { return; } const item = getOuterMemberExpression(node.argument); - if (isStateMemberExpression(item)) { + if (utils.isStateMemberExpression(item)) { const mutations = (component && component.mutations) || []; mutations.push(item); components.set(node, { diff --git a/lib/rules/state-in-constructor.js b/lib/rules/state-in-constructor.js index f416ffde8c..ebfeb65605 100644 --- a/lib/rules/state-in-constructor.js +++ b/lib/rules/state-in-constructor.js @@ -43,8 +43,7 @@ module.exports = { AssignmentExpression(node) { if ( option === 'never' && - node.left.object.type === 'ThisExpression' && - node.left.property.name === 'state' && + utils.isStateMemberExpression(node.left) && utils.inConstructor() && utils.getParentES6Component() ) { diff --git a/lib/util/Components.js b/lib/util/Components.js index 28e834d665..d4d42a0c6b 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -321,6 +321,15 @@ function componentRule(rule, context) { return false; }, + /** + * Determine if the node is MemberExpression of `this.state` + * @param {Object} node The node to process + * @returns {Boolean} + */ + isStateMemberExpression: function(node) { + return node.type === 'MemberExpression' && node.object.type === 'ThisExpression' && node.property.name === 'state'; + }, + getReturnPropertyAndNode(ASTnode) { let property; let node = ASTnode; diff --git a/tests/lib/rules/state-in-constructor.js b/tests/lib/rules/state-in-constructor.js index fe4804b65d..8324b56cf8 100644 --- a/tests/lib/rules/state-in-constructor.js +++ b/tests/lib/rules/state-in-constructor.js @@ -198,6 +198,31 @@ ruleTester.run('state-in-constructor', rule, { } } ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + foobar = { bar: 0 } + } + render() { + return
Foo
+ } + } + ` + }, { + code: ` + class Foo extends React.Component { + constructor(props) { + super(props) + foobar = { bar: 0 } + } + render() { + return
Foo
+ } + } + `, + options: ['never'] }], invalid: [{ From 9415814005b140858c89784acb2fd944d3b061ea Mon Sep 17 00:00:00 2001 From: Kanitkorn S Date: Thu, 3 Jan 2019 18:30:31 +0900 Subject: [PATCH 7/7] Add `react/state-in-constructor` to the list of rule in readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a526a08ef8..f65b072f71 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,7 @@ Enable the rules that you would like to use. * [react/self-closing-comp](docs/rules/self-closing-comp.md): Prevent extra closing tags for components without children (fixable) * [react/sort-comp](docs/rules/sort-comp.md): Enforce component methods order (fixable) * [react/sort-prop-types](docs/rules/sort-prop-types.md): Enforce propTypes declarations alphabetical sorting +* [react/state-in-constructor](docs/rules/state-in-constructor.md): Enforce the state initialization style to be either in a constructor or with a class property * [react/style-prop-object](docs/rules/style-prop-object.md): Enforce style prop value being an object * [react/void-dom-elements-no-children](docs/rules/void-dom-elements-no-children.md): Prevent void DOM elements (e.g. ``, `
`) from receiving children