From 4b76fe8b0561baad7c498d2c81f214c07faafe4e Mon Sep 17 00:00:00 2001 From: Ishpaul Singh Date: Sat, 23 Dec 2023 17:09:11 +0530 Subject: [PATCH 1/2] adds the lint rule --- eslint-plugin-expensify/CONST.js | 1 + eslint-plugin-expensify/no-default-props.js | 58 +++++++ .../tests/no-default-props.test.js | 153 ++++++++++++++++++ 3 files changed, 212 insertions(+) create mode 100644 eslint-plugin-expensify/no-default-props.js create mode 100644 eslint-plugin-expensify/tests/no-default-props.test.js diff --git a/eslint-plugin-expensify/CONST.js b/eslint-plugin-expensify/CONST.js index 456be1f..abfc6dc 100644 --- a/eslint-plugin-expensify/CONST.js +++ b/eslint-plugin-expensify/CONST.js @@ -26,5 +26,6 @@ module.exports = { HAVE_DEFAULT_PROPS: 'Component must have default prop values.', ONYX_ONE_PARAM: 'The withOnyx HOC must be passed at least one argument.', MUST_USE_VARIABLE_FOR_ASSIGNMENT: '{{key}} must be assigned as a variable instead of direct assignment.', + NO_DEFAULT_PROPS: 'defaultProps should not be used in function components. Use default Arguments instead.', }, }; diff --git a/eslint-plugin-expensify/no-default-props.js b/eslint-plugin-expensify/no-default-props.js new file mode 100644 index 0000000..4d6fe75 --- /dev/null +++ b/eslint-plugin-expensify/no-default-props.js @@ -0,0 +1,58 @@ +const _ = require('underscore'); +const lodashGet = require('lodash/get'); +const message = require('./CONST').MESSAGE.NO_DEFAULT_PROPS; +const {isReactViewFile} = require('./utils'); + +module.exports = { + create: context => ({ + AssignmentExpression(node) { + // Only looking at react files + if (!isReactViewFile(context.getFilename())) { + return; + } + + // if the name of assignment is not defaultProps, we're good + if (lodashGet(node, 'left.property.name') !== 'defaultProps') { + return; + } + + // Find all the function in the parent node that returns a jsx element + const parent = lodashGet(node, 'parent.type') === 'FunctionDeclaration' ? node.parent : lodashGet(node, 'parent.parent'); + const functionComponents = _.filter(parent.body, (n) => { + if (['FunctionDeclaration', 'ExportNamedDeclaration'].indexOf(n.type) === -1) { + return false; + } + const body = n.type === 'ExportNamedDeclaration' ? lodashGet(n, 'declaration.body.body') : lodashGet(n, 'body.body'); + const isReturningJSX = _.filter(body, bodyNode => bodyNode.type === 'ReturnStatement' && lodashGet(bodyNode, 'argument.type') === 'JSXElement'); + if (_.isEmpty(isReturningJSX)) { + return false; + } + return true; + }); + + // If we don't have any function components, we're good + if (_.isEmpty(functionComponents)) { + return; + } + + // Find all the function component names + const functionComponentNames = _.map(functionComponents, (functionComponent) => { + if (functionComponent.type === 'FunctionDeclaration') { + return functionComponent.id.name; + } + return lodashGet(functionComponent, 'declaration.id.name'); + }); + + // check if the function component names includes the name of the object + if (!_.includes(functionComponentNames, lodashGet(node, 'left.object.name'))) { + return; + } + + // report the error + context.report({ + node, + message, + }); + }, + }), +}; diff --git a/eslint-plugin-expensify/tests/no-default-props.test.js b/eslint-plugin-expensify/tests/no-default-props.test.js new file mode 100644 index 0000000..14bcaea --- /dev/null +++ b/eslint-plugin-expensify/tests/no-default-props.test.js @@ -0,0 +1,153 @@ +const RuleTester = require('eslint').RuleTester; +const rule = require('../no-default-props'); +const message = require('../CONST').MESSAGE.NO_DEFAULT_PROPS; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + // To support use of < in HOC + jsx: true, + + // To support use of ... operator + experimentalObjectRestSpread: true, + }, + }, +}); + +ruleTester.run('no-default-props', rule, { + valid: [ + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{propWithDefault}
; + }`, + filename: '/src/components/Test.js', + }, + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{propWithDefault}
; + } + Test.displayName = 'Test';`, + filename: '/src/components/Test.js', + }, + { + // VALID AS TECHNICALLY THIS IS NOT A FUNCTION DECLARATION + code: ` + const Test = React.forwardRef((props, ref) => { + return
{props.propWithDefault}
; + }); + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + // VALID AS TECHNICALLY THIS IS NOT A FUNCTION DECLARATION + code: ` + const Test = React.memo((props) => { + return
{props.propWithDefault}
; + }); + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + code: ` + class Test extends React.Component { + constructor(props) { + super(props); + } + render() { + return
{this.props.propWithDefault}
; + } + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + code: `function Test(props) { + return
{props.propWithDefault}
; + } + Test.displayName = 'Test'; + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/libs/Test.js', + }, + ], + invalid: [ + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/components/Test.js', + errors: [{message}], + }, + { + code: ` + function Test(props) { + return
{props.propWithDefault}
; + } + Test.displayName = 'Test'; + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: `function Test(props, ref) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default React.forwardRef(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: `function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default React.memo(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: ` + function HOC(Component) { + function WrappedComponent({propWithDefault, ...props}) { + return ; + } + WrappedComponent.defaultProps = { propWithDefault: 'defaultValue' }; + return WrappedComponent; + }`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: ` + function HOC(Component) { + function WrappedComponent({propWithDefault, ...props}) { + return ; + } + WrappedComponent.defaultProps = { propWithDefault: 'defaultValue' }; + return WrappedComponent; + } + function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default HOC(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}, {message}], + }, + { + code: ` + export function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + ], +}); From b516d74c30c36eee5d03997155c16aa53b041a36 Mon Sep 17 00:00:00 2001 From: Ishpaul Singh Date: Sat, 23 Dec 2023 17:13:37 +0530 Subject: [PATCH 2/2] adds the lint rule --- eslint-plugin-expensify/CONST.js | 1 + eslint-plugin-expensify/no-default-props.js | 58 +++++++ .../tests/no-default-props.test.js | 153 ++++++++++++++++++ 3 files changed, 212 insertions(+) create mode 100644 eslint-plugin-expensify/no-default-props.js create mode 100644 eslint-plugin-expensify/tests/no-default-props.test.js diff --git a/eslint-plugin-expensify/CONST.js b/eslint-plugin-expensify/CONST.js index 456be1f..abfc6dc 100644 --- a/eslint-plugin-expensify/CONST.js +++ b/eslint-plugin-expensify/CONST.js @@ -26,5 +26,6 @@ module.exports = { HAVE_DEFAULT_PROPS: 'Component must have default prop values.', ONYX_ONE_PARAM: 'The withOnyx HOC must be passed at least one argument.', MUST_USE_VARIABLE_FOR_ASSIGNMENT: '{{key}} must be assigned as a variable instead of direct assignment.', + NO_DEFAULT_PROPS: 'defaultProps should not be used in function components. Use default Arguments instead.', }, }; diff --git a/eslint-plugin-expensify/no-default-props.js b/eslint-plugin-expensify/no-default-props.js new file mode 100644 index 0000000..4d6fe75 --- /dev/null +++ b/eslint-plugin-expensify/no-default-props.js @@ -0,0 +1,58 @@ +const _ = require('underscore'); +const lodashGet = require('lodash/get'); +const message = require('./CONST').MESSAGE.NO_DEFAULT_PROPS; +const {isReactViewFile} = require('./utils'); + +module.exports = { + create: context => ({ + AssignmentExpression(node) { + // Only looking at react files + if (!isReactViewFile(context.getFilename())) { + return; + } + + // if the name of assignment is not defaultProps, we're good + if (lodashGet(node, 'left.property.name') !== 'defaultProps') { + return; + } + + // Find all the function in the parent node that returns a jsx element + const parent = lodashGet(node, 'parent.type') === 'FunctionDeclaration' ? node.parent : lodashGet(node, 'parent.parent'); + const functionComponents = _.filter(parent.body, (n) => { + if (['FunctionDeclaration', 'ExportNamedDeclaration'].indexOf(n.type) === -1) { + return false; + } + const body = n.type === 'ExportNamedDeclaration' ? lodashGet(n, 'declaration.body.body') : lodashGet(n, 'body.body'); + const isReturningJSX = _.filter(body, bodyNode => bodyNode.type === 'ReturnStatement' && lodashGet(bodyNode, 'argument.type') === 'JSXElement'); + if (_.isEmpty(isReturningJSX)) { + return false; + } + return true; + }); + + // If we don't have any function components, we're good + if (_.isEmpty(functionComponents)) { + return; + } + + // Find all the function component names + const functionComponentNames = _.map(functionComponents, (functionComponent) => { + if (functionComponent.type === 'FunctionDeclaration') { + return functionComponent.id.name; + } + return lodashGet(functionComponent, 'declaration.id.name'); + }); + + // check if the function component names includes the name of the object + if (!_.includes(functionComponentNames, lodashGet(node, 'left.object.name'))) { + return; + } + + // report the error + context.report({ + node, + message, + }); + }, + }), +}; diff --git a/eslint-plugin-expensify/tests/no-default-props.test.js b/eslint-plugin-expensify/tests/no-default-props.test.js new file mode 100644 index 0000000..14bcaea --- /dev/null +++ b/eslint-plugin-expensify/tests/no-default-props.test.js @@ -0,0 +1,153 @@ +const RuleTester = require('eslint').RuleTester; +const rule = require('../no-default-props'); +const message = require('../CONST').MESSAGE.NO_DEFAULT_PROPS; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + // To support use of < in HOC + jsx: true, + + // To support use of ... operator + experimentalObjectRestSpread: true, + }, + }, +}); + +ruleTester.run('no-default-props', rule, { + valid: [ + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{propWithDefault}
; + }`, + filename: '/src/components/Test.js', + }, + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{propWithDefault}
; + } + Test.displayName = 'Test';`, + filename: '/src/components/Test.js', + }, + { + // VALID AS TECHNICALLY THIS IS NOT A FUNCTION DECLARATION + code: ` + const Test = React.forwardRef((props, ref) => { + return
{props.propWithDefault}
; + }); + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + // VALID AS TECHNICALLY THIS IS NOT A FUNCTION DECLARATION + code: ` + const Test = React.memo((props) => { + return
{props.propWithDefault}
; + }); + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + code: ` + class Test extends React.Component { + constructor(props) { + super(props); + } + render() { + return
{this.props.propWithDefault}
; + } + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + }, + { + code: `function Test(props) { + return
{props.propWithDefault}
; + } + Test.displayName = 'Test'; + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/libs/Test.js', + }, + ], + invalid: [ + { + code: ` + function Test({ propWithDefault = 'defaultValue' }) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/components/Test.js', + errors: [{message}], + }, + { + code: ` + function Test(props) { + return
{props.propWithDefault}
; + } + Test.displayName = 'Test'; + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: `function Test(props, ref) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default React.forwardRef(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: `function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default React.memo(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: ` + function HOC(Component) { + function WrappedComponent({propWithDefault, ...props}) { + return ; + } + WrappedComponent.defaultProps = { propWithDefault: 'defaultValue' }; + return WrappedComponent; + }`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + { + code: ` + function HOC(Component) { + function WrappedComponent({propWithDefault, ...props}) { + return ; + } + WrappedComponent.defaultProps = { propWithDefault: 'defaultValue' }; + return WrappedComponent; + } + function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' }; + export default HOC(Test);`, + filename: '/src/pages/Test.js', + errors: [{message}, {message}], + }, + { + code: ` + export function Test(props) { + return
{props.propWithDefault}
; + } + Test.defaultProps = { propWithDefault: 'defaultValue' };`, + filename: '/src/pages/Test.js', + errors: [{message}], + }, + ], +});