Add custom eslint rule for warning and invariant

See #2869

Checks that the *second* argument of warning and invariant are a literal
string, or a concatination of literal strings, and that the number of
arguments is correct based on the number of %s substrings.

This commit also fixes a few places where the existing code had broken
error messages!

The rule itself is pretty straightforward, although adding the tests
ended up being a bit painful, as eslint-tester depends on mocha, and
therefore needs to be run in a separate grunt task.

.eslintrc

@@ -73,3 +73,7 @@ rules:
   # JSX
   react/jsx-uses-react: 2
   react/jsx-quotes: [2, "double"]
+
+  # CUSTOM RULES
+  # the second argument of warning/invariant should be a literal string
+  warning-and-invariant-args: 2

Gruntfile.js

@@ -13,6 +13,7 @@ var npmReactToolsTasks = require('./grunt/tasks/npm-react-tools');
 var versionCheckTask = require('./grunt/tasks/version-check');
 var gemReactSourceTasks = require('./grunt/tasks/gem-react-source');
 var eslintTask = require('./grunt/tasks/eslint');
+var testEslintRulesTask = require('./grunt/tasks/test-eslint-rules');
 
 module.exports = function(grunt) {
 
@@ -117,6 +118,7 @@ module.exports = function(grunt) {
   ]);
   grunt.registerTask('build:test', [
     'delete-build-modules',
+    'test:eslint-rules',
     'jsx:test',
     'version-check',
     'populist:test',
@@ -135,6 +137,8 @@ module.exports = function(grunt) {
 
   grunt.registerTask('coverage:parse', require('./grunt/tasks/coverage-parse'));
 
+  grunt.registerTask('test:eslint-rules', testEslintRulesTask);
+
   grunt.registerTask('test:webdriver:phantomjs', [
     'connect',
     'webdriver-phantomjs',

eslint-rules/__tests__/warning-and-invariant-args-test.js

@@ -0,0 +1,70 @@
+/**
+ * Copyright 2015, 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 eslint = require('eslint');
+var ESLintTester = require('eslint-tester');
+var eslintTester = new ESLintTester(eslint.linter);
+
+eslintTester.addRuleTest('eslint-rules/warning-and-invariant-args', {
+  valid: [
+    "warning(true, 'hello, world');",
+    "warning(true, 'expected %s, got %s', 42, 24);",
+    "invariant(true, 'hello, world');",
+    "invariant(true, 'expected %s, got %s', 42, 24);",
+  ],
+  invalid: [
+    {
+      code: "warning('hello, world');",
+      errors: [
+        {
+          message: 'warning takes at least two arguments',
+        },
+      ],
+    },
+    {
+      code: 'warning(true, null);',
+      errors: [
+        {
+          message: 'The second argument to warning must be a string literal',
+        },
+      ],
+    },
+    {
+      code: 'var g = 5; invariant(true, g);',
+      errors: [
+        {
+          message: 'The second argument to invariant must be a string literal',
+        },
+      ],
+    },
+    {
+      code: "warning(true, 'expected %s, got %s');",
+      errors: [
+        {
+          message: 'Expected 4 arguments in call to warning, but got 2 ' +
+                   'based on the number of "%s" substitutions',
+        },
+      ],
+    },
+    {
+      code: "warning(true, 'foobar', 'junk argument');",
+      errors: [
+        {
+          message: 'Expected 2 arguments in call to warning, but got 3 ' +
+                   'based on the number of "%s" substitutions',
+        },
+      ],
+    },
+  ],
+});
+

eslint-rules/warning-and-invariant-args.js

@@ -0,0 +1,79 @@
+/**
+ * Copyright 2015, 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';
+
+/**
+ * The warning() and invariant() functions take format strings as their second
+ * argument.
+ */
+
+module.exports = function(context) {
+  // we also allow literal strings and concatinated literal strings
+  function getLiteralString(node) {
+    if (node.type === 'Literal' && typeof node.value === 'string') {
+      return node.value;
+    } else if (node.type === 'BinaryExpression' && node.operator === '+') {
+      var l = getLiteralString(node.left);
+      var r = getLiteralString(node.right);
+      if (l !== null && r !== null) {
+        return l + r;
+      }
+    }
+    return null;
+  }
+
+  return {
+    CallExpression: function(node) {
+      // This could be a little smarter by checking context.getScope() to see
+      // how warning/invariant was defined.
+      var isWarningOrInvariant =
+        node.callee.type === 'Identifier' &&
+        (node.callee.name === 'warning' || node.callee.name === 'invariant');
+      if (!isWarningOrInvariant) {
+        return;
+      }
+      if (node.arguments.length < 2) {
+        context.report(
+          node,
+          '{{name}} takes at least two arguments',
+          {name: node.callee.name}
+        );
+        return;
+      }
+      var str = getLiteralString(node.arguments[1]);
+      if (str === null) {
+        context.report(
+          node,
+          'The second argument to {{name}} must be a string literal',
+          {name: node.callee.name}
+        );
+        return;
+      }
+      // count the number of string substitutions, plus the first two args
+      var expectedNArgs = (str.match(/%s/g) || []).length + 2;
+      if (node.arguments.length !== expectedNArgs) {
+        context.report(
+          node,
+          'Expected {{expectedNArgs}} arguments in call to {{name}}, but ' +
+          'got {{length}} based on the number of "%s" substitutions',
+          {
+            expectedNArgs: expectedNArgs,
+            name: node.callee.name,
+            length: node.arguments.length,
+          }
+        );
+      }
+    },
+  };
+};
+
+module.exports.schema = [];

grunt/tasks/eslint.js

@@ -6,7 +6,7 @@ module.exports = function() {
   var done = this.async();
   grunt.util.spawn({
     cmd: 'node_modules/.bin/eslint',
-    args: ['.'],
+    args: ['.', '--rulesdir=eslint-rules'],
     opts: {stdio: 'inherit'}, // allows colors to passthrough
   }, function(err, result, code) {
     if (err) {
@@ -14,9 +14,6 @@ module.exports = function() {
     } else {
       grunt.log.ok('Lint passed (but may contain warnings)');
     }
-    if (result.stdout.length) {
-      grunt.log.writeln(result.stdout);
-    }
 
     done(code === 0);
   });

grunt/tasks/test-eslint-rules.js

@@ -0,0 +1,20 @@
+'use strict';
+
+var grunt = require('grunt');
+
+module.exports = function() {
+  var done = this.async();
+  grunt.util.spawn({
+    cmd: 'node_modules/eslint-tester/node_modules/mocha/bin/mocha',
+    args: ['eslint-rules/__tests__'],
+    opts: {stdio: 'inherit'}, // allows colors to passthrough
+  }, function(err, result, code) {
+    if (err) {
+      grunt.log.error('Custom linter rules are broken');
+    } else {
+      grunt.log.ok('Custom linter rules are okay');
+    }
+
+    done(code === 0);
+  });
+};

package.json

@@ -41,6 +41,7 @@
     "es5-shim": "^4.0.0",
     "eslint": "^0.21.2",
     "eslint-plugin-react": "^2.3.0",
+    "eslint-tester": "^0.7.0",
     "grunt": "~0.4.2",
     "grunt-cli": "^0.1.13",
     "grunt-compare-size": "~0.4.0",

src/addons/ReactFragment.js

@@ -104,7 +104,7 @@ var ReactFragment = {
       if (typeof object !== 'object' || !object || Array.isArray(object)) {
         warning(
           false,
-          'React.addons.createFragment only accepts a single object.',
+          'React.addons.createFragment only accepts a single object. Got: %s',
           object
         );
         return object;

src/isomorphic/classic/element/ReactElementValidator.js

@@ -166,7 +166,8 @@ function warnAndMonitorForKeyUse(message, element, parentType) {
 
   warning(
     false,
-    message + '%s%s See https://fb.me/react-warning-keys for more information.',
+    '%s%s%s See https://fb.me/react-warning-keys for more information.',
+    message,
     parentOrOwnerAddendum,
     childOwnerAddendum
   );

src/renderers/dom/client/ReactMount.js

@@ -736,7 +736,8 @@ var ReactMount = {
           warning(
             false,
             'ReactMount: Root element has been removed from its original ' +
-            'container. New container:', rootElement.parentNode
+            'container. New container: %s',
+            rootElement.parentNode
           );
         }
       }

src/renderers/shared/reconciler/ReactInstanceHandles.js

@@ -204,7 +204,7 @@ function traverseParentPath(start, stop, cb, arg, skipFirst, skipLast) {
       depth++ < MAX_TREE_DEPTH,
       'traverseParentPath(%s, %s, ...): Detected an infinite loop while ' +
       'traversing the React DOM ID tree. This may be due to malformed IDs: %s',
-      start, stop
+      start, stop, id
     );
   }
 }