Skip to content

fix(linter/no-array-sort): skip non compare fn sort arguments#22752

Merged
camc314 merged 2 commits into
oxc-project:mainfrom
gaurav0107:fix/22487-linter-unicorn-no-array-sort-reports-non
May 28, 2026
Merged

fix(linter/no-array-sort): skip non compare fn sort arguments#22752
camc314 merged 2 commits into
oxc-project:mainfrom
gaurav0107:fix/22487-linter-unicorn-no-array-sort-reports-non

Conversation

@gaurav0107

Copy link
Copy Markdown
Contributor

Summary

Closes #22487.

unicorn/no-array-sort currently flags every member-call named .sort(...),
including query-builder APIs like Mongoose's Model.find().sort({ field: 1 }).
The receiver is not an Array, so the diagnostic is a false positive.

This change narrows the rule by inspecting the single argument's shape.
Array.prototype.sort(compareFn?) accepts either zero arguments or a function-
shaped value. A single object, string, numeric, template-literal, array-literal,
or unary-numeric argument is therefore incompatible with Array#sort. Skipping
diagnostics in those cases removes the false positives without weakening
detection of the real misuse patterns this rule targets (array.sort(),
[...array].sort(), array.sort(compareFn)).

Changes

crates/oxc_linter/src/rules/unicorn/no_array_sort.rs:

  • New helper is_non_compare_fn_argument early-returns from Rule::run when
    the call has exactly one argument matching the non-compareFn shapes listed
    above.
  • The in-source // TODO: Get these passing? block at the previous
    no_array_sort.rs:156 enumerated exactly these false-positive cases. Those
    TODO entries are now active pass tests.
  • Added three new pass regression tests drawn directly from the issue body
    (User.find().sort({ createdAt: -1 }), User.find().sort("-createdAt"),
    Post.find({ published: true }).sort({ updatedAt: "desc" })).
  • All existing fail and expect_fix cases are unchanged. The snapshot file
    snapshots/unicorn_no_array_sort.snap is unchanged because the new pass
    cases produce no diagnostics.

Test plan

  • cargo test --all-features -p oxc_linter no_array_sort1 passed; 0 failed
  • cargo test --all-features -p oxc_linter unicorn::148 passed; 0 failed
  • cargo fmt -- --check crates/oxc_linter/src/rules/unicorn/no_array_sort.rs — clean
  • Manual reasoning on the AST argument shapes:
    • array.sort(compareFn)Argument::Identifier (or arrow/function expr); not skipped, still reported
    • array.sort((a, b) => a - b)Argument::ArrowFunctionExpression; not skipped, still reported
    • array.sort(({a}) => a) — destructuring lives inside the arrow body; argument is still ArrowFunctionExpression; not skipped
    • query.sort(-1) / query.sort(+1)Argument::UnaryExpression over a NumericLiteral; explicitly skipped
    • array.sort(...spread)Argument::SpreadElement; the existing earlier early-return handles this

Notes

  • Disclosure: This change was prepared with AI assistance per the project's
    AI usage policy. The diagnostic narrowing follows the in-source TODO list
    the maintainer already curated, and all logic, tests, and reasoning were
    reviewed before submission.
  • The fix is intentionally conservative and type-info-free, in line with the
    rest of the unicorn rules. If a future direction prefers gating this kind
    of receiver inference behind --type-aware, this PR's logic can be
    reused or moved without disturbing public API.

Closes oxc-project#22487.

Narrow `unicorn/no-array-sort` to skip `.sort(...)` calls whose single
argument cannot be a valid `Array#sort` `compareFn`. `Array.prototype.sort`
accepts either zero arguments or a function-shaped value; an object,
string, numeric, template-literal, array-literal, or unary-numeric
argument is therefore inconsistent with `Array#sort` and almost always
indicates a query-builder API like Mongoose's
`Model.find().sort({ field: 1 })`.

The in-source TODO list at `no_array_sort.rs:156` already enumerated
exactly these false-positive shapes; this change promotes those cases
from commented TODOs to active `pass` tests and adds the issue body's
Mongoose reproduction as a named regression case.
@gaurav0107 gaurav0107 marked this pull request as ready for review May 27, 2026 06:00
@gaurav0107 gaurav0107 requested a review from camc314 as a code owner May 27, 2026 06:00
@camc314 camc314 added the A-linter Area - Linter label May 27, 2026
@camc314 camc314 self-assigned this May 28, 2026
@camc314 camc314 requested a review from Copilot May 28, 2026 11:13
@camc314 camc314 changed the title fix(linter): no-array-sort skip non-array literal arguments fix(linter/no-array-sort): skip non compare fn sort arguments May 28, 2026

@camc314 camc314 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you!

@camc314 camc314 merged commit 9f2f709 into oxc-project:main May 28, 2026
30 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR narrows unicorn/no-array-sort to avoid false positives for non-array .sort(...) APIs such as Mongoose query sorting.

