[Fix] no-arrow-function-lifecycle: fix invalid autofix from a concise arrow method to a regular one

Fixes #3145

Author: ljharb

CHANGELOG.md

@@ -9,10 +9,12 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 * [`no-invalid-html-attribute`]: allow `link` `rel` to have `apple-touch-icon`, `mask-icon` ([#3132][] @ljharb)
 * [`no-unused-class-component-methods`]: add `getChildContext` lifecycle method ([#3136][] @yoyo837)
 * [`prop-types`]: fix false positives on renames in object destructuring ([#3142][] @golopot)
+* [`no-arrow-function-lifecycle`]: fix invalid autofix from a concise arrow method to a regular one ([#3145][] @ljharb)
 
 ### Changed
 * [readme] fix syntax typo ([#3141][] @moselhy)
 
+[#3145]: https://github.com/yannickcr/eslint-plugin-react/issue/3145
 [#3142]: https://github.com/yannickcr/eslint-plugin-react/pull/3142
 [#3141]: https://github.com/yannickcr/eslint-plugin-react/pull/3141
 [#3136]: https://github.com/yannickcr/eslint-plugin-react/pull/3136

lib/rules/no-arrow-function-lifecycle.js

@@ -12,6 +12,20 @@ const astUtil = require('../util/ast');
 const docsUrl = require('../util/docsUrl');
 const lifecycleMethods = require('../util/lifecycleMethods');
 
+function getText(node) {
+  const params = node.value.params.map((p) => p.name);
+
+  if (node.type === 'Property') {
+    return `: function(${params.join(', ')}) `;
+  }
+
+  if (node.type === 'ClassProperty' || node.type === 'PropertyDefinition') {
+    return `(${params.join(', ')}) `;
+  }
+
+  return null;
+}
+
 module.exports = {
   meta: {
     docs: {
@@ -25,20 +39,6 @@ module.exports = {
   },
 
   create: Components.detect((context, components, utils) => {
-    function getText(node) {
-      const params = node.value.params.map((p) => p.name);
-
-      if (node.type === 'Property') {
-        return `: function(${params.join(', ')}) `;
-      }
-
-      if (node.type === 'ClassProperty' || node.type === 'PropertyDefinition') {
-        return `(${params.join(', ')}) `;
-      }
-
-      return null;
-    }
-
     /**
      * @param {Array} properties list of component properties
      */
@@ -57,16 +57,63 @@ module.exports = {
         ).indexOf(propertyName) > -1;
 
         if (nodeType === 'ArrowFunctionExpression' && isLifecycleMethod) {
-          const range = [node.key.range[1], node.value.body.range[0]];
-          const text = getText(node);
+          const body = node.value.body;
+          const isBlockBody = body.type === 'BlockStatement';
+          const sourceCode = context.getSourceCode();
+
+          let nextComment = [];
+          let previousComment = [];
+          let bodyRange;
+          if (!isBlockBody) {
+            const previousToken = sourceCode.getTokenBefore(body);
+
+            if (sourceCode.getCommentsBefore) {
+              // eslint >=4.x
+              previousComment = sourceCode.getCommentsBefore(body);
+            } else {
+              // eslint 3.x
+              const potentialComment = sourceCode.getTokenBefore(body, { includeComments: true });
+              previousComment = previousToken === potentialComment ? [] : [potentialComment];
+            }
+
+            if (sourceCode.getCommentsAfter) {
+              // eslint >=4.x
+              nextComment = sourceCode.getCommentsAfter(body);
+            } else {
+              // eslint 3.x
+              const potentialComment = sourceCode.getTokenAfter(body, { includeComments: true });
+              const nextToken = sourceCode.getTokenAfter(body);
+              nextComment = nextToken === potentialComment ? [] : [potentialComment];
+            }
+            bodyRange = [
+              (previousComment.length > 0 ? previousComment[0] : body).range[0],
+              (nextComment.length > 0 ? nextComment[nextComment.length - 1] : body).range[1],
+            ];
+          }
+          const headRange = [
+            node.key.range[1],
+            (previousComment.length > 0 ? previousComment[0] : body).range[0],
+          ];
 
           context.report({
             node,
             message: '{{propertyName}} is a React lifecycle method, and should not be an arrow function or in a class field. Use an instance method instead.',
             data: {
               propertyName,
             },
-            fix: (fixer) => fixer.replaceTextRange(range, text),
+            fix(fixer) {
+              if (!sourceCode.getCommentsAfter) {
+                // eslint 3.x
+                return isBlockBody && fixer.replaceTextRange(headRange, getText(node));
+              }
+              return [].concat(
+                fixer.replaceTextRange(headRange, getText(node)),
+                isBlockBody ? [] : fixer.replaceTextRange(
+                  bodyRange,
+                  `{ return ${previousComment.map((x) => sourceCode.getText(x)).join('')}${sourceCode.getText(body)}${nextComment.map((x) => sourceCode.getText(x)).join('')}; }`
+                )
+              );
+            },
           });
         }
       });

tests/lib/rules/no-arrow-function-lifecycle.js

@@ -5,7 +5,9 @@
 
 'use strict';
 
+const semver = require('semver');
 const RuleTester = require('eslint').RuleTester;
+const eslintPkg = require('eslint/package.json');
 const rule = require('../../../lib/rules/no-arrow-function-lifecycle');
 
 const parsers = require('../../helpers/parsers');
@@ -949,5 +951,41 @@ ruleTester.run('no-arrow-function-lifecycle', rule, {
         }
       `,
     },
+    {
+      code: `
+        class Hello extends React.Component {
+          render = () => <div />
+        }
+      `,
+      features: ['class fields'],
+      errors: [{ message: 'render is a React lifecycle method, and should not be an arrow function or in a class field. Use an instance method instead.' }],
+      output: semver.satisfies(eslintPkg.version, '> 3') ? `
+        class Hello extends React.Component {
+          render() { return <div />; }
+        }
+      ` : `
+        class Hello extends React.Component {
+          render = () => <div />
+        }
+      `,
+    },
+    {
+      code: `
+        class Hello extends React.Component {
+          render = () => /*first*/<div />/*second*/
+        }
+      `,
+      features: ['class fields'],
+      errors: [{ message: 'render is a React lifecycle method, and should not be an arrow function or in a class field. Use an instance method instead.' }],
+      output: semver.satisfies(eslintPkg.version, '> 3') ? `
+        class Hello extends React.Component {
+          render() { return /*first*/<div />/*second*/; }
+        }
+      ` : `
+        class Hello extends React.Component {
+          render = () => /*first*/<div />/*second*/
+        }
+      `,
+    },
   ]),
 });