javascript icon indicating copy to clipboard operation
javascript copied to clipboard

Non-Enforced Rules Should be Clearly Documented in the README.md

Open Zamiell opened this issue 4 years ago • 7 comments

  1. The users of this style guide will probably expect that all of the rules that it prescribes will be enforced by eslint. However, this is not the case - there is a secret, non-documented segmentation where some rules are enforced and others are not, because they would be "too noisy on a legacy codebase". An example of a problematic rule like this is covered in issue #2020. I propose that all of these exceptions should be clearly documented in the README.md file and not hidden from the end-user. Specifically, I propose that the suffix of each rule in question is modified, e.g. something along the lines of eslint: prefer-const, no-const-assign --> eslint: prefer-const (not enforced), no-const-assign (not enforced).

  2. As a separate but related issue, some style prescriptions in particular are not lintable. For example, the comparison shortcuts rule is not lintable, as mentioned in issue #1489. However, just like the previous point, this is opaque to the end user. Specifically, I propose that a suffix is added to each rule in question, e.g. something along the lines of (not lintable). Adding these suffixes would further inform the end-user about what is being linted and what is not being linted in a much more explicit way.

While I don't have time to perform a PR myself, I wanted to open an issue so that at least these problems are more visible.

In closing, I want to thank the maintainers for this excellent style guide.

Zamiell avatar Apr 07 '20 00:04 Zamiell

Many guidelines aren’t enforceable via eslint; I’m not sure of the benefit here of calling them out. You’re responsible for reading, knowing, and applying the guide - the eslint config is just a tool to help you automate some of that.

ljharb avatar Apr 07 '20 04:04 ljharb

Hello ljharb,

Many guidelines aren’t enforceable via eslint; I’m not sure of the benefit here of calling them out.

What percentage are not enforceable? Below 33%-ish? If the percentage is relatively small, I think there is value in calling them out, as it would not add very much clutter and help clarify a lot!

You’re responsible for reading, knowing, and applying the guide

Is this in response to both (1) and (2), above, or just (2)?

Essentially, you seem to be saying that: "We provide the ESLint rules alongside the style guide with no warranty and no guarantee that they are necessarily correct and no guarantee that they actually match the style guide. We only care about keeping the style guide accurate and not necessarily the ESLint rules."

But is that really a correct paraphrasing? At least for me, it seems highly implied that the repository maintainers are making a best-effort to make the provided ESLint rules match the style guide as much as possible. (Especially when end-users like me observe the "eslint" section suffixes scattered all throughout the style guide.)

Zamiell avatar Apr 08 '20 17:04 Zamiell

@ljharb I agree with @Zamiell. As a user of this repo when I read the documentation explaining a guideline (i.e. 10.8) and that guideline references a eslint rule (object-curly-newline) I assume the provided linter will "catch" that rule. A lot of value would be added if there was some sort of indication as to which rules are enforced and which are "recommend" but not enforced.

Also, thanks for your work! This project is very helpful

kagant15 avatar Apr 08 '20 17:04 kagant15

@Zamiell We care about keeping the shared config accurate, but we absolutely do not have "exhaustive coverage" as a goal of the shared config.

@kagant15 it is a fair point that if we're going to link to a rule but not enable it, that should be called out in the guide somehow - perhaps by linking to our configuration of the rule (which has the eslint rule docs in a comment) instead of the eslint rule docs directly?

ljharb avatar Apr 08 '20 17:04 ljharb

but we absolutely do not have "exhaustive coverage" as a goal of the shared config.

To be clear, I'm not proposing that anyone should code linting rules for things that are not lintable. Rather, I would define "exhaustive coverage" as a matter of documentation - by default, assume that every style prescription is linted, and specify an eslint suffix with all respective ESLint rule, and specify a (not lintable) suffix for the ones that aren't.

In this context, exhaustive coverage would be a great feature for the style guide to have! Correct me if I am wrong, but isn't the guide like 98% of the way there already? I am skimming through it now and it seems like nearly every single section already has an ESLint suffix. All that would have to be done is a few annotations of (not lintable) under the rules that are not lintable and (not enforced) under the few rules that are not enforced.

We care about keeping the shared config accurate,

Ok, but then we get back to (1) in my original post - the shared confg is not accurate, as demonstrated in #2020. So you seem to be giving me misleading answers here.

Zamiell avatar Apr 08 '20 17:04 Zamiell

By accurate, I only mean, the shared config should not enforce a style that contradicts the guide.

ljharb avatar Apr 08 '20 21:04 ljharb

@ljharb - I thinking linking to the configuration would be a step in the right direction but taking that a step further I would actually prefer what @Zamiell is suggesting. Some sort of (not lintable) (not enforced) text next to the rule itself in the README would go a long way

kagant15 avatar Apr 09 '20 12:04 kagant15