Skip to content

Rule proposal: Boolean variable naming #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sindresorhus opened this issue May 12, 2019 · 7 comments · Fixed by #1318
Closed

Rule proposal: Boolean variable naming #515

sindresorhus opened this issue May 12, 2019 · 7 comments · Fixed by #1318
Labels
enhancement: new plugin rule New rule request for eslint-plugin has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@sindresorhus
Copy link

sindresorhus commented May 12, 2019

I would like to enforce that variables and properties with a boolean use the prefix is, has, can, did, and will. For example: .visible.isVisible.

I often see .visible, .open, etc, in code, and it's not clear whether they are booleans, contain something visible/open, or whether they contain a function that makes something visible or opens something.

Fail

const visible = true;

const rainbow = false;

Pass

const isVisible = true;

const hasRainbow = false;
@sindresorhus sindresorhus added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels May 12, 2019
@swyxio
Copy link

swyxio commented May 12, 2019

i often use past/future tense stuff for dirty-flag type operations - thoughts? didChange, willCleanup.

@sindresorhus
Copy link
Author

@sw-yx Ah yes, forgot about those. They make sense too.

@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed triage Waiting for team members to take a look labels May 12, 2019
@platinumazure
Copy link
Contributor

platinumazure commented May 12, 2019

Seems this could be generalized as sort of like core id-match, but applied to specific types. Configuration could look something like:

{
    "typescript/typed-id-match": [
        "error",
        { "types": [/* ... */], "pattern": "^(is|has|can|did|will)$" },
        { /* ... */ }
    ]
}

@golopot
Copy link
Contributor

golopot commented May 21, 2019

Should this rule deal with properties? How to deal with property names you cannot control, for example, other people's api?

// it probably does not make sense to warn on this
const options = {capture: true, passive: false};
someExternalApi(options);

@golopot
Copy link
Contributor

golopot commented May 21, 2019

Does this rule cover boolean valued functions?

@bradzacher
Copy link
Member

There's no reason the rule couldn't support all of these things and have options to enable/disable checks.

@loilo
Copy link

loilo commented Aug 26, 2019

Side note: Another possible prefix that may be considered is should. This is very semantically similar to will, but in some cases I find should the more fitting choice (e.g. when used as a function parameter).

As an example from the code that induced this comment:

function formatArray(data: any[], shouldIgnoreEmpty = false) {
  if (data.length === 0 && shouldIgnoreEmpty) {
    return ''
  } else {
    return JSON.stringify(data)
  }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: new plugin rule New rule request for eslint-plugin has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants