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

Automate docs with eslint-doc-generator #2582

Merged
merged 1 commit into from Nov 4, 2022

Conversation

bmish
Copy link
Contributor

@bmish bmish commented Oct 29, 2022

I built this CLI tool eslint-doc-generator for automating the generation of the README rules list table and rule doc title/notices for ESLint plugins. It follows common documentation conventions from this and other top ESLint plugins and will help us standardize documentation across ESLint plugins (and generally improve the usability of custom rules through better documentation and streamline the process of adding new rules). It has 100% test coverage. It's already used in other popular plugins like eslint-plugin-react.

Notable changes:

  • Moved rule categories from README to common meta.docs.category property and used eslint-doc-generator --split-by meta.docs.category to continue displaying multiple lists in the README
  • Moved rule descriptions from README to standard meta.docs.description property which is consumable by tooling
  • Enabled eslint-plugin/require-meta-docs-description for ensuring we have consistent descriptions defined in all rules

Rule doc headers now have consistent titles and helpful notices about autofixers and what configs the rules are enabled/disabled/warn in. The README rules list now has an attractive table containing much more information about the rules.

@bmish bmish force-pushed the eslint-doc-generator branch 5 times, most recently from f140c87 to aa96296 Compare October 29, 2022 18:11
docs/rules/default.md Outdated Show resolved Hide resolved
@@ -1,4 +1,6 @@
# import/extensions - Ensure consistent use of file extension within the import path
Copy link
Member

Choose a reason for hiding this comment

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

i'd prefer to see this tagline preserved somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few of these rule docs had the rule description in their title. Note that these descriptions are all preserved in meta.docs.description which is used in the README rules table. If you wanted, we could include the rule descriptions in all the titles using the option --rule-doc-title-format option. But for the most part, these descriptions are already captured in the rule doc bodies.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's useful for the exact phrase to appear somewhere in the doc - it doesn't have to be in the title ofc, but somewhere. Can that be automated/enforced somehow?

Copy link
Contributor Author

@bmish bmish Oct 30, 2022

Choose a reason for hiding this comment

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

Today, there are many plugins that include the rule description directly in the rule doc title, which is why we have --rule-doc-title-format desc-parens-prefix-name. That's probably the best/easiest way to achieve this within existing conventions.

That said, if that's still not satisfactory, then I'm thinking of some possible options that could achieve what you want.

Potential Option Description Upside Downsides
--rule-doc-header-include-description Whether to include the rule description below any notices at the bottom of the rule doc header (default false). The rule description will be auto-generated in the rule doc header. This isn't always the best place to put the rule description. Often, rule docs provide general context first, and then provide the description later (potentially in a "Rule details" section).
--rule-doc-section-include-description Specifies the name of the section of the rule doc that should begin with the rule description. Pass option with no section name to include the description at the bottom of the rule doc header. Ensures the rule description is mentioned in the appropriate section, often a "Rule details" section. This is a more flexible version of --rule-doc-header-include-description. Most complex implementation.
--rule-doc-mention-description Whether to require the rule description to be mentioned somewhere in the rule doc (default false). Allows greatest flexibility in placement. Not auto-generated. The descriptions could end up placed in inconsistent positions.

My recommendation in order of preference:

  1. Leave as-is. Rule docs already capture the spirit of the description even if the wording doesn't exactly match.
  2. Include rule descriptions in the rule doc titles.
  3. Add --rule-doc-section-include-description option or --rule-doc-mention-description.

Let me know what you think. If we want to implement a new option, then it would probably be best to file an issue for this and avoid blocking the overall PR, since this only affected a few rule docs anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I agree it needn't block the PR, regardless.

I think a simple tagline is important top-level context, so I'd prefer --rule-doc-section-include-description. Is that already implemented? If so, let's use it! If not, we can address it in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not implemented but I filed bmish/eslint-doc-generator#192 to track this.

docs/rules/no-absolute-path.md Show resolved Hide resolved
docs/rules/no-deprecated.md Outdated Show resolved Hide resolved
docs/rules/no-dynamic-require.md Show resolved Hide resolved
docs/rules/no-extraneous-dependencies.md Show resolved Hide resolved
docs/rules/no-restricted-paths.md Show resolved Hide resolved
docs/rules/no-unassigned-import.md Show resolved Hide resolved
src/rules/consistent-type-specifier-style.js Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
| [no-empty-named-blocks](docs/rules/no-empty-named-blocks.md) | Forbid empty named import blocks. | | 🔧 | 💡 | |
| [no-extraneous-dependencies](docs/rules/no-extraneous-dependencies.md) | Forbid the use of extraneous packages. | | | | |
| [no-mutable-exports](docs/rules/no-mutable-exports.md) | Forbid the use of mutable exports with `var` or `let`. | | | | |
| [no-named-as-default](docs/rules/no-named-as-default.md) | Forbid use of exported name as identifier of default export. | ☑️<sup>⚠️</sup> <br>🚸<sup>⚠️</sup> | | | |
Copy link
Member

