eslint: forbid boolean function arguments

[bajtos]

Function arguments of type boolean are considered as a bad practice because they make the code difficult to read and reason about.

Consider the following code:

ctx.getBinding('repositories.todo', true);

What does the true flag mean? There is no way to tell when reading the code using the function, we have to look up the API definition and/or implementation to understand.

Compare with the current implementation which is following the best practices:

ctx.getBinding('repositories.todo', {optional: true});

Now it's more clear that we want to treat the binding as optional and don't trigger an error when it's not found.

Let's improve our eslint configuration to automatically detect and reject boolean arguments. Eslint rule to use: no-inferrable-types

Further reading:

• Martin Fowler on Flag Arguments
• Is it wrong to use a boolean parameter to determine behavior

Acceptance criteria

• [ ] A commit modifying our eslint configuration to enable the new check. This is a breaking change, see Describe incompatibilites for release notes
• [ ] One or more commits fixing any violations of this new rule in our existing code. For functions (methods) that are part our public API, decide whether to disable this rule via a code comment, change the method signature in a breaking (semver-major) change or implement both variants (with an options arg, with a boolean arg for backwards compatibility) to avoid a breaking change.

Breaking changes must be committed in such way that they don't trigger semver-major release of packages that are not affected.

[achrinza]

Is no-inferrable-types the correct eslint rule? AFAIK it's to prevent unnecessary verbosity when declaring a variable or parameter type.

Though it's still a good rule to enable nonetheless.

[raymondfeng]

@achrinza no-inferrable-types is already enforced to avoid something like const x : string = 'abc';.

[bajtos]

Is no-inferrable-types the correct eslint rule? AFAIK it's to prevent unnecessary verbosity when declaring a variable or parameter type.

Hmm, that's a good question. Now that I am re-reading the documentation for no-inferrable-types, I am not sure why I thought this is the rule to use 🙈

[bajtos]

I reviewed eslint rules and did not find any that we could use to enforce this coding style. I am closing this issue as it is not actionable now.

[jtbandes]

Hi 👋 I was looking for a similar rule lint rule for another project and came across this issue. Thought you might like to know that I just implemented a rule in @foxglove/eslint-plugin called @foxglove/no-boolean-parameters. More info here: https://github.com/foxglove/eslint-plugin/tree/main/rules#rules

[achrinza]

Thanks, @jtbandes! Will see if we can incorporate it into our monorepo.