Docs: Fixes incorrect example

[vermiceli]

This commit also updates the documentation for the parameters to
include the fact that the first parameter is the rule setting and not a
global to be restricted.

What is the purpose of this pull request? (put an "X" next to item)

[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)
I've fixed an incorrect example in the documentation and clarified an incorrect sentence in the parameter explanation.

Is there anything you'd like reviewers to focus on?
no

[jsf-clabot]

All committers have signed the CLA.

[aladdin-add]

The usage of options was documented here: https://eslint.org/docs/user-guide/configuring#configuring-rules. is there something worth to be added in that part?

[vermiceli]

Although it is defined somewhere else, the original sentence is factually incorrect. Furthermore, if a developer doesn't read the page you linked, they will not realize that 'error' is not just a global.

We should be clear in the documentation so that developers won't get confused. What advantage is there in leaving the documentation incorrect?

[ilyavolodin]

Hmm.. This is a strange case. On one hand, your change makes it clearer to understand that rule takes a flat array of options, and the first one is control option. On the other hand, we always assume that it's clear that first item in the config array is always control option, and we never mention it in any other rule documentation.
In all honesty, I think I would much more prefer to change configuration for this rule to accept an array of strings as a second item. I.e. ["off", ["string1", "string2", ...]]. We would need to make it backwards compatible, so that it wouldn't be a breaking change, but I think a flat array is just confusing.

[vermiceli]

Hmm.. This is a strange case. On one hand, your change makes it clearer to understand that rule takes a flat array of options, and the first one is control option. On the other hand, we always assume that it's clear that first item in the config array is always control option, and we never mention it in any other rule documentation.
In all honesty, I think I would much more prefer to change configuration for this rule to accept an array of strings as a second item. I.e. ["off", ["string1", "string2", ...]]. We would need to make it backwards compatible, so that it wouldn't be a breaking change, but I think a flat array is just confusing.

I agree with you that a single flat list is not the best design choice. When I made this pull request, I didn't realize this documentation oversight was pervasive across all documentation.

The reason I created this pull request is I was using a project that enabled eslint by default that had no-restricted-globals-update warnings. I simply found the docs for the rule and only read that page's documentation, which led me to think error was a global method/variable to ignore, which wasn't the case.

I think when writing documentation, we have to assume not everyone is going to start from the beginning. We should be as helpful as possible, which will make beginners starting with eslint have an easier time onboarding.

[ilyavolodin]

@eslint/eslint-team What do you think? Should we merge this as a one-off, or should we update documentation everywhere (or leave everything as is)?

[platinumazure]

I'd be okay with a one-off if it avoids confusion for this rule (which takes a variadic list of options); but I also think we should change our rule doc template to have a small sidebar reminding about rule configuration, maybe something that could be toggled to expand so it's not in users' way if they already know about the commonalities in rule configuration.

[platinumazure]

I'm going to add this to the TSC agenda to get visibility into the discussion (and ensure the TSC has a chance to consider the information architecture implications across the whole project).

TSC Summary

This PR was created due to confusion around the intersection of (1) how ESLint rule options work generally, and (2) the specific option schema for this rule, which is a variadic list that comes after the severity. This lead to user confusion (about whether "error" was a restricted global vs a severity). This PR definitely makes the documentation for this rule more clear, but there's a concern about whether this sets a precedent (i.e., whether we also need to update all rule docs to remind users about the severity option).

TSC Question

1. Should we accept this PR?
2. Should we establish a working group, or otherwise take on a task to improve information architecture around rule severity as part of (yet distinct from) rule options?

[not-an-aardvark]

In the June 20th TSC meeting, the TSC decided that we don't want to have one-off explanations like this on individual rule pages -- if we want to make things more clear it would be better to have a blurb in the rule page template that automatically gets added for every rule. (Sorry about the delay in reporting the decision back on this issue.)

[vermiceli]

I disagree with this result, but it's not my place. If the documentation template can be implemented to make rules more clear, I suppose it's an ok compromise.

Meanwhile, I made another pull request #12045 that contains the non-contentious code change to correct the incorrect parameter, which I believe shows that even an experienced eslint developer gets the rule setting and the globals confused due to their similarity.

[ilyavolodin]

Closing this PR based on the TSC decision. A smaller change was already merged in, and we want to fix this issue in a broader way through documentation template or other means. Thanks for PR!