Choose a reason for hiding this comment

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

it seems like the superscripted emoji is still rendering on a new line, and isn't staying attached to the base emoji?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh. I fixed some other issues with this but looks like I'm still struggling to get the emoji superscript to stay attached to its base character. Tracking this with: bmish/eslint-doc-generator#204

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried everything to fix the emoji superscripts but they turned out to be too fragile. This led me to remove the superscripts entirely and switch to what I believe is a significantly better information architecture with a separate rules list column per severity and many other improvements:

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Looks great!

A possible enhancement would be if there's a way to provide tooltips (like an img alt) on all the emojis.

@bmish
Copy link
Contributor Author

bmish commented Nov 2, 2022

@ljharb based on my research, it's not possible to add alt text / tooltips on emojis in GitHub markdown. It could be nice but hopefully it won't be needed anyway.

@ljharb ljharb closed this Nov 3, 2022
@ljharb ljharb reopened this Nov 3, 2022
@bmish
Copy link
Contributor Author

bmish commented Nov 3, 2022

Seems like there's an unrelated failure on CI with continuous-integration/appveyor/pr — AppVeyor build failed? I'd recommend merging and addressing that separately.

  1) dynamic imports no-unused-modules valid 
        import App from './App';
      :
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

@ljharb
Copy link
Member

ljharb commented Nov 3, 2022

@bmish i'll rerun it til it passes :-)

