Skip to content

Commit

Permalink
[Fix] no-arrow-function-lifecycle: fix invalid autofix from a conci…
Browse files Browse the repository at this point in the history
…se arrow method to a regular one

Fixes #3145
  • Loading branch information
ljharb committed Nov 19, 2021
1 parent 8f00023 commit eb81897
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
81 changes: 64 additions & 17 deletions lib/rules/no-arrow-function-lifecycle.js
Expand Up @@ -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: {
Expand All @@ -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
*/
Expand All @@ -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('')}; }`
)
);
},
});
}
});
Expand Down
38 changes: 38 additions & 0 deletions tests/lib/rules/no-arrow-function-lifecycle.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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*/
}
`,
},
]),
});

0 comments on commit eb81897

Please sign in to comment.