Merge pull request #9082 from acdlite/nobooleanorstringconstructors

acdlite · web-flow · commit 0585ee8b2206 · 2017-03-01T10:55:42.000-08:00
Enforce no boolean or string constructors
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -55,6 +55,7 @@ module.exports = {
     // CUSTOM RULES
     // the second argument of warning/invariant should be a literal string
     'react-internal/warning-and-invariant-args': ERROR,
+    'react-internal/no-primitive-constructors': ERROR,
   },
 
   globals: {
diff --git a/eslint-rules/__tests__/no-primitive-constructors-test.js b/eslint-rules/__tests__/no-primitive-constructors-test.js
@@ -0,0 +1,50 @@
+/**
+ * Copyright 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @emails react-core
+ */
+
+'use strict';
+
+var rule = require('../no-primitive-constructors');
+var RuleTester = require('eslint').RuleTester;
+var ruleTester = new RuleTester();
+
+ruleTester.run('eslint-rules/no-primitive-constructors', rule, {
+  valid: [
+    '!!obj',
+    "'' + obj",
+    '+string',
+  ],
+  invalid: [
+    {
+      code: 'Boolean(obj)',
+      errors: [
+        {
+          message: 'Do not use the Boolean constructor. To cast a value to a boolean, use double negation: !!value',
+        },
+      ],
+    },
+    {
+      code: 'String(obj)',
+      errors: [
+        {
+          message: 'Do not use the String constructor. To cast a value to a string, concat it with the empty string (unless it\'s a symbol, which has different semantics): \'\' + value',
+        },
+      ],
+    },
+    {
+      code: 'Number(string)',
+      errors: [
+        {
+          message: 'Do not use the Number constructor. To cast a value to a number, use the plus operator: +value',
+        },
+      ],
+    },
+  ],
+});
diff --git a/eslint-rules/index.js b/eslint-rules/index.js
@@ -3,5 +3,6 @@
 module.exports = {
   rules: {
     'warning-and-invariant-args': require('./warning-and-invariant-args'),
+    'no-primitive-constructors': require('./no-primitive-constructors'),
   },
 };
diff --git a/eslint-rules/no-primitive-constructors.js b/eslint-rules/no-primitive-constructors.js
@@ -0,0 +1,52 @@
+/**
+ * Copyright 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @emails react-core
+*/
+
+'use strict';
+
+module.exports = function(context) {
+  function report(node, name, msg) {
+    context.report(node, `Do not use the ${name} constructor. ${msg}`);
+  }
+
+  function check(node) {
+    const name = node.callee.name;
+    switch (name) {
+      case 'Boolean':
+        report(
+          node,
+          name,
+          'To cast a value to a boolean, use double negation: !!value'
+        );
+        break;
+      case 'String':
+        report(
+          node,
+          name,
+          'To cast a value to a string, concat it with the empty string ' +
+          '(unless it\'s a symbol, which has different semantics): ' +
+          '\'\' + value'
+        );
+        break;
+      case 'Number':
+        report(
+          node,
+          name,
+          'To cast a value to a number, use the plus operator: +value'
+        );
+        break;
+    }
+  }
+
+  return {
+    CallExpression: check,
+    NewExpression: check,
+  };
+};
diff --git a/package.json b/package.json
@@ -46,7 +46,7 @@
     "eslint-plugin-babel": "^3.3.0",
     "eslint-plugin-flowtype": "^2.25.0",
     "eslint-plugin-react": "^6.7.1",
-    "eslint-plugin-react-internal": "file:eslint-rules",
+    "eslint-plugin-react-internal": "file:./eslint-rules",
     "fbjs": "^0.8.9",
     "fbjs-scripts": "^0.6.0",
     "flow-bin": "^0.37.0",
diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -1,3 +1,11 @@
+eslint-rules/__tests__/no-primitive-constructors-test.js
+* !!obj
+* '' + obj
+* +string
+* Boolean(obj)
+* String(obj)
+* Number(string)
+
 eslint-rules/__tests__/warning-and-invariant-args-test.js
 * warning(true, 'hello, world');
 * warning(true, 'expected %s, got %s', 42, 24);
diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js
@@ -196,8 +196,9 @@ var DOMRenderer = ReactFiberReconciler({
         typeof props.children === 'string' ||
         typeof props.children === 'number'
       ) {
+        const string = '' + props.children;
         const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null);
-        validateDOMNesting(null, String(props.children), null, ownAncestorInfo);
+        validateDOMNesting(null, string, null, ownAncestorInfo);
       }
       parentNamespace = hostContextDev.namespace;
     } else {
@@ -237,8 +238,9 @@ var DOMRenderer = ReactFiberReconciler({
         typeof newProps.children === 'string' ||
         typeof newProps.children === 'number'
       )) {
+        const string = '' + newProps.children;
         const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null);
-        validateDOMNesting(null, String(newProps.children), null, ownAncestorInfo);
+        validateDOMNesting(null, string, null, ownAncestorInfo);
       }
     }
     return diffProperties(domElement, type, oldProps, newProps, rootContainerInstance);
