Add `better-boolean-variable-names` #2446

axetroy · 2024-09-08T08:09:29Z

Perhaps prefer-readable-boolean-variable-names is a better name?

axetroy · 2024-09-08T09:18:34Z

@fregante Found the issue in the codebase, can I fix it directly in this PR, or in separate PR?

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/expiring-todo-comments.js
Error:   181:9  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/explicit-length-check.js
Error:   170:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/filename-case.js
Error:   169:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/no-for-loop.js
Error:   340:10  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names
Error:   355:10  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/no-keyword-prefix.js
Error:   60:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names
Error:   71:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/no-process-exit.js
Error:   15:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names
Error:   24:6  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/prefer-default-parameters.js
Error:   100:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/prefer-spread.js
Error:   59:8  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/rules/prefer-ternary.js
Error:    40:8   error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names
Error:   137:10  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names
Error:   217:9   error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

/home/runner/work/eslint-plugin-unicorn/eslint-plugin-unicorn/test/utils/snapshot-rule-tester.mjs
Error:   144:9  error  Prefer readable Boolean variable names  unicorn/better-boolean-variable-names

✖ 15 problems (15 errors, 0 warnings)

Error: Process completed with exit code 1.

fregante · 2024-09-08T11:04:40Z

Note that I’m not a maintainer, I just help triage the issues and have lots of opinions. 😅 I’ll leave that to @fisker

axetroy · 2024-09-08T11:40:38Z

@fregante OK, can you give me some advice, should I check the function name?

eg.

function completed(): boolean {}

const isCompleted = completed();

After fix:

function isCompleted1(): boolean {}

const isCompleted = isCompleted1();

Applying this rule to functions seems to cause naming problems

fregante · 2024-09-08T14:49:53Z

Heh, good point.

What should be called isCompleted? The function or the constant? Both make sense.

In your (corner) case it’s only an issue because you’re “caching” the value. Generally Sindre has preferred appending an underscore when the exact name is unavailable, like function_ and arguments_. So the constant would be isCompleted_ or e.g. isCompletedLocal. I don’t have an exact solution to this.

By the way I think it would hasCompleted or isComplete

fregante · 2024-09-08T14:51:17Z

docs/rules/better-boolean-variable-names.md

+<!-- end auto-generated rule header -->
+<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
+
+When it is clear that an expression is a Boolean value, the variable name should start with `is`/`was`/`has`/`can`/`should` to improve readability.


Also: will am are have were got does

fregante · 2024-09-08T14:55:37Z

docs/rules/better-boolean-variable-names.md

