Skip to content

feat: add doctest-annotation-spacing ESLint rule #8842

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.

Sign up for GitHub

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
Open
Planeshifter wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

feat: add doctest-annotation-spacing ESLint rule #8842

Planeshifter wants to merge 4 commits into from

Conversation

@Planeshifter
Copy link
Member

@Planeshifter Planeshifter commented Dec 6, 2025

Resolves stdlib-js/metr-issue-tracker#127.

Description

What is the purpose of this pull request?

This pull request:

  • add an ESLint rule to enforce spacing after a returns annotation (and also that there is only one space following //)

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

PR was generated by Claude Code according to my specifications.

@stdlib-js/reviewers

@Planeshifter
feat: add doctest-annotation-spacing ESLint rule
20b7235 
This rule enforces spacing in return annotations in single-line
comments (`// returns` and `// =>`). It validates:

- Exactly 1 space between `//` and the annotation keyword
- Configurable spacing after the keyword (default: 1 space)

---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: passed
  - task: lint_package_json
    status: passed
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: passed
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: passed
  - task: lint_javascript_tests
    status: passed
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: passed
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---
@Planeshifter
docs: use JavaScript code blocks in README examples
850c135 
Also register the rule in the ESLint plugin namespace.

---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
  - task: lint_filenames
    status: passed
  - task: lint_editorconfig
    status: passed
  - task: lint_markdown
    status: passed
  - task: lint_package_json
    status: na
  - task: lint_repl_help
    status: na
  - task: lint_javascript_src
    status: passed
  - task: lint_javascript_cli
    status: na
  - task: lint_javascript_examples
    status: na
  - task: lint_javascript_tests
    status: na
  - task: lint_javascript_benchmarks
    status: na
  - task: lint_python
    status: na
  - task: lint_r
    status: na
  - task: lint_c_src
    status: na
  - task: lint_c_examples
    status: na
  - task: lint_c_benchmarks
    status: na
  - task: lint_c_tests_fixtures
    status: na
  - task: lint_shell
    status: na
  - task: lint_typescript_declarations
    status: passed
  - task: lint_typescript_tests
    status: na
  - task: lint_license_headers
    status: passed
---
@stdlib-bot stdlib-bot added Tools Issue or pull request related to project tooling. Needs Review A pull request which needs code review. labels Dec 6, 2025
@Planeshifter Planeshifter changed the title Philipp/eslint doctest annotation spacing feat: add doctest-annotation-spacing ESLint rule Dec 6, 2025
@kgryte kgryte removed the Needs Review A pull request which needs code review. label Dec 7, 2025
kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/README.md
// returns 3.14
```

Note: The spacing before the annotation keyword is always enforced to be exactly 1 space and is not configurable.
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

@Planeshifter What is the rationale for spacing being configurable after the keyword but not before?

@kgryte
docs: update example
b79f72f 
Signed-off-by: Athan <kgryte@gmail.com>
kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/README.md
{
'ruleId': 'doctest-annotation-spacing',
'severity': 2,
'message': 'Return annotation `returns` should be followed by 1 space(s). Found 13 space(s).',
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

"Return annotation keyword returns"

IIUC, the entire comment is an "annotation".

@kgryte
docs: update example
bfa5a50 
Signed-off-by: Athan <kgryte@gmail.com>
kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/defaults.json
@@ -0,0 +1,3 @@
{
"numSpaces": 1
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

@Planeshifter Were you to support space configuration before and after keyword: beforeSpaces and afterSpaces.

kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/main.js
* - match a `//` literal
*
* - `(\s*)`
* - capture zero or more whitespace characters (the spacing before the keyword)
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

@Planeshifter I don't think this is actually what you want. \s matches multiple different types of whitespace, not just . Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions/Character_classes#types.

I think we should be explicit here. E.g., we should not be allowing tab characters or various Unicode whitespace characters. Just a regular space character.

kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/main.js
spacingAfter = matches[ 4 ].length;

// Check spacing before keyword (must be exactly 1 space):
if ( spacingBefore !== 1 ) {
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

As mentioned above, seems arbitrary to allow configuration of one but not the other.

kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/main.js
// Check spacing before keyword (must be exactly 1 space):
if ( spacingBefore !== 1 ) {
if ( prefix ) {
msg = 'Return annotation `' + prefix + ' ' + keyword + '` should have exactly 1 space after `//`. Found ' + spacingBefore + ' space(s).';
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

We have string/format. We should use it here and below.

kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/main.js
// Check spacing after keyword (configurable):
expected = opts.numSpaces;
if ( spacingAfter !== expected ) {
msg = 'Return annotation `' + keyword + '` should be followed by ' + expected + ' space(s). Found ' + spacingAfter + ' space(s).';
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

Suggested change
msg = 'Return annotation `' + keyword + '` should be followed by ' + expected + ' space(s). Found ' + spacingAfter + ' space(s).';
msg = 'Return annotation keyword `' + keyword + '` should be followed by ' + expected + ' space(s). Found ' + spacingAfter + ' space(s).';

kgryte
kgryte reviewed Dec 7, 2025
View reviewed changes
lib/node_modules/@stdlib/_tools/eslint/rules/doctest-annotation-spacing/lib/main.js
'properties': {
'numSpaces': {
'type': 'integer',
'minimum': 1
Copy link
Member

@kgryte kgryte Dec 7, 2025

Choose a reason for hiding this comment

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

If we want this to be a "general" thing eventually which others can use, I'd allow the minimum to be 0.

kgryte
kgryte requested changes Dec 7, 2025
View reviewed changes
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

Left an initial round of comments.

@kgryte kgryte added the Needs Changes Pull request which needs changes before being merged. label Dec 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kgryte kgryte kgryte requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

Needs Changes Pull request which needs changes before being merged. Tools Issue or pull request related to project tooling.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RFC]: Add ESLint rule to enforce spacing after a returns annotation

4 participants

@Planeshifter @kgryte @stdlib-bot