Skip to content

Commit

Permalink
[Fix] function-component-definition: replace var by const in ce…
Browse files Browse the repository at this point in the history 
…rtain situations

Signed-off-by: Xavier Le Cunff <[email protected]>
Co-authored-by: Xavier Le Cunff <[email protected]>
Co-authored-by: Simeon Cheeseman <[email protected]>
  • Loading branch information
@JohnBerd @SimeonC @ljharb
3 people authored and ljharb committed Mar 20, 2022
1 parent 18de0a6 commit 5069def
Show file tree
Hide file tree
Showing 3 changed files with 254 additions and 59 deletions.
30 changes: 15 additions & 15 deletions docs/rules/function-component-definition.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ Examples of **incorrect** code for this rule:

```jsx
// function expression for named component
var Component = function (props) {
const Component = function (props) {
return <div>{props.content}</div>;
};

// arrow function for named component
var Component = (props) => {
const Component = (props) => {
return <div>{props.content}</div>;
};

Expand Down Expand Up @@ -49,11 +49,11 @@ Examples of **incorrect** code for this rule:
```jsx
// only function declarations for named components
// [2, { "namedComponents": "function-declaration" }]
var Component = function (props) {
const Component = function (props) {
return <div />;
};

var Component = (props) => {
const Component = (props) => {
return <div />;
};

Expand All @@ -63,7 +63,7 @@ function Component (props) {
return <div />;
};

var Component = (props) => {
const Component = (props) => {
return <div />;
};

Expand All @@ -73,7 +73,7 @@ function Component (props) {
return <div />;
};

var Component = function (props) {
const Component = function (props) {
return <div />;
};

Expand Down Expand Up @@ -107,13 +107,13 @@ function Component (props) {

// only function expressions for named components
// [2, { "namedComponents": "function-expression" }]
var Component = function (props) {
const Component = function (props) {
return <div />;
};

// only arrow functions for named components
// [2, { "namedComponents": "arrow-function" }]
var Component = (props) => {
const Component = (props) => {
return <div />;
};

Expand Down Expand Up @@ -170,11 +170,11 @@ The following patterns can **not** be autofixed in TypeScript:
```tsx
// function expressions and arrow functions that have type annotations cannot be autofixed to function declarations
// [2, { "namedComponents": "function-declaration" }]
var Component: React.FC<Props> = function (props) {
const Component: React.FC<Props> = function (props) {
return <div />;
};

var Component: React.FC<Props> = (props) => {
const Component: React.FC<Props> = (props) => {
return <div />;
};

Expand All @@ -184,7 +184,7 @@ function Component<T>(props: Props<T>) {
return <div />;
};

var Component = function <T>(props: Props<T>) {
const Component = function <T>(props: Props<T>) {
return <div />;
};

Expand All @@ -203,13 +203,13 @@ The following patterns can be autofixed in TypeScript:
```tsx
// autofix to function expression with type annotation
// [2, { "namedComponents": "function-expression" }]
var Component: React.FC<Props> = (props) => {
const Component: React.FC<Props> = (props) => {
return <div />;
};

// autofix to arrow function with type annotation
// [2, { "namedComponents": "function-expression" }]
var Component: React.FC<Props> = function (props) {
const Component: React.FC<Props> = function (props) {
return <div />;
};

Expand All @@ -219,7 +219,7 @@ function Component<T extends {}>(props: Props<T>) {
return <div />;
}

var Component = function <T extends {}>(props: Props<T>) {
const Component = function <T extends {}>(props: Props<T>) {
return <div />;
};

Expand All @@ -229,7 +229,7 @@ function Component<T1, T2>(props: Props<T1, T2>) {
return <div />;
}

var Component = function <T1, T2>(props: Props<T2>) {
const Component = function <T1, T2>(props: Props<T2>) {
return <div />;
};

Expand Down
142 changes: 106 additions & 36 deletions lib/rules/function-component-definition.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,19 @@ const reportC = require('../util/report');
// ------------------------------------------------------------------------------

function buildFunction(template, parts) {
return Object.keys(parts)
.reduce((acc, key) => acc.replace(`{${key}}`, () => (parts[key] || '')), template);
return Object.keys(parts).reduce(
(acc, key) => acc.replace(`{${key}}`, () => parts[key] || ''),
template
);
}

const NAMED_FUNCTION_TEMPLATES = {
'function-declaration': 'function {name}{typeParams}({params}){returnType} {body}',
'arrow-function': 'var {name}{typeAnnotation} = {typeParams}({params}){returnType} => {body}',
'function-expression': 'var {name}{typeAnnotation} = function{typeParams}({params}){returnType} {body}',
'function-declaration':
'function {name}{typeParams}({params}){returnType} {body}',
'arrow-function':
'{varType} {name}{typeAnnotation} = {typeParams}({params}){returnType} => {body}',
'function-expression':
'{varType} {name}{typeAnnotation} = function{typeParams}({params}){returnType} {body}',
};

const UNNAMED_FUNCTION_TEMPLATES = {
Expand All @@ -32,14 +37,20 @@ const UNNAMED_FUNCTION_TEMPLATES = {

function hasOneUnconstrainedTypeParam(node) {
if (node.typeParameters) {
return node.typeParameters.params.length === 1 && !node.typeParameters.params[0].constraint;
return (
node.typeParameters.params.length === 1
&& !node.typeParameters.params[0].constraint
);
}

return false;
}

function hasName(node) {
return node.type === 'FunctionDeclaration' || node.parent.type === 'VariableDeclarator';
return (
node.type === 'FunctionDeclaration'
|| node.parent.type === 'VariableDeclarator'
);
}

function getNodeText(prop, source) {
Expand All @@ -52,25 +63,29 @@ function getName(node) {
return node.id.name;
}

if (node.type === 'ArrowFunctionExpression' || node.type === 'FunctionExpression') {
if (
node.type === 'ArrowFunctionExpression'
|| node.type === 'FunctionExpression'
) {
return hasName(node) && node.parent.id.name;
}
}

function getParams(node, source) {
if (node.params.length === 0) return null;
return source.slice(node.params[0].range[0], node.params[node.params.length - 1].range[1]);
return source.slice(
node.params[0].range[0],
node.params[node.params.length - 1].range[1]
);
}

function getBody(node, source) {
const range = node.body.range;

if (node.body.type !== 'BlockStatement') {
return [
'{',
` return ${source.slice(range[0], range[1])}`,
'}',
].join('\n');
return ['{', ` return ${source.slice(range[0], range[1])}`, '}'].join(
'\n'
);
}

return source.slice(range[0], range[1]);
Expand All @@ -79,13 +94,20 @@ function getBody(node, source) {
function getTypeAnnotation(node, source) {
if (!hasName(node) || node.type === 'FunctionDeclaration') return;

if (node.type === 'ArrowFunctionExpression' || node.type === 'FunctionExpression') {
if (
node.type === 'ArrowFunctionExpression'
|| node.type === 'FunctionExpression'
) {
return getNodeText(node.parent.id.typeAnnotation, source);
}
}

function isUnfixableBecauseOfExport(node) {
return node.type === 'FunctionDeclaration' && node.parent && node.parent.type === 'ExportDefaultDeclaration';
return (
node.type === 'FunctionDeclaration'
&& node.parent
&& node.parent.type === 'ExportDefaultDeclaration'
);
}

function isFunctionExpressionWithName(node) {
Expand Down Expand Up @@ -116,12 +138,22 @@ module.exports = {
properties: {
namedComponents: {
oneOf: [
{ enum: ['function-declaration', 'arrow-function', 'function-expression'] },
{
enum: [
'function-declaration',
'arrow-function',
'function-expression',
],
},
{
type: 'array',
items: {
type: 'string',
enum: ['function-declaration', 'arrow-function', 'function-expression'],
enum: [
'function-declaration',
'arrow-function',
'function-expression',
],
},
},
],
Expand All @@ -145,9 +177,14 @@ module.exports = {

create: Components.detect((context, components) => {
const configuration = context.options[0] || {};
let fileVarType = 'var';

const namedConfig = [].concat(configuration.namedComponents || 'function-declaration');
const unnamedConfig = [].concat(configuration.unnamedComponents || 'function-expression');
const namedConfig = [].concat(
configuration.namedComponents || 'function-declaration'
);
const unnamedConfig = [].concat(
configuration.unnamedComponents || 'function-expression'
);

function getFixer(node, options) {
const sourceCode = context.getSourceCode();
Expand All @@ -156,18 +193,33 @@ module.exports = {
const typeAnnotation = getTypeAnnotation(node, source);

if (options.type === 'function-declaration' && typeAnnotation) return;
if (options.type === 'arrow-function' && hasOneUnconstrainedTypeParam(node)) return;
if (
options.type === 'arrow-function'
&& hasOneUnconstrainedTypeParam(node)
) return;
if (isUnfixableBecauseOfExport(node)) return;
if (isFunctionExpressionWithName(node)) return;
let varType = fileVarType;
if (
(node.type === 'FunctionExpression'
|| node.type === 'ArrowFunctionExpression')
&& node.parent.type === 'VariableDeclarator'
) {
varType = node.parent.parent.kind;
}

return (fixer) => fixer.replaceTextRange(options.range, buildFunction(options.template, {
typeAnnotation,
typeParams: getNodeText(node.typeParameters, source),
params: getParams(node, source),
returnType: getNodeText(node.returnType, source),
body: getBody(node, source),
name: getName(node),
}));
return (fixer) => fixer.replaceTextRange(
options.range,
buildFunction(options.template, {
typeAnnotation,
typeParams: getNodeText(node.typeParameters, source),
params: getParams(node, source),
returnType: getNodeText(node.returnType, source),
body: getBody(node, source),
name: getName(node),
varType,
})
);
}

function report(node, options) {
Expand All @@ -188,9 +240,10 @@ module.exports = {
fixerOptions: {
type: namedConfig[0],
template: NAMED_FUNCTION_TEMPLATES[namedConfig[0]],
range: node.type === 'FunctionDeclaration'
? node.range
: node.parent.parent.range,
range:
node.type === 'FunctionDeclaration'
? node.range
: node.parent.parent.range,
},
});
}
Expand All @@ -209,11 +262,28 @@ module.exports = {
// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

const validatePairs = [];
let hasES6OrJsx = false;
return {
FunctionDeclaration(node) { validate(node, 'function-declaration'); },
ArrowFunctionExpression(node) { validate(node, 'arrow-function'); },
FunctionExpression(node) { validate(node, 'function-expression'); },
FunctionDeclaration(node) {
validatePairs.push([node, 'function-declaration']);
},
ArrowFunctionExpression(node) {
validatePairs.push([node, 'arrow-function']);
},
FunctionExpression(node) {
validatePairs.push([node, 'function-expression']);
},
VariableDeclaration(node) {
hasES6OrJsx = hasES6OrJsx || node.kind === 'const' || node.kind === 'let';
},
'Program:exit'() {
if (hasES6OrJsx) fileVarType = 'const';
validatePairs.forEach((pair) => validate(pair[0], pair[1]));
},
'ImportDeclaration, ExportNamedDeclaration, ExportDefaultDeclaration, ExportAllDeclaration, ExportSpecifier, ExportDefaultSpecifier, JSXElement, TSExportAssignment, TSImportEqualsDeclaration'() {
hasES6OrJsx = true;
},
};
}),
};
Loading

0 comments on commit 5069def

Please sign in to comment.