@ljharb ljharb merged commit e85c694 into import-js:main Nov 4, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jan 13, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) | devDependencies | minor | [`2.26.0` -> `2.27.4`](https://renovatebot.com/diffs/npm/eslint-plugin-import/2.26.0/2.27.4) |

---

### Release Notes

<details>
<summary>import-js/eslint-plugin-import</summary>

### [`v2.27.4`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2274---2023-01-11)

[Compare Source](import-js/eslint-plugin-import@v2.27.3...v2.27.4)

##### Fixed

-   `semver` should be a prod dep (\[[#&#8203;2668](import-js/eslint-plugin-import#2668)])

### [`v2.27.3`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2273---2023-01-11)

[Compare Source](import-js/eslint-plugin-import@v2.27.2...v2.27.3)

##### Fixed

-   \[`no-empty-named-blocks`]: rewrite rule to only check import declarations (\[[#&#8203;2666](import-js/eslint-plugin-import#2666)])

### [`v2.27.2`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2272---2023-01-11)

[Compare Source](import-js/eslint-plugin-import@v2.27.1...v2.27.2)

##### Fixed

-   \[`no-duplicates`]: do not unconditionally require `typescript` (\[[#&#8203;2665](import-js/eslint-plugin-import#2665)])

### [`v2.27.1`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2271---2023-01-11)

[Compare Source](import-js/eslint-plugin-import@v2.27.0...v2.27.1)

##### Fixed

-   `array.prototype.flatmap` should be a prod dep (\[[#&#8203;2664](import-js/eslint-plugin-import#2664)], thanks \[[@&#8203;cristobal](https://github.com/cristobal)])

### [`v2.27.0`](https://github.com/import-js/eslint-plugin-import/blob/HEAD/CHANGELOG.md#&#8203;2270---2023-01-11)

[Compare Source](import-js/eslint-plugin-import@v2.26.0...v2.27.0)

##### Added

-   \[`newline-after-import`]: add `considerComments` option (\[[#&#8203;2399](import-js/eslint-plugin-import#2399)], thanks \[[@&#8203;pri1311](https://github.com/pri1311)])
-   \[`no-cycle`]: add `allowUnsafeDynamicCyclicDependency` option (\[[#&#8203;2387](import-js/eslint-plugin-import#2387)], thanks \[[@&#8203;GerkinDev](https://github.com/GerkinDev)])
-   \[`no-restricted-paths`]: support arrays for `from` and `target` options (\[[#&#8203;2466](import-js/eslint-plugin-import#2466)], thanks \[[@&#8203;AdriAt360](https://github.com/AdriAt360)])
-   \[`no-anonymous-default-export`]: add `allowNew` option (\[[#&#8203;2505](import-js/eslint-plugin-import#2505)], thanks \[[@&#8203;DamienCassou](https://github.com/DamienCassou)])
-   \[`order`]: Add `distinctGroup` option (\[[#&#8203;2395](import-js/eslint-plugin-import#2395)], thanks \[[@&#8203;hyperupcall](https://github.com/hyperupcall)])
-   \[`no-extraneous-dependencies`]: Add `includeInternal` option (\[[#&#8203;2541](import-js/eslint-plugin-import#2541)], thanks \[[@&#8203;bdwain](https://github.com/bdwain)])
-   \[`no-extraneous-dependencies`]: Add `includeTypes` option (\[[#&#8203;2543](import-js/eslint-plugin-import#2543)], thanks \[[@&#8203;bdwain](https://github.com/bdwain)])
-   \[`order`]: new `alphabetize.orderImportKind` option to sort imports with same path based on their kind (`type`, `typeof`) (\[[#&#8203;2544](import-js/eslint-plugin-import#2544)], thanks \[[@&#8203;stropho](https://github.com/stropho)])
-   \[`consistent-type-specifier-style`]: add rule (\[[#&#8203;2473](import-js/eslint-plugin-import#2473)], thanks \[[@&#8203;bradzacher](https://github.com/bradzacher)])
-   Add \[`no-empty-named-blocks`] rule (\[[#&#8203;2568](import-js/eslint-plugin-import#2568)], thanks \[[@&#8203;guilhermelimak](https://github.com/guilhermelimak)])
-   \[`prefer-default-export`]: add "target" option (\[[#&#8203;2602](import-js/eslint-plugin-import#2602)], thanks \[[@&#8203;azyzz228](https://github.com/azyzz228)])
-   \[`no-absolute-path`]: add fixer (\[[#&#8203;2613](import-js/eslint-plugin-import#2613)], thanks \[[@&#8203;adipascu](https://github.com/adipascu)])
-   \[`no-duplicates`]: support inline type import with `inlineTypeImport` option (\[[#&#8203;2475](import-js/eslint-plugin-import#2475)], thanks \[[@&#8203;snewcomer](https://github.com/snewcomer)])

##### Fixed

-   \[`order`]: move nested imports closer to main import entry (\[[#&#8203;2396](import-js/eslint-plugin-import#2396)], thanks \[[@&#8203;pri1311](https://github.com/pri1311)])
-   \[`no-restricted-paths`]: fix an error message (\[[#&#8203;2466](import-js/eslint-plugin-import#2466)], thanks \[[@&#8203;AdriAt360](https://github.com/AdriAt360)])
-   \[`no-restricted-paths`]: use `Minimatch.match` instead of `minimatch` to comply with Windows Native paths (\[[#&#8203;2466](import-js/eslint-plugin-import#2466)], thanks \[[@&#8203;AdriAt360](https://github.com/AdriAt360)])
-   \[`order`]: require with member expression could not be fixed if alphabetize.order was used (\[[#&#8203;2490](import-js/eslint-plugin-import#2490)], thanks \[[@&#8203;msvab](https://github.com/msvab)])
-   \[`order`]: leave more space in rankings for consecutive path groups (\[[#&#8203;2506](import-js/eslint-plugin-import#2506)], thanks \[[@&#8203;Pearce-Ropion](https://github.com/Pearce-Ropion)])
-   \[`no-cycle`]: add ExportNamedDeclaration statements to dependencies (\[[#&#8203;2511](import-js/eslint-plugin-import#2511)], thanks \[[@&#8203;BenoitZugmeyer](https://github.com/BenoitZugmeyer)])
-   \[`dynamic-import-chunkname`]: prevent false report on a valid webpack magic comment (\[[#&#8203;2330](import-js/eslint-plugin-import#2330)], thanks \[[@&#8203;mhmadhamster](https://github.com/mhmadhamster)])
-   \[`export`]: do not error on TS export overloads (\[[#&#8203;1590](import-js/eslint-plugin-import#1590)], thanks \[[@&#8203;ljharb](https://github.com/ljharb)])
-   \[`no-unresolved`], \[`extensions`]: ignore type only exports (\[[#&#8203;2436](import-js/eslint-plugin-import#2436)], thanks \[[@&#8203;Lukas-Kullmann](https://github.com/Lukas-Kullmann)])
-   `ExportMap`: add missing param to function (\[[#&#8203;2589](import-js/eslint-plugin-import#2589)], thanks \[[@&#8203;Fdawgs](https://github.com/Fdawgs)])
-   \[`no-unused-modules`]: `checkPkgFieldObject` filters boolean fields from checks (\[[#&#8203;2598](import-js/eslint-plugin-import#2598)], thanks \[[@&#8203;mpint](https://github.com/mpint)])
-   \[`no-cycle`]: accept Flow `typeof` imports, just like `type` (\[[#&#8203;2608](import-js/eslint-plugin-import#2608)], thanks \[[@&#8203;gnprice](https://github.com/gnprice)])
-   \[`no-import-module-exports`]: avoid a false positive for import variables (\[[#&#8203;2315](import-js/eslint-plugin-import#2315)], thanks \[[@&#8203;BarryThePenguin](https://github.com/BarryThePenguin)])

##### Changed

-   \[Tests] \[`named`]: Run all TypeScript test (\[[#&#8203;2427](import-js/eslint-plugin-import#2427)], thanks \[[@&#8203;ProdigySim](https://github.com/ProdigySim)])
-   \[readme] note use of typescript in readme `import/extensions` section (\[[#&#8203;2440](import-js/eslint-plugin-import#2440)], thanks \[[@&#8203;OutdatedVersion](https://github.com/OutdatedVersion)])
-   \[Docs] \[`order`]: use correct default value (\[[#&#8203;2392](import-js/eslint-plugin-import#2392)], thanks \[[@&#8203;hyperupcall](https://github.com/hyperupcall)])
-   \[meta] replace git.io link in comments with the original URL (\[[#&#8203;2444](import-js/eslint-plugin-import#2444)], thanks \[[@&#8203;liby](https://github.com/liby)])
-   \[Docs] remove global install in readme (\[[#&#8203;2412](import-js/eslint-plugin-import#2412)], thanks \[[@&#8203;aladdin-add](https://github.com/aladdin-add)])
-   \[readme] clarify `eslint-import-resolver-typescript` usage (\[[#&#8203;2503](import-js/eslint-plugin-import#2503)], thanks \[[@&#8203;JounQin](https://github.com/JounQin)])
-   \[Refactor] \[`no-cycle`]: Add per-run caching of traversed paths (\[[#&#8203;2419](import-js/eslint-plugin-import#2419)], thanks \[[@&#8203;nokel81](https://github.com/nokel81)])
-   \[Performance] `ExportMap`: add caching after parsing for an ambiguous module (\[[#&#8203;2531](import-js/eslint-plugin-import#2531)], thanks \[[@&#8203;stenin-nikita](https://github.com/stenin-nikita)])
-   \[Docs] \[`no-useless-path-segments`]: fix paths (\[[#&#8203;2424](import-js/eslint-plugin-import#2424)], thanks \[[@&#8203;s-h-a-d-o-w](https://github.com/s-h-a-d-o-w)])
-   \[Tests] \[`no-cycle`]: add passing test cases (\[[#&#8203;2438](import-js/eslint-plugin-import#2438)], thanks \[[@&#8203;georeith](https://github.com/georeith)])
-   \[Refactor] \[`no-extraneous-dependencies`] improve performance using cache (\[[#&#8203;2374](import-js/eslint-plugin-import#2374)], thanks \[[@&#8203;meowtec](https://github.com/meowtec)])
-   \[meta] `CONTRIBUTING.md`: mention inactive PRs (\[[#&#8203;2546](import-js/eslint-plugin-import#2546)], thanks \[[@&#8203;stropho](https://github.com/stropho)])
-   \[readme] make json for setting groups multiline (\[[#&#8203;2570](import-js/eslint-plugin-import#2570)], thanks \[[@&#8203;bertyhell](https://github.com/bertyhell)])
-   \[Tests] \[`no-restricted-paths`]: Tests for `import type` statements (\[[#&#8203;2459](import-js/eslint-plugin-import#2459)], thanks \[[@&#8203;golergka](https://github.com/golergka)])
-   \[Tests] \[`no-restricted-paths`]: fix one failing `import type` test case, submitted by \[[@&#8203;golergka](https://github.com/golergka)], thanks \[[@&#8203;azyzz228](https://github.com/azyzz228)]
-   \[Docs] automate docs with eslint-doc-generator (\[[#&#8203;2582](import-js/eslint-plugin-import#2582)], thanks \[[@&#8203;bmish](https://github.com/bmish)])
-   \[readme] Increase clarity around typescript configuration (\[[#&#8203;2588](import-js/eslint-plugin-import#2588)], thanks \[[@&#8203;Nfinished](https://github.com/Nfinished)])
-   \[Docs] update `eslint-doc-generator` to v1.0.0 (\[[#&#8203;2605](import-js/eslint-plugin-import#2605)], thanks \[[@&#8203;bmish](https://github.com/bmish)])
-   \[Perf] \[`no-cycle`], \[`no-internal-modules`], \[`no-restricted-paths`]: use `anyOf` instead of `oneOf` (thanks \[[@&#8203;ljharb](https://github.com/ljharb)], \[[@&#8203;remcohaszing](https://github.com/remcohaszing)])

</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:eyJjcmVhdGVkSW5WZXIiOiIzNC45Ny41IiwidXBkYXRlZEluVmVyIjoiMzQuMTAwLjEifQ==-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1724
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants