From 5069def4d464f2bb64df4ffe2ac4f702ed594e52 Mon Sep 17 00:00:00 2001 From: Xavier Le Cunff <31244713+JohnBerd@users.noreply.github.com> Date: Sun, 20 Mar 2022 16:15:51 +0100 Subject: [PATCH] [Fix] `function-component-definition`: replace `var` by `const` in certain situations Signed-off-by: Xavier Le Cunff Co-authored-by: Xavier Le Cunff Co-authored-by: Simeon Cheeseman --- docs/rules/function-component-definition.md | 30 ++-- lib/rules/function-component-definition.js | 142 +++++++++++++----- .../rules/function-component-definition.js | 141 ++++++++++++++++- 3 files changed, 254 insertions(+), 59 deletions(-) diff --git a/docs/rules/function-component-definition.md b/docs/rules/function-component-definition.md index b869373d20..9c798d17e4 100644 --- a/docs/rules/function-component-definition.md +++ b/docs/rules/function-component-definition.md @@ -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
{props.content}
; }; // arrow function for named component -var Component = (props) => { +const Component = (props) => { return
{props.content}
; }; @@ -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
; }; -var Component = (props) => { +const Component = (props) => { return
; }; @@ -63,7 +63,7 @@ function Component (props) { return
; }; -var Component = (props) => { +const Component = (props) => { return
; }; @@ -73,7 +73,7 @@ function Component (props) { return
; }; -var Component = function (props) { +const Component = function (props) { return
; }; @@ -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
; }; // only arrow functions for named components // [2, { "namedComponents": "arrow-function" }] -var Component = (props) => { +const Component = (props) => { return
; }; @@ -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 = function (props) { +const Component: React.FC = function (props) { return
; }; -var Component: React.FC = (props) => { +const Component: React.FC = (props) => { return
; }; @@ -184,7 +184,7 @@ function Component(props: Props) { return
; }; -var Component = function (props: Props) { +const Component = function (props: Props) { return
; }; @@ -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) => { +const Component: React.FC = (props) => { return
; }; // autofix to arrow function with type annotation // [2, { "namedComponents": "function-expression" }] -var Component: React.FC = function (props) { +const Component: React.FC = function (props) { return
; }; @@ -219,7 +219,7 @@ function Component(props: Props) { return
; } -var Component = function (props: Props) { +const Component = function (props: Props) { return
; }; @@ -229,7 +229,7 @@ function Component(props: Props) { return
; } -var Component = function (props: Props) { +const Component = function (props: Props) { return
; }; diff --git a/lib/rules/function-component-definition.js b/lib/rules/function-component-definition.js index 8f20e63366..ade4b5b9c2 100644 --- a/lib/rules/function-component-definition.js +++ b/lib/rules/function-component-definition.js @@ -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 = { @@ -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) { @@ -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]); @@ -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) { @@ -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', + ], }, }, ], @@ -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(); @@ -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) { @@ -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, }, }); } @@ -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; + }, }; }), }; diff --git a/tests/lib/rules/function-component-definition.js b/tests/lib/rules/function-component-definition.js index dbeb1b65b5..5040d03f5a 100644 --- a/tests/lib/rules/function-component-definition.js +++ b/tests/lib/rules/function-component-definition.js @@ -57,6 +57,10 @@ ruleTester.run('function-component-definition', rule, { code: 'var Hello = (props) => { return
}', options: [{ namedComponents: 'arrow-function' }], }, + { + code: 'const Hello = (props) => { return
}', + options: [{ namedComponents: 'arrow-function' }], + }, { code: 'function Hello(props) { return
}', options: [{ namedComponents: 'function-declaration' }], @@ -65,6 +69,10 @@ ruleTester.run('function-component-definition', rule, { code: 'var Hello = function(props) { return
}', options: [{ namedComponents: 'function-expression' }], }, + { + code: 'const Hello = function(props) { return
}', + options: [{ namedComponents: 'function-expression' }], + }, { code: 'function Hello() { return function() { return
} }', options: [{ unnamedComponents: 'function-expression' }], @@ -77,6 +85,10 @@ ruleTester.run('function-component-definition', rule, { code: 'var Foo = React.memo(function Foo() { return

})', options: [{ namedComponents: 'function-declaration' }], }, + { + code: 'const Foo = React.memo(function Foo() { return

})', + options: [{ namedComponents: 'function-declaration' }], + }, { // shouldn't trigger this rule since functions stating with a lowercase // letter are not considered components @@ -404,7 +416,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = (props) => { + const Hello = (props) => { return

; } `, @@ -467,6 +479,56 @@ ruleTester.run('function-component-definition', rule, { options: [{ namedComponents: 'function-expression' }], errors: [{ messageId: 'function-expression' }], }, + { + code: ` + let Hello = (props) => { + return
; + } + `, + output: ` + let Hello = function(props) { + return
; + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + }, + { + code: ` + let Hello; + Hello = (props) => { + return
; + } + `, + output: ` + let Hello; + Hello = function(props) { + return
; + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + }, + { + code: ` + let Hello = (props) => { + return
; + } + Hello = function(props) { + return ; + } + `, + output: ` + let Hello = function(props) { + return
; + } + Hello = function(props) { + return ; + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + }, { code: ` function Hello(props) { @@ -474,7 +536,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = function(props) { + const Hello = function(props) { return
; } `, @@ -554,7 +616,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = (props: Test) => { + const Hello = (props: Test) => { return
; } `, @@ -584,7 +646,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = function(props: Test) { + const Hello = function(props: Test) { return
; } `, @@ -592,6 +654,69 @@ ruleTester.run('function-component-definition', rule, { errors: [{ messageId: 'function-expression' }], features: ['types'], }, + { + code: ` + function Hello(props: Test) { + return React.createElement('div'); + } + `, + output: ` + var Hello = function(props: Test) { + return React.createElement('div'); + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + features: ['types'], + }, + { + code: ` + import * as React from 'react'; + function Hello(props: Test) { + return React.createElement('div'); + } + `, + output: ` + import * as React from 'react'; + const Hello = function(props: Test) { + return React.createElement('div'); + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + features: ['types'], + }, + { + code: ` + export function Hello(props: Test) { + return React.createElement('div'); + } + `, + output: ` + export const Hello = function(props: Test) { + return React.createElement('div'); + } + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + features: ['types'], + }, + { + code: ` + function Hello(props) { + return React.createElement('div'); + } + export default Hello; + `, + output: ` + const Hello = function(props) { + return React.createElement('div'); + } + export default Hello; + `, + options: [{ namedComponents: 'function-expression' }], + errors: [{ messageId: 'function-expression' }], + }, { code: ` var Hello = (props: Test) => { @@ -674,7 +799,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = (props: Test) => { + const Hello = (props: Test) => { return
; } `, @@ -704,7 +829,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = function(props: Test) { + const Hello = function(props: Test) { return
; } `, @@ -874,7 +999,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - export var Hello = (props) => { + export const Hello = (props) => { return
; } `, @@ -934,7 +1059,7 @@ ruleTester.run('function-component-definition', rule, { } `, output: ` - var Hello = (props) => { + const Hello = (props) => { return
; } `,