Add custom config to use double negation instead of Boolean #96

ShridharGoel · 2024-04-28T20:26:59Z

ishpaul777 · 2024-05-01T21:05:10Z

eslint-plugin-expensify/use-double-negation-instead-of-boolean.js

+                        fix(fixer) {
+                            const sourceCode = context.getSourceCode();
+                            const argumentText = sourceCode.getText(node.arguments[0]);
+                            return fixer.replaceText(node, `!!${argumentText}`);
+                        },


this will fail in this case Boolean(random1 || random2)

eslint-plugin-expensify/use-double-negation-instead-of-boolean.js

ishpaul777 · 2024-05-01T21:06:18Z

rules/expensify.js

@@ -15,6 +15,8 @@ module.exports = {
        'rulesdir/no-call-actions-from-actions': 'error',
        'rulesdir/no-api-side-effects-method': 'error',
        'rulesdir/prefer-localization': 'error',
+        'rulesdir/onyx-props-must-have-default': 'error',


ShridharGoel · 2024-05-02T12:23:57Z

@ishpaul777 Updated.

ishpaul777 · 2024-05-02T20:48:31Z

eslint-plugin-expensify/tests/prefer-double-negation.test.js

+const goodExample2 = '!!(test1 || test2)';
+
+const badExample1 = 'Boolean(test)';
+const badExample2 = 'Boolean(test1 || test2)';


Can you please add more test cases to cover more cases like &&, test1 && test2 || test3, test ? '' : "random" etc.

bump @ShridharGoel!

Will update this soon.

ishpaul777 · 2024-05-08T14:57:30Z

eslint-plugin-expensify/tests/prefer-double-negation.test.js

+ruleTester.run('use-!!-instead-of-Boolean()', rule, {
+    valid: [
+        {
+            code: goodExample1,
+        },
+        {
+            code: goodExample2,
+        },
+        {
+            code: goodExample3,
+        },
+        {
+            code: goodExample4,
+        },
+        {
+            code: goodExample5,
+        },
+        {
+            code: goodExample6,
+        },
+    ],


NAB, but can you please use the code here directly like we use in other test files, for better readability

This is last one, rest LGTM!

aldo-expensify · 2024-05-13T18:10:13Z

@ShridharGoel seems like you have some misconfiguration in your dev env and only some of the commits are signed:

I can't merge like this

Signing commits is mentioned as a requirement here: https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#begin-coding-your-solution-in-a-pull-request

aldo-expensify · 2024-05-13T18:12:13Z

eslint-plugin-expensify/tests/prefer-double-negation.test.js

+    },
+});
+
+ruleTester.run('use-!!-instead-of-Boolean()', rule, {


Suggested change

ruleTester.run('use-!!-instead-of-Boolean()', rule, {

ruleTester.run('use-double-negation-instead-of-boolean()', rule, {

Maybe name it the same as the rule so it is easy to find by the name of the rule? same comment for the file name to use use-double-negation-instead-of-boolean.test.js instead of prefer-double-negation.test..js

Shouldn't it be Boolean() with a capital B?

aldo-expensify · 2024-05-13T18:12:36Z

eslint-plugin-expensify/prefer-double-negation.js

@@ -0,0 +1,31 @@
+const message = require('./CONST').MESSAGE.USE_DOUBLE_NEGATION_INSTEAD_OF_BOOLEAN;


Should we name this file use-double-negation-instead-of-boolean?

That was the initial name but was suggested to update: #96 (comment)

Will change it again.

aldo-expensify

Need to fix commits, maybe the easiest way is to just create a new branch were you commit again

Left some minor comments

ShridharGoel · 2024-05-13T20:33:27Z

Will get back to this tomorrow.

fabioh8010 · 2024-05-15T09:25:20Z

@ShridharGoel kind bump 🙂

ishpaul777 · 2024-05-16T19:15:33Z

it seems to have a failing test on main (unrelated to PR)

ishpaul777 · 2024-05-28T20:00:54Z

hello 👋 @aldo-expensify, is the merge freeze ongoing, when can we expect this PR to be merged ?

aldo-expensify · 2024-05-29T02:56:46Z

@ishpaul777 I understand that the merge freeze has been lifted for the App and eslint-config-expensify repos.

@ShridharGoel can you resolve conflicts 🙏

ShridharGoel · 2024-05-29T06:20:59Z

@aldo-expensify Done.

aldo-expensify · 2024-05-29T17:21:43Z

Thank you @ShridharGoel

github-actions · 2024-05-29T17:22:55Z

🚀 Published in 2.0.50

ShridharGoel mentioned this pull request Apr 28, 2024

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] [TS Migration] Standartize between Boolean or !! Expensify/App#39126

Closed

ishpaul777 reviewed May 1, 2024

View reviewed changes

eslint-plugin-expensify/use-double-negation-instead-of-boolean.js Show resolved Hide resolved

ishpaul777 reviewed May 1, 2024

View reviewed changes

ishpaul777 reviewed May 2, 2024

View reviewed changes

ShridharGoel requested a review from ishpaul777 May 8, 2024 13:56

ishpaul777 reviewed May 8, 2024

View reviewed changes

ShridharGoel requested a review from ishpaul777 May 9, 2024 14:53

ishpaul777 approved these changes May 9, 2024

View reviewed changes

aldo-expensify approved these changes May 13, 2024

View reviewed changes

aldo-expensify reviewed May 13, 2024

View reviewed changes

aldo-expensify requested changes May 13, 2024

View reviewed changes

ShridharGoel force-pushed the boolean-update branch 7 times, most recently from 0f258cb to e6e0c69 Compare May 16, 2024 18:50

Add custom config to use double negation instead of Boolean()

1515072

ShridharGoel force-pushed the boolean-update branch from e6e0c69 to 1515072 Compare May 16, 2024 18:52

Merge branch 'main' into boolean-update

2964450

ShridharGoel requested a review from aldo-expensify May 16, 2024 18:57

aldo-expensify previously approved these changes May 16, 2024

View reviewed changes

aldo-expensify changed the title ~~Add custom config to use double negation instead of Boolean~~ [HOLD merge freeze] Add custom config to use double negation instead of Boolean May 16, 2024

Merge branch 'main' into boolean-update

30a3dbf

ShridharGoel dismissed aldo-expensify’s stale review via 30a3dbf May 29, 2024 06:20

aldo-expensify approved these changes May 29, 2024

View reviewed changes

aldo-expensify changed the title ~~[HOLD merge freeze] Add custom config to use double negation instead of Boolean~~ Add custom config to use double negation instead of Boolean May 29, 2024

aldo-expensify merged commit 89be844 into Expensify:main May 29, 2024

roryabraham mentioned this pull request May 30, 2024

[No QA] Use double negation instead of Boolean() Expensify/App#42492

Merged

48 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom config to use double negation instead of Boolean #96

Add custom config to use double negation instead of Boolean #96

ShridharGoel commented Apr 28, 2024 •

edited

Loading

ishpaul777 May 1, 2024

ishpaul777 May 1, 2024

ShridharGoel commented May 2, 2024

ishpaul777 May 2, 2024

ishpaul777 May 6, 2024

ShridharGoel May 6, 2024

ishpaul777 May 8, 2024

ishpaul777 May 8, 2024

aldo-expensify commented May 13, 2024

aldo-expensify May 13, 2024

ShridharGoel May 16, 2024

aldo-expensify May 13, 2024

ShridharGoel May 13, 2024

aldo-expensify left a comment

ShridharGoel commented May 13, 2024

fabioh8010 commented May 15, 2024

ishpaul777 commented May 16, 2024 •

edited

Loading

ishpaul777 commented May 28, 2024 •

edited

Loading

aldo-expensify commented May 29, 2024 •

edited

Loading

ShridharGoel commented May 29, 2024

aldo-expensify commented May 29, 2024

github-actions bot commented May 29, 2024

	ruleTester.run('use-!!-instead-of-Boolean()', rule, {
	ruleTester.run('use-double-negation-instead-of-boolean()', rule, {

		@@ -0,0 +1,31 @@
		const message = require('./CONST').MESSAGE.USE_DOUBLE_NEGATION_INSTEAD_OF_BOOLEAN;

Add custom config to use double negation instead of Boolean #96

Add custom config to use double negation instead of Boolean #96

Conversation

ShridharGoel commented Apr 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShridharGoel commented May 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aldo-expensify commented May 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aldo-expensify left a comment

Choose a reason for hiding this comment

ShridharGoel commented May 13, 2024

fabioh8010 commented May 15, 2024

ishpaul777 commented May 16, 2024 • edited Loading

ishpaul777 commented May 28, 2024 • edited Loading

aldo-expensify commented May 29, 2024 • edited Loading

ShridharGoel commented May 29, 2024

aldo-expensify commented May 29, 2024

github-actions bot commented May 29, 2024

ShridharGoel commented Apr 28, 2024 •

edited

Loading

ishpaul777 commented May 16, 2024 •

edited

Loading

ishpaul777 commented May 28, 2024 •

edited

Loading

aldo-expensify commented May 29, 2024 •

edited

Loading