Skip to content
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

Jump to bottom

docs: improve context.getScope() docs #16417

Merged
merged 7 commits into from Oct 25, 2022
Merged

docs: improve context.getScope() docs #16417

merged 7 commits into from Oct 25, 2022

Conversation

bpmutter
Copy link
Contributor

Prerequisites checklist

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

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

What changes did you make? (Give an overview)

add some more detail and clarity to the context.getScope() docs.

Fixes #16148

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

nothing in particular

@bpmutter
docs: improve context.getScope() docs
ef67d14 
add some more detail and clarity to the context.getScope() docs.

Fixes eslint#16148
@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon documentation Relates to ESLint's documentation labels Oct 13, 2022
@bpmutter bpmutter marked this pull request as draft October 13, 2022 00:42
@netlify
Copy link

netlify bot commented Oct 13, 2022 •
edited

✅ Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 8165a85
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/63544763ba23d900096b1b64
😎 Deploy Preview https://deploy-preview-16417--docs-eslint.netlify.app/developer-guide/working-with-rules
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bpmutter
Copy link
Contributor Author

@nzakas i'm fairly on the fence here about the necessity of a cope example here, as initially proposed by #16148. based on my (admittedly limited) understanding of context.getScope(), there's not a generalizably useful example.

with this being said, i think there could be a bit more explanation added for the context.getScope() method.
in the ## The Context Object section, it says about context.getScope(), "This information can be used to track references to variables." could you expand a bit on how this works and why it's useful to do this?

also are there any other frequent use cases for context.getScope() that you think are worth including/further explaining in the docs?

@nzakas
Copy link
Member

nzakas commented Oct 13, 2022

I think the problem is that we don't really have any good examples in the docs, so adding some would be useful. In general, I think places to dig in are:

  1. Finding the variables declared in a given scope. Given any scope object, you can get the variable defined within that scope through the variables property.
  2. Tracking variable references. We end up using this a lot. Basically, if you go through scope.variables, each Variable instance has a references property that contains Reference objects. Each Reference object refers to a location where the variable is being referenced in the source code (the entire source code, not just the scope).
  3. Locating variable definitions. Similarly, anytime you can get a Variable instance, you can use that to determine where that variable was defined by using the defs property

@bpmutter
Copy link
Contributor Author

@nzakas i've updated the docs per your feedback. note that i didn't include a code example b/c i still don't see a clear way to provide a concise example, per the others on this page. however, i did link to a couple of rules implementing context.getScope().

please let me know what you think of this approach, and and any further changes you'd like me to make as part of this docs update.

