Skip to content

Commit 3187bf9

Browse files
yannickcryannickcr
committed
Fix prefer-stateless-function crash (fixes #544)
1 parent 1a3d3d1 commit 3187bf9

File tree

2 files changed

+131
-44
lines changed

2 files changed

+131
-44
lines changed

‎lib/rules/prefer-stateless-function.js‎

Lines changed: 118 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -55,57 +55,131 @@ module.exports = Components.detect(function(context, components, utils) {
5555
}
5656

5757
/**
58-
* Checks whether the constructor body is a redundant super call.
58+
* Checks whether a given array of statements is a single call of `super`.
5959
* @see ESLint no-useless-constructor rule
60-
* @param {Array} body - constructor body content.
61-
* @param {Array} ctorParams - The params to check against super call.
62-
* @returns {boolean} true if the construtor body is redundant
60+
* @param {ASTNode[]} body - An array of statements to check.
61+
* @returns {boolean} `true` if the body is a single call of `super`.
6362
*/
64-
function isRedundantSuperCall(body, ctorParams) {
65-
if (
66-
body.length !== 1 ||
67-
body[0].type !== 'ExpressionStatement' ||
68-
body[0].expression.callee.type !== 'Super'
69-
) {
63+
function isSingleSuperCall(body) {
64+
return (
65+
body.length === 1 &&
66+
body[0].type === 'ExpressionStatement' &&
67+
body[0].expression.type === 'CallExpression' &&
68+
body[0].expression.callee.type === 'Super'
69+
);
70+
}
71+
72+
/**
73+
* Checks whether a given node is a pattern which doesn't have any side effects.
74+
* Default parameters and Destructuring parameters can have side effects.
75+
* @see ESLint no-useless-constructor rule
76+
* @param {ASTNode} node - A pattern node.
77+
* @returns {boolean} `true` if the node doesn't have any side effects.
78+
*/
79+
function isSimple(node) {
80+
return node.type === 'Identifier' || node.type === 'RestElement';
81+
}
82+
83+
/**
84+
* Checks whether a given array of expressions is `...arguments` or not.
85+
* `super(...arguments)` passes all arguments through.
86+
* @see ESLint no-useless-constructor rule
87+
* @param {ASTNode[]} superArgs - An array of expressions to check.
88+
* @returns {boolean} `true` if the superArgs is `...arguments`.
89+
*/
90+
function isSpreadArguments(superArgs) {
91+
return (
92+
superArgs.length === 1 &&
93+
superArgs[0].type === 'SpreadElement' &&
94+
superArgs[0].argument.type === 'Identifier' &&
95+
superArgs[0].argument.name === 'arguments'
96+
);
97+
}
98+
99+
/**
100+
* Checks whether given 2 nodes are identifiers which have the same name or not.
101+
* @see ESLint no-useless-constructor rule
102+
* @param {ASTNode} ctorParam - A node to check.
103+
* @param {ASTNode} superArg - A node to check.
104+
* @returns {boolean} `true` if the nodes are identifiers which have the same
105+
* name.
106+
*/
107+
function isValidIdentifierPair(ctorParam, superArg) {
108+
return (
109+
ctorParam.type === 'Identifier' &&
110+
superArg.type === 'Identifier' &&
111+
ctorParam.name === superArg.name
112+
);
113+
}
114+
115+
/**
116+
* Checks whether given 2 nodes are a rest/spread pair which has the same values.
117+
* @see ESLint no-useless-constructor rule
118+
* @param {ASTNode} ctorParam - A node to check.
119+
* @param {ASTNode} superArg - A node to check.
120+
* @returns {boolean} `true` if the nodes are a rest/spread pair which has the
121+
* same values.
122+
*/
123+
function isValidRestSpreadPair(ctorParam, superArg) {
124+
return (
125+
ctorParam.type === 'RestElement' &&
126+
superArg.type === 'SpreadElement' &&
127+
isValidIdentifierPair(ctorParam.argument, superArg.argument)
128+
);
129+
}
130+
131+
/**
132+
* Checks whether given 2 nodes have the same value or not.
133+
* @see ESLint no-useless-constructor rule
134+
* @param {ASTNode} ctorParam - A node to check.
135+
* @param {ASTNode} superArg - A node to check.
136+
* @returns {boolean} `true` if the nodes have the same value or not.
137+
*/
138+
function isValidPair(ctorParam, superArg) {
139+
return (
140+
isValidIdentifierPair(ctorParam, superArg) ||
141+
isValidRestSpreadPair(ctorParam, superArg)
142+
);
143+
}
144+
145+
/**
146+
* Checks whether the parameters of a constructor and the arguments of `super()`
147+
* have the same values or not.
148+
* @see ESLint no-useless-constructor rule
149+
* @param {ASTNode} ctorParams - The parameters of a constructor to check.
150+
* @param {ASTNode} superArgs - The arguments of `super()` to check.
151+
* @returns {boolean} `true` if those have the same values.
152+
*/
153+
function isPassingThrough(ctorParams, superArgs) {
154+
if (ctorParams.length !== superArgs.length) {
70155
return false;
71156
}
72157

73-
var superArgs = body[0].expression.arguments;
74-
var firstSuperArg = superArgs[0];
75-
var lastSuperArgIndex = superArgs.length - 1;
76-
var lastSuperArg = superArgs[lastSuperArgIndex];
77-
var isSimpleParameterList = ctorParams.every(function(param) {
78-
return param.type === 'Identifier' || param.type === 'RestElement';
79-
});
80-
81-
/**
82-
* Checks if a super argument is the same with constructor argument
83-
* @param {ASTNode} arg argument node
84-
* @param {number} index argument index
85-
* @returns {boolean} true if the arguments are same, false otherwise
86-
*/
87-
function isSameIdentifier(arg, index) {
88-
return (
89-
arg.type === 'Identifier' &&
90-
arg.name === ctorParams[index].name
91-
);
158+
for (var i = 0; i < ctorParams.length; ++i) {
159+
if (!isValidPair(ctorParams[i], superArgs[i])) {
160+
return false;
161+
}
92162
}
93163

94-
var spreadsArguments =
95-
superArgs.length === 1 &&
96-
firstSuperArg.type === 'SpreadElement' &&
97-
firstSuperArg.argument.name === 'arguments';
98-
99-
var passesParamsAsArgs =
100-
superArgs.length === ctorParams.length &&
101-
superArgs.every(isSameIdentifier) ||
102-
superArgs.length <= ctorParams.length &&
103-
superArgs.slice(0, -1).every(isSameIdentifier) &&
104-
lastSuperArg.type === 'SpreadElement' &&
105-
ctorParams[lastSuperArgIndex].type === 'RestElement' &&
106-
lastSuperArg.argument.name === ctorParams[lastSuperArgIndex].argument.name;
107-
108-
return isSimpleParameterList && (spreadsArguments || passesParamsAsArgs);
164+
return true;
165+
}
166+
167+
/**
168+
* Checks whether the constructor body is a redundant super call.
169+
* @see ESLint no-useless-constructor rule
170+
* @param {Array} body - constructor body content.
171+
* @param {Array} ctorParams - The params to check against super call.
172+
* @returns {boolean} true if the construtor body is redundant
173+
*/
174+
function isRedundantSuperCall(body, ctorParams) {
175+
return (
176+
isSingleSuperCall(body) &&
177+
ctorParams.every(isSimple) &&
178+
(
179+
isSpreadArguments(body[0].expression.arguments) ||
180+
isPassingThrough(ctorParams, body[0].expression.arguments)
181+
)
182+
);
109183
}
110184

111185
/**

‎tests/lib/rules/prefer-stateless-function.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,19 @@ ruleTester.run('prefer-stateless-function', rule, {
112112
'}'
113113
].join('\n'),
114114
parserOptions: parserOptions
115+
}, {
116+
// Has a constructor (2)
117+
code: [
118+
'class Foo extends React.Component {',
119+
' constructor() {',
120+
' foo;',
121+
' }',
122+
' render() {',
123+
' return <div>{this.props.foo}</div>;',
124+
' }',
125+
'}'
126+
].join('\n'),
127+
parserOptions: parserOptions
115128
}, {
116129
// Use this.bar
117130
code: [

0 commit comments

Comments
 (0)