Repository navigation
chore: clean-up - #15566
chore: clean-up#15566
Conversation
Propagates fix from f22d513 ("chore: fix JavaScript lint errors") to sibling packages declaring a nested function which references no binding local to its immediate enclosing function, tripping `stdlib/no-unnecessary-nested-functions`. Each declaration is dedented one scope level, as in the source commit. The JSDoc block travels with the declaration and each body is unchanged apart from one tab of indentation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SzTKV4KFPDZcVJF3NqTL9E
Reverts the dedent in `_tools/github/rank-followers` and `_tools/github/rank-users`, leaving both files as on `develop`. `test/test.cli.js` in each package fails identically on `develop` (2 and 4 assertions respectively): `bin/cli` keeps `format` and `delimiter` in locals used only by the `csv`/`json` writers and never assigns them onto `opts`, while `test/fixtures/cli_format.js.txt` and `cli_delimiter.js.txt` assert on `opts.format` and `opts.delimiter`, which are therefore always `undefined`. `cli_format.js.txt` additionally passes `--format=json` where the test expects `csv`. Touching either package makes those tests "affected", so the propagation blocks on a defect it does not introduce and cannot fix without widening into an unrelated test correction. Both sites remain valid and are logged for a follow-up run once the CLI tests are corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SzTKV4KFPDZcVJF3NqTL9E
|
The failure is pre-existing on
|
| package | test/test.cli.js on this branch |
on develop |
|---|---|---|
_tools/github/rank-followers |
25 tests, 2 failing | 25 tests, 2 failing |
_tools/github/rank-users |
23 tests, 4 failing | 23 tests, 4 failing |
CI stopped at rank-followers, so the four rank-users failures had not yet surfaced.
Root cause
bin/cli never assigns format or delimiter onto opts. Both stay in locals consumed only by the csv/json writers:
fmt = flags.format || 'csv';
delimiter = flags.delimiter || ',';opts only ever receives useragent, username, method and token. But test/fixtures/cli_format.js.txt and cli_delimiter.js.txt mock ./../lib and report opts.format / opts.delimiter back to the test, so those values are structurally always undefined — hence expected: 'csv', actual: undefined. cli_format.js.txt also pushes --format=json while the test asserts 'csv', so the fixture and its assertion disagree even on intent.
The assertions therefore test something the CLI is not designed to do. This is a test/fixture defect, not a library one.
Why the two sites were dropped rather than fixed here
The patches to those two files are pure dedents of an internal done callback, and the failing tests never execute them — the fixtures replace ./../lib wholesale via proxyquire. But Run affected tests derives its scope from the directories this PR touches, so touching either package makes the broken tests run. Correcting them means rewriting two fixtures and their assertions to exercise the CLI's actual output formatting, which is unrelated to the nested-function pattern this PR propagates.
Both sites remain valid and are logged for a follow-up run once the CLI tests are corrected. I found no open PR addressing them.
Proposed patch, for whoever picks this up
Assert on what the CLI actually emits rather than on the mocked opts. For the --delimiter case, keep the fixture's --format=json and have the mock return a { results, data } shape, then assert that stdout contains the delimiter in the CSV header; for the --format case, drop the opts.format assertion and instead assert that --format=json yields parseable JSON on stdout while the default yields a username,value CSV header. Alternatively, if format and delimiter are meant to reach the library, the fix belongs in bin/cli and the assertions can stay.
I have not spent a CI re-run: the failures are deterministic assertion mismatches, and running the same tests against an unmodified develop tree is stronger evidence than a re-run would be.
Generated by Claude Code
Reverts the dedent in `_tools/github/user-details`, leaving the file as on `develop`. `.github/workflows/scripts/run_affected_tests/run` expands the changed-package set with every package that `require()`s one of them, and `_tools/github/rank-followers` requires `@stdlib/_tools/github/user-details`. Touching `user-details` therefore runs `rank-followers`'s test suite, which fails on `develop`: 2 assertions in `test/test.cli.js` and 22 in `test/test.pipeline.js`. Dropping this site removes `rank-followers` from the affected set. Reproducing the workflow's scoping locally now yields 13 packages and 77 test files, all passing. The site remains valid and is logged for a follow-up run once `rank-followers`'s tests are corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SzTKV4KFPDZcVJF3NqTL9E
|
Correcting my previous comment: dropping
dependents=$(find lib/node_modules/@stdlib -type f -name '*.js' \
-exec grep -ol -E "require\( [']${escaped_package}['] \)" {} +)
The pre-existing breakage is larger than I first reportedReproducing the workflow's scoping locally and running all 83 resulting test files found
Both are Current stateNine propagation sites remain: Verified locally before pushing, by reproducing the workflow's own scoping: 13 affected packages, 77 test files, all passing. The target lint rule reports zero violations across the nine files, and all seven dedented Three sites are dropped and logged for a follow-up once Generated by Claude Code |
|
The one remaining failure, Mechanism
make test-javascript-cov TESTS_FILTER=".*/${pkg}/test/.*" C8_FLAGS="-n 'lib/node_modules/@stdlib/${pkg}/**'"
$(QUIET) $(FIND_TESTS_CMD) | grep '^[\/]\|^[a-zA-Z]:[/\]' | while read -r test; do ...the This is visible in the job log: every test that ran passed ( Scope of the defectAny PR touching a non- Proposed fix, for a separate PRSkip packages with no tests, before line 151: if ! find "lib/node_modules/@stdlib/${pkg}/test" -name 'test*.js' -print -quit 2>/dev/null | grep -q .; then
echo "No tests found for ${pkg}; skipping coverage." >&2
continue
fiHardening Why not worked around hereThe only in-PR workarounds are to drop the seven No CI re-run spent: the failure is deterministic and structural, so a re-run would reproduce it exactly. Final stateNine propagation sites: Generated by Claude Code |
|
As the CI failure is not due to changes introduced in this PR, I'll go ahead and merge. |
_tools/github and repl
Description
Propagating fixes merged to
developbetween 2026-09-25 13:09 UTC (947861b6) and 2026-09-26 13:09 UTC (05ed6e856) to sibling packages carrying the same defect.Pattern: nested function declaration not depending on its immediate enclosing function
Commit
f22d513bresolved astdlib/no-unnecessary-nested-functionsviolation by dedenting a nestedonRenderdeclaration one scope level in_tools/docs/www/readme-database/lib/render.js; because that rule is enabled at severityerrorfor source files (etc/eslint/rules/stdlib.js:4465) and the JavaScript lint workflow samples a rotating subset of files per run, every remaining violation is a latent lint failure. This PR applies the same one-level dedent at the twelve remaining sites whose flagged declaration only depends on bindings in a grandparent function scope. In each case, the JSDoc block moves with the declaration and the function body is unchanged apart from one tab of indentation.f22d513b(chore: fix JavaScript lint errors)_tools/github/{get,rank-followers,rank-users,user-details}: dedentdoneinlib/factory.js.repl: dedentclbkinlib/commands/load.js.repl/presentation: dedentonDraininlib/commands/{first_fragment,next_slide,prev,prev_slide,redraw,reload_presentation,run_slide}.js.Related Issues
No.
Questions
Yes — one, concerning the rule rather than this diff.
stdlib/no-unnecessary-nested-functionsemits "should be moved to outer function scope" for declarations whose only legal destination is module scope. stdlib lints withsourceType: 'script'plusenv.node/env.commonjs(etc/eslint/parser-options/source_type.js,etc/eslint/env/index.js), which setsecmaFeatures.globalReturn, so eslint-scope wraps the module body in a scope of typefunction. The rule skips a dependency only whendepScope.typeis'global'or'module'(lib/main.js), so a nested function whose sole free identifiers are module-levelvars is reported as an outer-function-scope violation. Eight of the twenty candidates in this sweep are mislabeled this way. Worth a separate fix to the rule.Other
Validation
Linterinstance and swept over all 22,377 files matchinglib/node_modules/@stdlib/**/lib/**/*.js. The harness was validated by reproducing CI's exact report against the pre-fix blob ofrender.js(82:3).**/benchmark/*.js,**/examples/*.js,**/test/*.jsand*.mdare excluded, peretc/eslint/overrides/index.js. The sweep found 29 violations: 20 reported as "outer function scope", 9 as "module scope".node --checkclean.f22d513bshape anddocs/style-guides/javascript, then applied all 12 together in a disposable worktree and re-ran the rule. All 12 approved without rewrite.f22d513b.Deliberately excluded:
@privatehelper out of the exported function entirely, above// MAIN //, which requires introducing a// FUNCTIONS //section; a different and larger restructuring than the source commit's._tools/browserify/string(onDelete),iter/empty(factory),net/simple-http-server(onClose) andplot/sparklines/base/ctor(onRender). Reported as outer-function-scope violations but mislabeled per the question above: each is a direct child of a top-level function whose only free identifiers are module-levelvars, so the fix is a module-scope hoist. Behavior-preserving, but out of this pattern's shape; none of the four files has a// FUNCTIONS //section.array/base/{entries2views,nested2views,rekey-views,zip2views}(Datum). Hard rejections: each call decoratesDatum.prototypewith call-specific accessors viasetNonEnumerableReadOnly/setReadWriteAccessor, which define properties as non-configurable. Hoisting gives every call one shared prototype, so the second invocation throwsTypeError: Cannot redefine property. Both validation agents demonstrated the throw.14b80fb6(docs: fix examples, wrappingdtype( x )inString( … )in README usage snippets): 19 matching sites across 11 files, all dropped.@stdlib/ndarray/dtypeand@stdlib/array/dtypereturn primitive strings, so the wrapper is a runtime no-op; thestdlib/doctestrule executes the snippet and compares values, so it passes either way, and no style guide requires it.69c38fa9(test: avoid out-of-memory error): no sibling usescartesianPower( dtypes(), N )withN >= 5. The nearest neighbour,quaternary-tiling-block-sizeatN = 4, does not exhaust memory, and applying the rotation would cut coverage from 65,536 combinations to 16 — a coverage-for-cost trade for a maintainer, not a defect fix.f9e32db7(test: increase tolerance to account for compiler optimizations): 173 textual matches forisAlmostSameValue( y, expected[ i ], 0 )understats/base/dists/**, none propagated. Zero ULPs is correct wherever the expression does not contract, the appropriate tolerance is per-formula, and confirming a site requires an FMA-enabled native build.fa7647de(chore: clean-up): per-plugin JSDoc prose polish and a runtime-no-op array-construction change in private dev tooling. No defect at any sibling.836f90b9,62cbbac0and947861b6: pattern searches returned zero remaining sites; each source commit already covered every occurrence.5a64ba79,4c05b095,ea33a97f,dc56a799and77e92e53:docs:updates to auto-populated namespace tables of contents, related-packages sections and equation SVGs. Generator-owned — the fix belongs in the generator.test: migrate … to ULP-based assertionscommits and everyfeat:commit in the window: not fix patterns.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by Claude Code on behalf of @Planeshifter as an automated propagation of fixes merged to
developover the prior 24 hours. Candidate source commits were filtered for generalizable patterns, sibling sites located via a runnable pattern signature, and each proposed patch independently validated by parallel reviewer agents before commits were applied in the primary worktree. A maintainer should audit and promote out of draft.Generated by Claude Code