Fix prefer-stateless-function crash (fixes #544)

jsx-eslint · Apr 12, 2016 · 3187bf9 · 3187bf9
1 parent 1a3d3d1
commit 3187bf9
Show file tree

Hide file tree

Showing 2 changed files with 131 additions and 44 deletions.
diff --git a/lib/rules/prefer-stateless-function.js b/lib/rules/prefer-stateless-function.js
@@ -55,57 +55,131 @@ module.exports = Components.detect(function(context, components, utils) {
   }
 
   /**
-   * Checks whether the constructor body is a redundant super call.
+   * Checks whether a given array of statements is a single call of `super`.
    * @see ESLint no-useless-constructor rule
-   * @param {Array} body - constructor body content.
-   * @param {Array} ctorParams - The params to check against super call.
-   * @returns {boolean} true if the construtor body is redundant
+   * @param {ASTNode[]} body - An array of statements to check.
+   * @returns {boolean} `true` if the body is a single call of `super`.
    */
-  function isRedundantSuperCall(body, ctorParams) {
-    if (
-      body.length !== 1 ||
-      body[0].type !== 'ExpressionStatement' ||
-      body[0].expression.callee.type !== 'Super'
-    ) {
+  function isSingleSuperCall(body) {
+    return (
+      body.length === 1 &&
+      body[0].type === 'ExpressionStatement' &&
+      body[0].expression.type === 'CallExpression' &&
+      body[0].expression.callee.type === 'Super'
+    );
+  }
+
+  /**
+   * Checks whether a given node is a pattern which doesn't have any side effects.
+   * Default parameters and Destructuring parameters can have side effects.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode} node - A pattern node.
+   * @returns {boolean} `true` if the node doesn't have any side effects.
+   */
+  function isSimple(node) {
+    return node.type === 'Identifier' || node.type === 'RestElement';
+  }
+
+  /**
+   * Checks whether a given array of expressions is `...arguments` or not.
+   * `super(...arguments)` passes all arguments through.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode[]} superArgs - An array of expressions to check.
+   * @returns {boolean} `true` if the superArgs is `...arguments`.
+   */
+  function isSpreadArguments(superArgs) {
+    return (
+      superArgs.length === 1 &&
+      superArgs[0].type === 'SpreadElement' &&
+      superArgs[0].argument.type === 'Identifier' &&
+      superArgs[0].argument.name === 'arguments'
+    );
+  }
+
+  /**
+   * Checks whether given 2 nodes are identifiers which have the same name or not.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode} ctorParam - A node to check.
+   * @param {ASTNode} superArg - A node to check.
+   * @returns {boolean} `true` if the nodes are identifiers which have the same
+   *      name.
+   */
+  function isValidIdentifierPair(ctorParam, superArg) {
+    return (
+      ctorParam.type === 'Identifier' &&
+      superArg.type === 'Identifier' &&
+      ctorParam.name === superArg.name
+    );
+  }
+
+  /**
+   * Checks whether given 2 nodes are a rest/spread pair which has the same values.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode} ctorParam - A node to check.
+   * @param {ASTNode} superArg - A node to check.
+   * @returns {boolean} `true` if the nodes are a rest/spread pair which has the
+   *      same values.
+   */
+  function isValidRestSpreadPair(ctorParam, superArg) {
+    return (
+      ctorParam.type === 'RestElement' &&
+      superArg.type === 'SpreadElement' &&
+      isValidIdentifierPair(ctorParam.argument, superArg.argument)
+    );
+  }
+
+  /**
+   * Checks whether given 2 nodes have the same value or not.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode} ctorParam - A node to check.
+   * @param {ASTNode} superArg - A node to check.
+   * @returns {boolean} `true` if the nodes have the same value or not.
+   */
+  function isValidPair(ctorParam, superArg) {
+    return (
+      isValidIdentifierPair(ctorParam, superArg) ||
+      isValidRestSpreadPair(ctorParam, superArg)
+    );
+  }
+
+  /**
+   * Checks whether the parameters of a constructor and the arguments of `super()`
+   * have the same values or not.
+   * @see ESLint no-useless-constructor rule
+   * @param {ASTNode} ctorParams - The parameters of a constructor to check.
+   * @param {ASTNode} superArgs - The arguments of `super()` to check.
+   * @returns {boolean} `true` if those have the same values.
+   */
+  function isPassingThrough(ctorParams, superArgs) {
+    if (ctorParams.length !== superArgs.length) {
       return false;
     }
 
-    var superArgs = body[0].expression.arguments;
-    var firstSuperArg = superArgs[0];
-    var lastSuperArgIndex = superArgs.length - 1;
-    var lastSuperArg = superArgs[lastSuperArgIndex];
-    var isSimpleParameterList = ctorParams.every(function(param) {
-      return param.type === 'Identifier' || param.type === 'RestElement';
-    });
-
-    /**
-     * Checks if a super argument is the same with constructor argument
-     * @param {ASTNode} arg argument node
-     * @param {number} index argument index
-     * @returns {boolean} true if the arguments are same, false otherwise
-     */
-    function isSameIdentifier(arg, index) {
-      return (
-        arg.type === 'Identifier' &&
-        arg.name === ctorParams[index].name
-      );
+    for (var i = 0; i < ctorParams.length; ++i) {
+      if (!isValidPair(ctorParams[i], superArgs[i])) {
+        return false;
+      }
     }
 
-    var spreadsArguments =
-      superArgs.length === 1 &&
-      firstSuperArg.type === 'SpreadElement' &&
-      firstSuperArg.argument.name === 'arguments';
-
-    var passesParamsAsArgs =
-      superArgs.length === ctorParams.length &&
-      superArgs.every(isSameIdentifier) ||
-      superArgs.length <= ctorParams.length &&
-      superArgs.slice(0, -1).every(isSameIdentifier) &&
-      lastSuperArg.type === 'SpreadElement' &&
-      ctorParams[lastSuperArgIndex].type === 'RestElement' &&
-      lastSuperArg.argument.name === ctorParams[lastSuperArgIndex].argument.name;
-
-    return isSimpleParameterList && (spreadsArguments || passesParamsAsArgs);
+    return true;
+  }
+
+  /**
+   * Checks whether the constructor body is a redundant super call.
+   * @see ESLint no-useless-constructor rule
+   * @param {Array} body - constructor body content.
+   * @param {Array} ctorParams - The params to check against super call.
+   * @returns {boolean} true if the construtor body is redundant
+   */
+  function isRedundantSuperCall(body, ctorParams) {
+    return (
+      isSingleSuperCall(body) &&
+      ctorParams.every(isSimple) &&
+      (
+        isSpreadArguments(body[0].expression.arguments) ||
+        isPassingThrough(ctorParams, body[0].expression.arguments)
+      )
+    );
   }
 
   /**

diff --git a/tests/lib/rules/prefer-stateless-function.js b/tests/lib/rules/prefer-stateless-function.js
@@ -112,6 +112,19 @@ ruleTester.run('prefer-stateless-function', rule, {
         '}'
       ].join('\n'),
       parserOptions: parserOptions
+    }, {
+      // Has a constructor (2)
+      code: [
+        'class Foo extends React.Component {',
+        '  constructor() {',
+        '    foo;',
+        '  }',
+        '  render() {',
+        '    return <div>{this.props.foo}</div>;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parserOptions: parserOptions
     }, {
       // Use this.bar
       code: [