Skip to content

Commit 9b74c46

Browse files
yannickcryannickcr
committed
Rewrite display-name and no-multi-comp rules (fixes #24)
1 parent 5963e2c commit 9b74c46

File tree

4 files changed

+346
-101
lines changed

4 files changed

+346
-101
lines changed

‎lib/rules/display-name.js‎

Lines changed: 145 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,125 @@
1010

1111
module.exports = function(context) {
1212

13-
var hasDisplayName = false;
13+
var components = {};
1414

15+
var MISSING_MESSAGE = 'Component definition is missing display name';
16+
var MISSING_MESSAGE_NAMED_COMP = '{{component}} component definition is missing display name';
17+
18+
var defaultClassName = 'eslintReactComponent';
19+
20+
/**
21+
* Get the component id from an ASTNode
22+
* @param {ASTNode} node The AST node being checked.
23+
* @returns {String} The component id.
24+
*/
25+
function getComponentId(node) {
26+
if (
27+
node.type === 'MemberExpression' &&
28+
node.property && node.property.name === 'displayName' &&
29+
node.object && components[node.object.name]
30+
) {
31+
return node.object.name;
32+
}
33+
34+
var scope = context.getScope();
35+
while (scope && scope.type !== 'class') {
36+
scope = scope.upper;
37+
}
38+
39+
if (scope) {
40+
return scope.block.id.name;
41+
}
42+
43+
return defaultClassName;
44+
}
45+
46+
/**
47+
* Get the component from an ASTNode
48+
* @param {ASTNode} node The AST node being checked.
49+
* @returns {Object} The component object.
50+
*/
51+
function getComponent(node) {
52+
var id = getComponentId(node);
53+
if (!components[id]) {
54+
components[id] = {
55+
name: id,
56+
node: node,
57+
hasDisplayName: false
58+
};
59+
}
60+
return components[id];
61+
}
62+
63+
/**
64+
* Detect if we are in a React component by checking the render method
65+
* @param {ASTNode} node The AST node being checked.
66+
*/
67+
function detectReactComponent(node) {
68+
var scope = context.getScope();
69+
if (
70+
(node.argument.type === 'Literal' && (node.argument.value !== null && node.argument.value !== false)) &&
71+
(node.argument.type !== 'JSXElement') &&
72+
(scope.block.parent.key.name === 'render')
73+
) {
74+
return;
75+
}
76+
var component = getComponent(node);
77+
component.isComponentDefinition = true;
78+
}
79+
80+
/**
81+
* Checks if we are inside a component definition
82+
* @param {ASTNode} node The AST node being checked.
83+
* @returns {Boolean} True if we are inside a component definition, false if not.
84+
*/
1585
function isComponentDefinition(node) {
16-
return (
86+
var isES5Component = Boolean(
87+
node.parent &&
88+
node.parent.callee &&
89+
node.parent.callee.object &&
90+
node.parent.callee.property &&
91+
node.parent.callee.object.name === 'React' &&
92+
node.parent.callee.property.name === 'createClass'
93+
);
94+
var isES6Component = getComponent(node).isComponentDefinition;
95+
return isES5Component || isES6Component;
96+
}
97+
98+
/**
99+
* Checks if we are declaring a display name
100+
* @param {ASTNode} node The AST node being checked.
101+
* @returns {Boolean} True if we are declaring a display name, false if not.
102+
*/
103+
function isDisplayNameDeclaration(node) {
104+
return Boolean(
17105
node &&
18-
node.callee &&
19-
node.callee.object &&
20-
node.callee.property &&
21-
node.callee.object.name === 'React' &&
22-
node.callee.property.name === 'createClass'
106+
node.name === 'displayName'
107+
);
108+
}
109+
110+
/**
111+
* Mark a prop type as declared
112+
* @param {ASTNode} node The AST node being checked.
113+
*/
114+
function markDisplayNameAsDeclared(node) {
115+
var component = getComponent(node);
116+
component.hasDisplayName = true;
117+
}
118+
119+
/**
120+
* Reports missing display name for a given component
121+
* @param {String} id The id of the component to process
122+
*/
123+
function reportMissingDisplayName(id) {
124+
if (!components[id] || components[id].hasDisplayName === true) {
125+
return;
126+
}
127+
context.report(
128+
components[id].node,
129+
id === defaultClassName ? MISSING_MESSAGE : MISSING_MESSAGE_NAMED_COMP, {
130+
component: id
131+
}
23132
);
24133
}
25134

@@ -29,32 +138,50 @@ module.exports = function(context) {
29138

30139
return {
31140

32-
ObjectExpression: function(node) {
141+
MemberExpression: function(node) {
142+
if (!isDisplayNameDeclaration(node.property)) {
143+
return;
144+
}
145+
markDisplayNameAsDeclared(node);
146+
},
33147

34-
if (!isComponentDefinition(node.parent)) {
148+
ObjectExpression: function(node) {
149+
if (!isComponentDefinition(node)) {
35150
return;
36151
}
37152

153+
// Search for the displayName declaration
38154
node.properties.forEach(function(property) {
39-
var keyName = property.key.name || property.key.value;
40-
if (keyName === 'displayName') {
41-
hasDisplayName = true;
155+
if (!isDisplayNameDeclaration(property.key)) {
156+
return;
42157
}
158+
markDisplayNameAsDeclared(node);
43159
});
44160
},
45161

46162
'ObjectExpression:exit': function(node) {
47-
48-
if (!isComponentDefinition(node.parent)) {
163+
if (!isComponentDefinition(node)) {
49164
return;
50165
}
51166

52-
if (!hasDisplayName) {
53-
context.report(node, 'Component definition is missing display name');
167+
// Report missing display name for all ES5 classes
168+
reportMissingDisplayName(defaultClassName);
169+
170+
// Reset the ES5 default object
171+
components[defaultClassName] = null;
172+
},
173+
174+
'Program:exit': function() {
175+
// Report missing display name for all ES6 classes
176+
for (var component in components) {
177+
if (!components.hasOwnProperty(component)) {
178+
continue;
179+
}
180+
reportMissingDisplayName(component);
54181
}
182+
},
55183

56-
hasDisplayName = false;
57-
}
184+
ReturnStatement: detectReactComponent
58185
};
59186

60187
};

‎lib/rules/no-multi-comp.js‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,80 @@
1010

1111
module.exports = function(context) {
1212

13-
var componentCounter = 0;
13+
var components = [];
14+
15+
var MULTI_COMP_MESSAGE = 'Declare only one React component per file';
16+
17+
/**
18+
* Checks if we are inside a component definition
19+
* @param {ASTNode} node The AST node being checked.
20+
* @returns {Boolean} True if we are inside a component definition, false if not.
21+
*/
22+
function isComponentDefinition(node) {
23+
return Boolean(
24+
node.parent &&
25+
node.parent.callee &&
26+
node.parent.callee.object &&
27+
node.parent.callee.property &&
28+
node.parent.callee.object.name === 'React' &&
29+
node.parent.callee.property.name === 'createClass'
30+
);
31+
}
32+
33+
/**
34+
* Get the component from an ASTNode
35+
* @param {ASTNode} node The AST node being checked.
36+
* @returns {Object} The component object.
37+
*/
38+
function getComponent() {
39+
var scope = context.getScope();
40+
while (scope && scope.type !== 'class') {
41+
if (isComponentDefinition(scope.block.parent.parent)) {
42+
return scope.block.parent.parent.parent.callee;
43+
}
44+
scope = scope.upper;
45+
}
46+
47+
return scope.block;
48+
}
49+
50+
/**
51+
* Detect if we are in a React component by checking the render method
52+
* @param {ASTNode} node The AST node being checked.
53+
*/
54+
function detectReactComponent(node) {
55+
var scope = context.getScope();
56+
if (
57+
(node.argument.type === 'Literal' && (node.argument.value !== null && node.argument.value !== false)) &&
58+
(node.argument.type !== 'JSXElement') &&
59+
(scope.block.parent.key.name === 'render')
60+
) {
61+
return;
62+
}
63+
components.push(getComponent(node));
64+
}
65+
66+
/**
67+
* Reports missing display name for a given component
68+
* @param {String} id The id of the component to process
69+
*/
70+
function reportMultiComponent() {
71+
if (components.length <= 1) {
72+
return;
73+
}
74+
75+
for (var i = 1, j = components.length; i < j; i++) {
76+
context.report(components[i], MULTI_COMP_MESSAGE);
77+
}
78+
}
1479

1580
// --------------------------------------------------------------------------
1681
// Public
1782
// --------------------------------------------------------------------------
1883

1984
return {
20-
MemberExpression: function(node) {
21-
if (node.object.name === 'React' && node.property.name === 'createClass' && ++componentCounter > 1) {
22-
context.report(node, 'Declare only one React component per file');
23-
}
24-
}
85+
'Program:exit': reportMultiComponent,
86+
87+
ReturnStatement: detectReactComponent
2588
};
2689
};

‎tests/lib/rules/display-name.js‎

Lines changed: 57 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -18,46 +18,61 @@ var ESLintTester = require('eslint-tester');
1818
var eslintTester = new ESLintTester(eslint);
1919
eslintTester.addRuleTest('lib/rules/display-name', {
2020

21-
valid: [
22-
{
23-
code: '\
24-
var Hello = React.createClass({\
25-
displayName: \'Hello\',\
26-
render: function() {\
27-
return <div>Hello {this.props.name}</div>;\
28-
}\
29-
});',
30-
ecmaFeatures: {
31-
jsx: true
32-
}
33-
}, {
34-
code: '\
35-
var createClass = function() {};\
36-
var Hello = createClass({\
37-
render: function() {\
38-
return <div>Hello {this.props.name}</div>;\
39-
}\
40-
});',
41-
ecmaFeatures: {
42-
jsx: true
43-
}
44-
}
45-
],
21+
valid: [{
22+
code: [
23+
'var Hello = React.createClass({',
24+
' displayName: \'Hello\',',
25+
' render: function() {',
26+
' return <div>Hello {this.props.name}</div>;',
27+
' }',
28+
'});'
29+
].join('\n'),
30+
ecmaFeatures: {
31+
jsx: true
32+
}
33+
}, {
34+
code: [
35+
'class Hello extends React.Component {',
36+
' render() {',
37+
' return <div>Hello {this.props.name}</div>;',
38+
' }',
39+
'}',
40+
'Hello.displayName = \'Hello\''
41+
].join('\n'),
42+
ecmaFeatures: {
43+
classes: true,
44+
jsx: true
45+
}
46+
}],
4647

47-
invalid: [
48-
{
49-
code: '\
50-
var Hello = React.createClass({\
51-
render: function() {\
52-
return <div>Hello {this.props.name}</div>;\
53-
}\
54-
});',
55-
ecmaFeatures: {
56-
jsx: true
57-
},
58-
errors: [{
59-
message: 'Component definition is missing display name'
60-
}]
61-
}
62-
]
63-
});
48+
invalid: [{
49+
code: [
50+
'var Hello = React.createClass({',
51+
' render: function() {',
52+
' return <div>Hello {this.props.name}</div>;',
53+
' }',
54+
'});'
55+
].join('\n'),
56+
ecmaFeatures: {
57+
jsx: true
58+
},
59+
errors: [{
60+
message: 'Component definition is missing display name'
61+
}]
62+
}, {
63+
code: [
64+
'class Hello extends React.Component {',
65+
' render() {',
66+
' return <div>Hello {this.props.name}</div>;',
67+
' }',
68+
'}'
69+
].join('\n'),
70+
ecmaFeatures: {
71+
classes: true,
72+
jsx: true
73+
},
74+
errors: [{
75+
message: 'Hello component definition is missing display name'
76+
}]
77+
}
78+
]});

0 commit comments

Comments
 (0)