Skip to content

Commit c4cef42

Browse files
jsnajdrjsnajdr
committed
For deprecated lifecycle methods, report identifier node instead of the class node
When the `no-deprecated` rule finds a deprecated lifecycle method, it reports the whole class declaration as the AST node for the error. Doesn't look good in editor and hides all other ESLint errors inside the class. This patch changes the reported AST node to the method name identifier. The errors are much better targeted at the infringing code and look much better when shown in editor UI. Added also test coverage to check for the reported AST node type.
1 parent f80e744 commit c4cef42

File tree

3 files changed

+72
-37
lines changed

3 files changed

+72
-37
lines changed

‎lib/rules/no-deprecated.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,19 +106,19 @@ module.exports = {
106106
);
107107
}
108108

109-
function checkDeprecation(node, method) {
110-
if (!isDeprecated(method)) {
109+
function checkDeprecation(node, methodName, methodNode) {
110+
if (!isDeprecated(methodName)) {
111111
return;
112112
}
113113
const deprecated = getDeprecated();
114-
const version = deprecated[method][0];
115-
const newMethod = deprecated[method][1];
116-
const refs = deprecated[method][2];
114+
const version = deprecated[methodName][0];
115+
const newMethod = deprecated[methodName][1];
116+
const refs = deprecated[methodName][2];
117117
context.report({
118-
node: node,
118+
node: methodNode || node,
119119
message: DEPRECATED_MESSAGE,
120120
data: {
121-
oldMethod: method,
121+
oldMethod: methodName,
122122
version,
123123
newMethod: newMethod ? `, use ${newMethod} instead` : '',
124124
refs: refs ? `, see ${refs}` : ''
@@ -150,7 +150,10 @@ module.exports = {
150150
*/
151151
function getLifeCycleMethods(node) {
152152
const properties = astUtil.getComponentProperties(node);
153-
return properties.map(property => astUtil.getPropertyName(property));
153+
return properties.map(property => ({
154+
name: astUtil.getPropertyName(property),
155+
node: astUtil.getPropertyNameNode(property)
156+
}));
154157
}
155158

156159
/**
@@ -160,7 +163,7 @@ module.exports = {
160163
function checkLifeCycleMethods(node) {
161164
if (utils.isES5Component(node) || utils.isES6Component(node)) {
162165
const methods = getLifeCycleMethods(node);
163-
methods.forEach(method => checkDeprecation(node, method));
166+
methods.forEach(method => checkDeprecation(node, method.name, method.node));
164167
}
165168
}
166169

‎lib/util/ast.js

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,27 @@ function findReturnStatement(node) {
2828
}
2929

3030
/**
31-
* Get properties name
31+
* Get node with property's name
3232
* @param {Object} node - Property.
33-
* @returns {String} Property name.
33+
* @returns {Object} Property name node.
3434
*/
35-
function getPropertyName(node) {
35+
function getPropertyNameNode(node) {
3636
if (node.key || ['MethodDefinition', 'Property'].indexOf(node.type) !== -1) {
37-
return node.key.name;
37+
return node.key;
3838
} else if (node.type === 'MemberExpression') {
39-
return node.property.name;
39+
return node.property;
4040
}
41-
return '';
41+
return null;
42+
}
43+
44+
/**
45+
* Get properties name
46+
* @param {Object} node - Property.
47+
* @returns {String} Property name.
48+
*/
49+
function getPropertyName(node) {
50+
const nameNode = getPropertyNameNode(node);
51+
return nameNode ? nameNode.name : '';
4252
}
4353

4454
/**
@@ -88,6 +98,7 @@ function isNodeFirstInLine(context, node) {
8898
module.exports = {
8999
findReturnStatement: findReturnStatement,
90100
getPropertyName: getPropertyName,
101+
getPropertyNameNode: getPropertyNameNode,
91102
getComponentProperties: getComponentProperties,
92103
isNodeFirstInLine: isNodeFirstInLine
93104
};

‎tests/lib/rules/no-deprecated.js

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,21 @@ ruleTester.run('no-deprecated', rule, {
223223
message: errorMessage(
224224
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
225225
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
226-
)
226+
),
227+
type: 'Identifier'
227228
},
228229
{
229230
message: errorMessage(
230231
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
231232
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
232-
)
233+
),
234+
type: 'Identifier'
233235
},
234236
{
235237
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
236238
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
237-
)
239+
),
240+
type: 'Identifier'
238241
}
239242
]
240243
},
@@ -253,18 +256,21 @@ ruleTester.run('no-deprecated', rule, {
253256
message: errorMessage(
254257
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
255258
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
256-
)
259+
),
260+
type: 'Identifier'
257261
},
258262
{
259263
message: errorMessage(
260264
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
261265
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
262-
)
266+
),
267+
type: 'Identifier'
263268
},
264269
{
265270
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
266271
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
267-
)
272+
),
273+
type: 'Identifier'
268274
}
269275
]
270276
},
@@ -281,18 +287,21 @@ ruleTester.run('no-deprecated', rule, {
281287
message: errorMessage(
282288
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
283289
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
284-
)
290+
),
291+
type: 'Identifier'
285292
},
286293
{
287294
message: errorMessage(
288295
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
289296
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
290-
)
297+
),
298+
type: 'Identifier'
291299
},
292300
{
293301
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
294302
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
295-
)
303+
),
304+
type: 'Identifier'
296305
}
297306
]
298307
},
@@ -309,18 +318,21 @@ ruleTester.run('no-deprecated', rule, {
309318
message: errorMessage(
310319
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
311320
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
312-
)
321+
),
322+
type: 'Identifier'
313323
},
314324
{
315325
message: errorMessage(
316326
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
317327
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
318-
)
328+
),
329+
type: 'Identifier'
319330
},
320331
{
321332
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
322333
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
323-
)
334+
),
335+
type: 'Identifier'
324336
}
325337
]
326338
},
@@ -337,18 +349,21 @@ ruleTester.run('no-deprecated', rule, {
337349
message: errorMessage(
338350
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
339351
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
340-
)
352+
),
353+
type: 'Identifier'
341354
},
342355
{
343356
message: errorMessage(
344357
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
345358
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
346-
)
359+
),
360+
type: 'Identifier'
347361
},
348362
{
349363
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
350364
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
351-
)
365+
),
366+
type: 'Identifier'
352367
}
353368
]
354369
},
@@ -365,23 +380,26 @@ ruleTester.run('no-deprecated', rule, {
365380
message: errorMessage(
366381
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
367382
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
368-
)
383+
),
384+
type: 'Identifier'
369385
},
370386
{
371387
message: errorMessage(
372388
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
373389
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
374-
)
390+
),
391+
type: 'Identifier'
375392
},
376393
{
377394
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
378395
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
379-
)
396+
),
397+
type: 'Identifier'
380398
}
381399
]
382400
},
383401
{
384-
code: `
402+
code: `un
385403
class Foo extends React.Component {
386404
constructor() {}
387405
componentWillMount() {}
@@ -394,18 +412,21 @@ ruleTester.run('no-deprecated', rule, {
394412
message: errorMessage(
395413
'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount',
396414
'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount'
397-
)
415+
),
416+
type: 'Identifier'
398417
},
399418
{
400419
message: errorMessage(
401420
'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps',
402421
'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops'
403-
)
422+
),
423+
type: 'Identifier'
404424
},
405425
{
406426
message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate',
407427
'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate'
408-
)
428+
),
429+
type: 'Identifier'
409430
}
410431
]
411432
}

0 commit comments

Comments
 (0)