Check usage of arguments in methods (#250)

* Add failing test for wrong `arguments` transform

* Check usage of `arguments` in methods

Fixes #163.

Although the issue mentions `class` transform, the problem likely was in
the `manual-bind-to-arrow` transform; `class` transform had checks to
avoid function transformation when `arguments` object was used in a
method.

* Moved the `doesNotUseArguments` function into a separate util.

Author: kgrz

transforms/__testfixtures__/manual-bind-to-arrow/manual-bind-to-arrow11.input.js

@@ -0,0 +1,12 @@
+class Component extends React.Component {
+  constructor() {
+    super();
+    this.onClick = this.onClick.bind(this);
+  }
+
+  onClick() {
+    if (fn) {
+      fn.apply(this, Array.prototype.slice.call(arguments, 1))
+    }
+  }
+}

transforms/__testfixtures__/manual-bind-to-arrow/manual-bind-to-arrow11.output.js

@@ -2,7 +2,7 @@
  * Copyright 2015-present, Facebook, Inc.
  *
  * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree. 
+ * LICENSE file in the root directory of this source tree.
  *
  */
 
@@ -26,7 +26,8 @@ var TESTS = [
   7,
   8,
   9,
-  10
+  10,
+  11
 ];
 
 TESTS.forEach(test => {

transforms/__tests__/manual-bind-to-arrow-test.js

@@ -15,6 +15,7 @@ module.exports = (file, api, options) => {
 
   require('./utils/array-polyfills');
   const ReactUtils = require('./utils/ReactUtils')(j);
+  const doesNotUseArguments = require('./utils/doesNotUseArguments')(j);
 
   const printOptions = options.printOptions || {
     quote: 'single',
@@ -236,26 +237,6 @@ module.exports = (file, api, options) => {
     return true;
   };
 
-  const doesNotUseArguments = classPath => {
-    const hasArguments =
-      j(classPath)
-        .find(j.Identifier, { name: 'arguments' })
-        .size() > 0;
-    if (hasArguments) {
-      console.warn(
-        file.path +
-          ': `' +
-          ReactUtils.directlyGetComponentName(classPath) +
-          '` ' +
-          'was skipped because `arguments` was found in your functions. ' +
-          'Arrow functions do not expose an `arguments` object; ' +
-          'consider changing to use ES6 spread operator and re-run this script.'
-      );
-      return false;
-    }
-    return true;
-  };
-
   const isGetInitialStateConstructorSafe = getInitialState => {
     if (!getInitialState) {
       return true;

transforms/class.js

@@ -21,6 +21,7 @@
 
 export default function transformer(file, api, options) {
   const j = api.jscodeshift;
+  const doesNotUseArguments = require('./utils/doesNotUseArguments')(j);
 
   const printOptions = options.printOptions || {};
   var root = j(file.source);
@@ -114,16 +115,18 @@ export default function transformer(file, api, options) {
       .filter(
         path =>
           path.node.key.type === 'Identifier' &&
-          path.node.key.name === methodName
+          path.node.key.name === methodName &&
+          doesNotUseArguments(path, file.path)
       );
 
     // Do not remove the binding if there's no corresponding method to turn
-    // into an arrow function
+    // into an arrow function, or if the method uses `arguments` keyword inside
+    // it.
     if (methods.size() === 0) {
       return;
     }
     methods.replaceWith(path => createArrowProperty(path.node));
-    
+
     // Remove the line
     // this.method = this.method.bind(this);
     j(path.parentPath).remove();

transforms/manual-bind-to-arrow.js

@@ -0,0 +1,40 @@
+/**
+ * Copyright 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+'use strict';
+
+module.exports = function(j) {
+  const ReactUtils = require('./ReactUtils')(j);
+
+  const doesNotUseArguments = (path, filepath) => {
+    const hasArguments =
+      j(path)
+        .find(j.Identifier, { name: 'arguments' })
+        .size() > 0;
+
+    if (hasArguments) {
+      var warnStr = '';
+
+      if (filepath) {
+        warnStr += filepath + ': ';
+      }
+
+      warnStr += '`' + ReactUtils.directlyGetComponentName(path) + '` ' +
+        'was skipped because `arguments` was found in your functions. ' +
+        'Arrow functions do not expose an `arguments` object; ' +
+        'consider changing to use ES6 spread operator and re-run this script.';
+
+      console.warn(warnStr);
+
+      return false;
+    }
+    return true;
+  };
+
+  return doesNotUseArguments;
+};