From c4cef420ec6abc65d4e38c0e19553b15f8295efc Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 26 Jun 2018 11:44:11 +0200 Subject: [PATCH 1/3] 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. --- lib/rules/no-deprecated.js | 21 ++++++----- lib/util/ast.js | 23 ++++++++--- tests/lib/rules/no-deprecated.js | 65 +++++++++++++++++++++----------- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/lib/rules/no-deprecated.js b/lib/rules/no-deprecated.js index c5dee54f5a..f5499deb7b 100644 --- a/lib/rules/no-deprecated.js +++ b/lib/rules/no-deprecated.js @@ -106,19 +106,19 @@ module.exports = { ); } - function checkDeprecation(node, method) { - if (!isDeprecated(method)) { + function checkDeprecation(node, methodName, methodNode) { + if (!isDeprecated(methodName)) { return; } const deprecated = getDeprecated(); - const version = deprecated[method][0]; - const newMethod = deprecated[method][1]; - const refs = deprecated[method][2]; + const version = deprecated[methodName][0]; + const newMethod = deprecated[methodName][1]; + const refs = deprecated[methodName][2]; context.report({ - node: node, + node: methodNode || node, message: DEPRECATED_MESSAGE, data: { - oldMethod: method, + oldMethod: methodName, version, newMethod: newMethod ? `, use ${newMethod} instead` : '', refs: refs ? `, see ${refs}` : '' @@ -150,7 +150,10 @@ module.exports = { */ function getLifeCycleMethods(node) { const properties = astUtil.getComponentProperties(node); - return properties.map(property => astUtil.getPropertyName(property)); + return properties.map(property => ({ + name: astUtil.getPropertyName(property), + node: astUtil.getPropertyNameNode(property) + })); } /** @@ -160,7 +163,7 @@ module.exports = { function checkLifeCycleMethods(node) { if (utils.isES5Component(node) || utils.isES6Component(node)) { const methods = getLifeCycleMethods(node); - methods.forEach(method => checkDeprecation(node, method)); + methods.forEach(method => checkDeprecation(node, method.name, method.node)); } } diff --git a/lib/util/ast.js b/lib/util/ast.js index 6161cb4513..a2c7696ea6 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -28,17 +28,27 @@ function findReturnStatement(node) { } /** - * Get properties name + * Get node with property's name * @param {Object} node - Property. - * @returns {String} Property name. + * @returns {Object} Property name node. */ -function getPropertyName(node) { +function getPropertyNameNode(node) { if (node.key || ['MethodDefinition', 'Property'].indexOf(node.type) !== -1) { - return node.key.name; + return node.key; } else if (node.type === 'MemberExpression') { - return node.property.name; + return node.property; } - return ''; + return null; +} + +/** + * Get properties name + * @param {Object} node - Property. + * @returns {String} Property name. + */ +function getPropertyName(node) { + const nameNode = getPropertyNameNode(node); + return nameNode ? nameNode.name : ''; } /** @@ -88,6 +98,7 @@ function isNodeFirstInLine(context, node) { module.exports = { findReturnStatement: findReturnStatement, getPropertyName: getPropertyName, + getPropertyNameNode: getPropertyNameNode, getComponentProperties: getComponentProperties, isNodeFirstInLine: isNodeFirstInLine }; diff --git a/tests/lib/rules/no-deprecated.js b/tests/lib/rules/no-deprecated.js index 2472ce5848..363adf2b53 100644 --- a/tests/lib/rules/no-deprecated.js +++ b/tests/lib/rules/no-deprecated.js @@ -223,18 +223,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, @@ -253,18 +256,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, @@ -281,18 +287,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, @@ -309,18 +318,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, @@ -337,18 +349,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, @@ -365,23 +380,26 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] }, { - code: ` + code: `un class Foo extends React.Component { constructor() {} componentWillMount() {} @@ -394,18 +412,21 @@ ruleTester.run('no-deprecated', rule, { message: errorMessage( 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' - ) + ), + type: 'Identifier' }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' - ) + ), + type: 'Identifier' }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' - ) + ), + type: 'Identifier' } ] } From a5ded4dd82f5f193493df445466886eb50d5703c Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 26 Jun 2018 20:39:01 +0200 Subject: [PATCH 2/3] Add line/column checks in tests for deprecated lifecycle methods --- tests/lib/rules/no-deprecated.js | 128 +++++++++++++++++++++++++------ 1 file changed, 106 insertions(+), 22 deletions(-) diff --git a/tests/lib/rules/no-deprecated.js b/tests/lib/rules/no-deprecated.js index 363adf2b53..3449767674 100644 --- a/tests/lib/rules/no-deprecated.js +++ b/tests/lib/rules/no-deprecated.js @@ -224,20 +224,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 3, + column: 11, + endLine: 3, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 30 } ] }, @@ -257,20 +269,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 13, + endLine: 4, + endColumn: 31 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 13, + endLine: 5, + endColumn: 38 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 6, + column: 13, + endLine: 6, + endColumn: 32 } ] }, @@ -288,20 +312,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 3, + column: 11, + endLine: 3, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 30 } ] }, @@ -319,20 +355,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 3, + column: 11, + endLine: 3, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 30 } ] }, @@ -350,20 +398,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 3, + column: 11, + endLine: 3, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 30 } ] }, @@ -381,25 +441,37 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 3, + column: 11, + endLine: 3, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 30 } ] }, { - code: `un + code: ` class Foo extends React.Component { constructor() {} componentWillMount() {} @@ -413,20 +485,32 @@ ruleTester.run('no-deprecated', rule, { 'componentWillMount', '16.3.0', 'UNSAFE_componentWillMount', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillmount' ), - type: 'Identifier' + type: 'Identifier', + line: 4, + column: 11, + endLine: 4, + endColumn: 29 }, { message: errorMessage( 'componentWillReceiveProps', '16.3.0', 'UNSAFE_componentWillReceiveProps', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops' ), - type: 'Identifier' + type: 'Identifier', + line: 5, + column: 11, + endLine: 5, + endColumn: 36 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', 'https://reactjs.org/docs/react-component.html#unsafe_componentwillupdate' ), - type: 'Identifier' + type: 'Identifier', + line: 6, + column: 11, + endLine: 6, + endColumn: 30 } ] } From 248c2ca50af4e7fdb765b73551ada09b6b7c7307 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 26 Jun 2018 23:17:23 +0200 Subject: [PATCH 3/3] Remove endLine/Column tests: not supported in ESLint 3 --- tests/lib/rules/no-deprecated.js | 84 ++++++++------------------------ 1 file changed, 21 insertions(+), 63 deletions(-) diff --git a/tests/lib/rules/no-deprecated.js b/tests/lib/rules/no-deprecated.js index 3449767674..12ab519904 100644 --- a/tests/lib/rules/no-deprecated.js +++ b/tests/lib/rules/no-deprecated.js @@ -226,9 +226,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 3, - column: 11, - endLine: 3, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -237,9 +235,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -247,9 +243,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 30 + column: 11 } ] }, @@ -271,9 +265,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 13, - endLine: 4, - endColumn: 31 + column: 13 }, { message: errorMessage( @@ -282,9 +274,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 13, - endLine: 5, - endColumn: 38 + column: 13 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -292,9 +282,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 6, - column: 13, - endLine: 6, - endColumn: 32 + column: 13 } ] }, @@ -314,9 +302,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 3, - column: 11, - endLine: 3, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -325,9 +311,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -335,9 +319,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 30 + column: 11 } ] }, @@ -357,9 +339,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 3, - column: 11, - endLine: 3, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -368,9 +348,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -378,9 +356,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 30 + column: 11 } ] }, @@ -400,9 +376,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 3, - column: 11, - endLine: 3, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -411,9 +385,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -421,9 +393,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 30 + column: 11 } ] }, @@ -443,9 +413,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 3, - column: 11, - endLine: 3, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -454,9 +422,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -464,9 +430,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 30 + column: 11 } ] }, @@ -487,9 +451,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 4, - column: 11, - endLine: 4, - endColumn: 29 + column: 11 }, { message: errorMessage( @@ -498,9 +460,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 5, - column: 11, - endLine: 5, - endColumn: 36 + column: 11 }, { message: errorMessage('componentWillUpdate', '16.3.0', 'UNSAFE_componentWillUpdate', @@ -508,9 +468,7 @@ ruleTester.run('no-deprecated', rule, { ), type: 'Identifier', line: 6, - column: 11, - endLine: 6, - endColumn: 30 + column: 11 } ] }