Changes:

  • Adds an argument-shape helper to skip .sort(...) calls with single non-compare-function literal-style arguments.
  • Promotes prior TODO false-positive cases into passing tests.
  • Adds Mongoose regression pass cases from the linked issue.

Comment thread crates/oxc_linter/src/rules/unicorn/no_array_sort.rs
@codspeed-hq

codspeed-hq Bot commented May 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 52 skipped benchmarks1


Comparing gaurav0107:fix/22487-linter-unicorn-no-array-sort-reports-non (1242956) with main (27268a0)

Open in CodSpeed

Footnotes

  1. 52 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

graphite-app Bot pushed a commit that referenced this pull request May 28, 2026
Follow-on from the review comment on #22752: unwrap parenthesized `.sort(...)` arguments before deciding whether they are non-compareFn query-builder arguments.
camc314 pushed a commit that referenced this pull request Jun 1, 2026
# Oxlint
### 🚀 Features

- e4b1f46 linter/typescript: Implement `method-signature-style` rule
(#22679) (Mikhail Baev)
- bc462ca linter/vue: Implement no-reserved-component-names rule
(#22741) (bab)
- ef9e751 linter/vue: Implement component-definition-name-casing rule
(#22818) (bab)
- d67f51a linter/vue: Implement require-prop-type-constructor rule
(#22708) (bab)
- 1444f82 linter/promise/spec-only: Add `Promise.try` to `Promise`
static methods (#22812) (Ben Saufley)
- 8422e8b linter/jsdoc: Implement `require-yields-description` rule
(#22805) (Mikhail Baev)
- fe93f97 linter/eslint: Implement `prefer-named-capture-group` rule
(#22759) (Sebastian Poxhofer)
- 1a7798b linter: Add suggestion for `unicorn/no-new-array` (#22682)
(Sysix)

### 🐛 Bug Fixes

- 760a9f9 linter: Report errors when writing to the filesystem (#22881)
(camc314)
- e5a2748 linter: Avoid no-unreachable false positive after conditional
loop (#22869) (camc314)
- 39d92d6 linter/arrow-body-style: Preserve comments within function
(#22854) (Sysix)
- 3d13e29 parser: Reject `declare` in an already-ambient context
(TS1038) (#22850) (Boshen)
- 5152854 parser: Reject statements in ambient contexts (TS1036)
(#22849) (Boshen)
- 2eafea6 parser: Reject function implementations in ambient contexts
(TS1183) (#22845) (Boshen)
- c645615 parser: Reject incompatible class member modifiers (#22843)
(Boshen)
- 4a1ca4a linter/export: Detect duplicate explicit exports (#22798)
(camc314)
- 0a9a735 linter/no-loop-func: Allow safe let closures (#22811)
(camc314)
- 1599f11 linter: Align lsp extends default plugins (#22788) (camc314)
- db32ec9 linter/no-accumulating-spread: Use loop as primary span
(#22800) (camc314)
- 33ec6b4 linter/consistent-test-it: Avoid adjacent describe leakage
(#22796) (camc314)
- 2606069 linter/no-array-sort: Unwrap parenthesized sort args (#22794)
(camc314)
- 9f2f709 linter/no-array-sort: Skip non compare fn sort arguments
(#22752) (Gaurav Dubey)
- 27268a0 linter/no-else-return: Preserve statement boundary in fixer
(#22687) (camc314)
- d9cb6d8 linter/no-empty-function: Allow functions callbacks with
`allow: functions` (#22764) (camc314)
- a40a314 linter/no-shadow-restricted-names: Ignore enum members
(#22762) (camc314)
- 82366d9 linter/no-cond-assign: Align ternary handling (#22761)
(camc314)

### 📚 Documentation

- 5e113ba linter: Add license notices for ported ESLint plugins (#22768)
(Boshen)
# Oxfmt
### 🚀 Features

- d75cbbf oxfmt: Format `parser:json` files by `oxc_formatter_json`
(#22709) (leaysgur)
- 49db054 formatter_json: Implement `oxc_formatter_json` (json variant
only) (#22641) (leaysgur)
- 9c71f2e ast, codegen, formatter: Add `WithClauseKeyword::as_str`
helper and use it (#22791) (camc314)

### 🐛 Bug Fixes

- d3cdd62 oxfmt: Skip formatting for whitespace-only file (#22780)
(leaysgur)
- 23f0cc8 formatter: Don't move comments inside variable declaration in
for in loop (#22776) (leaysgur)
- f200c40 formatter: Don't move comments inside variable declaration in
for of loop (#22773) (Leonabcd123)

### 📚 Documentation

- 845f393 oxfmt,formatter,formatter_json,formatter_core: Add/update
AGENTS.md (#22873) (leaysgur)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linter: unicorn/no-array-sort reports non-array .sort() calls

3 participants