Skip to content

fix(formatter): remove extra outer parentheses on return with JSDoc type cast#21109

Merged
leaysgur merged 4 commits intooxc-project:mainfrom
babu-ch:fix/return-typecast-extra-parens
Apr 9, 2026
Merged

fix(formatter): remove extra outer parentheses on return with JSDoc type cast#21109
leaysgur merged 4 commits intooxc-project:mainfrom
babu-ch:fix/return-typecast-extra-parens

Conversation

@babu-ch
Copy link
Copy Markdown
Contributor

@babu-ch babu-ch commented Apr 7, 2026

closes #20183

@babu-ch babu-ch requested review from Dunqing and leaysgur as code owners April 7, 2026 08:45
@github-actions github-actions Bot added A-formatter Area - Formatter C-bug Category - Bug labels Apr 7, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 7, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing babu-ch:fix/return-typecast-extra-parens (8e00c8c) with main (a950f55)

Open in CodSpeed

Footnotes

  1. 7 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.

@babu-ch babu-ch marked this pull request as draft April 7, 2026 09:04
@babu-ch babu-ch force-pushed the fix/return-typecast-extra-parens branch from baecad5 to c59b0b4 Compare April 7, 2026 09:11
@babu-ch babu-ch marked this pull request as ready for review April 7, 2026 09:46
Copy link
Copy Markdown
Member

@leaysgur leaysgur left a comment

Choose a reason for hiding this comment

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

I found one case that still produces extra outer parentheses.

The is_own_line_comment_or_multi_line_comment closure isn't covered by the same type_cast_comment_end filter, so a comment between object and .property inside type cast parens still triggers outer parenthesization.

INPUT:

function type_cast_member_expression() {
  return (
    /** @type {X} */ (
      obj
        // comment
        .prop
    )
  );
}

Actual:

function type_cast_member_expression() {
  return (
    /** @type {X} */ (
      obj
        // comment
        .prop
    )
  );
}

Expected:

function type_cast_member_expression() {
  return /** @type {X} */ (
    obj
      // comment
      .prop
  );
}

@babu-ch
Copy link
Copy Markdown
Contributor Author

babu-ch commented Apr 8, 2026

@leaysgur
You're right, that case was also necessary! I have added it

@leaysgur

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@leaysgur
Copy link
Copy Markdown
Member

leaysgur commented Apr 9, 2026

/oxfmt-ecosys

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

Oxfmt Ecosystem CI

suite oxfmt@latest refs/pull/21109/head branch
eggjs/egg
monkeytypegame/monkeytype
cnpm/cnpmcore
formatjs/formatjs
vercel/turborepo
vuejs/pinia
actualbudget/actual
fuma-nama/fumadocs
vuejs/core
AmanVarshney01/create-better-t-stack
fastify/fastify-vite
huggingface/huggingface.js
tale/headplane
dyad-sh/dyad
lichess-org/lila
getsentry/sentry-javascript
Comfy-Org/ComfyUI_frontend
cloudflare/agents
openclaw/openclaw
aidenybai/react-grab
rolldown/rolldown
mastodon/mastodon
npmx-dev/npmx.dev

Copy link
Copy Markdown
Member

@leaysgur leaysgur left a comment

Choose a reason for hiding this comment

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

Thanks!

One minor note (not related to this PR):
the type_cast_member_expression.js test case has a pre-existing difference from Prettier's output.

Prettier hoists the comment before the member expression, while oxc_formatter keeps it between the object and property:

// Prettier
return /** @type {X} */ (
    // comment
    obj.prop
);

// oxc_formatter
return /** @type {X} */ (
    obj
      // comment
      .prop
);

This exists on main already, so not a concern for this PR — just flagging it as a known discrepancy.

@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Apr 9, 2026
@graphite-app
Copy link
Copy Markdown
Contributor

graphite-app Bot commented Apr 9, 2026

Merge activity

  • Apr 9, 3:16 AM UTC: @babu-ch we removed the merge queue label because we could not find a Graphite account associated with your GitHub profile.

You must have a Graphite account in order to use the merge queue. Create an account and try again using this link

@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 9, 2026
@leaysgur leaysgur merged commit d13fd37 into oxc-project:main Apr 9, 2026
32 of 33 checks passed
camc314 pushed a commit that referenced this pull request Apr 13, 2026
# Oxlint
### 💥 BREAKING CHANGES