+
+```js
+const gotModifyRights = isGotPreviewRights() && isGotDownloadRights(); // ❌
+const isGotModifyRights = isGotPreviewRights() && isGotDownloadRights(); // ✅


Why? I think got is right.

const gotMilk = true

Here is the response from AI:

The word "got" is generally not suitable as a prefix for Boolean variables because it does not conform to the conventions for naming Boolean variables. Boolean variable names typically use verbs or adverbs to clearly express their Boolean value, such as "is," "has," "can," etc.

Using "got" as a prefix may result in variable names that are not sufficiently clear, thereby reducing the readability and maintainability of the code. Therefore, it is recommended to avoid using "got" as a prefix for Boolean variables.

Better choices include the following prefixes:

is: isUserLoggedIn

has: hasAdminPrivileges

can: canEditProfile

should: shouldDisplayNotification

will: willSendEmail

needs: needsPasswordReset

These prefixes can more clearly express the meaning of Boolean variables.

fregante · 2024-09-08T14:58:52Z

docs/rules/better-boolean-variable-names.md

+  'unicorn/prefer-readable-boolean-variable-names': [
+    'error',
+    {
+      prefixes: ['will', 'allows']


This gives me doubts about this rule. Having to configure your own verbs is a smell to me. allows should be valid by default… maybe? Or should it be doesAllowDeletion? What situations are we missing?

I don’t really want¹ a rule that bugs you for names that are fine and that might be difficult to get right.

Footnotes

meaning: if we add this rule to the recommended and then we get complaints, adding it would be a net loss. ↩

How about we add more prefixes to covering most scenarios? eg.

["is","was","has","had","will","would","can","could","should","shall","does","needs","must","includes","enables","supports"]

These prefixes are treated as valid names, but only a few suggested fixes are provided.

sindresorhus · 2024-09-08T17:53:32Z

docs/rules/better-boolean-variable-names.md

+```js
+const completed = Boolean('true'); // ❌
+const isCompleted = Boolean('true'); // ✅
+```
+
+```js
+const completed = new Boolean('true'); // ❌
+const isCompleted = new Boolean('true'); // ✅
+```


I would drop these. No one would code like this.

sindresorhus · 2024-09-08T17:55:51Z

rules/better-boolean-variable-names.js

+/**
+Capitalize the first letter of a string
+
+@param {string} str
+@returns {string}
+*/
+function capitalize(string_) {
+	return string_.charAt(0).toUpperCase() + string_.slice(1);
+}


I think this should be un utils.

Also:

Suggested change

/**

Capitalize the first letter of a string

@param {string} str

@returns {string}

*/

function capitalize(string_) {

return string_.charAt(0).toUpperCase() + string_.slice(1);

}

/**

Capitalize the first letter of a string.

@param {string} string

@returns {string}

*/

function capitalize(string) {

return string.charAt(0).toUpperCase() + string.slice(1);

}

sindresorhus · 2024-09-08T17:57:19Z

rules/utils/is-boolean.js

+@param {import('estree').Expression} node
+@param {number} [deep] - The current recursion depth. Users do not need to pass this parameter.
+*/
+function isBooleanExpression(context, node, deep = 0) {


Suggested change

function isBooleanExpression(context, node, deep = 0) {

function isBooleanExpression(context, node, depth = 0) {

sindresorhus · 2024-09-08T17:59:40Z

rules/utils/rename-variable.js

+/**
+Replace the string between start and end with replacement.
+
+@param {string} str


Does not match actual parameter.

sindresorhus · 2024-09-08T17:59:48Z

rules/utils/rename-variable.js

+function isExportedVariable(identifier) {
+	return (
+		identifier
+        && identifier.parent?.type === 'VariableDeclarator'


Use tab-indentation.

sindresorhus · 2024-09-08T18:00:05Z

rules/utils/rename-variable.js

+	 * ```js
+	 * export const foo = 1
+	 * ```
+	 *


Don't prefix comment lines with *

sindresorhus · 2024-09-08T18:01:23Z

docs/rules/better-boolean-variable-names.md

@@ -0,0 +1,162 @@
+# Prefer readable Boolean variable names


Suggested change

# Prefer readable Boolean variable names

# Prefer readable boolean variable names

sindresorhus · 2024-09-08T18:02:32Z

docs/rules/better-boolean-variable-names.md

+<!-- end auto-generated rule header -->
+<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
+
+When it is clear that an expression is a Boolean value, the variable name should start with `is`/`was`/`has`/`can`/`should` to improve readability.


Suggested change

When it is clear that an expression is a Boolean value, the variable name should start with `is`/`was`/`has`/`can`/`should` to improve readability.

Boolean variable names should start with `is`/`was`/`has`/`can`/`should` for clarity and readability.

sindresorhus · 2024-09-08T18:02:44Z

docs/rules/better-boolean-variable-names.md

+
+```js
+{
+  'unicorn/prefer-readable-boolean-variable-names': [


Tab-indentation.

axetroy · 2024-09-09T08:38:06Z

rules/utils/is-boolean.js

+@param {number} [depth] - The current recursion depth. Users do not need to pass this parameter.
+@returns {boolean}
+*/
+function isBooleanExpression(context, node, depth = 0) {


I know there is a method isBooleanNode, but it is not comprehensive enough, and it cannot track the type of Identifier.

eslint-plugin-unicorn/rules/utils/boolean.js

Lines 32 to 37 in 461b01c

function isBooleanNode(node) {

if (

isLogicNot(node)

|| isLogicNotArgument(node)

|| isBooleanCall(node)

|| isBooleanCallArgument(node)

So, I need more advice, should I use the same method.

axetroy force-pushed the better-boolean-var-name branch from 7e59c84 to 7d18b38 Compare September 8, 2024 08:15

axetroy changed the title ~~Add better-boolean-var-names~~ Add better-boolean-variable-names Sep 8, 2024

axetroy marked this pull request as draft September 8, 2024 08:21

axetroy force-pushed the better-boolean-var-name branch 3 times, most recently from f1bc082 to bc3134e Compare September 8, 2024 08:59

axetroy marked this pull request as ready for review September 8, 2024 10:16

axetroy force-pushed the better-boolean-var-name branch 2 times, most recently from d5882a4 to c0b0b0d Compare September 8, 2024 11:02

axetroy force-pushed the better-boolean-var-name branch 2 times, most recently from bed94be to 63f8643 Compare September 8, 2024 11:27

fregante reviewed Sep 8, 2024

View reviewed changes

sindresorhus requested changes Sep 8, 2024

View reviewed changes

axetroy force-pushed the better-boolean-var-name branch from 63f8643 to d412470 Compare September 9, 2024 03:06

axetroy requested a review from sindresorhus September 9, 2024 07:53

axetroy commented Sep 9, 2024

View reviewed changes

axetroy added 8 commits September 10, 2024 09:45

Add better-boolean-var-names

1a253c7

update

99a3c90

remove

6f9224d

fix infinite recursion

32ca80d

add more test

dd753d0

fix lint in codebase

fd763fb

update description

e9bfca4

update docs

983a51d

axetroy added 22 commits September 10, 2024 09:45

update docs

c39e780

update

401ea80

fix

c75def7

fix

d92b7b4

indent

14a31be

indent

5d3082c

remove validate function name

963348c

update

0834654

update docs

b74ce48

add test for conflicting rename

8e1463b

use existing rename-variable

1143b2c

move capitalize to utils

b061e6a

fix

50938b7

update test

8fb817e

update test

16a32c3

rename report to reportProblem

090337c

Simplify isBooleanExpression

c94f75c

Simplify isBooleanExpression2

89caa0c

Simplify isBooleanExpression

5e4bf73

fix lint

d2b44e3

update

ee34f51

add test

2077601

axetroy force-pushed the better-boolean-var-name branch from a40eb9c to 2077601 Compare September 10, 2024 01:46

remove unused message template context

bfba029

axetroy closed this Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `better-boolean-variable-names` #2446

Add `better-boolean-variable-names` #2446

axetroy commented Sep 8, 2024 •

edited

Loading

axetroy commented Sep 8, 2024

fregante commented Sep 8, 2024

axetroy commented Sep 8, 2024

fregante commented Sep 8, 2024

fregante Sep 8, 2024 •

edited

Loading

fregante Sep 8, 2024

axetroy Sep 9, 2024

fregante Sep 8, 2024 •

edited

Loading

axetroy Sep 9, 2024 •

edited

Loading

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

sindresorhus Sep 8, 2024

axetroy Sep 9, 2024

	function isBooleanExpression(context, node, deep = 0) {
	function isBooleanExpression(context, node, depth = 0) {

	# Prefer readable Boolean variable names
	# Prefer readable boolean variable names

	When it is clear that an expression is a Boolean value, the variable name should start with `is`/`was`/`has`/`can`/`should` to improve readability.
	Boolean variable names should start with `is`/`was`/`has`/`can`/`should` for clarity and readability.

	function isBooleanNode(node) {
	if (
	isLogicNot(node)
	\|\| isLogicNotArgument(node)
	\|\| isBooleanCall(node)
	\|\| isBooleanCallArgument(node)

Add better-boolean-variable-names #2446

Add better-boolean-variable-names #2446

Conversation

axetroy commented Sep 8, 2024 • edited Loading

axetroy commented Sep 8, 2024

fregante commented Sep 8, 2024

axetroy commented Sep 8, 2024

fregante commented Sep 8, 2024

fregante Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fregante Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

Footnotes

axetroy Sep 9, 2024 • edited Loading

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

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

Choose a reason for hiding this comment

Add `better-boolean-variable-names` #2446

Add `better-boolean-variable-names` #2446

axetroy commented Sep 8, 2024 •

edited

Loading

fregante Sep 8, 2024 •

edited

Loading

fregante Sep 8, 2024 •

edited

Loading

axetroy Sep 9, 2024 •

edited

Loading