@@ -411,9 +413,9 @@ var ReactDOM = {
     }
 
     if (__DEV__) {
-      const isRootRenderedBySomeReact = Boolean(container._reactRootContainer);
+      const isRootRenderedBySomeReact = !!container._reactRootContainer;
       const rootEl = getReactRootElementInContainer(container);
-      const hasNonRootReactChild = Boolean(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl));
+      const hasNonRootReactChild = !!(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl));
 
       warning(
         !hasNonRootReactChild ||
diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js
@@ -134,7 +134,7 @@ var ReactDOMSelect = {
     var value = props.value;
     node._wrapperState = {
       initialValue: value != null ? value : props.defaultValue,
-      wasMultiple: Boolean(props.multiple),
+      wasMultiple: !!props.multiple,
     };
 
     if (
@@ -153,11 +153,11 @@ var ReactDOMSelect = {
       didWarnValueDefaultValue = true;
     }
 
-    node.multiple = Boolean(props.multiple);
+    node.multiple = !!props.multiple;
     if (value != null) {
-      updateOptions(node, Boolean(props.multiple), value);
+      updateOptions(node, !!props.multiple, value);
     } else if (props.defaultValue != null) {
-      updateOptions(node, Boolean(props.multiple), props.defaultValue);
+      updateOptions(node, !!props.multiple, props.defaultValue);
     }
   },
 
@@ -168,18 +168,18 @@ var ReactDOMSelect = {
     node._wrapperState.initialValue = undefined;
 
     var wasMultiple = node._wrapperState.wasMultiple;
-    node._wrapperState.wasMultiple = Boolean(props.multiple);
+    node._wrapperState.wasMultiple = !!props.multiple;
 
     var value = props.value;
     if (value != null) {
-      updateOptions(node, Boolean(props.multiple), value);
-    } else if (wasMultiple !== Boolean(props.multiple)) {
+      updateOptions(node, !!props.multiple, value);
+    } else if (wasMultiple !== !!props.multiple) {
       // For simplicity, reapply `defaultValue` if `multiple` is toggled.
       if (props.defaultValue != null) {
-        updateOptions(node, Boolean(props.multiple), props.defaultValue);
+        updateOptions(node, !!props.multiple, props.defaultValue);
       } else {
         // Revert the select back to its default unselected state.
-        updateOptions(node, Boolean(props.multiple), props.multiple ? [] : '');
+        updateOptions(node, !!props.multiple, props.multiple ? [] : '');
       }
     }
   },
@@ -189,7 +189,7 @@ var ReactDOMSelect = {
     var value = props.value;
 
     if (value != null) {
-      updateOptions(node, Boolean(props.multiple), value);
+      updateOptions(node, !!props.multiple, value);
     }
   },
 };
diff --git a/src/renderers/dom/shared/ReactBrowserEventEmitter.js b/src/renderers/dom/shared/ReactBrowserEventEmitter.js
@@ -154,7 +154,7 @@ var topEventMapping = {
 /**
  * To ensure no conflicts with other potential React instances on the page
  */
-var topListenersIDKey = '_reactListenersID' + String(Math.random()).slice(2);
+var topListenersIDKey = '_reactListenersID' + ('' + Math.random()).slice(2);
 
 function getListeningForDocument(mountAt) {
   // In IE8, `mountAt` is a host object and doesn't have `hasOwnProperty`
diff --git a/src/renderers/dom/shared/ReactDOMComponentTree.js b/src/renderers/dom/shared/ReactDOMComponentTree.js
@@ -31,7 +31,7 @@ var internalEventHandlersKey = '__reactEventHandlers$' + randomKey;
  */
 function shouldPrecacheNode(node, nodeID) {
   return (node.nodeType === 1 &&
-          node.getAttribute(ATTR_NAME) === String(nodeID)) ||
+          node.getAttribute(ATTR_NAME) === ('' + nodeID)) ||
          (node.nodeType === 8 &&
           node.nodeValue === ' react-text: ' + nodeID + ' ') ||
          (node.nodeType === 8 &&
diff --git a/src/renderers/dom/shared/validateDOMNesting.js b/src/renderers/dom/shared/validateDOMNesting.js
@@ -400,7 +400,7 @@ if (__DEV__) {
         childTag,
         ancestorInstance,
         ancestorTag,
-        Boolean(invalidParent)
+        !!invalidParent
       ) + '.';
     } else {
       addendum = getCurrentFiberStackAddendum();
diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js
@@ -185,7 +185,7 @@ if (__DEV__) {
       return;
     }
 
-    validateDOMNesting(null, String(content), this, this._ancestorInfo);
+    validateDOMNesting(null, '' + content, this, this._ancestorInfo);
     this._contentDebugID = contentDebugID;
     if (hasExistingContent) {
       ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content);
diff --git a/src/renderers/dom/stack/client/ReactMount.js b/src/renderers/dom/stack/client/ReactMount.js
@@ -440,7 +440,7 @@ var ReactMount = {
         callback === null || typeof callback === 'function',
         'render(...): Expected the last optional `callback` argument to be a ' +
         'function. Instead received: %s.',
-        String(callback)
+        '' + callback
       );
     }
     if (!React.isValidElement(nextElement)) {
diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js b/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js
@@ -135,7 +135,7 @@ var ReactDOMSelect = {
     inst._wrapperState = {
       initialValue: value != null ? value : props.defaultValue,
       listeners: null,
-      wasMultiple: Boolean(props.multiple),
+      wasMultiple: !!props.multiple,
     };
 
     if (
@@ -169,18 +169,18 @@ var ReactDOMSelect = {
     inst._wrapperState.initialValue = undefined;
 
     var wasMultiple = inst._wrapperState.wasMultiple;
-    inst._wrapperState.wasMultiple = Boolean(props.multiple);
+    inst._wrapperState.wasMultiple = !!props.multiple;
 
     var value = props.value;
     if (value != null) {
-      updateOptions(inst, Boolean(props.multiple), value);
-    } else if (wasMultiple !== Boolean(props.multiple)) {
+      updateOptions(inst, !!props.multiple, value);
+    } else if (wasMultiple !== !!props.multiple) {
       // For simplicity, reapply `defaultValue` if `multiple` is toggled.
       if (props.defaultValue != null) {
-        updateOptions(inst, Boolean(props.multiple), props.defaultValue);
+        updateOptions(inst, !!props.multiple, props.defaultValue);
       } else {
         // Revert the select back to its default unselected state.
-        updateOptions(inst, Boolean(props.multiple), props.multiple ? [] : '');
+        updateOptions(inst, !!props.multiple, props.multiple ? [] : '');
       }
     }
   },
@@ -191,7 +191,7 @@ var ReactDOMSelect = {
       var value = props.value;
 
       if (value != null) {
-        updateOptions(inst, Boolean(props.multiple), value);
+        updateOptions(inst, !!props.multiple, value);
       }
     }
   },
diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js
@@ -95,7 +95,7 @@ function coerceRef(current: Fiber | null, element: ReactElement) {
         'bug in React. Please file an issue.',
         mixedRef
       );
-      const stringRef = String(mixedRef);
+      const stringRef = '' + mixedRef;
       // Check if previous string ref matches new string ref
       if (current !== null && current.ref !== null && current.ref._stringRef === stringRef) {
         return current.ref;
@@ -117,7 +117,8 @@ function coerceRef(current: Fiber | null, element: ReactElement) {
 
 function throwOnInvalidObjectType(returnFiber : Fiber, newChild : Object) {
   if (returnFiber.type !== 'textarea') {
-    const childrenString = String(newChild);
+    // $FlowFixMe - Intentional cast to string
+    const childrenString = '' + newChild;
     let addendum = '';
     if (__DEV__) {
       addendum =
diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js
@@ -47,7 +47,8 @@ if (__DEV__) {
       '%s(...): Expected the last optional `callback` argument to be a ' +
       'function. Instead received: %s.',
       callerName,
-      String(callback)
+      // $FlowFixMe - Intentional cast to string
+      '' + callback
     );
   };
 }
diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js
@@ -167,7 +167,8 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
         callback === null || typeof callback === 'function',
         'render(...): Expected the last optional `callback` argument to be a ' +
         'function. Instead received: %s.',
-        String(callback)
+        // $FlowFixMe - Intentional cast to string
+        '' + callback
       );
     }
     addTopLevelUpdate(current, nextState, callback, priorityLevel);
diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
@@ -776,7 +776,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
       'by a bug in React. Please file an issue.'
     );
     isPerformingWork = true;
-    const isPerformingDeferredWork = Boolean(deadline);
+    const isPerformingDeferredWork = !!deadline;
 
     // This outer loop exists so that we can restart the work loop after
     // catching an error. It also lets us flush Task work at the end of a
@@ -1034,7 +1034,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
   function hasCapturedError(fiber : Fiber) : boolean {
     // TODO: capturedErrors should store the boundary instance, to avoid needing
     // to check the alternate.
-    return Boolean(
+    return (
       capturedErrors !== null &&
       (capturedErrors.has(fiber) || (fiber.alternate !== null && capturedErrors.has(fiber.alternate)))
     );
@@ -1043,7 +1043,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
   function isFailedBoundary(fiber : Fiber) : boolean {
     // TODO: failedBoundaries should store the boundary instance, to avoid
     // needing to check the alternate.
-    return Boolean(
+    return (
       failedBoundaries !== null &&
       (failedBoundaries.has(fiber) || (fiber.alternate !== null && failedBoundaries.has(fiber.alternate)))
     );
diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js
diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
diff --git a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js
diff --git a/src/renderers/shared/utils/validateCallback.js b/src/renderers/shared/utils/validateCallback.js
diff --git a/src/shared/utils/traverseAllChildren.js b/src/shared/utils/traverseAllChildren.js
diff --git a/src/test/reactComponentExpect.js b/src/test/reactComponentExpect.js
diff --git a/yarn.lock b/yarn.lock