- 382958a span: [**BREAKING**] Remove re-exports of string types from
`oxc_span` crate (#21246) (overlookmotel)
- c4aedfa str: [**BREAKING**] Add `static_ident!` macro (#21245)
(overlookmotel)
- 7354f3c linter: [**BREAKING**] Error on no matched files (#21144)
(camc314)

### 🚀 Features

- 91f2c79 linter/eslint-jest-plugin: Implemented
`prefer-importing-jest-globals` rule (#21303) (Said Atrahouch)
- a02f32c linter: Add release version for existing rules (#21363)
(camchenry)
- b9e93da linter: Allow tagging rules with release version (#21362)
(camchenry)
- f99ecda oxlint: Gate `vite.config.ts` recognition behind `VP_VERSION`
env var (#21298) (leaysgur)
- cf459d3 linter: Implement suggestion for `no-empty-function` rule
(#21347) (Mikhail Baev)
- 7213d61 linter: Adding pending suggestions fix to `valid_expect`
rules. (#21249) (Said Atrahouch)
- ae45312 linter: Introduce `--type-check-only` flag (#21184) (camc314)
- 1ce8b90 linter: Implemented `valid-expect-in-promise` vitest and jest
rule (#21170) (Said Atrahouch)
- 39f7fda linter: Add auto-fix to `unicorn/prefer-default-parameters`
(#21166) (yefan)
- 15574bc linter/unicorn: Implement consistent-template-literal-escape
(#21126) (AliceLanniste)
- c5c8c03 linter/prefer-readonly-parameter-types: Move rule from nursery
to pedantic (#21114) (camc314)
- 1893be1 linter/no-useless-default-assignment: Move rule from nursery
to correctness (#21113) (camc314)
- 5462ff9 linter/strict-void-return: Move rule from nursery to pedantic
(#21115) (camc314)
- c2989bd linter/no-unnecessary-type-parameters: Move rule from nursery
to suspicious (#21112) (camc314)
- 79d339a linter/no-unnecessary-qualifier: Move rule from nursery to
style (#21111) (camc314)

### 🐛 Bug Fixes

- b577efc linter/unicorn: Handle optional chaining in
`prefer-array-flat` and `no-invalid-remove-event-listener` (#21299)
(Mikhail Baev)
- 5e55735 oxlint/lsp: Skip .git directories in LSP walkers (#21316)
(camc314)
- ec7f6ed oxlint, oxfmt: Apply `check_for_writer_error` to `.flush()`
(#21343) (Craig Morrison)
- a17a08a linter/no-useless-assignment: Handle continue edges in loop
analysis (#21358) (camc314)
- a0eac12 linter/array-type: Move match to first stmt (#21357) (camc314)
- 1b3abc3 linter: Exclude boundary tokens from JSXText whitespace check
in isSpaceBetweenTokens (#21313) (bab)
- ecbcf5e linter: More info to summary output for GitHub formatter
(#21330) (Théo LUDWIG)
- a0a8c62 linter/no-fallthrough: Check from start of switch case for
empty lines (#21324) (Josh Cartmell)
- 36f0bc4 linter/no-cycle: Report all cyclic dependencies inside a file
(#21259) (camc314)
- 3f80536 linter: Ignore regex flags other than `g`/`u`/`v` in
`prefer-string-replace-all` (#21203) (bab)
- f21d3aa linter/unicorn: Report on optional in
`require-number-to-fixed-digits-argument` rule (#21207) (Mikhail Baev)
- af8e122 linter: Render each config error as a separate diagnostic
(#21120) (bab)
- a950f55 linter/unicorn: Do not report on optionals in
`no-single-promise-in-promise-methods` (#21157) (Mikhail Baev)
- 472f8ee linter: Mark complete comment for unused disable directives +
lsp fix (#21092) (copilot-swe-agent)
- edd0865 linter/no-array-index-key: False positive when index is inside
an expression within a template literal (#21123) (bab)
- 7e8d520 linter/unicorn: Report on optional `foo?.postMessage` in
`require-post-message-target-origin` rule (#21104) (Mikhail Baev)

### ⚡ Performance

- addcd02 napi/parser, linter/plugins: Raw transfer deserializer for
`Vec`s use shift instead of multiply where possible (#21142)
(overlookmotel)
- 3068ded napi/parser, linter/plugins: Shift before add when calculating
positions in raw transfer deserializer (#21141) (overlookmotel)
- eb400b8 napi/parser, linter/plugins: Remove `uint32` buffer view
(#21140) (overlookmotel)
- 7a86613 linter/plugins: Use `Int32Array`s for tokens and comments
buffers (#21136) (overlookmotel)
- 8c51121 napi/parser, linter/plugins: Raw transfer deserialize `Span`
fields as `i32`s (#21135) (overlookmotel)
- bc1bcdd napi/parser, linter/plugins: Inline trivial raw transfer field
deserializers into node object definitions (#21134) (overlookmotel)
- c0278ab napi/parser, linter/plugins: Use `Int32Array` in raw transfer
deserializer (#21132) (overlookmotel)
- 43482c7 linter/plugins: Use `>>` not `>>>` in binary search loops
(#21129) (overlookmotel)

### 📚 Documentation

- 7888280 linter: Move config docs for `no-restricted-exports` (#21360)
(camchenry)
- 162d26c linter: Improve docs for `typescript/array-type` (#21356)
(camchenry)
- a2dbaec linter: Add missing docs for options for
`typescript/class-literal-property-style` (#21355) (camchenry)
- 79593eb linter: Improve docs for
`typescript/consistent-type-assertions` (#21353) (camchenry)
- f9d20d2 linter: Move config option docs for
`typescript/no-empty-object-type` (#21352) (camchenry)
- a8f650d linter: Add missing config option docs for
`prefer-string-start-ends-with` (#21332) (camchenry)
- cfd8a4f linter: Don't rely on old eslint doc for available globals
(#21334) (Nicolas Le Cam)
- 03865fa linter: Jest/prefer-snapshot-hint: add doc comment for
snapshot hint mode (#21290) (camchenry)
- a6fe09b linter: Add missing docs for config options in `react` plugin
(#21289) (camchenry)
- 60eaf47 linter: Add missing docs for config options in unicorn plugin
(#21288) (camchenry)
- c3c2055 linter: `jsx-a11y/label-has-associated-control`: document the
`assert` options (#21287) (camchenry)
- a928ed9 linter: Add missing config docs for vitest plugin rules
(#21285) (camchenry)
- 7e07c7c linter: `id-length`: move enum docs to doc comments (#21281)
(camchenry)
- 9746bdf linter: Add missing docs for `class-methods-use-this` config
(#21278) (camchenry)
- 6ffe7a5 linter: Move docs for `Target` variant onto enum (#21277)
(camchenry)
- 305350d linter/plugins: Correct comments (#21130) (overlookmotel)
# Oxfmt
### 💥 BREAKING CHANGES

- 382958a span: [**BREAKING**] Remove re-exports of string types from
`oxc_span` crate (#21246) (overlookmotel)

### 🚀 Features

- e3081e1 oxfmt: Gate `vite.config.ts` recognition behind `VP_VERSION`
env var (#21295) (leaysgur)
- 5b0b573 oxfmt: Update prettier to 3.8.2 (#21294) (leaysgur)
- 0d67834 oxfmt: Show hint for all files are ignored case (#21154)
(leaysgur)

### 🐛 Bug Fixes

- 2871fc2 oxfmt: Non idempotent formatting on comments in TS (#20449)
(Cat Chen)
- ec7f6ed oxlint, oxfmt: Apply `check_for_writer_error` to `.flush()`
(#21343) (Craig Morrison)
- 1a8c225 formatter: Preserve newline between self-closing JSX element
and single-char text (#21149) (Justin Mecham)
- 407b725 oxfmt: Indent dangling comments in empty enum with block
indent (#21163) (Leonabcd123)
- d13fd37 formatter: Remove extra outer parentheses on return with JSDoc
type cast (#21109) (bab)
- 22babde oxfmt: Fix unicode char escaping (#21162) (leaysgur)
- 4da53e5 formatter: Preserve trailing comma in TSX arrow functions with
default type params (#21151) (Justin Mecham)
- 94fe774 oxfmt: Handle paths with consecutive leading slashes (#21155)
(leaysgur)
- 50c389b oxfmt: Support `.editorconfig` `quote_type` (#20989)
(leaysgur)

### ⚡ Performance

- 0ce619f formatter: Use `Allocator::alloc_concat_strs_array` instead of
`StringBuilder::from_strs_array_in` (#21339) (overlookmotel)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

formatter: Return statement with JSDoc type cast gets extra outer parentheses

2 participants