Skip to content

Commit 3516a81

Browse files
committed
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.
1 parent 81d0e8b commit 3516a81

File tree

4 files changed

+54
-22
lines changed

4 files changed

+54
-22
lines changed

lib/rules/prop-types.js

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,6 @@ module.exports = {
6262

6363
const MISSING_MESSAGE = '\'{{name}}\' is missing in props validation';
6464

65-
/**
66-
* Check if we are in a class constructor
67-
* @return {boolean} true if we are in a class constructor, false if not
68-
*/
69-
function inConstructor() {
70-
let scope = context.getScope();
71-
while (scope) {
72-
if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
73-
return true;
74-
}
75-
scope = scope.upper;
76-
}
77-
return false;
78-
}
79-
8065
/**
8166
* Check if we are in a class constructor
8267
* @return {boolean} true if we are in a class constructor, false if not
@@ -290,7 +275,7 @@ module.exports = {
290275
function getPropertyName(node) {
291276
const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node));
292277
const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component();
293-
const isNotInConstructor = !inConstructor();
278+
const isNotInConstructor = !utils.inConstructor();
294279
const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps();
295280
const isNotInShouldComponentUpdate = !inShouldComponentUpdate();
296281
if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps
@@ -382,7 +367,7 @@ module.exports = {
382367
// let {firstname} = props
383368
const directDestructuring =
384369
PROPS_REGEX.test(node.init.name) &&
385-
(utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps())
370+
(utils.getParentStatelessComponent() || utils.inConstructor() || inComponentWillReceiveProps())
386371
;
387372

388373
if (thisDestructuring) {
@@ -416,7 +401,7 @@ module.exports = {
416401
name: name,
417402
allNames: allNames,
418403
node: (
419-
!isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ?
404+
!isDirectProp && !utils.inConstructor() && !inComponentWillReceiveProps() ?
420405
node.parent.property :
421406
node.property
422407
)
@@ -502,7 +487,7 @@ module.exports = {
502487
const directDestructuring =
503488
destructuring &&
504489
PROPS_REGEX.test(node.init.name) &&
505-
(utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps())
490+
(utils.getParentStatelessComponent() || utils.inConstructor() || inComponentWillReceiveProps())
506491
;
507492

508493
if (!thisDestructuring && !directDestructuring) {

lib/rules/state-in-constructor.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ module.exports = {
3232
option === 'always' &&
3333
!node.static &&
3434
node.key.name === 'state' &&
35-
utils.isES6Component(node.parent.parent)
35+
utils.getParentES6Component()
3636
) {
3737
context.report({
3838
node,
@@ -45,8 +45,8 @@ module.exports = {
4545
option === 'never' &&
4646
node.left.object.type === 'ThisExpression' &&
4747
node.left.property.name === 'state' &&
48-
node.parent.parent.parent.parent.kind === 'constructor' &&
49-
utils.isES6Component(node.parent.parent.parent.parent.parent.parent)
48+
utils.inConstructor() &&
49+
utils.getParentES6Component()
5050
) {
5151
context.report({
5252
node,

lib/util/Components.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,21 @@ function componentRule(rule, context) {
306306
return calledOnReact;
307307
},
308308

309+
/**
310+
* Check if we are in a class constructor
311+
* @return {boolean} true if we are in a class constructor, false if not
312+
*/
313+
inConstructor: function() {
314+
let scope = context.getScope();
315+
while (scope) {
316+
if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
317+
return true;
318+
}
319+
scope = scope.upper;
320+
}
321+
return false;
322+
},
323+
309324
getReturnPropertyAndNode(ASTnode) {
310325
let property;
311326
let node = ASTnode;

tests/lib/rules/state-in-constructor.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,20 @@ ruleTester.run('state-in-constructor', rule, {
184184
}
185185
`,
186186
options: ['never']
187+
}, {
188+
code: `
189+
class Foo extends React.Component {
190+
constructor(props) {
191+
super(props)
192+
if (foobar) {
193+
this.state = { bar: 0 }
194+
}
195+
}
196+
render() {
197+
return <div>Foo</div>
198+
}
199+
}
200+
`
187201
}],
188202

189203
invalid: [{
@@ -293,5 +307,23 @@ ruleTester.run('state-in-constructor', rule, {
293307
errors: [{
294308
message: 'State initialization should be in a class property'
295309
}]
310+
}, {
311+
code: `
312+
class Foo extends React.Component {
313+
constructor(props) {
314+
super(props)
315+
if (foobar) {
316+
this.state = { bar: 0 }
317+
}
318+
}
319+
render() {
320+
return <div>Foo</div>
321+
}
322+
}
323+
`,
324+
options: ['never'],
325+
errors: [{
326+
message: 'State initialization should be in a class property'
327+
}]
296328
}]
297329
});

0 commit comments

Comments
 (0)