fix: tolerate trailing inline comment in _tools/eslint/rules/jsdoc-doctest#11632
Draft
Planeshifter wants to merge 1 commit intodevelopfrom
Draft
fix: tolerate trailing inline comment in _tools/eslint/rules/jsdoc-doctest#11632Planeshifter wants to merge 1 commit intodevelopfrom
_tools/eslint/rules/jsdoc-doctest#11632Planeshifter wants to merge 1 commit intodevelopfrom
Conversation
…octest`
The `RE_ANNOTATION` matcher required `;\n//` literally, so any statement
carrying a trailing inline `// ...` comment between the `;` and the `\n`
caused the entire `// returns X` assertion to be silently skipped. Allow
an optional `[ \t]*(?://[^\n]*)?` between the semicolon and newline so
math-narration and other inline comments no longer disable the doctest.
---
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: na
- 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: 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
---
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request:
_tools/eslint/rules/jsdoc-doctest— Relaxes theRE_ANNOTATIONmatcher inlib/main.jsso that an optional trailing inline// ...comment between the statement-terminating;and the newline preceding a// returns X(or// => X,// throws X) annotation no longer disables the doctest. Previously the regex required;\n//literally, which meant any statement carrying a math-narration / explanatory trailing comment caused the rule to silently skip the assertion. Adds five fixture-driven test cases (two valid, three invalid) covering bothvar-annotated andconsole.log-annotated forms, the autofix output (which preserves the trailing comment and only rewrites the// returnsannotation), and the edge case where the trailing inline comment itself contains a;.Related Issues
This pull request surfaced from review of #11584, where a stale
// returns 0.5annotation inchebyshev-seriesf's JSDoc example slipped past the doctest rule because of this exact gap. With this fix in place, the rule would have flagged that bug at lint time, and will now also catch the same pattern still present in the siblingchebyshev-seriespackage.No.
Questions
No.
Other
Verification.
node test/test.js(from the rule's package directory): all 5 tape tests pass, including the 5 newly added fixtures.chebyshev-seriesfexample confirms the old regex returned 0 matches (silently skipping both annotations), while the new regex returns 2 matches and correctly surfaces the stale// returns 0.5.Why the old regex missed it. The chebyshev example reads:
The old
;\n\/\/literal inRE_ANNOTATIONrequired the semicolon to be followed immediately by a newline. The space +// 1*T_0(0) + 0.5*T_1(0)between;and\nbroke the match, so the assertion was never executed and the wrong return value was never compared.Risk surface.
[ \t]*(?:\/\/[^\n]*)?is non-capturing; existing capture indices (arr[1]variable name,arr[2]annotation kind,arr[3]arrow-fn name,arr[4]expected value) are unchanged.arr[0]is fed back tovm.runInContextat line 300 — appended trailing line comments are stripped at JS parse time, so execution is unaffected.replace( tag.description, arr[ 0 ], str )at line 318) still works because the enlargedarr[0]remains a verbatim substring of the JSDoc description; the trailing inline comment is preserved verbatim in the autofix output, as covered by the new invalid fixtures'outputassertions.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by Claude (Opus 4.7) under my review. The bug was identified during follow-up review of PR #11584; Claude traced it to the exact regex literal, drafted the one-character-region regex change, added paired valid/invalid fixtures, and verified with both the existing tape suite and a direct microtest against the real-world chebyshev snippet that previously slipped through.
@stdlib-js/reviewers