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

test: ensure that CLI options are alphabetical #55790

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 42 additions & 42 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -706,25 +706,6 @@ Make built-in language features like `eval` and `new Function` that generate
code from strings throw an exception instead. This does not affect the Node.js
`node:vm` module.

### `--expose-gc`

<!-- YAML
added:
- v22.3.0
- v20.18.0
-->

> Stability: 1 - Experimental. This flag is inherited from V8 and is subject to
> change upstream.

This flag will expose the gc extension from V8.

```js
if (globalThis.gc) {
globalThis.gc();
}
```

### `--dns-result-order=order`

<!-- YAML
Expand Down Expand Up @@ -962,17 +943,6 @@ files with no extension will be treated as WebAssembly if they begin with the
WebAssembly magic number (`\0asm`); otherwise they will be treated as ES module
JavaScript.

### `--experimental-transform-types`

<!-- YAML
added: v22.7.0
-->

> Stability: 1.1 - Active development

Enables the transformation of TypeScript-only syntax into JavaScript code.
Implies `--experimental-strip-types` and `--enable-source-maps`.

### `--experimental-eventsource`

<!-- YAML
Expand Down Expand Up @@ -1052,6 +1022,18 @@ following permissions are restricted:
* WASI - manageable through [`--allow-wasi`][] flag
* Addons - manageable through [`--allow-addons`][] flag

### `--experimental-print-required-tla`

<!-- YAML
added:
- v22.0.0
- v20.17.0
-->

If the ES module being `require()`'d contains top-level `await`, this flag
allows Node.js to evaluate the module, try to locate the
top-level awaits, and print their location to help users find them.

### `--experimental-require-module`

<!-- YAML
Expand Down Expand Up @@ -1166,6 +1148,17 @@ added: v22.3.0

Enable [snapshot testing][] in the test runner.

### `--experimental-transform-types`

<!-- YAML
added: v22.7.0
-->

> Stability: 1.1 - Active development

Enables the transformation of TypeScript-only syntax into JavaScript code.
Implies `--experimental-strip-types` and `--enable-source-maps`.

### `--experimental-vm-modules`

<!-- YAML
Expand Down Expand Up @@ -1211,6 +1204,25 @@ added: v22.4.0

Enable experimental [`Web Storage`][] support.

### `--expose-gc`

<!-- YAML
added:
- v22.3.0
- v20.18.0
-->

> Stability: 1 - Experimental. This flag is inherited from V8 and is subject to
> change upstream.

This flag will expose the gc extension from V8.

```js
if (globalThis.gc) {
globalThis.gc();
}
```

### `--force-context-aware`

<!-- YAML
Expand Down Expand Up @@ -1919,18 +1931,6 @@ changes:

Identical to `-e` but prints the result.

### `--experimental-print-required-tla`

<!-- YAML
added:
- v22.0.0
- v20.17.0
-->

If the ES module being `require()`'d contains top-level `await`, this flag
allows Node.js to evaluate the module, try to locate the
top-level awaits, and print their location to help users find them.

### `--prof`

<!-- YAML
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-cli-node-options-docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,20 @@ for (const [, envVar, config] of nodeOptionsCC.matchAll(addOptionRE)) {
}
}

{

const start = /^## Options/m;
const end = /^## Environment variables/m;
const filteredCLIText = cliText.slice(cliText.search(start), cliText.search(end)).trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

The trimming is not useful, is it?

Suggested change
const filteredCLIText = cliText.slice(cliText.search(start), cliText.search(end)).trim();
const filteredCLIText = cliText.slice(cliText.search(start), cliText.search(end));

const cliOptionPattern = /^### `(--[a-zA-Z0-9-]+)`/mg;
const options = Array.from(filteredCLIText.matchAll(cliOptionPattern)).map((match) => match[1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const options = Array.from(filteredCLIText.matchAll(cliOptionPattern)).map((match) => match[1]);
const options = Array.from(filteredCLIText.matchAll(cliOptionPattern), (match) => match[1]);


const sortedOptions = [...options].sort();
/* eslint-disable no-restricted-syntax */
assert.deepStrictEqual(options, sortedOptions, 'CLI options are not in alphabetical order');
Comment on lines +134 to +136
Copy link
Contributor

@aduh95 aduh95 Nov 13, 2024

Choose a reason for hiding this comment

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

Suggested change
const sortedOptions = [...options].sort();
/* eslint-disable no-restricted-syntax */
assert.deepStrictEqual(options, sortedOptions, 'CLI options are not in alphabetical order');
assert.deepStrictEqual(options, options.toSorted());

Copy link
Contributor

Choose a reason for hiding this comment

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

We should not disable lint rules like that:


Comment on lines +127 to +137
Copy link
Contributor

@aduh95 aduh95 Nov 13, 2024

Choose a reason for hiding this comment

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

IMO we also want env variables and V8 options to be sorted, it makes little sense to reduce it to our CLI options

Suggested change
const start = /^## Options/m;
const end = /^## Environment variables/m;
const filteredCLIText = cliText.slice(cliText.search(start), cliText.search(end)).trim();
const cliOptionPattern = /^### `(--[a-zA-Z0-9-]+)`/mg;
const options = Array.from(filteredCLIText.matchAll(cliOptionPattern)).map((match) => match[1]);
const sortedOptions = [...options].sort();
/* eslint-disable no-restricted-syntax */
assert.deepStrictEqual(options, sortedOptions, 'CLI options are not in alphabetical order');
const sections = /^## (.+)$/mg;
const cliOptionPattern = /^### (?:`-\w.*`, )?`([^`]+)`/mg;
let match;
let previousIndex = 0;
do {
const sectionTitle = match?.[1];
match = sections.exec(cliText);
const filteredCLIText = cliText.slice(previousIndex, match?.index);
const options = Array.from(filteredCLIText.matchAll(cliOptionPattern), (match) => match[1]);
assert.deepStrictEqual(options, options.toSorted(), `${sectionTitle} subsections are not in alphabetical order`);
previousIndex = match?.index;
} while (match);

I can also do that as part of a follow-up PR

}

// add alias handling
manPagesOptions.delete('-trace-events-enabled');

Expand Down
Loading