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.
Benjamin Woodruff committed
Jun 3, 2015 at 15:09 UTC
17e704675491e84b9c944684d4732823c584cefe
11 files changed
+185
-8
.eslintrc
+4
@@ -73,3 +73,7 @@ rules:
73
# JSX
74
react/jsx-uses-react: 2
75
react/jsx-quotes: [2, "double"]
76
+
77
+ # CUSTOM RULES
78
+ # the second argument of warning/invariant should be a literal string
79
+ warning-and-invariant-args: 2
Gruntfile.js
+4
@@ -13,6 +13,7 @@ var npmReactToolsTasks = require('./grunt/tasks/npm-react-tools');
13
var versionCheckTask = require('./grunt/tasks/version-check');
14
var gemReactSourceTasks = require('./grunt/tasks/gem-react-source');
15
var eslintTask = require('./grunt/tasks/eslint');
16
+var testEslintRulesTask = require('./grunt/tasks/test-eslint-rules');
17
18
module.exports = function(grunt) {
19
@@ -117,6 +118,7 @@ module.exports = function(grunt) {
118
]);
119
grunt.registerTask('build:test', [
120
'delete-build-modules',
121
+ 'test:eslint-rules',
122
'jsx:test',
123
'version-check',
124
'populist:test',
@@ -135,6 +137,8 @@ module.exports = function(grunt) {
137
138
grunt.registerTask('coverage:parse', require('./grunt/tasks/coverage-parse'));
139
140
+ grunt.registerTask('test:eslint-rules', testEslintRulesTask);
141
+
142
grunt.registerTask('test:webdriver:phantomjs', [
143
'connect',
144
'webdriver-phantomjs',
eslint-rules/__tests__/warning-and-invariant-args-test.js
new
+70
@@ -0,0 +1,70 @@
1
+/**
2
+ * Copyright 2015, Facebook, Inc.
3
+ * All rights reserved.
4
+ *
5
+ * This source code is licensed under the BSD-style license found in the
6
+ * LICENSE file in the root directory of this source tree. An additional grant
7
+ * of patent rights can be found in the PATENTS file in the same directory.
8
+ *
9
+ * @emails react-core
10
+ */
11
+
12
+'use strict';
13
+
14
+var eslint = require('eslint');
15
+var ESLintTester = require('eslint-tester');
16
+var eslintTester = new ESLintTester(eslint.linter);
17
+
18
+eslintTester.addRuleTest('eslint-rules/warning-and-invariant-args', {
19
+ valid: [
20
+ "warning(true, 'hello, world');",
21
+ "warning(true, 'expected %s, got %s', 42, 24);",
22
+ "invariant(true, 'hello, world');",
23
+ "invariant(true, 'expected %s, got %s', 42, 24);",
24
+ ],
25
+ invalid: [
26
+ {
27
+ code: "warning('hello, world');",
28
+ errors: [
29
+ {
30
+ message: 'warning takes at least two arguments',
31
+ },
32
+ ],
33
+ },
34
+ {
35
+ code: 'warning(true, null);',
36
+ errors: [
37
+ {
38
+ message: 'The second argument to warning must be a string literal',
39
+ },
40
+ ],
41
+ },
42
+ {
43
+ code: 'var g = 5; invariant(true, g);',
44
+ errors: [
45
+ {
46
+ message: 'The second argument to invariant must be a string literal',
47
+ },
48
+ ],
49
+ },
50
+ {
51
+ code: "warning(true, 'expected %s, got %s');",
52
+ errors: [
53
+ {
54
+ message: 'Expected 4 arguments in call to warning, but got 2 ' +
55
+ 'based on the number of "%s" substitutions',
56
+ },
57
+ ],
58
+ },
59
+ {
60
+ code: "warning(true, 'foobar', 'junk argument');",
61
+ errors: [
62
+ {
63
+ message: 'Expected 2 arguments in call to warning, but got 3 ' +
64
+ 'based on the number of "%s" substitutions',
65
+ },
66
+ ],
67
+ },
68
+ ],
69
+});
70
+
eslint-rules/warning-and-invariant-args.js
new
+79
@@ -0,0 +1,79 @@
1
+/**
2
+ * Copyright 2015, Facebook, Inc.
3
+ * All rights reserved.
4
+ *
5
+ * This source code is licensed under the BSD-style license found in the
6
+ * LICENSE file in the root directory of this source tree. An additional grant
7
+ * of patent rights can be found in the PATENTS file in the same directory.
8
+ *
9
+ * @emails react-core
10
+*/
11
+
12
+'use strict';
13
+
14
+/**
15
+ * The warning() and invariant() functions take format strings as their second
16
+ * argument.
17
+ */
18
+
19
+module.exports = function(context) {
20
+ // we also allow literal strings and concatinated literal strings
21
+ function getLiteralString(node) {
22
+ if (node.type === 'Literal' && typeof node.value === 'string') {
23
+ return node.value;
24
+ } else if (node.type === 'BinaryExpression' && node.operator === '+') {
25
+ var l = getLiteralString(node.left);
26
+ var r = getLiteralString(node.right);
27
+ if (l !== null && r !== null) {
28
+ return l + r;
29
+ }
30
+ }
31
+ return null;
32
+ }
33
+
34
+ return {
35
+ CallExpression: function(node) {
36
+ // This could be a little smarter by checking context.getScope() to see
37
+ // how warning/invariant was defined.
38
+ var isWarningOrInvariant =
39
+ node.callee.type === 'Identifier' &&
40
+ (node.callee.name === 'warning' || node.callee.name === 'invariant');
41
+ if (!isWarningOrInvariant) {
42
+ return;
43
+ }
44
+ if (node.arguments.length < 2) {
45
+ context.report(
46
+ node,
47
+ '{{name}} takes at least two arguments',
48
+ {name: node.callee.name}
49
+ );
50
+ return;
51
+ }
52
+ var str = getLiteralString(node.arguments[1]);
53
+ if (str === null) {
54
+ context.report(
55
+ node,
56
+ 'The second argument to {{name}} must be a string literal',
57
+ {name: node.callee.name}
58
+ );
59
+ return;
60
+ }
61
+ // count the number of string substitutions, plus the first two args
62
+ var expectedNArgs = (str.match(/%s/g) || []).length + 2;
63
+ if (node.arguments.length !== expectedNArgs) {
64
+ context.report(
65
+ node,
66
+ 'Expected {{expectedNArgs}} arguments in call to {{name}}, but ' +
67
+ 'got {{length}} based on the number of "%s" substitutions',
68
+ {
69
+ expectedNArgs: expectedNArgs,
70
+ name: node.callee.name,
71
+ length: node.arguments.length,
72
+ }
73
+ );
74
+ }
75
+ },
76
+ };
77
+};
78
+
79
+module.exports.schema = [];
grunt/tasks/eslint.js
+1
-4
@@ -6,7 +6,7 @@ module.exports = function() {
6
var done = this.async();
7
grunt.util.spawn({
8
cmd: 'node_modules/.bin/eslint',
9
- args: ['.'],
9
+ args: ['.', '--rulesdir=eslint-rules'],
10
opts: {stdio: 'inherit'}, // allows colors to passthrough
11
}, function(err, result, code) {
12
if (err) {
@@ -14,9 +14,6 @@ module.exports = function() {
14
} else {
15
grunt.log.ok('Lint passed (but may contain warnings)');
16
}
17
- if (result.stdout.length) {
18
- grunt.log.writeln(result.stdout);
19
- }
17
18
done(code === 0);
19
});
grunt/tasks/test-eslint-rules.js
new
+20
@@ -0,0 +1,20 @@
1
+'use strict';
2
+
3
+var grunt = require('grunt');
4
+
5
+module.exports = function() {
6
+ var done = this.async();
7
+ grunt.util.spawn({
8
+ cmd: 'node_modules/eslint-tester/node_modules/mocha/bin/mocha',
9
+ args: ['eslint-rules/__tests__'],
10
+ opts: {stdio: 'inherit'}, // allows colors to passthrough
11
+ }, function(err, result, code) {
12
+ if (err) {
13
+ grunt.log.error('Custom linter rules are broken');
14
+ } else {
15
+ grunt.log.ok('Custom linter rules are okay');
16
+ }
17
+
18
+ done(code === 0);
19
+ });
20
+};
package.json
+1
@@ -41,6 +41,7 @@
41
"es5-shim": "^4.0.0",
42
"eslint": "^0.21.2",
43
"eslint-plugin-react": "^2.3.0",
44
+ "eslint-tester": "^0.7.0",
45
"grunt": "~0.4.2",
46
"grunt-cli": "^0.1.13",
47
"grunt-compare-size": "~0.4.0",
src/addons/ReactFragment.js
+1
-1
@@ -104,7 +104,7 @@ var ReactFragment = {
104
if (typeof object !== 'object' || !object || Array.isArray(object)) {
105
warning(
106
false,
107
- 'React.addons.createFragment only accepts a single object.',
107
+ 'React.addons.createFragment only accepts a single object. Got: %s',
108
object
109
);
110
return object;
src/isomorphic/classic/element/ReactElementValidator.js
+2
-1
@@ -166,7 +166,8 @@ function warnAndMonitorForKeyUse(message, element, parentType) {
166
167
warning(
168
false,
169
- message + '%s%s See https://fb.me/react-warning-keys for more information.',
169
+ '%s%s%s See https://fb.me/react-warning-keys for more information.',
170
+ message,
171
parentOrOwnerAddendum,
172
childOwnerAddendum
173
);
src/renderers/dom/client/ReactMount.js
+2
-1
@@ -736,7 +736,8 @@ var ReactMount = {
736
warning(
737
false,
738
'ReactMount: Root element has been removed from its original ' +
739
- 'container. New container:', rootElement.parentNode
739
+ 'container. New container: %s',
740
+ rootElement.parentNode
741
);
742
}
743
}
src/renderers/shared/reconciler/ReactInstanceHandles.js
+1
-1
@@ -204,7 +204,7 @@ function traverseParentPath(start, stop, cb, arg, skipFirst, skipLast) {
204
depth++ < MAX_TREE_DEPTH,
205
'traverseParentPath(%s, %s, ...): Detected an infinite loop while ' +
206
'traversing the React DOM ID tree. This may be due to malformed IDs: %s',
207
- start, stop
207
+ start, stop, id
208
);
209
}
210
}