From 0804d20a3669265f7ce976a820abe1e6dfc4bd01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 22 Nov 2024 21:06:22 +0000 Subject: [PATCH 1/4] Initial implementation of no-default-values rule --- eslint-plugin-expensify/no-default-values.js | 86 ++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 eslint-plugin-expensify/no-default-values.js diff --git a/eslint-plugin-expensify/no-default-values.js b/eslint-plugin-expensify/no-default-values.js new file mode 100644 index 0000000..b76ead3 --- /dev/null +++ b/eslint-plugin-expensify/no-default-values.js @@ -0,0 +1,86 @@ +module.exports = { + name: 'no-default-values', + meta: { + type: 'problem', + docs: { + description: 'Enforce boolean conditions in React conditional rendering', + recommended: 'error', + }, + schema: [], + messages: { + // eslint-disable-next-line max-len + disallowedNumberDefault: 'Default the number ID to `CONST.DEFAULT_NUMBER_ID` instead. See: https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-IDs', + // eslint-disable-next-line max-len + disallowedStringDefault: 'Do not default string IDs to any value. See: https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-IDs', + }, + }, + create(context) { + function createPatternRegex(pattern) { + return new RegExp(pattern.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'), 'g'); + } + + const sourceCode = context.getSourceCode(); + const text = sourceCode.text; // This gets all the text in the file + + const disallowedNumberDefaults = [ + ' ?? -1', + ' || -1', + ' : -1', + ]; + + const disallowedStringDefaults = [ + " ?? '-1'", + "ID ?? ''", + "id ?? ''", + "ID ?? '0'", + "id ?? '0'", + " || '-1'", + "ID || ''", + "id || ''", + "ID || '0'", + "id || '0'", + " : '-1'", + " : '0'", + ]; + + disallowedNumberDefaults.forEach((pattern) => { + const regex = createPatternRegex(pattern); + let match; + while ((match = regex.exec(text)) !== null) { + const index = match.index; + + const defaultStr = match[0]; + const defaultStrPosition = sourceCode.getLocFromIndex(index); + + context.report({ + messageId: 'disallowedNumberDefault', + loc: { + start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, + end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, + }, + }); + } + }); + + disallowedStringDefaults.forEach((pattern) => { + const regex = createPatternRegex(pattern); + let match; + while ((match = regex.exec(text)) !== null) { + const index = match.index; + + const defaultStr = match[0]; + const defaultStrPosition = sourceCode.getLocFromIndex(index); + + context.report({ + messageId: 'disallowedStringDefault', + loc: { + start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, + end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, + }, + }); + } + }); + + return {}; + }, +}; From 98b7777de6ecabf690878d22760becac2c94d898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 28 Nov 2024 08:31:00 +0000 Subject: [PATCH 2/4] Refactor code and add tests --- eslint-plugin-expensify/no-default-values.js | 74 ++--- .../tests/no-default-values.test.js | 260 ++++++++++++++++++ 2 files changed, 297 insertions(+), 37 deletions(-) create mode 100644 eslint-plugin-expensify/tests/no-default-values.test.js diff --git a/eslint-plugin-expensify/no-default-values.js b/eslint-plugin-expensify/no-default-values.js index b76ead3..9d1429e 100644 --- a/eslint-plugin-expensify/no-default-values.js +++ b/eslint-plugin-expensify/no-default-values.js @@ -3,7 +3,7 @@ module.exports = { meta: { type: 'problem', docs: { - description: 'Enforce boolean conditions in React conditional rendering', + description: 'Restricts use of default number/string IDs in the project.', recommended: 'error', }, schema: [], @@ -19,13 +19,43 @@ module.exports = { return new RegExp(pattern.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'), 'g'); } + function searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, messageId) { + const regex = createPatternRegex(pattern); + let match = regex.exec(soureCodeStr); + while (match !== null) { + const index = match.index; + + const defaultStr = match[0]; + const defaultStrPosition = sourceCode.getLocFromIndex(index); + + context.report({ + messageId, + loc: { + start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, + end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, + }, + }); + + match = regex.exec(soureCodeStr); + } + } + const sourceCode = context.getSourceCode(); - const text = sourceCode.text; // This gets all the text in the file + const soureCodeStr = sourceCode.text; // This gets all the text in the file const disallowedNumberDefaults = [ - ' ?? -1', - ' || -1', - ' : -1', + 'ID ?? -1', + 'id ?? -1', + 'ID ?? 0', + 'id ?? 0', + 'ID || -1', + 'id || -1', + 'ID || 0', + 'id || 0', + 'ID : -1', + 'id : -1', + 'ID : 0', + 'id : 0', ]; const disallowedStringDefaults = [ @@ -44,41 +74,11 @@ module.exports = { ]; disallowedNumberDefaults.forEach((pattern) => { - const regex = createPatternRegex(pattern); - let match; - while ((match = regex.exec(text)) !== null) { - const index = match.index; - - const defaultStr = match[0]; - const defaultStrPosition = sourceCode.getLocFromIndex(index); - - context.report({ - messageId: 'disallowedNumberDefault', - loc: { - start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, - end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, - }, - }); - } + searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, 'disallowedNumberDefault'); }); disallowedStringDefaults.forEach((pattern) => { - const regex = createPatternRegex(pattern); - let match; - while ((match = regex.exec(text)) !== null) { - const index = match.index; - - const defaultStr = match[0]; - const defaultStrPosition = sourceCode.getLocFromIndex(index); - - context.report({ - messageId: 'disallowedStringDefault', - loc: { - start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, - end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, - }, - }); - } + searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, 'disallowedStringDefault'); }); return {}; diff --git a/eslint-plugin-expensify/tests/no-default-values.test.js b/eslint-plugin-expensify/tests/no-default-values.test.js new file mode 100644 index 0000000..1729125 --- /dev/null +++ b/eslint-plugin-expensify/tests/no-default-values.test.js @@ -0,0 +1,260 @@ +const RuleTester = require('eslint').RuleTester; +const rule = require('../no-default-values'); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + }, +}); + +ruleTester.run('no-default-values', rule, { + valid: [ + // Number IDs + { + code: 'const accountID = report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'const accountID = account?.id ?? CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'currentState = currentState?.routes[currentState.index ?? -1].state;', + }, + { + code: 'currentState = currentState?.routes[currentState.index ?? 0].state;', + }, + { + code: 'const accountID = report?.ownerAccountID || CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'const accountID = account?.id || CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'currentState = currentState?.routes[currentState.index || -1].state;', + }, + { + code: 'currentState = currentState?.routes[currentState.index || 0].state;', + }, + { + code: 'const managerID = report ? report.managerID : CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'const accountID = account ? account.id : CONST.DEFAULT_NUMBER_ID;', + }, + { + code: 'options.sort((method) => (method.value === exportMethod ? -1 : 0));', + }, + + // String IDs + { + code: 'const reportID = report?.reportID;', + }, + { + code: 'const iconName = icon.name ?? \'\'', + }, + { + code: 'const index = tempIndex ?? \'0\'', + }, + { + code: 'const iconName = icon.name || \'\'', + }, + { + code: 'const index = tempIndex || \'0\'', + }, + ], + invalid: [ + // Number IDs + { + code: 'const accountID = report?.ownerAccountID ?? -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const reportID = report?.id ?? -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const accountID = report?.ownerAccountID ?? 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const reportID = report?.id ?? 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const accountID = report?.ownerAccountID || -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const reportID = report?.id || -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const accountID = report?.ownerAccountID || 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const reportID = report?.id || 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const managerID = report ? report.managerID : -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const accountID = account ? account.id : -1;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const managerID = report ? report.managerID : 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + { + code: 'const accountID = account ? account.id : 0;', + errors: [{ + messageId: 'disallowedNumberDefault', + }], + }, + + // String IDs + { + code: 'const reportID = report?.reportID ?? \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const currentReportID = Navigation.getTopmostReportId() ?? \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const currentReportID = navigationRef?.isReady?.() ? Navigation.getTopmostReportId() ?? \'-1\' : \'-1\';', + errors: [ + { + messageId: 'disallowedStringDefault', + }, + { + messageId: 'disallowedStringDefault', + }, + ], + }, + { + code: 'const reportID = report?.reportID ?? \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const policyID = policy?.id ?? \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report?.reportID ?? \'\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const policyID = policy?.id ?? \'\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report?.reportID ?? \'0\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const policyID = policy?.id ?? \'0\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report?.reportID || \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const currentReportID = Navigation.getTopmostReportId() || \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const currentReportID = navigationRef?.isReady?.() ? Navigation.getTopmostReportId() || \'-1\' : \'-1\';', + errors: [ + { + messageId: 'disallowedStringDefault', + }, + { + messageId: 'disallowedStringDefault', + }, + ], + }, + { + code: 'const reportID = report?.reportID || \'\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const policyID = policy?.id || \'\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report?.reportID || \'0\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const policyID = policy?.id || \'0\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report ? report.reportID : \'-1\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + { + code: 'const reportID = report ? report.reportID : \'0\';', + errors: [{ + messageId: 'disallowedStringDefault', + }], + }, + ], +}); From b9aec0ac24814cd3efb7dcbcdcb3fb2de977f0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 28 Nov 2024 08:32:49 +0000 Subject: [PATCH 3/4] Rename ESLint rule --- .../{no-default-values.js => no-default-id-values.js} | 2 +- ...no-default-values.test.js => no-default-id-values.test.js} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename eslint-plugin-expensify/{no-default-values.js => no-default-id-values.js} (98%) rename eslint-plugin-expensify/tests/{no-default-values.test.js => no-default-id-values.test.js} (98%) diff --git a/eslint-plugin-expensify/no-default-values.js b/eslint-plugin-expensify/no-default-id-values.js similarity index 98% rename from eslint-plugin-expensify/no-default-values.js rename to eslint-plugin-expensify/no-default-id-values.js index 9d1429e..a215b52 100644 --- a/eslint-plugin-expensify/no-default-values.js +++ b/eslint-plugin-expensify/no-default-id-values.js @@ -1,5 +1,5 @@ module.exports = { - name: 'no-default-values', + name: 'no-default-id-values', meta: { type: 'problem', docs: { diff --git a/eslint-plugin-expensify/tests/no-default-values.test.js b/eslint-plugin-expensify/tests/no-default-id-values.test.js similarity index 98% rename from eslint-plugin-expensify/tests/no-default-values.test.js rename to eslint-plugin-expensify/tests/no-default-id-values.test.js index 1729125..e394199 100644 --- a/eslint-plugin-expensify/tests/no-default-values.test.js +++ b/eslint-plugin-expensify/tests/no-default-id-values.test.js @@ -1,5 +1,5 @@ const RuleTester = require('eslint').RuleTester; -const rule = require('../no-default-values'); +const rule = require('../no-default-id-values'); const ruleTester = new RuleTester({ parserOptions: { @@ -8,7 +8,7 @@ const ruleTester = new RuleTester({ }, }); -ruleTester.run('no-default-values', rule, { +ruleTester.run('no-default-id-values', rule, { valid: [ // Number IDs { From f784aff424f912738e1ad61d841ae8f1d87c24c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 28 Nov 2024 10:43:08 +0000 Subject: [PATCH 4/4] Move functions outside create() --- .../no-default-id-values.js | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/eslint-plugin-expensify/no-default-id-values.js b/eslint-plugin-expensify/no-default-id-values.js index a215b52..69272bd 100644 --- a/eslint-plugin-expensify/no-default-id-values.js +++ b/eslint-plugin-expensify/no-default-id-values.js @@ -1,3 +1,28 @@ +function createPatternRegex(pattern) { + return new RegExp(pattern.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'), 'g'); +} + +function searchForPatternsAndReport(context, sourceCode, soureCodeStr, pattern, messageId) { + const regex = createPatternRegex(pattern); + let match = regex.exec(soureCodeStr); + while (match !== null) { + const index = match.index; + + const defaultStr = match[0]; + const defaultStrPosition = sourceCode.getLocFromIndex(index); + + context.report({ + messageId, + loc: { + start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, + end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, + }, + }); + + match = regex.exec(soureCodeStr); + } +} + module.exports = { name: 'no-default-id-values', meta: { @@ -15,31 +40,6 @@ module.exports = { }, }, create(context) { - function createPatternRegex(pattern) { - return new RegExp(pattern.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'), 'g'); - } - - function searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, messageId) { - const regex = createPatternRegex(pattern); - let match = regex.exec(soureCodeStr); - while (match !== null) { - const index = match.index; - - const defaultStr = match[0]; - const defaultStrPosition = sourceCode.getLocFromIndex(index); - - context.report({ - messageId, - loc: { - start: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.indexOf(' ')}, - end: {line: defaultStrPosition.line, column: defaultStrPosition.column + defaultStr.length}, - }, - }); - - match = regex.exec(soureCodeStr); - } - } - const sourceCode = context.getSourceCode(); const soureCodeStr = sourceCode.text; // This gets all the text in the file @@ -74,11 +74,11 @@ module.exports = { ]; disallowedNumberDefaults.forEach((pattern) => { - searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, 'disallowedNumberDefault'); + searchForPatternsAndReport(context, sourceCode, soureCodeStr, pattern, 'disallowedNumberDefault'); }); disallowedStringDefaults.forEach((pattern) => { - searchForPatternsAndReport(sourceCode, soureCodeStr, pattern, 'disallowedStringDefault'); + searchForPatternsAndReport(context, sourceCode, soureCodeStr, pattern, 'disallowedStringDefault'); }); return {};