@bpmutter bpmutter marked this pull request as ready for review October 19, 2022 00:28
nzakas
nzakas requested changes Oct 19, 2022
View reviewed changes
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated Show resolved Hide resolved
docs/src/developer-guide/working-with-rules.md Outdated
* `variable.eslintExplicitGlobalComments` (`Comment[] | undefined`) ... The array of `/* globals */` directive comments which defined this global variable in the source code file. This property is `undefined` if there are no `/* globals */` directive comments.
* `variable.eslintImplicitGlobalSetting` (`"readonly" | "writable" | undefined`) ... The configured value in config files. This can be different from `variable.writeable` if there are `/* globals */` directive comments.
* [no-shadow](https://github.com/eslint/eslint/blob/main/lib/rules/no-shadow.js)
* [no-redeclare](https://github.com/eslint/eslint/blob/main/lib/rules/no-redeclare.js)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are good example files to link to. Can you also indicate what each rule is doing with getScope to help readers decide which one to click on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a sentence for each + link to each rule's documentation.

bpmutter and others added 3 commits October 19, 2022 21:32
@bpmutter @nzakas
Apply suggestions from NZ code review
94383f1 
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
@bpmutter
draft explanation of usage
59677fb
@bpmutter
copy edit
8165a85
@bpmutter
Copy link
Contributor Author

@nzakas implemented your feedback. back to you here!

nzakas
nzakas approved these changes Oct 25, 2022
View reviewed changes
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@nzakas nzakas merged commit 1581405 into eslint:main Oct 25, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Nov 10, 2022
Update dependency eslint to v8.27.0 (#1628)
7452726 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.26.0` -> `8.27.0`](https://renovatebot.com/diffs/npm/eslint/8.26.0/8.27.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.27.0`](https://github.com/eslint/eslint/releases/tag/v8.27.0)

[Compare Source](eslint/eslint@v8.26.0...v8.27.0)

#### Features

-   [`f14587c`](eslint/eslint@f14587c) feat: new `no-new-native-nonconstructor` rule ([#&#8203;16368](eslint/eslint#16368)) (Sosuke Suzuki)
-   [`978799b`](eslint/eslint@978799b) feat: add new rule `no-empty-static-block` ([#&#8203;16325](eslint/eslint#16325)) (Sosuke Suzuki)
-   [`69216ee`](eslint/eslint@69216ee) feat: no-empty suggest to add comment in empty BlockStatement ([#&#8203;16470](eslint/eslint#16470)) (Nitin Kumar)
-   [`319f0a5`](eslint/eslint@319f0a5) feat: use `context.languageOptions.ecmaVersion` in core rules ([#&#8203;16458](eslint/eslint#16458)) (Milos Djermanovic)

#### Bug Fixes

-   [`c3ce521`](eslint/eslint@c3ce521) fix: Ensure unmatched glob patterns throw an error ([#&#8203;16462](eslint/eslint#16462)) (Nicholas C. Zakas)
-   [`886a038`](eslint/eslint@886a038) fix: handle files with unspecified path in `getRulesMetaForResults` ([#&#8203;16437](eslint/eslint#16437)) (Francesco Trotta)

#### Documentation

-   [`ce93b42`](eslint/eslint@ce93b42) docs: Stylelint property-no-unknown ([#&#8203;16497](eslint/eslint#16497)) (Nick Schonning)
-   [`d2cecb4`](eslint/eslint@d2cecb4) docs: Stylelint declaration-block-no-shorthand-property-overrides ([#&#8203;16498](eslint/eslint#16498)) (Nick Schonning)
-   [`0a92805`](eslint/eslint@0a92805) docs: stylelint color-hex-case ([#&#8203;16496](eslint/eslint#16496)) (Nick Schonning)
-   [`74a5af4`](eslint/eslint@74a5af4) docs: fix stylelint error ([#&#8203;16491](eslint/eslint#16491)) (Milos Djermanovic)
-   [`324db1a`](eslint/eslint@324db1a) docs: explicit stylelint color related rules ([#&#8203;16465](eslint/eslint#16465)) (Nick Schonning)
-   [`94dc4f1`](eslint/eslint@94dc4f1) docs: use Stylelint for HTML files ([#&#8203;16468](eslint/eslint#16468)) (Nick Schonning)
-   [`cc6128d`](eslint/eslint@cc6128d) docs: enable stylelint declaration-block-no-duplicate-properties ([#&#8203;16466](eslint/eslint#16466)) (Nick Schonning)
-   [`d03a8bf`](eslint/eslint@d03a8bf) docs: Add heading to justification explanation ([#&#8203;16430](eslint/eslint#16430)) (Maritaria)
-   [`8a15968`](eslint/eslint@8a15968) docs: add Stylelint configuration and cleanup ([#&#8203;16379](eslint/eslint#16379)) (Nick Schonning)
-   [`9b0a469`](eslint/eslint@9b0a469) docs: note commit messages don't support scope ([#&#8203;16435](eslint/eslint#16435)) (Andy Edwards)
-   [`1581405`](eslint/eslint@1581405) docs: improve context.getScope() docs ([#&#8203;16417](eslint/eslint#16417)) (Ben Perlmutter)
-   [`b797149`](eslint/eslint@b797149) docs: update formatters template ([#&#8203;16454](eslint/eslint#16454)) (Milos Djermanovic)
-   [`5ac4de9`](eslint/eslint@5ac4de9) docs: fix link to formatters on the Core Concepts page ([#&#8203;16455](eslint/eslint#16455)) (Vladislav)
-   [`33313ef`](eslint/eslint@33313ef) docs: core-concepts: fix link to semi rule ([#&#8203;16453](eslint/eslint#16453)) (coderaiser)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xOS4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjEuMiJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1628
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Apr 24, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nzakas nzakas nzakas approved these changes

Assignees
No one assigned
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation triage An ESLint team member will look at this issue soon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change Request: Better documentation for Scopes
2 participants
@bpmutter @nzakas